DevTools: properly handle target suspension/resuming in NetworkProject

DevTools: properly handle target suspension/resuming in NetworkProject

NetworkProject adds UISourceCode in response to DebuggerModel.scriptParsed
event and removes UISourceCode in response to RuntimeModel.executionContextDestroyed.

In case of target suspension, the runtimeModel is not getting suspended and
does not destroy execution contexts. But DebuggerModel does get suspended and
later gets resumed, which results in duplicate scriptParsed events.

This patch starts listening to globalObjectCleared event in NetworkProject
to clean up scripts on target suspension.

Drive-by: promisify targetManager.suspendAllTargets(), make methods to 
enable/disable debugger private.

R=dgozman
BUG=670180
Review-Url: https://codereview.chromium.org/2832943002
Cr-Commit-Position: refs/heads/master@{#469791}
Committed: https://chromium.googlesource.com/chromium/src/+/50d089bc61f3dd1546a1a6f6267f097db24fb541

[lushnikov, 2017-04-21 00:35:41]

Description was changed from

==========
DevTools: implement RuntimeModel.suspendModel/resumeModel methods

Since NetworkProject relies solely on RuntimeModel.ExecutionContextDestroyed
event to remove UISourceCodes, this patch starts issuing this event
when the RuntimeModel gets suspended.

This patch:
- implements RuntimeModel.suspendModel/resumeModel methods
- adds tests to validate bindings in case of target suspension/resume.

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

to

==========
DevTools: implement RuntimeModel.suspendModel/resumeModel methods

Since NetworkProject relies solely on RuntimeModel.ExecutionContextDestroyed
event to remove UISourceCodes, this patch starts issuing this event
when the RuntimeModel gets suspended.

This patch:
- implements RuntimeModel.suspendModel/resumeModel methods
- adds tests to validate bindings in case of target suspension/resume.
- makes methods DebuggerModel.enableDebugger/disableDebugger private

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

[lushnikov, 2017-04-21 00:39:03]

please, take a look

[lushnikov, 2017-04-21 00:40:08]

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

[commit-bot: I haz the power, 2017-04-21 00:40:25]

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-21 01:49:41]

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

[commit-bot: I haz the power, 2017-04-21 01:49:42]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[lushnikov, 2017-04-21 03:41:20]

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

[commit-bot: I haz the power, 2017-04-21 03:41:42]

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-21 04:44:21]

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

[commit-bot: I haz the power, 2017-04-21 04:44:22]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[lushnikov, 2017-04-21 20:00:31]

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

[commit-bot: I haz the power, 2017-04-21 20:00:58]

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-21 21:17:47]

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

[commit-bot: I haz the power, 2017-04-21 21:17:48]

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

[dgozman, 2017-04-21 22:58:22]

We spent some time discussing this and decided to fix all the issues on higher
level. Let's have a discussion offline.

[lushnikov, 2017-05-04 22:18:43]

Description was changed from

==========
DevTools: implement RuntimeModel.suspendModel/resumeModel methods

Since NetworkProject relies solely on RuntimeModel.ExecutionContextDestroyed
event to remove UISourceCodes, this patch starts issuing this event
when the RuntimeModel gets suspended.

This patch:
- implements RuntimeModel.suspendModel/resumeModel methods
- adds tests to validate bindings in case of target suspension/resume.
- makes methods DebuggerModel.enableDebugger/disableDebugger private

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

to

==========
DevTools: properly handle target suspension/resuming in NetworkProject

NetworkProject adds UISourceCode in response to DebuggerModel.scriptParsed
event and removes UISourceCode in response to
RuntimeModel.executionContextDestroyed.

In case of target suspension, the runtimeModel is not getting suspended and
does not destroy execution contexts. But DebuggerModel does get suspended and
later gets resumed, which results in duplicate scriptParsed events.

This patch starts listening to globalObjectCleared event in NetworkProject
to clean up scripts on target suspension.

Drive-by: promisify targetManager.suspendAllTargets(), make methods to 
enable/disable debugger private.

R=dgozman
==========

[lushnikov, 2017-05-04 22:19:00]

Description was changed from

==========
DevTools: properly handle target suspension/resuming in NetworkProject

NetworkProject adds UISourceCode in response to DebuggerModel.scriptParsed
event and removes UISourceCode in response to
RuntimeModel.executionContextDestroyed.

In case of target suspension, the runtimeModel is not getting suspended and
does not destroy execution contexts. But DebuggerModel does get suspended and
later gets resumed, which results in duplicate scriptParsed events.

This patch starts listening to globalObjectCleared event in NetworkProject
to clean up scripts on target suspension.

Drive-by: promisify targetManager.suspendAllTargets(), make methods to 
enable/disable debugger private.

R=dgozman
==========

to

==========
DevTools: properly handle target suspension/resuming in NetworkProject

NetworkProject adds UISourceCode in response to DebuggerModel.scriptParsed
event and removes UISourceCode in response to
RuntimeModel.executionContextDestroyed.

In case of target suspension, the runtimeModel is not getting suspended and
does not destroy execution contexts. But DebuggerModel does get suspended and
later gets resumed, which results in duplicate scriptParsed events.

