Bug 203074

Summary: Make it possible to not include IPC Messages headers in other headers
Product: WebKit Reporter: Tim Horton <thorton>
Component: New BugsAssignee: Tim Horton <thorton>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, annulen, commit-queue, eric.carlson, ews-watchlist, ggaren, glenn, gyuyoung.kim, jer.noble, krollin, philipj, ryuan.choi, sam, sergio, simon.fraser, tsavell, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Description Tim Horton 2019-10-17 00:25:59 PDT 
Make it possible to not include IPC Messages headers in other headers
Comment 1 Tim Horton 2019-10-17 00:32:46 PDT 
Created attachment 381174 [details]
Patch
Comment 2 Tim Horton 2019-10-17 00:33:38 PDT 
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.
Comment 3 Tim Horton 2019-10-17 00:34:04 PDT 
I guess I have some CMake fixing to do too.
Comment 4 Alex Christensen 2019-10-17 08:39:35 PDT 
Comment on attachment 381174 [details]
Patch

Please also update Source/WebKit/Scripts/Makefile and run it to update the generator tests.
Comment 5 Tim Horton 2019-10-21 22:45:07 PDT 
Created attachment 381510 [details]
Patch
Comment 6 Tim Horton 2019-10-22 01:06:46 PDT 
Created attachment 381522 [details]
Patch
Comment 7 Tim Horton 2019-10-22 01:15:14 PDT 
(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 :(
Comment 8 Tim Horton 2019-10-22 01:19:43 PDT 
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
Comment 9 Alex Christensen 2019-10-22 07:07:40 PDT 
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!
Comment 10 Tim Horton 2019-10-22 10:09:25 PDT 
Created attachment 381557 [details]
Patch
Comment 11 Tim Horton 2019-10-22 10:51:41 PDT 
Created attachment 381564 [details]
Patch
Comment 12 Tim Horton 2019-10-22 11:17:26 PDT 
Created attachment 381568 [details]
Patch
Comment 13 Geoffrey Garen 2019-10-22 11:34:31 PDT 
Comment on attachment 381568 [details]
Patch

r=me
Comment 14 WebKit Commit Bot 2019-10-22 13:02:26 PDT 
Comment on attachment 381568 [details]
Patch

Clearing flags on attachment: 381568

Committed r251445: <https://trac.webkit.org/changeset/251445>
Comment 15 WebKit Commit Bot 2019-10-22 13:02:28 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 16 Radar WebKit Bug Importer 2019-10-22 13:03:27 PDT 
<rdar://problem/56511583>
Comment 17 Tim Horton 2019-10-22 13:09:14 PDT 
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
Comment 18 Truitt Savell 2019-10-22 14:53:20 PDT 
(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
Comment 19 Tim Horton 2019-10-22 15:37:04 PDT 
I'll take a peek!
Comment 20 Tim Horton 2019-10-22 15:59:47 PDT 
(In reply to Tim Horton from comment #19)
> I'll take a peek!

Fixed in r251464