Skip to content

Commit

Permalink
DevTools: carefully cleanup javascript sourcemaps
Browse files Browse the repository at this point in the history
This patch starts to detach JavaScript sourcemaps as the
associated scripts go away (e.g. parent frame got detached).

This patch makes javascript sourcemap management similar to
CSS domain:
- debuggerModel now has sourceMapManager
- CompilerScriptMapping listens only to SourceMapManager events

A few key differences arose:
- We make sure that there's only one sourceMap per
  (executionContext, script.sourceURL, script.sourceMapURL).
  This is to support "browsersync" scenario and "hot script reload".
- Since CompilerScriptMapping has a need for stubProjects,
  SourceMapManager is taught to send SourceMapWillAttach and
  SourceMapFailedToAttach events.

BUG=670180, 685552
R=dgozman

Review-Url: https://codereview.chromium.org/2784413003
Cr-Commit-Position: refs/heads/master@{#461303}
  • Loading branch information
aslushnikov authored and Commit bot committed Apr 1, 2017
1 parent 152f290 commit 074cd71
Show file tree
Hide file tree
Showing 9 changed files with 205 additions and 229 deletions.
2 changes: 1 addition & 1 deletion front_end/bindings/BlackboxManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ Bindings.BlackboxManager = class {

/**
* @param {!SDK.Script} script
* @param {?SDK.TextSourceMap} sourceMap
* @param {?SDK.SourceMap} sourceMap
* @return {!Promise<undefined>}
*/
sourceMapLoaded(script, sourceMap) {
Expand Down
313 changes: 107 additions & 206 deletions front_end/bindings/CompilerScriptMapping.js

Large diffs are not rendered by default.

5 changes: 1 addition & 4 deletions front_end/bindings/DebuggerWorkspaceBinding.js
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ Bindings.DebuggerWorkspaceBinding = class extends Common.Object {

/**
* @param {!SDK.Script} script
* @return {?SDK.TextSourceMap}
* @return {?SDK.SourceMap}
*/
sourceMapForScript(script) {
var modelData = this._debuggerModelToData.get(script.debuggerModel);
Expand Down Expand Up @@ -370,9 +370,6 @@ Bindings.DebuggerWorkspaceBinding.ModelData = class {
var script = /** @type {!SDK.Script} */ (event.data);
this._defaultMapping.addScript(script);
this._resourceMapping.addScript(script);

if (Common.moduleSetting('jsSourceMapsEnabled').get())
this._compilerMapping.addScript(script);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion front_end/bindings/ResourceScriptMapping.js
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@ Bindings.ResourceScriptFile = class extends Common.Object {
addSourceMapURL(sourceMapURL) {
if (!this._script)
return;
this._script.addSourceMapURL(sourceMapURL);
this._script.debuggerModel.setSourceMapURL(this._script, sourceMapURL);
}

/**
Expand Down
4 changes: 3 additions & 1 deletion front_end/platform/utilities.js
Original file line number Diff line number Diff line change
Expand Up @@ -1213,12 +1213,14 @@ Multimap.prototype = {
/**
* @param {K} key
* @param {V} value
* @return {boolean}
*/
remove: function(key, value) {
var values = this.get(key);
values.delete(value);
var result = values.delete(value);
if (!values.size)
this._map.delete(key);
return result;
},

/**
Expand Down
79 changes: 77 additions & 2 deletions front_end/sdk/DebuggerModel.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ SDK.DebuggerModel = class extends SDK.SDKModel {
this._agent = target.debuggerAgent();
this._runtimeModel = /** @type {!SDK.RuntimeModel} */ (target.model(SDK.RuntimeModel));

/** @type {!SDK.SourceMapManager<!SDK.Script>} */
this._sourceMapManager = new SDK.SourceMapManager(target);
/** @type {!Map<string, !SDK.Script>} */
this._sourceMapIdToScript = new Map();

/** @type {?SDK.DebuggerPausedDetails} */
this._debuggerPausedDetails = null;
/** @type {!Object.<string, !SDK.Script>} */
Expand All @@ -65,6 +70,28 @@ SDK.DebuggerModel = class extends SDK.SDKModel {

/** @type {!Map<string, string>} */
this._stringMap = new Map();
this._sourceMapManager.setEnabled(Common.moduleSetting('jsSourceMapsEnabled').get());
Common.moduleSetting('jsSourceMapsEnabled')
.addChangeListener(event => this._sourceMapManager.setEnabled(/** @type {boolean} */ (event.data)));
}

/**
* @param {string} executionContextId
* @param {string} sourceURL
* @param {?string} sourceMapURL
* @return {?string}
*/
static _sourceMapId(executionContextId, sourceURL, sourceMapURL) {
if (!sourceMapURL)
return null;
return executionContextId + ':' + sourceURL + ':' + sourceMapURL;
}

/**
* @return {!SDK.SourceMapManager<!SDK.Script>}
*/
sourceMapManager() {
return this._sourceMapManager;
}

/**
Expand Down Expand Up @@ -314,6 +341,10 @@ SDK.DebuggerModel = class extends SDK.SDKModel {
}

_reset() {
for (var scriptWithSourceMap of this._sourceMapIdToScript.values())
this._sourceMapManager.detachSourceMap(scriptWithSourceMap);
this._sourceMapIdToScript.clear();

this._scripts = {};
this._scriptsBySourceURL.clear();
this._stringMap.clear();
Expand Down Expand Up @@ -486,6 +517,18 @@ SDK.DebuggerModel = class extends SDK.SDKModel {
this.dispatchEventToListeners(SDK.DebuggerModel.Events.ParsedScriptSource, script);
else
this.dispatchEventToListeners(SDK.DebuggerModel.Events.FailedToParseScriptSource, script);

var sourceMapId = SDK.DebuggerModel._sourceMapId(script.executionContextId, script.sourceURL, script.sourceMapURL);
if (sourceMapId && !hasSyntaxError) {
// Consecutive script evaluations in the same execution context with the same sourceURL
// and sourceMappingURL should result in source map reloading.
var previousScript = this._sourceMapIdToScript.get(sourceMapId);
if (previousScript)
this._sourceMapManager.detachSourceMap(previousScript);
this._sourceMapIdToScript.set(sourceMapId, script);
this._sourceMapManager.attachSourceMap(script, script.sourceURL, script.sourceMapURL);
}

var isDiscardable = hasSyntaxError && script.isAnonymousScript();
if (isDiscardable) {
this._discardableScripts.push(script);
Expand All @@ -494,6 +537,38 @@ SDK.DebuggerModel = class extends SDK.SDKModel {
return script;
}

/**
* @param {!SDK.Script} script
* @param {string} newSourceMapURL
*/
setSourceMapURL(script, newSourceMapURL) {
var sourceMapId = SDK.DebuggerModel._sourceMapId(script.executionContextId, script.sourceURL, script.sourceMapURL);
if (sourceMapId && this._sourceMapIdToScript.get(sourceMapId) === script)
this._sourceMapIdToScript.delete(sourceMapId);
this._sourceMapManager.detachSourceMap(script);

script.sourceMapURL = newSourceMapURL;
sourceMapId = SDK.DebuggerModel._sourceMapId(script.executionContextId, script.sourceURL, script.sourceMapURL);
if (!sourceMapId)
return;
this._sourceMapIdToScript.set(sourceMapId, script);
this._sourceMapManager.attachSourceMap(script, script.sourceURL, script.sourceMapURL);
}

/**
* @param {!SDK.ExecutionContext} executionContext
*/
executionContextDestroyed(executionContext) {
var sourceMapIds = Array.from(this._sourceMapIdToScript.keys());
for (var sourceMapId of sourceMapIds) {
var script = this._sourceMapIdToScript.get(sourceMapId);
if (script.executionContextId === executionContext.id) {
this._sourceMapIdToScript.delete(sourceMapId);
this._sourceMapManager.detachSourceMap(script);
}
}
}

/**
* @param {!SDK.Script} script
*/
Expand Down Expand Up @@ -772,6 +847,7 @@ SDK.DebuggerModel = class extends SDK.SDKModel {
* @override
*/
dispose() {
this._sourceMapManager.dispose();
Common.moduleSetting('pauseOnExceptionEnabled').removeChangeListener(this._pauseOnExceptionStateChanged, this);
Common.moduleSetting('pauseOnCaughtException').removeChangeListener(this._pauseOnExceptionStateChanged, this);
Common.moduleSetting('enableAsyncStackTraces').removeChangeListener(this.asyncStackTracesStateChanged, this);
Expand Down Expand Up @@ -847,8 +923,7 @@ SDK.DebuggerModel.Events = {
DiscardedAnonymousScriptSource: Symbol('DiscardedAnonymousScriptSource'),
GlobalObjectCleared: Symbol('GlobalObjectCleared'),
CallFrameSelected: Symbol('CallFrameSelected'),
ConsoleCommandEvaluatedInSelectedCallFrame: Symbol('ConsoleCommandEvaluatedInSelectedCallFrame'),
SourceMapURLAdded: Symbol('SourceMapURLAdded')
ConsoleCommandEvaluatedInSelectedCallFrame: Symbol('ConsoleCommandEvaluatedInSelectedCallFrame')
};

/** @enum {string} */
Expand Down
1 change: 1 addition & 0 deletions front_end/sdk/RuntimeModel.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ SDK.RuntimeModel = class extends SDK.SDKModel {
var executionContext = this._executionContextById.get(executionContextId);
if (!executionContext)
return;
this.debuggerModel().executionContextDestroyed(executionContext);
this._executionContextById.delete(executionContextId);
this.dispatchEventToListeners(SDK.RuntimeModel.Events.ExecutionContextDestroyed, executionContext);
}
Expand Down
14 changes: 2 additions & 12 deletions front_end/sdk/Script.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 12 additions & 2 deletions front_end/sdk/SourceMapManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,8 @@ SDK.SourceMapManager = class extends SDK.SDKObject {
if (!this._isEnabled)
return;

this.dispatchEventToListeners(SDK.SourceMapManager.Events.SourceMapWillAttach, client);

if (this._sourceMapByURL.has(sourceMapURL)) {
attach.call(this, sourceMapURL, client);
return;
Expand Down Expand Up @@ -169,8 +171,13 @@ SDK.SourceMapManager = class extends SDK.SDKObject {
this._sourceMapLoadedForTest();
var clients = this._sourceMapURLToLoadingClients.get(sourceMapURL);
this._sourceMapURLToLoadingClients.removeAll(sourceMapURL);
if (!sourceMap || !clients.size)
if (!clients.size)
return;
if (!sourceMap) {
for (var client of clients)
this.dispatchEventToListeners(SDK.SourceMapManager.Events.SourceMapFailedToAttach, client);
return;
}
this._sourceMapByURL.set(sourceMapURL, sourceMap);
for (var client of clients)
attach.call(this, sourceMapURL, client);
Expand Down Expand Up @@ -217,7 +224,8 @@ SDK.SourceMapManager = class extends SDK.SDKObject {
if (!sourceMapURL)
return;
if (!this._sourceMapURLToClients.hasValue(sourceMapURL, client)) {
this._sourceMapURLToLoadingClients.remove(sourceMapURL, client);
if (this._sourceMapURLToLoadingClients.remove(sourceMapURL, client))
this.dispatchEventToListeners(SDK.SourceMapManager.Events.SourceMapFailedToAttach, client);
return;
}
this._sourceMapURLToClients.remove(sourceMapURL, client);
Expand All @@ -238,6 +246,8 @@ SDK.SourceMapManager = class extends SDK.SDKObject {
};

SDK.SourceMapManager.Events = {
SourceMapWillAttach: Symbol('SourceMapWillAttach'),
SourceMapFailedToAttach: Symbol('SourceMapFailedToAttach'),
SourceMapAttached: Symbol('SourceMapAttached'),
SourceMapDetached: Symbol('SourceMapDetached'),
SourceMapChanged: Symbol('SourceMapChanged')
Expand Down

0 comments on commit 074cd71

Please sign in to comment.