RESOLVED FIXED 60267
[Chromium] Web Inspector: provide initial implementation of worker inspector transport
https://bugs.webkit.org/show_bug.cgi?id=60267
Summary [Chromium] Web Inspector: provide initial implementation of worker inspector ...
Yury Semikhatsky
Reported 2011-05-05 08:19:26 PDT
[Chromium] Web Inspector: provide initial implementation of worker inspector transport
Attachments
Patch (28.28 KB, patch)
2011-05-05 08:29 PDT, Yury Semikhatsky
no flags
Patch (28.19 KB, patch)
2011-05-05 08:32 PDT, Yury Semikhatsky
no flags
Patch (26.01 KB, patch)
2011-05-20 08:57 PDT, Yury Semikhatsky
pfeldman: review+
webkit.review.bot: commit-queue-
Patch for landing (32.84 KB, patch)
2011-05-23 07:18 PDT, Yury Semikhatsky
no flags
Yury Semikhatsky
Comment 1 2011-05-05 08:29:13 PDT
Yury Semikhatsky
Comment 2 2011-05-05 08:32:31 PDT
Pavel Feldman
Comment 3 2011-05-17 08:15:30 PDT
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
Comment 4 2011-05-17 11:13:37 PDT
(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
Comment 5 2011-05-17 11:32:02 PDT
(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 6 2011-05-17 11:56:56 PDT
Comment on attachment 92419 [details] Patch Per discussion here, the patch could use some re-factoring. r- for now.
Yury Semikhatsky
Comment 7 2011-05-20 08:57:54 PDT
WebKit Review Bot
Comment 8 2011-05-20 10:22:01 PDT
Yury Semikhatsky
Comment 9 2011-05-23 07:18:56 PDT
Created attachment 94422 [details] Patch for landing
Yury Semikhatsky
Comment 10 2011-05-23 08:08:20 PDT
Note You need to log in before you can comment on or make changes to this bug.