Bug 203074 – Make it possible to not include IPC Messages headers in other headers

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Tim Horton]

Make it possible to not include IPC Messages headers in other headers

[Tim Horton]

Created attachment 381174 [details]
Patch

[Tim Horton]

Comment on attachment 381174 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=381174&action=review

> Source/WebKit/Scripts/webkit/messages.py:287
> +def forward_declarations_and_headers_for_replies(receiver):

I think we can de-duplicate a bunch of this code. Need to think about how I want to make it look.

[Tim Horton]

I guess I have some CMake fixing to do too.

[Alex Christensen]

Comment on attachment 381174 [details]
Patch

Please also update Source/WebKit/Scripts/Makefile and run it to update the generator tests.

[Tim Horton]

Created attachment 381510 [details]
Patch

[Tim Horton]

Created attachment 381522 [details]
Patch

[Tim Horton]

(In reply to Alex Christensen from comment #4)
> Comment on attachment 381174 [details]
> Patch
> 
> Please also update Source/WebKit/Scripts/Makefile and run it to update the
> generator tests.

Man, that would have been nice to know about when I was writing this :(

[Tim Horton]

Comment on attachment 381522 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=381522&action=review

> Source/WebKit/DerivedSources.make:212
> +	@echo Generating messages receiver for $*...

huh, don’t know why I pluralized that

> Source/WebKit/Scripts/webkit/LegacyMessages-expected.h:364
> +    using DelayedReply = GetPluginProcessConnectionDelayedReply;    static void send(std::unique_ptr<IPC::Encoder>&&, IPC::Connection&, const IPC::Connection::Handle& connectionHandle);

This is a bug

> Source/WebKit/UIProcess/WebPageProxy.cpp:259
> +    String message;

Shouldn’t do this, gotta give it its own file

[Alex Christensen]

Comment on attachment 381522 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=381522&action=review

>> Source/WebKit/Scripts/webkit/LegacyMessages-expected.h:364
>> +    using DelayedReply = GetPluginProcessConnectionDelayedReply;    static void send(std::unique_ptr<IPC::Encoder>&&, IPC::Connection&, const IPC::Connection::Handle& connectionHandle);
> 
> This is a bug

Hooray tests!

[Tim Horton]

Created attachment 381557 [details]
Patch

[Tim Horton]

Created attachment 381564 [details]
Patch

[Tim Horton]

Created attachment 381568 [details]
Patch

[Geoffrey Garen]

Comment on attachment 381568 [details]
Patch

r=me

[WebKit Commit Bot]

Comment on attachment 381568 [details]
Patch

Clearing flags on attachment: 381568

Committed r251445: <https://trac.webkit.org/changeset/251445>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/56511583>

[Tim Horton]

Comment on attachment 381568 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=381568&action=review

> Source/WebKit/Scripts/Makefile:2
> +	python ./generate-message-receiver.py webkit/test-superclass-messages.in --implementation webkit/MessageReceiverSuperclass-expected.cpp --header webkit/MessagesSuperclass-expected.h --reply-header webkit/MessagesRepliesSuperclassReplies-expected.h

... I typoed the word "Receiver" as "Replies" in the last filename here

[Truitt Savell]

(In reply to Tim Horton from comment #17)
> Comment on attachment 381568 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=381568&action=review
> 
> > Source/WebKit/Scripts/Makefile:2
> > +	python ./generate-message-receiver.py webkit/test-superclass-messages.in --implementation webkit/MessageReceiverSuperclass-expected.cpp --header webkit/MessagesSuperclass-expected.h --reply-header webkit/MessagesRepliesSuperclassReplies-expected.h
> 
> ... I typoed the word "Receiver" as "Replies" in the last filename here

It looks like this change may have caused 4 webkitpy test failures: https://build.webkit.org/builders/Apple%20Mojave%20Release%20WK2%20%28Tests%29/builds/7595

[Tim Horton]

I'll take a peek!

[Tim Horton]

(In reply to Tim Horton from comment #19)
> I'll take a peek!

Fixed in r251464