Bug 32314

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 Flags
patch
none
patch
pfeldman: review+
patch pfeldman: review+

Description Yury Semikhatsky 2009-12-09 01:55:41 PST
Currently devtools message passing interface allows to send messages with exactly 3 params, class and method names. Having a class describing message payload would add more flexibility.
Comment 1 Yury Semikhatsky 2009-12-09 02:17:40 PST
Created attachment 44525 [details]
patch
Comment 2 WebKit Review Bot 2009-12-09 02:21:36 PST
style-queue ran check-webkit-style on attachment 44525 [details] without any errors.
Comment 3 Yury Semikhatsky 2009-12-09 02:57:23 PST
Created attachment 44528 [details]
patch
Comment 4 WebKit Review Bot 2009-12-09 03:02:36 PST
style-queue ran check-webkit-style on attachment 44528 [details] without any errors.
Comment 5 Pavel Feldman 2009-12-09 03:18:26 PST
Comment on attachment 44528 [details]
patch

> +    virtual void sendMessageToFrontend(const WebDevToolsMessageData&) {}

nit: { } here and everywhere else.

>  
> +template <typename T> class WebVector;
> +

remove this.
Comment 6 Yury Semikhatsky 2009-12-09 05:21:30 PST
(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
Comment 7 Yury Semikhatsky 2009-12-09 06:55:47 PST
Reopening the bug since we need to add methods for dispatching devtools messages on the IO thread that accept WebDevToolsMessageData parameter.
Comment 8 Yury Semikhatsky 2009-12-09 06:56:34 PST
Created attachment 44534 [details]
patch
Comment 9 Pavel Feldman 2009-12-09 06:57:51 PST
Comment on attachment 44534 [details]
patch

Add TODO to remove the original one.
Comment 10 Yury Semikhatsky 2009-12-09 07:05:43 PST
(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 11 Darin Fisher (:fishd, Google) 2009-12-09 07:48:31 PST
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?
Comment 12 Pavel Feldman 2009-12-09 08:03:04 PST
(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?