This patch starts listening to globalObjectCleared event in NetworkProject
to clean up scripts on target suspension.

Drive-by: promisify targetManager.suspendAllTargets(), make methods to 
enable/disable debugger private.

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

[lushnikov, 2017-05-04 22:21:44]

please, take a look.

Implemented a new approach which listens to DebuggerModel.globalObjectCleared
event, as discussed offline.

[lushnikov, 2017-05-04 22:21:49]

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

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

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-05-04 23:59:33]

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

[commit-bot: I haz the power, 2017-05-04 23:59:34]

Dry run: This issue passed the CQ dry run.

[dgozman, 2017-05-05 06:10:55]

lgtm

https://codereview.chromium.org/2832943002/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/inspector/bindings/resources/sourcemap-script.js
(right):

https://codereview.chromium.org/2832943002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/inspector/bindings/resources/sourcemap-script.js:7:
//# sourceMappingURL=sourcemap-script.js.map
Don't you have to change the map as well?

https://codereview.chromium.org/2832943002/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/inspector/profiler/agents-disabled-check.html
(right):

https://codereview.chromium.org/2832943002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/inspector/profiler/agents-disabled-check.html:25:
if (message.startsWith('backend'))
Why this?

https://codereview.chromium.org/2832943002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/sdk/TargetManager.js (right):

https://codereview.chromium.org/2832943002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/TargetManager.js:36: throw new
Error('Already suspended');
Not sure this won't ever happen. Let's just return Promise.resolve.

[lushnikov, 2017-05-05 20:58:27]

https://codereview.chromium.org/2832943002/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/inspector/bindings/resources/sourcemap-script.js
(right):

https://codereview.chromium.org/2832943002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/inspector/bindings/resources/sourcemap-script.js:7:
//# sourceMappingURL=sourcemap-script.js.map
On 2017/05/05 06:10:54, dgozman wrote:
> Don't you have to change the map as well?

The sourcemap has a single entry which maps the beginning of the javascript file
to the beginning of the typescript file. 

I, however, accidentally dropped the changes to the ts file :) Brought them
back.

https://codereview.chromium.org/2832943002/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/inspector/profiler/agents-disabled-check.html
(right):

https://codereview.chromium.org/2832943002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/inspector/profiler/agents-disabled-check.html:25:
if (message.startsWith('backend'))
On 2017/05/05 06:10:54, dgozman wrote:
> Why this?

This is needed since i await for suspension and resume now. As a result, the
backend messages are reported as well.

https://codereview.chromium.org/2832943002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/sdk/TargetManager.js (right):

https://codereview.chromium.org/2832943002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/TargetManager.js:36: throw new
Error('Already suspended');
On 2017/05/05 06:10:55, dgozman wrote:
> Not sure this won't ever happen. Let's just return Promise.resolve.

Changed both here and in resumeAllTargets() to make them symmetrical. Done.

[lushnikov, 2017-05-05 21:24:21]

The CQ bit was checked by lushnikov@chromium.org

[lushnikov, 2017-05-05 21:24:21]

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/2832943002/#ps100001
(title: "address comments")

[commit-bot: I haz the power, 2017-05-05 21:25:32]

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-05-05 22:57:26]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1494019461293980,
"parent_rev": "129513138af401c216d3401159260cbc06577fb3", "commit_rev":
"50d089bc61f3dd1546a1a6f6267f097db24fb541"}

[commit-bot: I haz the power, 2017-05-05 22:58:01]

Description was changed from

==========
DevTools: properly handle target suspension/resuming in NetworkProject

NetworkProject adds UISourceCode in response to DebuggerModel.scriptParsed
event and removes UISourceCode in response to
RuntimeModel.executionContextDestroyed.

In case of target suspension, the runtimeModel is not getting suspended and
does not destroy execution contexts. But DebuggerModel does get suspended and
later gets resumed, which results in duplicate scriptParsed events.

This patch starts listening to globalObjectCleared event in NetworkProject
to clean up scripts on target suspension.

Drive-by: promisify targetManager.suspendAllTargets(), make methods to 
enable/disable debugger private.

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

to

==========
DevTools: properly handle target suspension/resuming in NetworkProject

NetworkProject adds UISourceCode in response to DebuggerModel.scriptParsed
event and removes UISourceCode in response to
RuntimeModel.executionContextDestroyed.

In case of target suspension, the runtimeModel is not getting suspended and
does not destroy execution contexts. But DebuggerModel does get suspended and
later gets resumed, which results in duplicate scriptParsed events.

This patch starts listening to globalObjectCleared event in NetworkProject
to clean up scripts on target suspension.

Drive-by: promisify targetManager.suspendAllTargets(), make methods to 
enable/disable debugger private.

R=dgozman
BUG=670180

Review-Url: https://codereview.chromium.org/2832943002
Cr-Commit-Position: refs/heads/master@{#469791}
Committed:
https://chromium.googlesource.com/chromium/src/+/50d089bc61f3dd1546a1a6f6267f...
==========

[commit-bot: I haz the power, 2017-05-05 22:58:02]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/50d089bc61f3dd1546a1a6f6267f...