[webkit-reviews] review denied: [Bug 180955] Add WKUIDelegatePrivate equivalent of WKPageContextMenuClient getContextMenuFromProposedMenuAsync : [Attachment 338270] Patch

[bugzilla-daemon at webkit.org]

Alex Christensen <achristensen at apple.com> has denied Jeff Miller
<jeffm at apple.com>'s request for review:
Bug 180955: Add WKUIDelegatePrivate equivalent of WKPageContextMenuClient
getContextMenuFromProposedMenuAsync
https://bugs.webkit.org/show_bug.cgi?id=180955

Attachment 338270: Patch

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




--- Comment #6 from Alex Christensen <achristensen at apple.com> ---
Comment on attachment 338270
  --> https://bugs.webkit.org/attachment.cgi?id=338270
Patch

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

> Source/WebKit/UIProcess/API/APIContextMenuClient.h:61
> -    virtual RetainPtr<NSMenu> menuFromProposedMenu(WebKit::WebPageProxy&,
NSMenu *menu, const WebKit::WebHitTestResultData&, API::Object*) { return menu;
}
> +    virtual bool useGetContextMenuFromProposedNSMenu() { return false; }
> +    virtual void getContextMenuFromProposedNSMenu(WebKit::WebPageProxy&,
NSMenu *proposedMenu, WebKit::WebContextMenuListenerProxy& listener,
API::Object* /* userData */) { listener.useContextMenuNSMenu(proposedMenu); }

This feels like the wrong approach.  I think the right approach would make
menuFromProposedMenu return void and have it have a
CompletionHandler<void(RetainPtr<NSMenu>)>&& as its last parameter.

> Source/WebKit/UIProcess/WebContextMenuListenerProxy.cpp:58
> +void WebContextMenuListenerProxy::useContextMenuNSMenu(NSMenu *menu)

The Linux build was indeed failing because it was trying to compile this.

> Source/WebKit/UIProcess/WebContextMenuListenerProxy.h:50
>      void useContextMenuItems(Vector<Ref<WebContextMenuItem>>&&);
> +    void useContextMenuNSMenu(NSMenu *);

It seems like we should package things so we don't need to add another function
here.