WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
43636
Add a type-safe IPC mechanism to WebKit2
https://bugs.webkit.org/show_bug.cgi?id=43636
Summary
Add a type-safe IPC mechanism to WebKit2
Adam Roben (:aroben)
Reported
2010-08-06 12:52:23 PDT
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.
Attachments
Patch
(5.96 KB, patch)
2010-08-06 14:38 PDT
,
Adam Roben (:aroben)
no flags
Details
Formatted Diff
Diff
Super-ugly work-in-progress patch (with autogeneration!)
(52.96 KB, patch)
2010-09-16 07:29 PDT
,
Adam Roben (:aroben)
no flags
Details
Formatted Diff
Diff
Autogenerate WebPage's message-receiving code and message types
(92.11 KB, patch)
2010-09-22 12:47 PDT
,
Adam Roben (:aroben)
andersca
: review+
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Adam Roben (:aroben)
Comment 1
2010-08-06 12:54:25 PDT
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);
Adam Roben (:aroben)
Comment 2
2010-08-06 12:55:58 PDT
We could presumably generate these message classes from IDL files, though that's not a requirement.
Adam Roben (:aroben)
Comment 3
2010-08-06 12:56:55 PDT
<
rdar://problem/8282462
>
Adam Roben (:aroben)
Comment 4
2010-08-06 13:55:50 PDT
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) { } } }
Adam Roben (:aroben)
Comment 5
2010-08-06 14:05:24 PDT
I think the destination and source in the above example are backwards. It should be: messages WebPageProxy -> WebPage {
Adam Roben (:aroben)
Comment 6
2010-08-06 14:38:18 PDT
Created
attachment 63762
[details]
Patch
Sam Weinig
Comment 7
2010-08-06 21:30:22 PDT
Perhaps the MouseEvent constructor should be explicit.
Anders Carlsson
Comment 8
2010-08-07 14:58:23 PDT
(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>
Adam Roben (:aroben)
Comment 9
2010-08-16 07:13:33 PDT
(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.
Adam Roben (:aroben)
Comment 10
2010-09-15 08:42:56 PDT
(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&>?
Anders Carlsson
Comment 11
2010-09-15 08:45:51 PDT
(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.
Adam Roben (:aroben)
Comment 12
2010-09-16 07:29:23 PDT
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!
Adam Roben (:aroben)
Comment 13
2010-09-16 15:06:04 PDT
Anders suggested adding a type blacklist to prevent certain types (e.g., enums, unsigned long, etc.) from being used as message parameters.
Adam Roben (:aroben)
Comment 14
2010-09-22 12:24:05 PDT
(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.
Anders Carlsson
Comment 15
2010-09-22 12:36:43 PDT
(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>
Adam Roben (:aroben)
Comment 16
2010-09-22 12:47:01 PDT
Created
attachment 68420
[details]
Autogenerate WebPage's message-receiving code and message types
WebKit Review Bot
Comment 17
2010-09-22 12:48:52 PDT
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.
Early Warning System Bot
Comment 18
2010-09-22 12:56:20 PDT
Attachment 68420
[details]
did not build on qt: Build output:
http://queues.webkit.org/results/4071101
Adam Roben (:aroben)
Comment 19
2010-09-22 13:50:57 PDT
Committed
r68079
: <
http://trac.webkit.org/changeset/68079
>
Csaba Osztrogonác
Comment 20
2010-09-22 22:21:48 PDT
(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. :)
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug