Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(50)

Issue 6990059: DevTools: devtools message plumbing between worker and page processes (Closed)

Created:
9 years, 7 months ago by yurys
Modified:
9 years, 7 months ago
Reviewers:
jam, Dmitry Titov, pfeldman
CC:
chromium-reviews, darin-cc_chromium.org, Jói, brettw-cc_chromium.org
Visibility:
Public.

Description

This change provides initial support for sending DevTools messages between Worker and Page processes. On the Page side devtools messages are handled by WorkerDevtoolsAgentProxy. It implements WebWorkerBase::DevToolsDelegate interface which isolates worker stuff under content/ from DevTools specifics. In the worker process it's WorkerDevtoolsAgent that is responsible for sending/receiving devtools messages. It implements WebWorkerStub::DevToolsDelegate and WebWorkerClientProxy::DevToolsDelegate which insulate worker stuff under content/ from the devtools implementation details. WorkerDevtoolsAgentProxy and WorkerDevtoolsAgent are counterparts of WebWorkerProxy and WebWorkerStub. Since devtools is not a part of HTML5 the new classes live under chrome/ ContentWorkerClient is introduced for notifying embedder about certain events when worker clients can be installed. BUG=13684 TEST=None Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=86991

Patch Set 1 #

Total comments: 15

Patch Set 2 : Moved worker devtools login under content/ #

Total comments: 14

Patch Set 3 : Comments addressed, WebWorkerBase::DevToolsDelegate removed #

Total comments: 2

Patch Set 4 : Removed extra line #

Unified diffs Side-by-side diffs Delta from patch set Stats (+334 lines, -171 lines) Patch
M chrome/browser/debugger/debugger_remote_service.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/debugger/devtools_handler.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/debugger/devtools_http_protocol_handler.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/debugger/devtools_manager.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/debugger/devtools_remote_service.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/debugger/extension_ports_remote_service.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/debugger/inspectable_tab_proxy.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/extensions/extension_debugger_api.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/extensions/extension_devtools_bridge.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/extensions/extension_devtools_browsertests.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/views/about_ipc_dialog.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/webui/devtools_ui.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/chrome_common.gypi View 1 2 3 1 chunk +0 lines, -1 line 0 comments Download
M chrome/common/common_message_generator.h View 1 1 chunk +0 lines, -1 line 0 comments Download
M chrome/common/devtools_messages.h View 1 1 chunk +0 lines, -142 lines 0 comments Download
M chrome/common/logging_chrome.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/renderer/devtools_agent.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/renderer/devtools_agent_filter.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M chrome/renderer/devtools_client.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M content/common/content_message_generator.h View 1 1 chunk +1 line, -0 lines 0 comments Download
content/common/devtools_messages.h View 1 3 chunks +18 lines, -3 lines 0 comments Download
M content/content_common.gypi View 1 2 3 1 chunk +1 line, -0 lines 0 comments Download
M content/content_renderer.gypi View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download
M content/content_worker.gypi View 1 1 chunk +2 lines, -0 lines 0 comments Download
M content/renderer/websharedworker_proxy.cc View 1 1 chunk +2 lines, -1 line 0 comments Download
M content/renderer/webworker_base.h View 1 2 3 chunks +7 lines, -1 line 0 comments Download
M content/renderer/webworker_base.cc View 1 2 4 chunks +10 lines, -1 line 0 comments Download
M content/renderer/webworker_proxy.h View 1 2 3 1 chunk +4 lines, -0 lines 0 comments Download
M content/renderer/webworker_proxy.cc View 1 2 3 chunks +24 lines, -2 lines 0 comments Download
A content/renderer/worker_devtools_agent_proxy.h View 1 2 3 1 chunk +50 lines, -0 lines 0 comments Download
A content/renderer/worker_devtools_agent_proxy.cc View 1 1 chunk +67 lines, -0 lines 0 comments Download
M content/worker/webworker_stub.h View 1 4 chunks +5 lines, -1 line 0 comments Download
M content/worker/webworker_stub.cc View 1 2 3 chunks +9 lines, -1 line 0 comments Download
M content/worker/webworkerclient_proxy.h View 1 2 3 chunks +7 lines, -0 lines 0 comments Download
M content/worker/webworkerclient_proxy.cc View 1 2 3 chunks +8 lines, -1 line 0 comments Download
A content/worker/worker_devtools_agent.h View 1 2 1 chunk +44 lines, -0 lines 0 comments Download
A content/worker/worker_devtools_agent.cc View 1 2 1 chunk +57 lines, -0 lines 0 comments Download

Messages

Total messages: 9 (0 generated)
yurys
9 years, 7 months ago (2011-05-24 16:34:37 UTC) #1
jam
Hi, I started reviewing this but then I hit the fact that even though the ...
9 years, 7 months ago (2011-05-24 20:21:31 UTC) #2
Dmitry Titov
Note dependency check failing on the try bot, and also couple tiny nits: http://codereview.chromium.org/6990059/diff/1/content/renderer/webworker_base.cc File ...
9 years, 7 months ago (2011-05-25 01:38:43 UTC) #3
yurys
I moved devtools_messages.h from chrome/common/ to content/common/ and put all new worker devtools classes under ...
9 years, 7 months ago (2011-05-25 17:28:29 UTC) #4
jam
On 2011/05/25 17:28:29, Yury Semikhatsky wrote: > I moved devtools_messages.h from chrome/common/ to content/common/ and ...
9 years, 7 months ago (2011-05-25 17:44:09 UTC) #5
jam
http://codereview.chromium.org/6990059/diff/7001/content/renderer/webworker_proxy.cc File content/renderer/webworker_proxy.cc (right): http://codereview.chromium.org/6990059/diff/7001/content/renderer/webworker_proxy.cc#newcode95 content/renderer/webworker_proxy.cc:95: { nit: brace brackets on previous line http://codereview.chromium.org/6990059/diff/7001/content/worker/webworkerclient_proxy.cc File ...
9 years, 7 months ago (2011-05-25 17:50:11 UTC) #6
yurys
All comments addressed. WebWorkerBase::DevToolsDelegate and WebWorkerClientProxy::DevToolsDelegate were removed in favor of using WorkerDevToolsAgent directly. http://codereview.chromium.org/6990059/diff/7001/content/renderer/webworker_proxy.cc ...
9 years, 7 months ago (2011-05-26 09:11:49 UTC) #7
jam
lgtm http://codereview.chromium.org/6990059/diff/11001/content/renderer/worker_devtools_agent_proxy.h File content/renderer/worker_devtools_agent_proxy.h (right): http://codereview.chromium.org/6990059/diff/11001/content/renderer/worker_devtools_agent_proxy.h#newcode39 content/renderer/worker_devtools_agent_proxy.h:39: nit: extra line
9 years, 7 months ago (2011-05-26 18:28:53 UTC) #8
yurys
9 years, 7 months ago (2011-05-27 07:45:43 UTC) #9
http://codereview.chromium.org/6990059/diff/11001/content/renderer/worker_dev...
File content/renderer/worker_devtools_agent_proxy.h (right):

http://codereview.chromium.org/6990059/diff/11001/content/renderer/worker_dev...
content/renderer/worker_devtools_agent_proxy.h:39: 
On 2011/05/26 18:28:53, John Abd-El-Malek wrote:
> nit: extra line

Done.

Powered by Google App Engine
This is Rietveld 408576698