[DevTools] Migrate usages of Target to RuntimeModel where makes sense

[DevTools] Migrate usages of Target to RuntimeModel where makes sense

This is in a preparation to removing always-initialized target.runtimeModel.
Instead we will only instantiate one for targets with JS support.

CL also fixes a lot of compilation errors (some of them were real bugs) which
are now suddenly being caught by closure.

BUG=none

Review-Url: https://codereview.chromium.org/2752403002
Cr-Commit-Position: refs/heads/master@{#457941}
Committed: https://chromium.googlesource.com/chromium/src/+/9f18e4305df8b79a0b0ef7d94430b7ea76dfe1f6

[dgozman, 2017-03-17 00:40:30]

Description was changed from

==========
[DevTools] Migrate usages of Target to RuntimeModel where makes sense

This is in a preparation to removing always-initialized target.runtimeModel.
Instead we will only instantiate one for targets with JS support.

BUG=none
==========

to

==========
[DevTools] Migrate usages of Target to RuntimeModel where makes sense

This is in a preparation to removing always-initialized target.runtimeModel.
Instead we will only instantiate one for targets with JS support.

CL also fixes a lot of compilation errors (some of them were real bugs) which
are now suddenly being caught by closure.

BUG=none
==========

[dgozman, 2017-03-17 00:40:36]

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

[dgozman, 2017-03-17 00:40:49]

dgozman@chromium.org changed reviewers:
+ alph@chromium.org

[dgozman, 2017-03-17 00:40:50]

Could you please take a look?

[commit-bot: I haz the power, 2017-03-17 00:41:14]

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-17 02:22:14]

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

[commit-bot: I haz the power, 2017-03-17 02:22:15]

Dry run: This issue passed the CQ dry run.

[dgozman, 2017-03-17 17:43:01]

dgozman@chromium.org changed reviewers:
+ caseq@chromium.org

[dgozman, 2017-03-17 17:43:01]

+caseq

[alph, 2017-03-17 18:20:27]

lgtm

[caseq, 2017-03-17 18:30:44]

lgtm

https://codereview.chromium.org/2752403002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/inspector/sources/debugger/source-map-http-header.html
(right):

https://codereview.chromium.org/2752403002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/inspector/sources/debugger/source-map-http-header.html:19:
var mainTarget = SDK.targetManager.mainTarget();
Nuke this.

https://codereview.chromium.org/2752403002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/inspector/sources/dont-diverge-script-evaluated-twice.html
(right):

https://codereview.chromium.org/2752403002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/inspector/sources/dont-diverge-script-evaluated-twice.html:28:
var mainTarget = SDK.targetManager.mainTarget();
ditto.

https://codereview.chromium.org/2752403002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/console_model/ConsoleModel.js
(right):

https://codereview.chromium.org/2752403002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console_model/ConsoleModel.js:464:
* @param {!SDK.RuntimeModel} runtimeModel
Meeh :( Perhaps keep target here?

https://codereview.chromium.org/2752403002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/profiler/ProfileView.js
(right):

https://codereview.chromium.org/2752403002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/profiler/ProfileView.js:404:
this._debuggerModel = target ? target.model(SDK.DebuggerModel) : null;
nit: we typically prefer "target && target.model(SDK.DebuggerModel)"

https://codereview.chromium.org/2752403002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/sdk/RemoteObject.js (right):

https://codereview.chromium.org/2752403002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/RemoteObject.js:127: if (object
instanceof SDK.RemoteObjectImpl && typeof object._unserializableValue !==
'undefined')
is this a related change?

https://codereview.chromium.org/2752403002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/RemoteObject.js:209: get
objectId() {
ditto. perhaps extract this?

https://codereview.chromium.org/2752403002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/snippets/ScriptSnippetModel.js
(right):

https://codereview.chromium.org/2752403002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/snippets/ScriptSnippetModel.js:224:
var mapping = this._mappingForDebuggerModel.get(debuggerModel);
nuke this, use one in closure?

https://codereview.chromium.org/2752403002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/snippets/ScriptSnippetModel.js:229:
executionContext.debuggerModel.scriptForId(/** @type {string} */ (scriptId ||
exceptionDetails.scriptId)));
just debuggerModel

[dgozman, 2017-03-17 21:43:46]

Thank you for review!

https://codereview.chromium.org/2752403002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/http/tests/inspector/sources/debugger/source-map-http-header.html
(right):

