DevTools: carefully cleanup javascript sourcemaps

DevTools: carefully cleanup javascript sourcemaps

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}
Committed: https://chromium.googlesource.com/chromium/src/+/625244e25254fb6479b3122d92145c596616a196

[lushnikov, 2017-03-30 23:52:16]

please, take a look

[lushnikov, 2017-03-30 23:52:54]

Description was changed from

==========
DevTools: carefully cleanup javascript sourcemaps

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 X script.sourceURL X 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
R=dgozman
==========

to

==========
DevTools: carefully cleanup javascript sourcemaps

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
R=dgozman
==========

[lushnikov, 2017-03-30 23:53:04]

The CQ bit was checked by lushnikov@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-30 23:54:07]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-31 02:32:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-31 02:32:37]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[lushnikov, 2017-03-31 02:40:21]

The CQ bit was checked by lushnikov@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-31 02:40:49]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[lushnikov, 2017-03-31 02:53:01]

Description was changed from

==========
DevTools: carefully cleanup javascript sourcemaps

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
R=dgozman
==========

to

==========
DevTools: carefully cleanup javascript sourcemaps

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
==========

[commit-bot: I haz the power, 2017-03-31 03:34:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-31 03:34:40]

Dry run: This issue passed the CQ dry run.

[dgozman, 2017-03-31 21:57:41]

Let's add a test for jsSourceMapsEnabled.

lgtm

https://codereview.chromium.org/2784413003/diff/1/third_party/WebKit/Source/d...
File
third_party/WebKit/Source/devtools/front_end/bindings/CompilerScriptMapping.js
(right):

https://codereview.chromium.org/2784413003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/bindings/CompilerScriptMapping.js:114:
var sourceMap = this._sourceMapManager.sourceMapForClient(script);
script ? ... : null;

https://codereview.chromium.org/2784413003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/bindings/CompilerScriptMapping.js:233:
if (!sourceMap)
|| this._scriptSources.has(script)

https://codereview.chromium.org/2784413003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/bindings/CompilerScriptMapping.js:281:
var sourceMap = this._sourceMapManager.sourceMapForClient(script);
script ? ... : null;

https://codereview.chromium.org/2784413003/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/sdk/DebuggerModel.js (right):

https://codereview.chromium.org/2784413003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/DebuggerModel.js:352:
this._sourceMapManager.detachSourceMap(scriptWithSourceMap);
Let's do this before clearing scripts.

https://codereview.chromium.org/2784413003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/DebuggerModel.js:525: var
previousSourceMapClient = this._sourceMapIdToScript.get(sourceMapId);
Let's add a comment.

https://codereview.chromium.org/2784413003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/DebuggerModel.js:526: if
(previousSourceMapClient)
previousScript

https://codereview.chromium.org/2784413003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/DebuggerModel.js:552: if
(!sourceMapId)
Figure out null vs undefined vs empty string.

https://codereview.chromium.org/2784413003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/DebuggerModel.js:565:
this._sourceMapManager.detachSourceMap(script);
Remove from this._sourceMapIdToScript as well.

https://codereview.chromium.org/2784413003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/DebuggerModel.js:848:
this._runtimeModel.removeEventListener(
Let's turn this into direct call 'executionContextDestroyed()'.

https://codereview.chromium.org/2784413003/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/sdk/SourceMapManager.js
(right):

https://codereview.chromium.org/2784413003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/SourceMapManager.js:135:
SDK.SourceMapManager.Events.SourceMapWillAttach, {client: client, sourceMapURL:
sourceMapURL});
drop sourceMapURL

https://codereview.chromium.org/2784413003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/SourceMapManager.js:180:
SDK.SourceMapManager.Events.SourceMapFailedToAttach, {client: client,
sourceMapURL: sourceMapURL});
drop sourceMapURL

https://codereview.chromium.org/2784413003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/SourceMapManager.js:232:
SDK.SourceMapManager.Events.SourceMapFailedToAttach, {client: client,
sourceMapURL: sourceMapURL});
drop sourceMapURL

https://codereview.chromium.org/2784413003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/SourceMapManager.js:254:
SourceMapWillAttach: Symbol('SourceMapWillAttach'),
SourceMapWillBeAttached

[lushnikov, 2017-03-31 23:49:08]

Patchset #4 (id:60001) has been deleted

[lushnikov, 2017-03-31 23:49:16]

Patchset #2 (id:20001) has been deleted

[lushnikov, 2017-03-31 23:49:22]

Patchset #2 (id:40001) has been deleted

[lushnikov, 2017-03-31 23:53:10]

https://codereview.chromium.org/2784413003/diff/1/third_party/WebKit/Source/d...
File
third_party/WebKit/Source/devtools/front_end/bindings/CompilerScriptMapping.js
(right):

https://codereview.chromium.org/2784413003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/bindings/CompilerScriptMapping.js:114:
var sourceMap = this._sourceMapManager.sourceMapForClient(script);
On 2017/03/31 21:57:40, dgozman wrote:
> script ? ... : null;

