Bug 205065 - Leverage the fact that WebPageProxy subclasses IPC::MessageSender to simplify IPC sending code
Summary: Leverage the fact that WebPageProxy subclasses IPC::MessageSender to simplify...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-12-10 08:53 PST by Chris Dumez
Modified: 2019-12-10 14:30 PST (History)
11 users (show)

See Also:

Attachments
Patch (121.57 KB, patch)
2019-12-10 09:05 PST, Chris Dumez 		no flags Details | Formatted Diff | Diff
Patch (127.36 KB, patch)
2019-12-10 09:25 PST, Chris Dumez 		no flags Details | Formatted Diff | Diff
Patch (126.55 KB, patch)
2019-12-10 12:00 PST, Chris Dumez 		no flags Details | Formatted Diff | Diff
Patch (135.22 KB, patch)
2019-12-10 12:36 PST, Chris Dumez 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2019-12-10 08:53:34 PST 
Leverage the fact that WebPageProxy subclasses IPC::MessageSender to simplify IPC sending code.
Comment 1 Chris Dumez 2019-12-10 09:05:41 PST 
Created attachment 385268 [details]
Patch
Comment 2 Chris Dumez 2019-12-10 09:25:42 PST 
Created attachment 385271 [details]
Patch
Comment 3 Chris Dumez 2019-12-10 12:00:46 PST 
Created attachment 385292 [details]
Patch
Comment 4 Alex Christensen 2019-12-10 12:10:48 PST 
Comment on attachment 385292 [details]
Patch

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

r=me with one important comment and one less important comment.

> Source/WebKit/Platform/IPC/MessageSender.h:83
> +        messageSenderConnection()->sendWithAsyncReply(WTFMove(message), WTFMove(completionHandler), destinationID, sendOptions);

What if the connection is null?  This should probably call sendMessage like AuxiliaryProcessProxy::sendWithAsyncReply does.

> Source/WebKit/UIProcess/WebPageProxy.h:511
> -    void systemPreviewActionTriggered(const WebCore::SystemPreviewInfo&, const String&) const;
> +    void systemPreviewActionTriggered(const WebCore::SystemPreviewInfo&, const String&);

Why not make send const?
Comment 5 Chris Dumez 2019-12-10 12:20:03 PST 
Comment on attachment 385292 [details]
Patch

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

>> Source/WebKit/Platform/IPC/MessageSender.h:83
>> +        messageSenderConnection()->sendWithAsyncReply(WTFMove(message), WTFMove(completionHandler), destinationID, sendOptions);
> 
> What if the connection is null?  This should probably call sendMessage like AuxiliaryProcessProxy::sendWithAsyncReply does.

Ok, I will look into this.

>> Source/WebKit/UIProcess/WebPageProxy.h:511
>> +    void systemPreviewActionTriggered(const WebCore::SystemPreviewInfo&, const String&);
> 
> Why not make send const?

If const, it was building because MessageSender::send() is not marked as const.
Comment 6 Chris Dumez 2019-12-10 12:36:33 PST 
Created attachment 385296 [details]
Patch
Comment 7 Chris Dumez 2019-12-10 12:42:27 PST 
(In reply to Chris Dumez from comment #5)
> Comment on attachment 385292 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=385292&action=review
> 
> >> Source/WebKit/Platform/IPC/MessageSender.h:83
> >> +        messageSenderConnection()->sendWithAsyncReply(WTFMove(message), WTFMove(completionHandler), destinationID, sendOptions);
> > 
> > What if the connection is null?  This should probably call sendMessage like AuxiliaryProcessProxy::sendWithAsyncReply does.
> 
> Ok, I will look into this.
> 
> >> Source/WebKit/UIProcess/WebPageProxy.h:511
> >> +    void systemPreviewActionTriggered(const WebCore::SystemPreviewInfo&, const String&);
> > 
> > Why not make send const?
> 
> If const, it was building because MessageSender::send() is not marked as
> const.

it was *NOT* building
Comment 8 Chris Dumez 2019-12-10 14:29:46 PST 
Comment on attachment 385296 [details]
Patch

Clearing flags on attachment: 385296

Committed r253346: <https://trac.webkit.org/changeset/253346>
Comment 9 Chris Dumez 2019-12-10 14:29:48 PST 
All reviewed patches have been landed.  Closing bug.
Comment 10 Radar WebKit Bug Importer 2019-12-10 14:30:59 PST 
<rdar://problem/57808886>