https://codereview.chromium.org/2752403002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/http/tests/inspector/sources/debugger/source-map-http-header.html:19:
var mainTarget = SDK.targetManager.mainTarget();
On 2017/03/17 18:30:43, caseq wrote:
> Nuke this.

Done.

https://codereview.chromium.org/2752403002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/inspector/sources/dont-diverge-script-evaluated-twice.html
(right):

https://codereview.chromium.org/2752403002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/inspector/sources/dont-diverge-script-evaluated-twice.html:28:
var mainTarget = SDK.targetManager.mainTarget();
On 2017/03/17 18:30:43, caseq wrote:
> ditto.

Done.

https://codereview.chromium.org/2752403002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/console_model/ConsoleModel.js
(right):

https://codereview.chromium.org/2752403002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/console_model/ConsoleModel.js:464:
* @param {!SDK.RuntimeModel} runtimeModel
On 2017/03/17 18:30:43, caseq wrote:
> Meeh :( Perhaps keep target here?

Since exception comes from a runtime, passing RuntimeModel is logical here.

https://codereview.chromium.org/2752403002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/profiler/ProfileView.js
(right):

https://codereview.chromium.org/2752403002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/profiler/ProfileView.js:404:
this._debuggerModel = target ? target.model(SDK.DebuggerModel) : null;
On 2017/03/17 18:30:43, caseq wrote:
> nit: we typically prefer "target && target.model(SDK.DebuggerModel)"

Done.

https://codereview.chromium.org/2752403002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/sdk/RemoteObject.js (right):

https://codereview.chromium.org/2752403002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/sdk/RemoteObject.js:127: if (object
instanceof SDK.RemoteObjectImpl && typeof object._unserializableValue !==
'undefined')
On 2017/03/17 18:30:44, caseq wrote:
> is this a related change?

Yeah, closure started to complain.

https://codereview.chromium.org/2752403002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/devtools/front_end/snippets/ScriptSnippetModel.js
(right):

https://codereview.chromium.org/2752403002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/snippets/ScriptSnippetModel.js:224:
var mapping = this._mappingForDebuggerModel.get(debuggerModel);
On 2017/03/17 18:30:44, caseq wrote:
> nuke this, use one in closure?

Done.

https://codereview.chromium.org/2752403002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/devtools/front_end/snippets/ScriptSnippetModel.js:229:
executionContext.debuggerModel.scriptForId(/** @type {string} */ (scriptId ||
exceptionDetails.scriptId)));
On 2017/03/17 18:30:44, caseq wrote:
> just debuggerModel

Done.

[dgozman, 2017-03-17 21:44:12]

The CQ bit was checked by dgozman@chromium.org

[dgozman, 2017-03-17 21:44:12]

The patchset sent to the CQ was uploaded after l-g-t-m from caseq@chromium.org,
alph@chromium.org
Link to the patchset: https://codereview.chromium.org/2752403002/#ps40001
(title: "review comments addressed")

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

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-18 01:20:26]

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

[commit-bot: I haz the power, 2017-03-18 01:20:27]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[dgozman, 2017-03-18 02:02:50]

The CQ bit was checked by dgozman@chromium.org

[commit-bot: I haz the power, 2017-03-18 02:03:13]

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-18 02:55:42]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1489802570106780,
"parent_rev": "86b3d88fbf2cced97264276fb2ec796e828ddf50", "commit_rev":
"9f18e4305df8b79a0b0ef7d94430b7ea76dfe1f6"}

[commit-bot: I haz the power, 2017-03-18 02:56:23]

Description was changed from

==========
[DevTools] Migrate usages of Target to RuntimeModel where makes sense

This is in a preparation to removing always-initialized target.runtimeModel.
Instead we will only instantiate one for targets with JS support.

CL also fixes a lot of compilation errors (some of them were real bugs) which
are now suddenly being caught by closure.

BUG=none
==========

to

==========
[DevTools] Migrate usages of Target to RuntimeModel where makes sense

This is in a preparation to removing always-initialized target.runtimeModel.
Instead we will only instantiate one for targets with JS support.

CL also fixes a lot of compilation errors (some of them were real bugs) which
are now suddenly being caught by closure.

BUG=none

Review-Url: https://codereview.chromium.org/2752403002
Cr-Commit-Position: refs/heads/master@{#457941}
Committed:
https://chromium.googlesource.com/chromium/src/+/9f18e4305df8b79a0b0ef7d94430...
==========

[commit-bot: I haz the power, 2017-03-18 02:56:24]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/9f18e4305df8b79a0b0ef7d94430...