Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
185463010:
DevTools: Introduce Target class which holds connection & models (Closed)
|
Created:
6 years, 9 months ago by sergeyv Modified:
6 years, 9 months ago CC:
blink-reviews, caseq+blink_chromium.org, loislo+blink_chromium.org, eustas+blink_chromium.org, alph+blink_chromium.org, yurys+blink_chromium.org, lushnikov+blink_chromium.org, vsevik+blink_chromium.org, pfeldman+blink_chromium.org, paulirish+reviews_chromium.org, apavlov+blink_chromium.org, devtools-reviews_chromium.org, aandrey+blink_chromium.org Base URL:
https://chromium.googlesource.com/chromium/blink.git@gr-RuntimeAgent Visibility:
Public. |
DescriptionDevTools: Introduce Target class which holds connection & models
BUG=
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=168741
Patch Set 1 #
Total comments: 14
Patch Set 2 : Address aandrey's comments #
Total comments: 6
Patch Set 3 : Address vsevik's comments #Patch Set 4 : Solve compilation issues #
Total comments: 4
Patch Set 5 : Address vsevik's comments #
Total comments: 6
Patch Set 6 : Address vsevik's comments #Patch Set 7 : Rebase on master #
Total comments: 10
Patch Set 8 : Address vsevik's comments #
Total comments: 2
Messages
Total messages: 41 (0 generated)
https://codereview.chromium.org/185463010/diff/1/Source/devtools/front_end/Co... File Source/devtools/front_end/ConsoleModel.js (right): https://codereview.chromium.org/185463010/diff/1/Source/devtools/front_end/Co... Source/devtools/front_end/ConsoleModel.js:38: this._agent = target.connection.agent("Console"); this should be typed for the js compiler, so either cast, or (better) encapsulate the "connection" inside target and expose explicit: target.consoleAgent(), target.pageAgent(),... that will return a proper @return types. https://codereview.chromium.org/185463010/diff/1/Source/devtools/front_end/Co... Source/devtools/front_end/ConsoleModel.js:43: target.connection.registerDispatcher("Console", new WebInspector.ConsoleDispatcher(this)); encapsulate connection? https://codereview.chromium.org/185463010/diff/1/Source/devtools/front_end/Ta... File Source/devtools/front_end/Target.js (right): https://codereview.chromium.org/185463010/diff/1/Source/devtools/front_end/Ta... Source/devtools/front_end/Target.js:2: * Copyright 2014 The Chromium Authors. All rights reserved. odd licence https://codereview.chromium.org/185463010/diff/1/Source/devtools/front_end/Ta... Source/devtools/front_end/Target.js:24: // give a chance to add listeners of WebInspector.Target.Events.TargetIsReady add {} braces https://codereview.chromium.org/185463010/diff/1/Source/devtools/front_end/Ta... Source/devtools/front_end/Target.js:41: _loadedWithCapabilities: function() this does not seem to be private https://codereview.chromium.org/185463010/diff/1/Source/devtools/front_end/Ta... Source/devtools/front_end/Target.js:61: this._targets = []; jsdoc https://codereview.chromium.org/185463010/diff/1/Source/devtools/front_end/in... File Source/devtools/front_end/inspector.js (right): https://codereview.chromium.org/185463010/diff/1/Source/devtools/front_end/in... Source/devtools/front_end/inspector.js:350: this.domAgent.addEventListener(WebInspector.DOMAgent.Events.InspectNodeRequested, this._inspectNodeRequested, this); domAgent is one for many targets? what is "target" then?
https://codereview.chromium.org/185463010/diff/1/Source/devtools/front_end/Co... File Source/devtools/front_end/ConsoleModel.js (right): https://codereview.chromium.org/185463010/diff/1/Source/devtools/front_end/Co... Source/devtools/front_end/ConsoleModel.js:38: this._agent = target.connection.agent("Console"); On 2014/03/04 10:05:16, aandrey wrote: > this should be typed for the js compiler, so either cast, or (better) > encapsulate the "connection" inside target and expose explicit: > target.consoleAgent(), target.pageAgent(),... that will return a proper @return > types. Partially done. I encapsulated connection. But, unfortunately, our generator does not create type ConsoleAgent, but instance of it, thus our compiler does not understand /** @type {!ConsoleAgent} */ I will handle this issue in a separate patch. https://codereview.chromium.org/185463010/diff/1/Source/devtools/front_end/Co... Source/devtools/front_end/ConsoleModel.js:43: target.connection.registerDispatcher("Console", new WebInspector.ConsoleDispatcher(this)); On 2014/03/04 10:05:16, aandrey wrote: > encapsulate connection? Done. https://codereview.chromium.org/185463010/diff/1/Source/devtools/front_end/Ta... File Source/devtools/front_end/Target.js (right): https://codereview.chromium.org/185463010/diff/1/Source/devtools/front_end/Ta... Source/devtools/front_end/Target.js:2: * Copyright 2014 The Chromium Authors. All rights reserved. On 2014/03/04 10:05:16, aandrey wrote: > odd licence Header was changed recently. http://www.chromium.org/developers/coding-style#TOC-File-headers https://codereview.chromium.org/185463010/diff/1/Source/devtools/front_end/Ta... Source/devtools/front_end/Target.js:24: // give a chance to add listeners of WebInspector.Target.Events.TargetIsReady On 2014/03/04 10:05:16, aandrey wrote: > add {} braces Done. https://codereview.chromium.org/185463010/diff/1/Source/devtools/front_end/Ta... Source/devtools/front_end/Target.js:41: _loadedWithCapabilities: function() On 2014/03/04 10:05:16, aandrey wrote: > this does not seem to be private There is no usages of this function out of this file and it should be used only by constructor. I cant see the reason to make it public. https://codereview.chromium.org/185463010/diff/1/Source/devtools/front_end/Ta... Source/devtools/front_end/Target.js:61: this._targets = []; On 2014/03/04 10:05:16, aandrey wrote: > jsdoc Done. https://codereview.chromium.org/185463010/diff/1/Source/devtools/front_end/in... File Source/devtools/front_end/inspector.js (right): https://codereview.chromium.org/185463010/diff/1/Source/devtools/front_end/in... Source/devtools/front_end/inspector.js:350: this.domAgent.addEventListener(WebInspector.DOMAgent.Events.InspectNodeRequested, this._inspectNodeRequested, this); On 2014/03/04 10:05:16, aandrey wrote: > domAgent is one for many targets? what is "target" then? It is hard to add support of multiple target in all codebase in one patch, so it is added step by step. Currently domAgent is one for all targets, but it will be changed in next patches.
https://codereview.chromium.org/185463010/diff/1/Source/devtools/front_end/Co... File Source/devtools/front_end/ConsoleModel.js (right): https://codereview.chromium.org/185463010/diff/1/Source/devtools/front_end/Co... Source/devtools/front_end/ConsoleModel.js:38: this._agent = target.connection.agent("Console"); On 2014/03/04 10:05:16, aandrey wrote: > this should be typed for the js compiler, so either cast, or (better) > encapsulate the "connection" inside target and expose explicit: > target.consoleAgent(), target.pageAgent(),... that will return a proper @return > types. Partially done. I encapsulated connection. But, unfortunately, our generator does not create type ConsoleAgent, but instance of it, thus our compiler does not understand /** @type {!ConsoleAgent} */ I will handle this issue in a separate patch. https://codereview.chromium.org/185463010/diff/1/Source/devtools/front_end/Co... Source/devtools/front_end/ConsoleModel.js:43: target.connection.registerDispatcher("Console", new WebInspector.ConsoleDispatcher(this)); On 2014/03/04 10:05:16, aandrey wrote: > encapsulate connection? Done. https://codereview.chromium.org/185463010/diff/1/Source/devtools/front_end/Ta... File Source/devtools/front_end/Target.js (right): https://codereview.chromium.org/185463010/diff/1/Source/devtools/front_end/Ta... Source/devtools/front_end/Target.js:2: * Copyright 2014 The Chromium Authors. All rights reserved. On 2014/03/04 10:05:16, aandrey wrote: > odd licence Header was changed recently. http://www.chromium.org/developers/coding-style#TOC-File-headers https://codereview.chromium.org/185463010/diff/1/Source/devtools/front_end/Ta... Source/devtools/front_end/Target.js:24: // give a chance to add listeners of WebInspector.Target.Events.TargetIsReady On 2014/03/04 10:05:16, aandrey wrote: > add {} braces Done. https://codereview.chromium.org/185463010/diff/1/Source/devtools/front_end/Ta... Source/devtools/front_end/Target.js:41: _loadedWithCapabilities: function() On 2014/03/04 10:05:16, aandrey wrote: > this does not seem to be private There is no usages of this function out of this file and it should be used only by constructor. I cant see the reason to make it public. https://codereview.chromium.org/185463010/diff/1/Source/devtools/front_end/Ta... Source/devtools/front_end/Target.js:61: this._targets = []; On 2014/03/04 10:05:16, aandrey wrote: > jsdoc Done. https://codereview.chromium.org/185463010/diff/1/Source/devtools/front_end/in... File Source/devtools/front_end/inspector.js (right): https://codereview.chromium.org/185463010/diff/1/Source/devtools/front_end/in... Source/devtools/front_end/inspector.js:350: this.domAgent.addEventListener(WebInspector.DOMAgent.Events.InspectNodeRequested, this._inspectNodeRequested, this); On 2014/03/04 10:05:16, aandrey wrote: > domAgent is one for many targets? what is "target" then? It is hard to add support of multiple target in all codebase in one patch, so it is added step by step. Currently domAgent is one for all targets, but it will be changed in next patches.
> https://codereview.chromium.org/185463010/diff/1/Source/devtools/front_end/Co... > Source/devtools/front_end/ConsoleModel.js:38: this._agent = > target.connection.agent("Console"); > On 2014/03/04 10:05:16, aandrey wrote: > > this should be typed for the js compiler, so either cast, or (better) > > encapsulate the "connection" inside target and expose explicit: > > target.consoleAgent(), target.pageAgent(),... that will return a proper > @return > > types. > > Partially done. I encapsulated connection. But, unfortunately, our generator > does not create type ConsoleAgent, but instance of it, thus our compiler does > not understand /** @type {!ConsoleAgent} */ > > I will handle this issue in a separate patch. Let's do it here, it's just a few lines in externs generator and it would help us to keep compilation as good as it was before. https://chromiumcodereview.appspot.com/185463010/diff/20001/Source/devtools/f... File Source/devtools/front_end/Target.js (right): https://chromiumcodereview.appspot.com/185463010/diff/20001/Source/devtools/f... Source/devtools/front_end/Target.js:84: WebInspector.TargetManager.Events = { Let's add this event once it is at least dispatched somewhere. https://chromiumcodereview.appspot.com/185463010/diff/20001/Source/devtools/f... Source/devtools/front_end/Target.js:94: newConnectionAvailable: function(connection) How about createTarget(connection, callback) where callback is function(Target) that is called once the target is ready? https://chromiumcodereview.appspot.com/185463010/diff/20001/Source/devtools/f... File Source/devtools/front_end/inspector.js (left): https://chromiumcodereview.appspot.com/185463010/diff/20001/Source/devtools/f... Source/devtools/front_end/inspector.js:321: WorkerAgent.canInspectWorkers(WebInspector._initializeCapability.bind(WebInspector, "canInspectWorkers", WebInspector._doLoadedDoneWithCapabilities.bind(WebInspector))); _initializeCapability can be removed now.
https://chromiumcodereview.appspot.com/185463010/diff/20001/Source/devtools/f... File Source/devtools/front_end/Target.js (right): https://chromiumcodereview.appspot.com/185463010/diff/20001/Source/devtools/f... Source/devtools/front_end/Target.js:84: WebInspector.TargetManager.Events = { On 2014/03/05 12:34:49, vsevik wrote: > Let's add this event once it is at least dispatched somewhere. Done. https://chromiumcodereview.appspot.com/185463010/diff/20001/Source/devtools/f... Source/devtools/front_end/Target.js:94: newConnectionAvailable: function(connection) On 2014/03/05 12:34:49, vsevik wrote: > How about createTarget(connection, callback) where callback is function(Target) > that is called once the target is ready? Done. https://chromiumcodereview.appspot.com/185463010/diff/20001/Source/devtools/f... File Source/devtools/front_end/inspector.js (left): https://chromiumcodereview.appspot.com/185463010/diff/20001/Source/devtools/f... Source/devtools/front_end/inspector.js:321: WorkerAgent.canInspectWorkers(WebInspector._initializeCapability.bind(WebInspector, "canInspectWorkers", WebInspector._doLoadedDoneWithCapabilities.bind(WebInspector))); On 2014/03/05 12:34:49, vsevik wrote: > _initializeCapability can be removed now. Done.
https://chromiumcodereview.appspot.com/185463010/diff/60001/Source/devtools/f... File Source/devtools/front_end/Target.js (right): https://chromiumcodereview.appspot.com/185463010/diff/60001/Source/devtools/f... Source/devtools/front_end/Target.js:37: _generateProtocolAgentsMethods: function() Can we generate them on prototype instead? https://chromiumcodereview.appspot.com/185463010/diff/60001/Source/devtools/s... File Source/devtools/scripts/generate_protocol_externs.py (left): https://chromiumcodereview.appspot.com/185463010/diff/60001/Source/devtools/s... Source/devtools/scripts/generate_protocol_externs.py:111: output_file.write("\n\n\nvar %sAgent = {};\n" % domain_name) Why did this move?
https://chromiumcodereview.appspot.com/185463010/diff/60001/Source/devtools/f... File Source/devtools/front_end/Target.js (right): https://chromiumcodereview.appspot.com/185463010/diff/60001/Source/devtools/f... Source/devtools/front_end/Target.js:37: _generateProtocolAgentsMethods: function() On 2014/03/06 14:49:45, vsevik wrote: > Can we generate them on prototype instead? Done. https://chromiumcodereview.appspot.com/185463010/diff/60001/Source/devtools/s... File Source/devtools/scripts/generate_protocol_externs.py (left): https://chromiumcodereview.appspot.com/185463010/diff/60001/Source/devtools/s... Source/devtools/scripts/generate_protocol_externs.py:111: output_file.write("\n\n\nvar %sAgent = {};\n" % domain_name) On 2014/03/06 14:49:45, vsevik wrote: > Why did this move? I've done this move, cause now we need to create Agent's prototype before instantiating Agent itself
lgtm https://chromiumcodereview.appspot.com/185463010/diff/80001/Source/devtools/f... File Source/devtools/front_end/InspectorBackend.js (right): https://chromiumcodereview.appspot.com/185463010/diff/80001/Source/devtools/f... Source/devtools/front_end/InspectorBackend.js:46: initProtocolAgentsClass: function() { { on the next line. This method should be private. https://chromiumcodereview.appspot.com/185463010/diff/80001/Source/devtools/f... Source/devtools/front_end/InspectorBackend.js:141: this._addProtocolAgentsMethodIfNeeded(domainAndMethod[0]); You could move this call to _agentPrototype and get rid of IfNeeded suffix and extra check. https://chromiumcodereview.appspot.com/185463010/diff/80001/Source/devtools/f... File Source/devtools/front_end/inspector.js (right): https://chromiumcodereview.appspot.com/185463010/diff/80001/Source/devtools/f... Source/devtools/front_end/inspector.js:317: Capabilities.canInspectWorkers = mainTarget.canInspectWorkers; Please move this to Target.
The CQ bit was checked by sergeyv@chromium.org
https://chromiumcodereview.appspot.com/185463010/diff/80001/Source/devtools/f... File Source/devtools/front_end/InspectorBackend.js (right): https://chromiumcodereview.appspot.com/185463010/diff/80001/Source/devtools/f... Source/devtools/front_end/InspectorBackend.js:46: initProtocolAgentsClass: function() { On 2014/03/06 16:30:08, vsevik wrote: > { on the next line. > This method should be private. Done. https://chromiumcodereview.appspot.com/185463010/diff/80001/Source/devtools/f... Source/devtools/front_end/InspectorBackend.js:141: this._addProtocolAgentsMethodIfNeeded(domainAndMethod[0]); On 2014/03/06 16:30:08, vsevik wrote: > You could move this call to _agentPrototype and get rid of IfNeeded suffix and > extra check. Done. https://chromiumcodereview.appspot.com/185463010/diff/80001/Source/devtools/f... File Source/devtools/front_end/inspector.js (right): https://chromiumcodereview.appspot.com/185463010/diff/80001/Source/devtools/f... Source/devtools/front_end/inspector.js:317: Capabilities.canInspectWorkers = mainTarget.canInspectWorkers; On 2014/03/06 16:30:08, vsevik wrote: > Please move this to Target. Done.
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sergeyv@chromium.org/185463010/100001
The CQ bit was unchecked by commit-bot@chromium.org
Failed to apply patch for Source/devtools/front_end/inspector.js:
While running patch -p1 --forward --force --no-backup-if-mismatch;
patching file Source/devtools/front_end/inspector.js
Hunk #4 FAILED at 322.
Hunk #5 succeeded at 392 (offset -1 lines).
1 out of 5 hunks FAILED -- saving rejects to file
Source/devtools/front_end/inspector.js.rej
Patch: Source/devtools/front_end/inspector.js
Index: Source/devtools/front_end/inspector.js
diff --git a/Source/devtools/front_end/inspector.js
b/Source/devtools/front_end/inspector.js
index
2baf581ab659c3ff310e8b8cd1484181e9e8927a..ef21e8ac4d89b7ab88eae412f245e29bc97df2b7
100644
--- a/Source/devtools/front_end/inspector.js
+++ b/Source/devtools/front_end/inspector.js
@@ -193,13 +193,6 @@ var WebInspector = {
return parsedURL ? parsedURL.host : "";
},
- _initializeCapability: function(name, callback, error, result)
- {
- Capabilities[name] = result;
- if (callback)
- callback();
- },
-
_debuggerPaused: function()
{
this.debuggerModel.removeEventListener(WebInspector.DebuggerModel.Events.DebuggerPaused,
this._debuggerPaused, this);
@@ -293,11 +286,8 @@ WebInspector.loaded = function()
WebInspector.doLoadedDone();
- // In case of loading as a web page with no bindings / harness, kick off
initialization manually.
- if (InspectorFrontendHost.isStub) {
+ if (InspectorFrontendHost.isStub)
InspectorFrontendAPI.dispatchQueryParameters(WebInspector.queryParamsObject);
- WebInspector._doLoadedDoneWithCapabilities();
- }
}
WebInspector.doLoadedDone = function()
@@ -317,14 +307,13 @@ WebInspector.doLoadedDone = function()
var connection = workerId ? new WebInspector.WorkerConnection(workerId) :
new InspectorBackendClass.MainConnection();
InspectorBackend.setConnection(connection);
-
PageAgent.canScreencast(WebInspector._initializeCapability.bind(WebInspector,
"canScreencast", null));
-
WorkerAgent.canInspectWorkers(WebInspector._initializeCapability.bind(WebInspector,
"canInspectWorkers",
WebInspector._doLoadedDoneWithCapabilities.bind(WebInspector)));
+ WebInspector.targetManager = new WebInspector.TargetManager();
+ WebInspector.targetManager.createTarget(connection,
WebInspector._doLoadedDoneWithCapabilities.bind(WebInspector));
}
-WebInspector._doLoadedDoneWithCapabilities = function()
+WebInspector._doLoadedDoneWithCapabilities = function(mainTarget)
{
new WebInspector.VersionController().updateVersion();
-
WebInspector.shortcutsScreen = new WebInspector.ShortcutsScreen();
this._registerShortcuts();
@@ -333,19 +322,13 @@ WebInspector._doLoadedDoneWithCapabilities = function()
WebInspector.shortcutsScreen.section(WebInspector.UIString("Elements
Panel"));
WebInspector.ShortcutsScreen.registerShortcuts();
- this.console = new WebInspector.ConsoleModel();
this.console.addEventListener(WebInspector.ConsoleModel.Events.ConsoleCleared,
this._resetErrorAndWarningCounts, this);
this.console.addEventListener(WebInspector.ConsoleModel.Events.MessageAdded,
this._updateErrorAndWarningCounts, this);
this.console.addEventListener(WebInspector.ConsoleModel.Events.RepeatCountUpdated,
this._updateErrorAndWarningCounts, this);
- this.networkManager = new WebInspector.NetworkManager();
- this.resourceTreeModel = new
WebInspector.ResourceTreeModel(this.networkManager);
- this.debuggerModel = new WebInspector.DebuggerModel();
+
this.debuggerModel.addEventListener(WebInspector.DebuggerModel.Events.DebuggerPaused,
this._debuggerPaused, this);
this.networkLog = new WebInspector.NetworkLog();
- this.domAgent = new WebInspector.DOMAgent();
this.domAgent.addEventListener(WebInspector.DOMAgent.Events.InspectNodeRequested,
this._inspectNodeRequested, this);
- this.workerManager = new
WebInspector.WorkerManager(Capabilities.canInspectWorkers);
- this.runtimeModel = new WebInspector.RuntimeModel(this.resourceTreeModel);
this.zoomManager = new WebInspector.ZoomManager();
@@ -404,7 +387,7 @@ WebInspector._doLoadedDoneWithCapabilities = function()
this.panels = {};
this.inspectorView = new WebInspector.InspectorView();
// Screencast controller creates a root view itself.
- if (Capabilities.canScreencast)
+ if (mainTarget.canScreencast)
this._screencastController = new WebInspector.ScreencastController();
else
this._createRootView();
The CQ bit was checked by sergeyv@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sergeyv@chromium.org/185463010/120001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_blink_rel
https://codereview.chromium.org/185463010/diff/120001/Source/devtools/front_e... File Source/devtools/front_end/InspectorBackend.js (right): https://codereview.chromium.org/185463010/diff/120001/Source/devtools/front_e... Source/devtools/front_end/InspectorBackend.js:46: _initProtocolAgentsClass: function() nit: _initProtocolAgentsConstructor https://codereview.chromium.org/185463010/diff/120001/Source/devtools/front_e... Source/devtools/front_end/InspectorBackend.js:62: _addProtocolAgentsMethod: function(domain) nit: _addAgentGetterMethodToProtocolAgentsPrototype https://codereview.chromium.org/185463010/diff/120001/Source/devtools/front_e... File Source/devtools/front_end/Target.js (right): https://codereview.chromium.org/185463010/diff/120001/Source/devtools/front_e... Source/devtools/front_end/Target.js:17: this.canScreenCast = false; Why is this needed? https://codereview.chromium.org/185463010/diff/120001/Source/devtools/front_e... Source/devtools/front_end/Target.js:32: _initializeCapability: function(name, callback, error, result) weird indent https://codereview.chromium.org/185463010/diff/120001/Source/devtools/front_e... Source/devtools/front_end/Target.js:45: Capabilities.canInspectWorkers = this.canInspectWorkers; Capabilities[name] = result; in _initializeCapability instead, let's this legacy code shorter and in one place.
https://codereview.chromium.org/185463010/diff/120001/Source/devtools/front_e... File Source/devtools/front_end/InspectorBackend.js (right): https://codereview.chromium.org/185463010/diff/120001/Source/devtools/front_e... Source/devtools/front_end/InspectorBackend.js:46: _initProtocolAgentsClass: function() On 2014/03/06 17:52:31, vsevik wrote: > nit: _initProtocolAgentsConstructor Done. https://codereview.chromium.org/185463010/diff/120001/Source/devtools/front_e... Source/devtools/front_end/InspectorBackend.js:62: _addProtocolAgentsMethod: function(domain) On 2014/03/06 17:52:31, vsevik wrote: > nit: _addAgentGetterMethodToProtocolAgentsPrototype Done. https://codereview.chromium.org/185463010/diff/120001/Source/devtools/front_e... File Source/devtools/front_end/Target.js (right): https://codereview.chromium.org/185463010/diff/120001/Source/devtools/front_e... Source/devtools/front_end/Target.js:17: this.canScreenCast = false; On 2014/03/06 17:52:31, vsevik wrote: > Why is this needed? Done. https://codereview.chromium.org/185463010/diff/120001/Source/devtools/front_e... Source/devtools/front_end/Target.js:32: _initializeCapability: function(name, callback, error, result) On 2014/03/06 17:52:31, vsevik wrote: > weird indent Done. https://codereview.chromium.org/185463010/diff/120001/Source/devtools/front_e... Source/devtools/front_end/Target.js:45: Capabilities.canInspectWorkers = this.canInspectWorkers; On 2014/03/06 17:52:31, vsevik wrote: > Capabilities[name] = result; in _initializeCapability instead, let's this legacy > code shorter and in one place. Done.
The CQ bit was checked by sergeyv@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sergeyv@chromium.org/185463010/140001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_blink
The CQ bit was checked by sergeyv@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sergeyv@chromium.org/185463010/140001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_blink_rel
The CQ bit was checked by sergeyv@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sergeyv@chromium.org/185463010/140001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_blink_rel
The CQ bit was checked by sergeyv@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sergeyv@chromium.org/185463010/140001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_blink_rel
The CQ bit was checked by sergeyv@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sergeyv@chromium.org/185463010/140001
Message was sent while issue was closed.
Change committed as 168741
Message was sent while issue was closed.
https://codereview.chromium.org/185463010/diff/140001/Source/devtools/front_e... File Source/devtools/front_end/Target.js (right): https://codereview.chromium.org/185463010/diff/140001/Source/devtools/front_e... Source/devtools/front_end/Target.js:25: setTimeout(this._loadedWithCapabilities.bind(this, callback), 0); This kills hosted mode - everything is initialized twice.
Message was sent while issue was closed.
https://codereview.chromium.org/185463010/diff/140001/Source/devtools/front_e... File Source/devtools/front_end/inspector.js (right): https://codereview.chromium.org/185463010/diff/140001/Source/devtools/front_e... Source/devtools/front_end/inspector.js:273: WebInspector.socket = new WebSocket(ws); What about web socket connection? It is not a main connection. You should either follow up with refactoring or revert this change. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
