Bug 60267 – [Chromium] Web Inspector: provide initial implementation of worker inspector transport

• Component: Web Inspector (Deprecated)
• Status: RESOLVED FIXED

[Yury Semikhatsky]

[Chromium] Web Inspector: provide initial implementation of worker inspector transport

[Yury Semikhatsky]

Created attachment 92418 [details]
Patch

[Yury Semikhatsky]

Created attachment 92419 [details]
Patch

[Pavel Feldman]

Comment on attachment 92419 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=92419&action=review

> Source/WebKit/chromium/public/WebWorkerDevToolsAgent.h:42
> +    virtual void connect(WebDevToolsFrontend*) = 0;

I'd suggest that you inline these methods into WebWorker + inline corresponding client methods into WebWorkerClient. I understand that WebWorkerImpl.cpp will get all the code from the WebWorkerDevToolsAgentImpl below, but I think both worker and worker debugger interfaces are thin enough to be combined in a single class. Adding David (+Dimich) to get the Worker's perspective.

[Dmitry Titov]

(In reply to comment #3)
> I'd suggest that you inline these methods into WebWorker + inline corresponding client methods into WebWorkerClient. I understand that WebWorkerImpl.cpp will get all the code from the WebWorkerDevToolsAgentImpl below, but I think both worker and worker debugger interfaces are thin enough to be combined in a single class.

I agree these interfaces, as in this patch, look thin enough to just add them into WebWorker/WebWorkerClient (or, actually, WebCommonWorkerClient),  with some method renaming. (WebWorker::connect() would be too ambiguous)

We already have methods like WebCommonWorkerClient::postConsoleMessageToWorkerObject() - I think these would be consumed by these new interfaces as well, right?

[Yury Semikhatsky]

(In reply to comment #4)
> (In reply to comment #3)
> > I'd suggest that you inline these methods into WebWorker + inline corresponding client methods into WebWorkerClient. I understand that WebWorkerImpl.cpp will get all the code from the WebWorkerDevToolsAgentImpl below, but I think both worker and worker debugger interfaces are thin enough to be combined in a single class.
> 
> I agree these interfaces, as in this patch, look thin enough to just add them into WebWorker/WebWorkerClient (or, actually, WebCommonWorkerClient),  with some method renaming. (WebWorker::connect() would be too ambiguous)
> 
> We already have methods like WebCommonWorkerClient::postConsoleMessageToWorkerObject() - I think these would be consumed by these new interfaces as well, right?
It depends on how we proceed with console implementation in worker context. Once we have inspector agents in WorkerContext it may make sense to collect the messages in the worker process and send them to the page inspector only when inspector window is opened.

[Dmitry Titov]

Comment on attachment 92419 [details]
Patch

Per discussion here, the patch could use some re-factoring. r- for now.

[Yury Semikhatsky]

Created attachment 94228 [details]
Patch

[WebKit Review Bot]

Comment on attachment 94228 [details]
Patch

Attachment 94228 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/8719421

[Yury Semikhatsky]

Created attachment 94422 [details]
Patch for landing

[Yury Semikhatsky]

Committed r87074: <http://trac.webkit.org/changeset/87074>