DevTools: clean up tests to not depend on NetworkProject.addFile method

DevTools: clean up tests to not depend on NetworkProject.addFile method

The NetworkProject.addFile method is adding a UISourceCode
for SourceMap source. However, multiple tests abuse the method for their
needs. This is unfortunate, since we plan to re-work bindings and
eliminate the public "NetworkProject.addFile" method altogether.

This patch migrates tests to other methods of adding UISourceCodes:
- a InspectorTest.addScriptUISourceCode method is introduced which
  adds scripts/contentScripts with proper urls in the main target.
- a InspectorTest.PageMock is introduced to add scripts in a multi-target
  tests

This implementation of pagemock supports only two methods:
- addScript, which adds a script to the mock page and reports it via
  Debugger.scriptParsed protocol event.
- reload(), which "reloads" page, killing all the added scripts

Since PageMock's speaks over a very stable devtools protocol,
little effort will be needed to maintain it. This is unlike the
current solution where tests abuse different public/private binding
APIs.

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

[lushnikov, 2017-03-15 05:53:36]

please, take a look

[lushnikov, 2017-03-15 05:53:49]

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

[commit-bot: I haz the power, 2017-03-15 05:54:03]

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-15 06:57:17]

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

[commit-bot: I haz the power, 2017-03-15 06:57:18]

Dry run: This issue passed the CQ dry run.

[lushnikov, 2017-03-15 23:38:31]

Description was changed from

==========
DevTools: introduce page-mock.js to support multi-target tests

This patch introduces InspectorTest.PageMock class - mock of a
web page which devtools can connect to.

This implementation of pagemock supports only one method - addScript,
which adds a script to the page and reports it via Debugger.scriptParsed
protocol event.

Since PageMock's speaks over a very stable devtools protocol,
little effort will be needed to maintain it. This is unlike the
current solution where tests abuse different public/private binding
APIs.

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

to

==========
DevTools: clean up tests to not depend on NetworkProject.addFile method

The NetworkProject.addFile method is adding a UISourceCode
for SourceMap source. However, multiple tests abuse the method for their
needs.

This patch migrates tests to other methods of adding UISourceCodes:
- a InspectorTest.addScriptUISourceCode method is introduced which
  adds scripts/contentScripts with proper urls in the main target.
- a InspectorTest.PageMock is introduced to add scripts in a multi-target
  tests

This implementation of pagemock supports only two methods:
- addScript, which adds a script to the mock page and reports it via
  Debugger.scriptParsed protocol event.
- reload(), which "reloads" page, killing all the added scripts

Since PageMock's speaks over a very stable devtools protocol,
little effort will be needed to maintain it. This is unlike the
current solution where tests abuse different public/private binding
APIs.

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

[lushnikov, 2017-03-15 23:38:36]

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

[commit-bot: I haz the power, 2017-03-15 23:39:16]

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-15 23:39:43]

Description was changed from

==========
DevTools: clean up tests to not depend on NetworkProject.addFile method

The NetworkProject.addFile method is adding a UISourceCode
for SourceMap source. However, multiple tests abuse the method for their
needs.

This patch migrates tests to other methods of adding UISourceCodes:
- a InspectorTest.addScriptUISourceCode method is introduced which
  adds scripts/contentScripts with proper urls in the main target.
- a InspectorTest.PageMock is introduced to add scripts in a multi-target
  tests

This implementation of pagemock supports only two methods:
- addScript, which adds a script to the mock page and reports it via
  Debugger.scriptParsed protocol event.
- reload(), which "reloads" page, killing all the added scripts

Since PageMock's speaks over a very stable devtools protocol,
little effort will be needed to maintain it. This is unlike the
current solution where tests abuse different public/private binding
APIs.

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

to

==========
DevTools: clean up tests to not depend on NetworkProject.addFile method

The NetworkProject.addFile method is adding a UISourceCode
for SourceMap source. However, multiple tests abuse the method for their
needs. This is unfortunate, since we plan to re-work bindings and
eliminate the public "NetworkProject.addFile" method altogether.

This patch migrates tests to other methods of adding UISourceCodes:
- a InspectorTest.addScriptUISourceCode method is introduced which
  adds scripts/contentScripts with proper urls in the main target.
- a InspectorTest.PageMock is introduced to add scripts in a multi-target
  tests

