DevTools: proper management of ResourceTreeModel in NetworkProject

DevTools: proper management of ResourceTreeModel in NetworkProject

This patch cleans up NetworkProject handling of ResourceTreeModel.

This patch:
- stops NetworkProject from listening to MainFrameNavigated event.
  Listening to frameDetached and frameWillNavigate is enough
- cleans up only those UISourceCodes which were created by
  resourceTreeModel (via _acceptsResource method).
- stops listening to suspendedStateChanged method. This is not needed
  anymore because cssmodel / debuggermodel / sourcemaps are properly
  added and removed in case of target suspension/resume.

This patch also fixes a bug: calling method removeUISourceCode on
ContentProviderBasedProject is incorrect. 

Drive-By: un-unrestrict ResourceTreeModel.

R=dgozman
BUG=670180
Review-Url: https://codereview.chromium.org/2833583003
Cr-Commit-Position: refs/heads/master@{#469815}
Committed: https://chromium.googlesource.com/chromium/src/+/07cf0b557a5fdf69dd0cd0bf95909a9d4c762e25

[lushnikov, 2017-04-20 02:50:02]

please, take a look

[lushnikov, 2017-04-20 02:50:09]

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

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

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

[lushnikov, 2017-04-20 02:52:43]

Description was changed from

==========
DevTools: proper management of ResourceTreeModel in NetworkProject

This patch cleans up NetworkProject handling of ResourceTreeModel.

This patch:
- stops NetworkProject from listening to MainFrameNavigated event.
  Listening to frameDetached and frameWillNavigate is enough
- cleans up only those UISourceCodes which were created by
  resourceTreeModel (via _acceptsResource method).
- stops listening to suspendedStateChanged method. This is not needed
  anymore because cssmodel / debuggermodel / sourcemaps are properly
  added and removed in case of target suspension/resume.

Drive-By: un-unrestrict ResourceTreeModel.

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

to

==========
DevTools: proper management of ResourceTreeModel in NetworkProject

This patch cleans up NetworkProject handling of ResourceTreeModel.

This patch:
- stops NetworkProject from listening to MainFrameNavigated event.
  Listening to frameDetached and frameWillNavigate is enough
- cleans up only those UISourceCodes which were created by
  resourceTreeModel (via _acceptsResource method).
- stops listening to suspendedStateChanged method. This is not needed
  anymore because cssmodel / debuggermodel / sourcemaps are properly
  added and removed in case of target suspension/resume.

This patch also fixes a bug: calling method removeUISourceCode on
ContentProviderBasedProject is incorrect. 

Drive-By: un-unrestrict ResourceTreeModel.

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

[lushnikov, 2017-04-20 02:52:50]

https://codereview.chromium.org/2833583003/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/bindings/NetworkProject.js
(left):

https://codereview.chromium.org/2833583003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/bindings/NetworkProject.js:383:
project.removeUISourceCode(resource.url);
this was a bug: it's incorrect to call project.removeUISourceCode for
ContentProviderBasedProjects. Proper way is calling removeFile.

[commit-bot: I haz the power, 2017-04-20 04:13:17]

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

[commit-bot: I haz the power, 2017-04-20 04:13:17]

Dry run: This issue passed the CQ dry run.

[dgozman, 2017-04-20 17:23:17]

https://codereview.chromium.org/2833583003/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/bindings/NetworkProject.js
(left):

https://codereview.chromium.org/2833583003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/bindings/NetworkProject.js:366: //
Never load document twice.
What was this about? Why it doesn't happen anymore?

https://codereview.chromium.org/2833583003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/bindings/NetworkProject.js:412:
_suspendStateChanged() {
Let's add a test for this.

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

https://codereview.chromium.org/2833583003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/ResourceTreeModel.js:104:
this._frames = new Map();
You can just remove this - it's only called from constructor. I'd just inline
the method.

[lushnikov, 2017-04-21 00:51:27]

https://codereview.chromium.org/2833583003/diff/1/third_party/WebKit/Source/d...
File third_party/WebKit/Source/devtools/front_end/bindings/NetworkProject.js
(left):

https://codereview.chromium.org/2833583003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/bindings/NetworkProject.js:366: //
Never load document twice.
On 2017/04/20 17:23:17, dgozman wrote:
> What was this about? Why it doesn't happen anymore?

This was introduced in https://codereview.chromium.org/1409223006
That was before the project-per-frame approach and made sense, not anymore since
we have per-frame distribution (and will have frame attribution later)

https://codereview.chromium.org/2833583003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/bindings/NetworkProject.js:412:
_suspendStateChanged() {
On 2017/04/20 17:23:17, dgozman wrote:
> Let's add a test for this.

Done in a separate patch.

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

https://codereview.chromium.org/2833583003/diff/1/third_party/WebKit/Source/d...
third_party/WebKit/Source/devtools/front_end/sdk/ResourceTreeModel.js:104:
this._frames = new Map();
On 2017/04/20 17:23:17, dgozman wrote:
> You can just remove this - it's only called from constructor. I'd just inline
> the method.

it is also called from test - i preferred to keep it as is.

https://codereview.chromium.org/2833583003/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/inspector/bindings/suspendtarget-bindings-expected.txt
(right):

https://codereview.chromium.org/2833583003/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/inspector/bindings/suspendtarget-bindings-expected.txt:66:
http://127.0.0.1:8000/inspector/bindings/resources/sourcemap-frame.html
since resourcetreemodel is not suspended, associated UISourceCodes don't go
away. I'm don't think there's a need to remove these.

[lushnikov, 2017-04-21 00:51:38]

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:51:53]

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:53:17]

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

[commit-bot: I haz the power, 2017-04-21 01:53:18]

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

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:58:00]

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-05 02:42:23]

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

