Summary: | [Chromium] Introduce a class for devtools message data | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Yury Semikhatsky <yurys> | ||||||||
Component: | Web Inspector (Deprecated) | Assignee: | Yury Semikhatsky <yurys> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | mnaganov, pfeldman, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Attachments: |
|
Description
Yury Semikhatsky
2009-12-09 01:55:41 PST
Created attachment 44525 [details]
patch
style-queue ran check-webkit-style on attachment 44525 [details] without any errors.
Created attachment 44528 [details]
patch
style-queue ran check-webkit-style on attachment 44528 [details] without any errors.
Comment on attachment 44528 [details] patch > + virtual void sendMessageToFrontend(const WebDevToolsMessageData&) {} nit: { } here and everywhere else. > > +template <typename T> class WebVector; > + remove this. (In reply to comment #5) > (From update of attachment 44528 [details]) > > + virtual void sendMessageToFrontend(const WebDevToolsMessageData&) {} > > nit: { } here and everywhere else. > Done. > > > > +template <typename T> class WebVector; > > + > > remove this. Done. Committing to http://svn.webkit.org/repository/webkit/trunk ... C WebKit/chromium/public/WebDevToolsFrontendClient.h => WebKit/chromium/public/WebDevToolsMessageData.h M WebKit/chromium/ChangeLog M WebKit/chromium/WebKit.gyp M WebKit/chromium/public/WebDevToolsAgent.h M WebKit/chromium/public/WebDevToolsAgentClient.h M WebKit/chromium/public/WebDevToolsFrontend.h M WebKit/chromium/public/WebDevToolsFrontendClient.h Committed r51897 Reopening the bug since we need to add methods for dispatching devtools messages on the IO thread that accept WebDevToolsMessageData parameter. Created attachment 44534 [details]
patch
Comment on attachment 44534 [details]
patch
Add TODO to remove the original one.
(In reply to comment #9) > (From update of attachment 44534 [details]) > Add TODO to remove the original one. Done. Committing to http://svn.webkit.org/repository/webkit/trunk ... M WebKit/chromium/ChangeLog M WebKit/chromium/public/WebDevToolsAgent.h M WebKit/chromium/public/WebDevToolsAgentClient.h Committed r51899 Comment on attachment 44534 [details] patch > + WEBKIT_API static bool dispatchMessageFromFrontendOnIOThread(const WebDevToolsMessageData&); It seems unfortunate that we need to mention things like "IOThread" in WebKit. That's an implementation detail of Chromium. Is it possible to avoid specifying details like this that cannot be understood without understanding Chromium? (In reply to comment #11) > (From update of attachment 44534 [details]) > > + WEBKIT_API static bool dispatchMessageFromFrontendOnIOThread(const WebDevToolsMessageData&); > > It seems unfortunate that we need to mention things like "IOThread" in WebKit. > That's > an implementation detail of Chromium. Is it possible to avoid specifying > details like > this that cannot be understood without understanding Chromium? It is named so on purpose. We need to execute these commands asynchronously wrt render thread. In our case it is IO thread that handles async dev tools agent <-> frontend communication. I do value abstraction a lot, but in this case I think we should be more transparent. It is interesting that devtools is the only feature that talks to WebKit/chromium on non-render thread. Or is there any other? |