This implementation of pagemock supports only two methods:
- addScript, which adds a script to the mock page and reports it via
  Debugger.scriptParsed protocol event.
- reload(), which "reloads" page, killing all the added scripts

Since PageMock's speaks over a very stable devtools protocol,
little effort will be needed to maintain it. This is unlike the
current solution where tests abuse different public/private binding
APIs.

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

[dgozman, 2017-03-16 00:17:10]

https://codereview.chromium.org/2747863007/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/http/tests/inspector/inspector-test.js
(right):

https://codereview.chromium.org/2747863007/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/inspector/inspector-test.js:55:
InspectorTest.addScriptUISourceCode = async function(url, content,
isContentScript, worldId) {
I'm worried this whether this forces everyone using async helper to be async as
well? Maybe avoid async yet?

https://codereview.chromium.org/2747863007/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/inspector/inspector-test.js:58:
content = `testRunner.evaluateScriptInIsolatedWorld(${worldId},
\`${content}\`)`;
Nice one!

https://codereview.chromium.org/2747863007/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/inspector/inspector-test.js:59:
InspectorTest.evaluateInPagePromise(content)
nit: missing semicolon

https://codereview.chromium.org/2747863007/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/http/tests/inspector/page-mock.js (right):

https://codereview.chromium.org/2747863007/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/inspector/page-mock.js:69:
addScript(url, content, hasSourceURL, isContentScript) {
Note that content scripts must have non-default execution context.

https://codereview.chromium.org/2747863007/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/inspector/page-mock.js:73: var auxData
= isContentScript ? {isDefault: false} : undefined;
this._executionContext.auxData

https://codereview.chromium.org/2747863007/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/inspector/page-mock.js:79: endLine: 0,
Let's call lineEndings on it and pass true endLine/endColumn?

https://codereview.chromium.org/2747863007/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/inspector/page-mock.js:96:
this._fireEvent("Runtime.executionContextDestroyed", {executionContextId:
this._executionContext.id});
I believe this one is not required.

https://codereview.chromium.org/2747863007/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/inspector/page-mock.js:132: resources:
[]
Should we list all scripts here?

https://codereview.chromium.org/2747863007/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/inspector/sources/debugger-ui/scripts-panel-expected.txt
(right):

https://codereview.chromium.org/2747863007/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/inspector/sources/debugger-ui/scripts-panel-expected.txt:6:
mock-page
We don't exercise the main target logic anymore, while it's very important in
real world. Can we fix that?

https://codereview.chromium.org/2747863007/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/inspector/sources/debugger-ui/scripts-panel.html
(right):

https://codereview.chromium.org/2747863007/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/inspector/sources/debugger-ui/scripts-panel.html:28:
Bindings.NetworkProject.forTarget(InspectorTest.mainTarget)._reset();
Eh? Unfortunate!

https://codereview.chromium.org/2747863007/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/inspector/sources/debugger-ui/switch-file.html
(right):

https://codereview.chromium.org/2747863007/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/inspector/sources/debugger-ui/switch-file.html:37:
uiSourceCodes.push(await InspectorTest.addScriptUISourceCode(files[i], ''));
I can feel how slow (and thus flaky) this is :-(

[commit-bot: I haz the power, 2017-03-16 01:12:22]

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

[commit-bot: I haz the power, 2017-03-16 01:12:23]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[lushnikov, 2017-03-16 06:30:50]

ptal!

https://codereview.chromium.org/2747863007/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/http/tests/inspector/inspector-test.js
(right):

https://codereview.chromium.org/2747863007/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/inspector/inspector-test.js:55:
InspectorTest.addScriptUISourceCode = async function(url, content,
isContentScript, worldId) {
On 2017/03/16 00:17:09, dgozman wrote:
> I'm worried this whether this forces everyone using async helper to be async
as
> well? Maybe avoid async yet?

Done. The async is actually not needed here.

https://codereview.chromium.org/2747863007/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/inspector/inspector-test.js:58:
content = `testRunner.evaluateScriptInIsolatedWorld(${worldId},
\`${content}\`)`;
On 2017/03/16 00:17:09, dgozman wrote:
> Nice one!

Thanks!

https://codereview.chromium.org/2747863007/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/inspector/inspector-test.js:59:
InspectorTest.evaluateInPagePromise(content)
On 2017/03/16 00:17:09, dgozman wrote:
> nit: missing semicolon

Done.

https://codereview.chromium.org/2747863007/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/http/tests/inspector/page-mock.js (right):

https://codereview.chromium.org/2747863007/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/inspector/page-mock.js:69:
addScript(url, content, hasSourceURL, isContentScript) {
On 2017/03/16 00:17:10, dgozman wrote:
> Note that content scripts must have non-default execution context.

Done.

https://codereview.chromium.org/2747863007/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/inspector/page-mock.js:73: var auxData
= isContentScript ? {isDefault: false} : undefined;
On 2017/03/16 00:17:10, dgozman wrote:
> this._executionContext.auxData

Done.

https://codereview.chromium.org/2747863007/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/inspector/page-mock.js:79: endLine: 0,
On 2017/03/16 00:17:09, dgozman wrote:
> Let's call lineEndings on it and pass true endLine/endColumn?

Done.

https://codereview.chromium.org/2747863007/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/inspector/page-mock.js:96:
this._fireEvent("Runtime.executionContextDestroyed", {executionContextId:
this._executionContext.id});
On 2017/03/16 00:17:09, dgozman wrote:
> I believe this one is not required.

We currently send it whenever the page gets reloaded

https://codereview.chromium.org/2747863007/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/inspector/page-mock.js:132: resources:
[]
On 2017/03/16 00:17:09, dgozman wrote:
> Should we list all scripts here?

I consider those to be evals for simplicity; renamed the method to make this
clear

https://codereview.chromium.org/2747863007/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/inspector/sources/debugger-ui/scripts-panel-expected.txt
(right):

https://codereview.chromium.org/2747863007/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/inspector/sources/debugger-ui/scripts-panel-expected.txt:6:
mock-page
On 2017/03/16 00:17:10, dgozman wrote:
> We don't exercise the main target logic anymore, while it's very important in
> real world. Can we fix that?

Done.

https://codereview.chromium.org/2747863007/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/inspector/sources/debugger-ui/scripts-panel.html
(right):

https://codereview.chromium.org/2747863007/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/inspector/sources/debugger-ui/scripts-panel.html:28:
Bindings.NetworkProject.forTarget(InspectorTest.mainTarget)._reset();
On 2017/03/16 00:17:10, dgozman wrote:
> Eh? Unfortunate!

Right, it looks like I'll have a binding-resetters for a while.

https://codereview.chromium.org/2747863007/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/inspector/sources/debugger-ui/switch-file.html
(right):

https://codereview.chromium.org/2747863007/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/inspector/sources/debugger-ui/switch-file.html:37:
uiSourceCodes.push(await InspectorTest.addScriptUISourceCode(files[i], ''));
On 2017/03/16 00:17:10, dgozman wrote:
> I can feel how slow (and thus flaky) this is :-(

Shouldn't be slow, but I migrated this onto PageMock.

[lushnikov, 2017-03-16 06:31:41]

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

[commit-bot: I haz the power, 2017-03-16 06:31:59]

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-16 07:43:08]

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

[commit-bot: I haz the power, 2017-03-16 07:43:09]

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-03-16 08:23:49]

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

[commit-bot: I haz the power, 2017-03-16 08:24:08]

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-16 09:39:14]

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

[commit-bot: I haz the power, 2017-03-16 09:39:15]

Dry run: This issue passed the CQ dry run.

[dgozman, 2017-03-16 18:04:01]

lgtm

https://codereview.chromium.org/2747863007/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/http/tests/inspector/page-mock.js (right):

https://codereview.chromium.org/2747863007/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/inspector/page-mock.js:119:
this._fireEvent("Page.frameNavigated", {frame: this._mainFrame});
Let's send all of the frame-related notifications.

https://codereview.chromium.org/2747863007/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/inspector/page-mock.js:131:
_enableRuntime(id, params) {
Let's name these "runtimeEnable", "pageGetResourceTree" and similar.

[lushnikov, 2017-03-16 20:59:22]

All done; also, made the harness slightly more general and nicer.

https://codereview.chromium.org/2747863007/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/http/tests/inspector/page-mock.js (right):

https://codereview.chromium.org/2747863007/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/inspector/page-mock.js:119:
this._fireEvent("Page.frameNavigated", {frame: this._mainFrame});
On 2017/03/16 18:04:01, dgozman wrote:
> Let's send all of the frame-related notifications.

Done.

https://codereview.chromium.org/2747863007/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/inspector/page-mock.js:131:
_enableRuntime(id, params) {
On 2017/03/16 18:04:01, dgozman wrote:
> Let's name these "runtimeEnable", "pageGetResourceTree" and similar.

Done.

[lushnikov, 2017-03-16 20:59:28]

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

[commit-bot: I haz the power, 2017-03-16 21:00:03]

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

[dgozman, 2017-03-16 21:12:56]

https://codereview.chromium.org/2747863007/diff/100001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/http/tests/inspector/page-mock.js (right):

https://codereview.chromium.org/2747863007/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/inspector/page-mock.js:41:
'Debugger.getScriptSource': this._debuggergetScriptSource,
typo in method name

https://codereview.chromium.org/2747863007/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/inspector/page-mock.js:203: if
(!this._enabledDomains.has(domain))
Use _isSupportedDomain instead.

[lushnikov, 2017-03-16 21:19:13]

https://codereview.chromium.org/2747863007/diff/100001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/http/tests/inspector/page-mock.js (right):

https://codereview.chromium.org/2747863007/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/inspector/page-mock.js:41:
'Debugger.getScriptSource': this._debuggergetScriptSource,
On 2017/03/16 21:12:56, dgozman wrote:
> typo in method name

Done.

https://codereview.chromium.org/2747863007/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/inspector/page-mock.js:203: if
(!this._enabledDomains.has(domain))
On 2017/03/16 21:12:56, dgozman wrote:
> Use _isSupportedDomain instead.

That's not needed since non-supported domain can't be enabled

[lushnikov, 2017-03-16 21:19:18]

The CQ bit was checked by lushnikov@chromium.org

[lushnikov, 2017-03-16 21:19:18]

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/2747863007/#ps120001
(title: "typo")

[commit-bot: I haz the power, 2017-03-16 21:20:10]

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-16 23:19:07]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1489699158188620,
"parent_rev": "4df290e53149721f6955c5fdfa29d2073da5da72", "commit_rev":
"e9ffec31f17a567b537b260f772d649a8f437154"}

[commit-bot: I haz the power, 2017-03-16 23:19:52]

Description was changed from

==========
DevTools: clean up tests to not depend on NetworkProject.addFile method

The NetworkProject.addFile method is adding a UISourceCode
for SourceMap source. However, multiple tests abuse the method for their
needs. This is unfortunate, since we plan to re-work bindings and
eliminate the public "NetworkProject.addFile" method altogether.

This patch migrates tests to other methods of adding UISourceCodes:
- a InspectorTest.addScriptUISourceCode method is introduced which
  adds scripts/contentScripts with proper urls in the main target.
- a InspectorTest.PageMock is introduced to add scripts in a multi-target
  tests

This implementation of pagemock supports only two methods:
- addScript, which adds a script to the mock page and reports it via
  Debugger.scriptParsed protocol event.
- reload(), which "reloads" page, killing all the added scripts

Since PageMock's speaks over a very stable devtools protocol,
little effort will be needed to maintain it. This is unlike the
current solution where tests abuse different public/private binding
APIs.

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

to

==========
DevTools: clean up tests to not depend on NetworkProject.addFile method

The NetworkProject.addFile method is adding a UISourceCode
for SourceMap source. However, multiple tests abuse the method for their
needs. This is unfortunate, since we plan to re-work bindings and
eliminate the public "NetworkProject.addFile" method altogether.

This patch migrates tests to other methods of adding UISourceCodes:
- a InspectorTest.addScriptUISourceCode method is introduced which
  adds scripts/contentScripts with proper urls in the main target.
- a InspectorTest.PageMock is introduced to add scripts in a multi-target
  tests

This implementation of pagemock supports only two methods:
- addScript, which adds a script to the mock page and reports it via
  Debugger.scriptParsed protocol event.
- reload(), which "reloads" page, killing all the added scripts

Since PageMock's speaks over a very stable devtools protocol,
little effort will be needed to maintain it. This is unlike the
current solution where tests abuse different public/private binding
APIs.

R=dgozman
BUG=670180

Review-Url: https://codereview.chromium.org/2747863007
Cr-Commit-Position: refs/heads/master@{#457601}
Committed:
https://chromium.googlesource.com/chromium/src/+/e9ffec31f17a567b537b260f772d...
==========

[commit-bot: I haz the power, 2017-03-16 23:19:53]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/e9ffec31f17a567b537b260f772d...