[commit-bot: I haz the power, 2017-05-05 02:42:24]

Dry run: This issue passed the CQ dry run.

[dgozman, 2017-05-05 21:20:29]

lgtm

https://codereview.chromium.org/2833583003/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/inspector/bindings/suspendtarget-navigator-expected.txt
(right):

https://codereview.chromium.org/2833583003/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/inspector/bindings/suspendtarget-navigator-expected.txt:52:
frame2 (sourcemap-frame.html)
Let's detach one frame during suspend.

https://codereview.chromium.org/2833583003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/bindings/NetworkProject.js
(right):

https://codereview.chromium.org/2833583003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/bindings/NetworkProject.js:231:
_populate() {
Not used anymore.

https://codereview.chromium.org/2833583003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/sdk/ResourceTreeModel.js
(right):

https://codereview.chromium.org/2833583003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/ResourceTreeModel.js:56:
this.mainFrame = null;
/** @type {?...} */

https://codereview.chromium.org/2833583003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/ResourceTreeModel.js:103:
_fetchResourceTree() {
Let's inline this.

[lushnikov, 2017-05-05 21:54:35]

All done, thanks!

https://codereview.chromium.org/2833583003/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/inspector/bindings/suspendtarget-navigator-expected.txt
(right):

https://codereview.chromium.org/2833583003/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/inspector/bindings/suspendtarget-navigator-expected.txt:52:
frame2 (sourcemap-frame.html)
On 2017/05/05 21:20:28, dgozman wrote:
> Let's detach one frame during suspend.

Done.

https://codereview.chromium.org/2833583003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/bindings/NetworkProject.js
(right):

https://codereview.chromium.org/2833583003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/bindings/NetworkProject.js:231:
_populate() {
On 2017/05/05 21:20:29, dgozman wrote:
> Not used anymore.

Done.

https://codereview.chromium.org/2833583003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/sdk/ResourceTreeModel.js
(right):

https://codereview.chromium.org/2833583003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/ResourceTreeModel.js:56:
this.mainFrame = null;
On 2017/05/05 21:20:29, dgozman wrote:
> /** @type {?...} */

Done.

https://codereview.chromium.org/2833583003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/ResourceTreeModel.js:103:
_fetchResourceTree() {
On 2017/05/05 21:20:29, dgozman wrote:
> Let's inline this.

Done.

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

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

[commit-bot: I haz the power, 2017-05-05 21:55:44]

Dry run: CQ is trying da patch.

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

[lushnikov, 2017-05-05 23:09:07]

The CQ bit was unchecked by lushnikov@chromium.org

[lushnikov, 2017-05-05 23:09:07]

The CQ bit was checked by lushnikov@chromium.org

[lushnikov, 2017-05-05 23:09:08]

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

[commit-bot: I haz the power, 2017-05-05 23:10: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-05-05 23:55:35]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1494025747944460,
"parent_rev": "53d3ccbec20a48789f3328737e2034a7798cb7c7", "commit_rev":
"07cf0b557a5fdf69dd0cd0bf95909a9d4c762e25"}

[commit-bot: I haz the power, 2017-05-05 23:56:20]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1494025747944460,
"parent_rev": "53d3ccbec20a48789f3328737e2034a7798cb7c7", "commit_rev":
"07cf0b557a5fdf69dd0cd0bf95909a9d4c762e25"}

[commit-bot: I haz the power, 2017-05-05 23:56:23]

Description was changed from

==========
DevTools: proper management of ResourceTreeModel in NetworkProject

This patch cleans up NetworkProject handling of ResourceTreeModel.

This patch:
- stops NetworkProject from listening to MainFrameNavigated event.
  Listening to frameDetached and frameWillNavigate is enough
- cleans up only those UISourceCodes which were created by
  resourceTreeModel (via _acceptsResource method).
- stops listening to suspendedStateChanged method. This is not needed
  anymore because cssmodel / debuggermodel / sourcemaps are properly
  added and removed in case of target suspension/resume.

This patch also fixes a bug: calling method removeUISourceCode on
ContentProviderBasedProject is incorrect. 

Drive-By: un-unrestrict ResourceTreeModel.

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

to

==========
DevTools: proper management of ResourceTreeModel in NetworkProject

This patch cleans up NetworkProject handling of ResourceTreeModel.

This patch:
- stops NetworkProject from listening to MainFrameNavigated event.
  Listening to frameDetached and frameWillNavigate is enough
- cleans up only those UISourceCodes which were created by
  resourceTreeModel (via _acceptsResource method).
- stops listening to suspendedStateChanged method. This is not needed
  anymore because cssmodel / debuggermodel / sourcemaps are properly
  added and removed in case of target suspension/resume.

This patch also fixes a bug: calling method removeUISourceCode on
ContentProviderBasedProject is incorrect. 

Drive-By: un-unrestrict ResourceTreeModel.

R=dgozman
BUG=670180

Review-Url: https://codereview.chromium.org/2833583003
Cr-Commit-Position: refs/heads/master@{#469815}
Committed:
https://chromium.googlesource.com/chromium/src/+/07cf0b557a5fdf69dd0cd0bf9590...
==========

[commit-bot: I haz the power, 2017-05-05 23:56:24]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/07cf0b557a5fdf69dd0cd0bf9590...