[webkit-reviews] review granted: [Bug 194216] [WPE] Do not use a sync IPC message to send the host FD to the web process : [Attachment 361049] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Carlos Garcia Campos
<cgarcia at igalia.com>'s request for review:
Bug 194216: [WPE] Do not use a sync IPC message to send the host FD to the web
process
https://bugs.webkit.org/show_bug.cgi?id=194216

Attachment 361049: Patch

https://bugs.webkit.org/attachment.cgi?id=361049&action=review




--- Comment #2 from Darin Adler <darin at apple.com> ---
Comment on attachment 361049
  --> https://bugs.webkit.org/attachment.cgi?id=361049
Patch

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

Not thrilled with the use of "FD" in the patch.

> Source/WebKit/UIProcess/API/wpe/PageClientImpl.h:146
> +    IPC::Attachment hostFD();

This should say "override" or "final"; applies even though the underlying
function is a pure virtual. My preference is final, but it seems we have a mix
in this file.

> Source/WebKit/UIProcess/API/wpe/WPEView.h:57
> +    virtual ~View() = default;

I suggest we consider just deleting this line entirely, because this is what
happens if we don’t declare the destructor at all, and I think it’s elegant to
just let that happen. OK this way too, though.

> Source/WebKit/UIProcess/PageClient.h:488
> +#if PLATFORM(WPE)
> +    virtual IPC::Attachment hostFD() = 0;
> +#endif

This is a messy pattern, but I suppose it’s what we are using for lots of other
things above, so it’s not great for me to complain just about this one.