Consider this code from WebPageProxy::mouseEvent: process()->send(WebPageMessage::MouseEvent, m_pageID, CoreIPC::In(event)); The compiler does not enforce that a WebMouseEvent is passed to CoreIPC::In when the message ID is WebPageMessage::MouseEvent. It's easy to make mistakes which will result in crashes on the other end of the connection. We should make our IPC type-safe.
Anders' current thinking is that messages should be classes whose constructors take arguments of the right types. Each class will have a static const member that is the message ID. So the above code will instead look like: process()->send(MouseEvent(event), m_pageID);
We could presumably generate these message classes from IDL files, though that's not a requirement.
<rdar://problem/8282462>
Here's some proposed syntax from Anders, if we generate these classes: messages WebPage -> WebPageProxy { MouseEvent(WebMouseEvent) RunJavaScriptInMainFrame(WebCore::String, uint64_t) } generates: namespace Message { namespace WebPage { enum { MouseEventID, RunJavaScriptInMainFrameID, } struct MouseEvent : Arguments1<WebMouseEvent> { static const unsigned messageID = Message::WebPage::MouseEventID; explicit MouseEvent(const WebMouseEvent& argument1) : Arguments1(argument1) } { }; struct RunJavaScriptInMainFrame : Arguments2<WebCore::String, uint64_t> { static const unsigned messageID = Message::WebPage::RunJavaScriptInMainFrameID; RunJavaScriptInMainFrame(const WebCore::String& argument1, uint64_t argument2) : Arguments2(argument1, argument2) { } } }
I think the destination and source in the above example are backwards. It should be: messages WebPageProxy -> WebPage {
Created attachment 63762 [details] Patch
Perhaps the MouseEvent constructor should be explicit.
(In reply to comment #4) > struct MouseEvent : Arguments1<WebMouseEvent> { This should be Arguments1<const MouseEvent&> I think. > struct RunJavaScriptInMainFrame : Arguments2<WebCore::String, uint64_t> { And this should be Arguments2<const WebCore::String&, uint64_t>
(In reply to comment #7) > Perhaps the MouseEvent constructor should be explicit. Definitely. (In reply to comment #8) > (In reply to comment #4) > > > struct MouseEvent : Arguments1<WebMouseEvent> { > > This should be Arguments1<const MouseEvent&> I think. > > > struct RunJavaScriptInMainFrame : Arguments2<WebCore::String, uint64_t> { > > And this should be Arguments2<const WebCore::String&, uint64_t> Hm, OK.
(In reply to comment #8) > (In reply to comment #4) > > > struct MouseEvent : Arguments1<WebMouseEvent> { > > This should be Arguments1<const MouseEvent&> I think. Do you mean Arguments1<const WebMouseEvent&>?
(In reply to comment #10) > (In reply to comment #8) > > (In reply to comment #4) > > > > > struct MouseEvent : Arguments1<WebMouseEvent> { > > > > This should be Arguments1<const MouseEvent&> I think. > > Do you mean Arguments1<const WebMouseEvent&>? Yes.
Created attachment 67795 [details] Super-ugly work-in-progress patch (with autogeneration!) Here's another ugly work-in-progress patch. WebPage.messages.in (ugly!) declares all the messages that WebPage receives. generate-messages-header.py generates WebPageMessages.h and generate-message-handler.py generates WebPageMessageHandler.cpp. You have to invoke the scripts manually for now, and I've placed the generated source files in the source tree for convenience. But you can get an idea of where I'm headed. Tips for cleaning up the Python would be much appreciated!
Anders suggested adding a type blacklist to prevent certain types (e.g., enums, unsigned long, etc.) from being used as message parameters.
(In reply to comment #13) > Anders suggested adding a type blacklist to prevent certain types (e.g., enums, unsigned long, etc.) from being used as message parameters. I'm going to handle this separately.
(In reply to comment #14) > (In reply to comment #13) > > Anders suggested adding a type blacklist to prevent certain types (e.g., enums, unsigned long, etc.) from being used as message parameters. > > I'm going to handle this separately. Maybe we should use a whitelist instead, unless your blacklist can handle things like Vector<unsigned long>
Created attachment 68420 [details] Autogenerate WebPage's message-receiving code and message types
Attachment 68420 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebKit2/Platform/CoreIPC/HandleMessage.h:6: More than one command on the same line [whitespace/newline] [4] WebKit2/Platform/CoreIPC/HandleMessage.h:7: More than one command on the same line [whitespace/newline] [4] WebKit2/Scripts/webkit2/messages.py:52: expected 2 blank lines, found 1 [pep8/E302] [5] WebKit2/Scripts/webkit2/messages_unittest.py:127: expected 2 blank lines, found 1 [pep8/E302] [5] Total errors found: 4 in 22 files If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 68420 [details] did not build on qt: Build output: http://queues.webkit.org/results/4071101
Committed r68079: <http://trac.webkit.org/changeset/68079>
(In reply to comment #19) > Committed r68079: <http://trac.webkit.org/changeset/68079> Andras fixed the Qt build: http://trac.webkit.org/changeset/68097 Thx bbandix, you rock. :)