DevTools: carefully cleanup script UISourceCodes

DevTools: carefully cleanup script UISourceCodes

This patch starts removing script UISourceCodes when the parent
execution context gets destroyed.

Note: this patch removes only script-related UISourceCodes in
NetworkProject. The "debugger://" UISourceCodes, created by
DefaultScriptMapping, are kept intact. They come handy to reveal
location in old stacktrace/messages.

With this patch, all UISourceCodes created by networkProject have
a lifetime, defined by their parent model:
- DebuggerModel defines life-time for Script-related UISourceCodes
- CSSModel defines life time for CSS-related UISourceCodes
- SASSSourceMapping/CompilerScriptMapping define life-time of SourceMap
  Sources
- ResourceModel defines life-time for document/font/images
  UISourceCodes

For this reason, the NavigatorView no longer needs to listen to
FrameDetached/FrameNavigated events.

Drive-By: move DebuggerModel.scripts to the native Map.

R=dgozman
BUG=670180
Review-Url: https://codereview.chromium.org/2795193003
Cr-Commit-Position: refs/heads/master@{#462645}
Committed: https://chromium.googlesource.com/chromium/src/+/96fe1d78f2701bb68c721282a5831026243e3a3d

[lushnikov, 2017-04-04 20:48:49]

this is a re-implementation of https://codereview.chromium.org/2739983002/ which
you've already liked.

Please, take a look!

[lushnikov, 2017-04-04 20:48:56]

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

[commit-bot: I haz the power, 2017-04-04 20:50:01]

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-04-04 22:41:45]

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

[commit-bot: I haz the power, 2017-04-04 22:41:46]

Dry run: This issue passed the CQ dry run.

[dgozman, 2017-04-05 18:22:56]

I don't like that DebuggerModel keeps the scripts, but notifies clients about
them being "removed". Let's figure out a better solution.

https://codereview.chromium.org/2795193003/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/http/tests/inspector/bindings/dynamic-bindings-frame-attach-detach-expected.txt
(right):

https://codereview.chromium.org/2795193003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/inspector/bindings/dynamic-bindings-frame-attach-detach-expected.txt:31:
[+] _test_attachFrame.js
What about this one?

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

https://codereview.chromium.org/2795193003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/DebuggerModel.js:53:
this._scripts = new Map();
Is it as performant? We have a lot of scripts!

https://codereview.chromium.org/2795193003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/DebuggerModel.js:358: return
Array.from(this._scripts.values());
Is this method performance-critical? Does it make sense to return an iterator or
map itself?

https://codereview.chromium.org/2795193003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/DebuggerModel.js:928:
ExecutionContextDestroyed: Symbol('ExecutionContextDestroyed'),
DiscardedScriptSources, and merge with DiscardedAnonymousScriptSource

[lushnikov, 2017-04-06 03:32:35]

please, take another look

https://codereview.chromium.org/2795193003/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/http/tests/inspector/bindings/dynamic-bindings-frame-attach-detach-expected.txt
(right):

https://codereview.chromium.org/2795193003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/inspector/bindings/dynamic-bindings-frame-attach-detach-expected.txt:31:
[+] _test_attachFrame.js
On 2017/04/05 18:22:55, dgozman wrote:
> What about this one?

This one is executed in the main frame

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

https://codereview.chromium.org/2795193003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/DebuggerModel.js:53:
this._scripts = new Map();
On 2017/04/05 18:22:55, dgozman wrote:
> Is it as performant? We have a lot of scripts!

According to 
https://docs.google.com/spreadsheets/d/1lQTC4ZG3XY33Gdw7Ibla30fz3HQHqWZ87DQjH...

Map is more performant for storing rather then for querying. And this is the
case we have!

https://codereview.chromium.org/2795193003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/DebuggerModel.js:358: return
Array.from(this._scripts.values());
On 2017/04/05 18:22:55, dgozman wrote:
> Is this method performance-critical? Does it make sense to return an iterator
or
> map itself?

No, it's used by blackbox model only

https://codereview.chromium.org/2795193003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/DebuggerModel.js:928:
ExecutionContextDestroyed: Symbol('ExecutionContextDestroyed'),
On 2017/04/05 18:22:55, dgozman wrote:
> DiscardedScriptSources, and merge with DiscardedAnonymousScriptSource

As discussed offline, i'm started to listen to
RuntimeModel.ExecutionContextDestroyed in NetworkPanel

[lushnikov, 2017-04-06 19:07:01]

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

[commit-bot: I haz the power, 2017-04-06 19:08:02]

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

[dgozman, 2017-04-06 19:36:58]

lgtm

[lushnikov, 2017-04-06 20:22:04]

The CQ bit was unchecked by lushnikov@chromium.org

[lushnikov, 2017-04-06 20:22:04]

The CQ bit was checked by lushnikov@chromium.org

[commit-bot: I haz the power, 2017-04-06 20:23:20]

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-06 22:16:24]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1491510124543410,
"parent_rev": "d3920ccc81711676a41982bcae3a29b4142c6838", "commit_rev":
"96fe1d78f2701bb68c721282a5831026243e3a3d"}

[commit-bot: I haz the power, 2017-04-06 22:17:15]

Description was changed from

==========
DevTools: carefully cleanup script UISourceCodes

This patch starts removing script UISourceCodes when the parent
execution context gets destroyed.

Note: this patch removes only script-related UISourceCodes in
NetworkProject. The "debugger://" UISourceCodes, created by
DefaultScriptMapping, are kept intact. They come handy to reveal
location in old stacktrace/messages.

With this patch, all UISourceCodes created by networkProject have
a lifetime, defined by their parent model:
- DebuggerModel defines life-time for Script-related UISourceCodes
- CSSModel defines life time for CSS-related UISourceCodes
- SASSSourceMapping/CompilerScriptMapping define life-time of SourceMap
  Sources
- ResourceModel defines life-time for document/font/images
  UISourceCodes

For this reason, the NavigatorView no longer needs to listen to
FrameDetached/FrameNavigated events.

Drive-By: move DebuggerModel.scripts to the native Map.

R=dgozman
BUG=670180
==========

to

==========
DevTools: carefully cleanup script UISourceCodes

This patch starts removing script UISourceCodes when the parent
execution context gets destroyed.

Note: this patch removes only script-related UISourceCodes in
NetworkProject. The "debugger://" UISourceCodes, created by
DefaultScriptMapping, are kept intact. They come handy to reveal
location in old stacktrace/messages.

With this patch, all UISourceCodes created by networkProject have
a lifetime, defined by their parent model:
- DebuggerModel defines life-time for Script-related UISourceCodes
- CSSModel defines life time for CSS-related UISourceCodes
- SASSSourceMapping/CompilerScriptMapping define life-time of SourceMap
  Sources
- ResourceModel defines life-time for document/font/images
  UISourceCodes

For this reason, the NavigatorView no longer needs to listen to
FrameDetached/FrameNavigated events.

Drive-By: move DebuggerModel.scripts to the native Map.

R=dgozman
BUG=670180

Review-Url: https://codereview.chromium.org/2795193003
Cr-Commit-Position: refs/heads/master@{#462645}
Committed:
https://chromium.googlesource.com/chromium/src/+/96fe1d78f2701bb68c721282a583...
==========

[commit-bot: I haz the power, 2017-04-06 22:17:16]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/96fe1d78f2701bb68c721282a583...