Bug 32314 – [Chromium] Introduce a class for devtools message data

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

[Yury Semikhatsky]

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.

[Yury Semikhatsky]

Created attachment 44525 [details]
patch

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 44525 [details] without any errors.

[Yury Semikhatsky]

Created attachment 44528 [details]
patch

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 44528 [details] without any errors.

[Pavel Feldman]

Comment on attachment 44528 [details]
patch

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

nit: { } here and everywhere else.

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

remove this.

[Yury Semikhatsky]

(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

[Yury Semikhatsky]

Reopening the bug since we need to add methods for dispatching devtools messages on the IO thread that accept WebDevToolsMessageData parameter.

[Yury Semikhatsky]

Created attachment 44534 [details]
patch

[Pavel Feldman]

Comment on attachment 44534 [details]
patch

Add TODO to remove the original one.

[Yury Semikhatsky]

(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

[Darin Fisher (:fishd, Google)]

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?

[Pavel Feldman]

(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?