Done.

https://codereview.chromium.org/2784413003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/bindings/CompilerScriptMapping.js:233:
if (!sourceMap)
On 2017/03/31 21:57:40, dgozman wrote:
> || this._scriptSources.has(script)

Done.

https://codereview.chromium.org/2784413003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/bindings/CompilerScriptMapping.js:281:
var sourceMap = this._sourceMapManager.sourceMapForClient(script);
On 2017/03/31 21:57:40, dgozman wrote:
> script ? ... : null;

Done.

https://codereview.chromium.org/2784413003/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/sdk/DebuggerModel.js (right):

https://codereview.chromium.org/2784413003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/DebuggerModel.js:352:
this._sourceMapManager.detachSourceMap(scriptWithSourceMap);
On 2017/03/31 21:57:41, dgozman wrote:
> Let's do this before clearing scripts.

Done.

https://codereview.chromium.org/2784413003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/DebuggerModel.js:525: var
previousSourceMapClient = this._sourceMapIdToScript.get(sourceMapId);
On 2017/03/31 21:57:41, dgozman wrote:
> Let's add a comment.

Done.

https://codereview.chromium.org/2784413003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/DebuggerModel.js:526: if
(previousSourceMapClient)
On 2017/03/31 21:57:40, dgozman wrote:
> previousScript

Done.

https://codereview.chromium.org/2784413003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/DebuggerModel.js:552: if
(!sourceMapId)
On 2017/03/31 21:57:41, dgozman wrote:
> Figure out null vs undefined vs empty string.

Not loading sourceMaps for all falsy values of souceMapId works fine here.

https://codereview.chromium.org/2784413003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/DebuggerModel.js:565:
this._sourceMapManager.detachSourceMap(script);
On 2017/03/31 21:57:41, dgozman wrote:
> Remove from this._sourceMapIdToScript as well.

Done.

https://codereview.chromium.org/2784413003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/DebuggerModel.js:848:
this._runtimeModel.removeEventListener(
On 2017/03/31 21:57:40, dgozman wrote:
> Let's turn this into direct call 'executionContextDestroyed()'.

Done.

https://codereview.chromium.org/2784413003/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/sdk/SourceMapManager.js
(right):

https://codereview.chromium.org/2784413003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/SourceMapManager.js:135:
SDK.SourceMapManager.Events.SourceMapWillAttach, {client: client, sourceMapURL:
sourceMapURL});
On 2017/03/31 21:57:41, dgozman wrote:
> drop sourceMapURL

Done.

https://codereview.chromium.org/2784413003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/SourceMapManager.js:180:
SDK.SourceMapManager.Events.SourceMapFailedToAttach, {client: client,
sourceMapURL: sourceMapURL});
On 2017/03/31 21:57:41, dgozman wrote:
> drop sourceMapURL

Done.

https://codereview.chromium.org/2784413003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/SourceMapManager.js:232:
SDK.SourceMapManager.Events.SourceMapFailedToAttach, {client: client,
sourceMapURL: sourceMapURL});
On 2017/03/31 21:57:41, dgozman wrote:
> drop sourceMapURL

Done.

https://codereview.chromium.org/2784413003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/SourceMapManager.js:254:
SourceMapWillAttach: Symbol('SourceMapWillAttach'),
On 2017/03/31 21:57:41, dgozman wrote:
> SourceMapWillBeAttached

Decided to keep this as-is: renamed event looked like an outlier among others.

[lushnikov, 2017-03-31 23:53:17]

The CQ bit was checked by lushnikov@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-31 23:53:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[lushnikov, 2017-03-31 23:54:08]

The CQ bit was unchecked by lushnikov@chromium.org

[lushnikov, 2017-03-31 23:54:08]

The CQ bit was checked by lushnikov@chromium.org

[lushnikov, 2017-03-31 23:54:09]

The patchset sent to the CQ was uploaded after l-g-t-m from dgozman@chromium.org
Link to the patchset: https://codereview.chromium.org/2784413003/#ps80001
(title: "address comments")

[commit-bot: I haz the power, 2017-03-31 23:54:35]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-01 01:37:59]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1491004448876610,
"parent_rev": "f0433e75d2158112feb6be25d52c60d0c7f80e77", "commit_rev":
"625244e25254fb6479b3122d92145c596616a196"}

[commit-bot: I haz the power, 2017-04-01 01:38:44]

Description was changed from

==========
DevTools: carefully cleanup javascript sourcemaps

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
==========

to

==========
DevTools: carefully cleanup javascript sourcemaps

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}
Committed:
https://chromium.googlesource.com/chromium/src/+/625244e25254fb6479b3122d9214...
==========

[commit-bot: I haz the power, 2017-04-01 01:38:46]

Committed patchset #2 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/625244e25254fb6479b3122d9214...