Bug 205065 – Leverage the fact that WebPageProxy subclasses IPC::MessageSender to simplify IPC sending code

• Component: WebKit2
• Status: RESOLVED FIXED

[Chris Dumez]

Leverage the fact that WebPageProxy subclasses IPC::MessageSender to simplify IPC sending code.

[Chris Dumez]

Created attachment 385268 [details]
Patch

[Chris Dumez]

Created attachment 385271 [details]
Patch

[Chris Dumez]

Created attachment 385292 [details]
Patch

[Alex Christensen]

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?

[Chris Dumez]

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.

[Chris Dumez]

Created attachment 385296 [details]
Patch

[Chris Dumez]

(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

[Chris Dumez]

Comment on attachment 385296 [details]
Patch

Clearing flags on attachment: 385296

Committed r253346: <https://trac.webkit.org/changeset/253346>

[Chris Dumez]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/57808886>