Merge sync and async code paths for getting context menus

https://bugs.webkit.org/show_bug.cgi?id=181423

Reviewed by Joseph Pecoraro.

What a mess.  We had a code path for asynchronous context menu generation and a different one for synchronous context menu generation.
This makes it so there is just one.  At the API level we see if there is an asynchronous delegate to call, then synchronous.
There is a subtle theoretical change in behaviour because m_page.contextMenuClient().showContextMenu is now called for the asynchronous
case and it wasn't before, but the one C API client that uses this has nullptr as it's WKPageShowContextMenuCallback, so we won't break anything!

* UIProcess/API/APIContextMenuClient.h:
(API::ContextMenuClient::getContextMenuFromProposedMenu):
(API::ContextMenuClient::getContextMenuFromProposedMenuAsync): Deleted.
* UIProcess/API/C/WKPage.cpp:
(WKPageSetPageContextMenuClient):
* UIProcess/API/glib/WebKitContextMenuClient.cpp:
* UIProcess/WebContextMenuProxy.h:
* UIProcess/gtk/WebContextMenuProxyGtk.cpp:
(WebKit::WebContextMenuProxyGtk::show):
(WebKit::WebContextMenuProxyGtk::showContextMenuWithItems):
* UIProcess/gtk/WebContextMenuProxyGtk.h:
* UIProcess/mac/WebContextMenuProxyMac.h:
* UIProcess/mac/WebContextMenuProxyMac.mm:
(WebKit::WebContextMenuProxyMac::showContextMenuWithItems):
(WebKit::WebContextMenuProxyMac::showContextMenu):
* UIProcess/wpe/WebContextMenuProxyWPE.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@227001 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Source/WebKit/CMakeLists.txt b/Source/WebKit/CMakeLists.txt
index cee99ed..b17f854 100644 (file)
--- a/Source/WebKit/CMakeLists.txt
+++ b/Source/WebKit/CMakeLists.txt
@@ -312,6 +312,7 @@ set(WebKit_SOURCES
     UIProcess/WebContextClient.cpp
     UIProcess/WebContextConnectionClient.cpp
     UIProcess/WebContextInjectedBundleClient.cpp
+    UIProcess/WebContextMenuListenerProxy.cpp
     UIProcess/WebContextMenuProxy.cpp
     UIProcess/WebCookieManagerProxy.cpp
     UIProcess/WebCookieManagerProxyClient.cpp

diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
index 14c15ab..bcbd1f0 100644 (file)
--- a/Source/WebKit/ChangeLog
+++ b/Source/WebKit/ChangeLog
@@ -1,3 +1,32 @@
+2018-01-16  Alex Christensen  <achristensen@webkit.org>
+
+        Merge sync and async code paths for getting context menus
+        https://bugs.webkit.org/show_bug.cgi?id=181423
+
+        Reviewed by Joseph Pecoraro.
+
+        What a mess.  We had a code path for asynchronous context menu generation and a different one for synchronous context menu generation.
+        This makes it so there is just one.  At the API level we see if there is an asynchronous delegate to call, then synchronous.
+        There is a subtle theoretical change in behaviour because m_page.contextMenuClient().showContextMenu is now called for the asynchronous
+        case and it wasn't before, but the one C API client that uses this has nullptr as it's WKPageShowContextMenuCallback, so we won't break anything!
+
+        * UIProcess/API/APIContextMenuClient.h:
+        (API::ContextMenuClient::getContextMenuFromProposedMenu):
+        (API::ContextMenuClient::getContextMenuFromProposedMenuAsync): Deleted.
+        * UIProcess/API/C/WKPage.cpp:
+        (WKPageSetPageContextMenuClient):
+        * UIProcess/API/glib/WebKitContextMenuClient.cpp:
+        * UIProcess/WebContextMenuProxy.h:
+        * UIProcess/gtk/WebContextMenuProxyGtk.cpp:
+        (WebKit::WebContextMenuProxyGtk::show):
+        (WebKit::WebContextMenuProxyGtk::showContextMenuWithItems):
+        * UIProcess/gtk/WebContextMenuProxyGtk.h:
+        * UIProcess/mac/WebContextMenuProxyMac.h:
+        * UIProcess/mac/WebContextMenuProxyMac.mm:
+        (WebKit::WebContextMenuProxyMac::showContextMenuWithItems):
+        (WebKit::WebContextMenuProxyMac::showContextMenu):
+        * UIProcess/wpe/WebContextMenuProxyWPE.h:
+
 2018-01-16  Michael Catanzaro  <mcatanzaro@igalia.com>
 
         Don't link WebKit target directly to JavaScriptCore

diff --git a/Source/WebKit/PlatformMac.cmake b/Source/WebKit/PlatformMac.cmake
index 9ab6c4a..78896c5 100644 (file)
--- a/Source/WebKit/PlatformMac.cmake
+++ b/Source/WebKit/PlatformMac.cmake
@@ -168,7 +168,6 @@ list(APPEND WebKit_SOURCES
 
     UIProcess/HighPerformanceGraphicsUsageSampler.cpp
     UIProcess/PerActivityStateCPUUsageSampler.cpp
-    UIProcess/WebContextMenuListenerProxy.cpp
     UIProcess/WebResourceLoadStatisticsStore.cpp
     UIProcess/WebResourceLoadStatisticsTelemetry.cpp
 

diff --git a/Source/WebKit/UIProcess/API/APIContextMenuClient.h b/Source/WebKit/UIProcess/API/APIContextMenuClient.h
index ba1e36b..2fe2494 100644 (file)
--- a/Source/WebKit/UIProcess/API/APIContextMenuClient.h
+++ b/Source/WebKit/UIProcess/API/APIContextMenuClient.h
@@ -27,6 +27,7 @@
 
 #if ENABLE(CONTEXT_MENUS)
 
+#include "WebContextMenuItem.h"
 #include "WebContextMenuListenerProxy.h"
 #include "WebHitTestResultData.h"
 #include <WebKit/WKBase.h>
@@ -40,7 +41,6 @@ class IntPoint;
 }
 
 namespace WebKit {
-class WebContextMenuItem;
 class WebContextMenuItemData;
 class WebPageProxy;
 }
@@ -51,8 +51,7 @@ class ContextMenuClient {
 public:
     virtual ~ContextMenuClient() { }
 
-    virtual bool getContextMenuFromProposedMenu(WebKit::WebPageProxy&, const Vector<Ref<WebKit::WebContextMenuItem>>& /* proposedMenu */, Vector<Ref<WebKit::WebContextMenuItem>>& /* customMenu */, const WebKit::WebHitTestResultData&, API::Object* /* userData */) { return false; }
-    virtual bool getContextMenuFromProposedMenuAsync(WebKit::WebPageProxy&, const Vector<Ref<WebKit::WebContextMenuItem>>& /* proposedMenu */, WebKit::WebContextMenuListenerProxy*, const WebKit::WebHitTestResultData&, API::Object* /* userData */) { return false; }
+    virtual void getContextMenuFromProposedMenu(WebKit::WebPageProxy&, Vector<Ref<WebKit::WebContextMenuItem>>&& proposedMenu, WebKit::WebContextMenuListenerProxy& listener, const WebKit::WebHitTestResultData&, API::Object* /* userData */) { listener.useContextMenuItems(WTFMove(proposedMenu)); }
     virtual void customContextMenuItemSelected(WebKit::WebPageProxy&, const WebKit::WebContextMenuItemData&) { }
     virtual bool showContextMenu(WebKit::WebPageProxy&, const WebCore::IntPoint&, const Vector<Ref<WebKit::WebContextMenuItem>>&) { return false; }
     virtual bool hideContextMenu(WebKit::WebPageProxy&) { return false; }

diff --git a/Source/WebKit/UIProcess/API/C/WKPage.cpp b/Source/WebKit/UIProcess/API/C/WKPage.cpp
index 7669d7b..af2bded 100644 (file)
--- a/Source/WebKit/UIProcess/API/C/WKPage.cpp
+++ b/Source/WebKit/UIProcess/API/C/WKPage.cpp
@@ -818,13 +818,29 @@ void WKPageSetPageContextMenuClient(WKPageRef pageRef, const WKPageContextMenuCl
         }
 
     private:
-        bool getContextMenuFromProposedMenu(WebPageProxy& page, const Vector<Ref<WebKit::WebContextMenuItem>>& proposedMenuVector, Vector<Ref<WebKit::WebContextMenuItem>>& customMenu, const WebHitTestResultData& hitTestResultData, API::Object* userData) override
+        void getContextMenuFromProposedMenu(WebPageProxy& page, Vector<Ref<WebKit::WebContextMenuItem>>&& proposedMenuVector, WebKit::WebContextMenuListenerProxy& contextMenuListener, const WebHitTestResultData& hitTestResultData, API::Object* userData) override
         {
-            if (!m_client.getContextMenuFromProposedMenu && !m_client.getContextMenuFromProposedMenu_deprecatedForUseWithV0)
-                return false;
+            if (m_client.base.version >= 4 && m_client.getContextMenuFromProposedMenuAsync) {
+                Vector<RefPtr<API::Object>> proposedMenuItems;
+                proposedMenuItems.reserveInitialCapacity(proposedMenuVector.size());
+                
+                for (const auto& menuItem : proposedMenuVector)
+                    proposedMenuItems.uncheckedAppend(menuItem.ptr());
+                
+                auto webHitTestResult = API::HitTestResult::create(hitTestResultData);
+                m_client.getContextMenuFromProposedMenuAsync(toAPI(&page), toAPI(API::Array::create(WTFMove(proposedMenuItems)).ptr()), toAPI(&contextMenuListener), toAPI(webHitTestResult.ptr()), toAPI(userData), m_client.base.clientInfo);
+                return;
+            }
+            
+            if (!m_client.getContextMenuFromProposedMenu && !m_client.getContextMenuFromProposedMenu_deprecatedForUseWithV0) {
+                contextMenuListener.useContextMenuItems(WTFMove(proposedMenuVector));
+                return;
+            }
 
-            if (m_client.base.version >= 2 && !m_client.getContextMenuFromProposedMenu)
-                return false;
+            if (m_client.base.version >= 2 && !m_client.getContextMenuFromProposedMenu) {
+                contextMenuListener.useContextMenuItems(WTFMove(proposedMenuVector));
+                return;
+            }
 
             Vector<RefPtr<API::Object>> proposedMenuItems;
             proposedMenuItems.reserveInitialCapacity(proposedMenuVector.size());
@@ -841,9 +857,9 @@ void WKPageSetPageContextMenuClient(WKPageRef pageRef, const WKPageContextMenuCl
 
             RefPtr<API::Array> array = adoptRef(toImpl(newMenu));
 
-            customMenu.clear();
-
+            Vector<Ref<WebContextMenuItem>> customMenu;
             size_t newSize = array ? array->size() : 0;
+            customMenu.reserveInitialCapacity(newSize);
             for (size_t i = 0; i < newSize; ++i) {
                 WebContextMenuItem* item = array->at<WebContextMenuItem>(i);
                 if (!item) {
@@ -851,27 +867,10 @@ void WKPageSetPageContextMenuClient(WKPageRef pageRef, const WKPageContextMenuCl
                     continue;
                 }
 
-                customMenu.append(*item);
+                customMenu.uncheckedAppend(*item);
             }
 
-            return true;
-        }
-
-        bool getContextMenuFromProposedMenuAsync(WebPageProxy& page, const Vector<Ref<WebKit::WebContextMenuItem>>& proposedMenuVector, WebKit::WebContextMenuListenerProxy* contextMenuListener, const WebHitTestResultData& hitTestResultData, API::Object* userData) override
-        {
-            if (m_client.base.version < 4 || !m_client.getContextMenuFromProposedMenuAsync)
-                return false;
-
-            Vector<RefPtr<API::Object>> proposedMenuItems;
-            proposedMenuItems.reserveInitialCapacity(proposedMenuVector.size());
-
-            for (const auto& menuItem : proposedMenuVector)
-                proposedMenuItems.uncheckedAppend(menuItem.ptr());
-
-            RefPtr<API::HitTestResult> webHitTestResult = API::HitTestResult::create(hitTestResultData);
-            m_client.getContextMenuFromProposedMenuAsync(toAPI(&page), toAPI(API::Array::create(WTFMove(proposedMenuItems)).ptr()), toAPI(contextMenuListener), toAPI(webHitTestResult.get()), toAPI(userData), m_client.base.clientInfo);
-
-            return true;
+            contextMenuListener.useContextMenuItems(WTFMove(customMenu));
         }
 
         void customContextMenuItemSelected(WebPageProxy& page, const WebContextMenuItemData& itemData) override

diff --git a/Source/WebKit/UIProcess/API/glib/WebKitContextMenuClient.cpp b/Source/WebKit/UIProcess/API/glib/WebKitContextMenuClient.cpp
index bfea5f9..97e9256 100644 (file)
--- a/Source/WebKit/UIProcess/API/glib/WebKitContextMenuClient.cpp
+++ b/Source/WebKit/UIProcess/API/glib/WebKitContextMenuClient.cpp
@@ -34,7 +34,7 @@ public:
     }
 
 private:
-    bool getContextMenuFromProposedMenu(WebPageProxy&, const Vector<Ref<WebContextMenuItem>>& proposedMenu, Vector<Ref<WebContextMenuItem>>&, const WebHitTestResultData& hitTestResultData, API::Object* userData) override
+    void getContextMenuFromProposedMenu(WebPageProxy&, Vector<Ref<WebKit::WebContextMenuItem>>&& proposedMenu, WebKit::WebContextMenuListenerProxy& contextMenuListener, const WebHitTestResultData& hitTestResultData, API::Object* userData) override
     {
         GRefPtr<GVariant> variant;
         if (userData) {
@@ -48,7 +48,7 @@ private:
         for (auto& item : proposedMenu)
             menuItems.uncheckedAppend(item->data());
         webkitWebViewPopulateContextMenu(m_webView, menuItems, hitTestResultData, variant.get());
-        return true;
+        contextMenuListener.useContextMenuItems({ });
     }
 
     WebKitWebView* m_webView;

diff --git a/Source/WebKit/UIProcess/WebContextMenuProxy.h b/Source/WebKit/UIProcess/WebContextMenuProxy.h
index e68912c..3f05f43 100644 (file)
--- a/Source/WebKit/UIProcess/WebContextMenuProxy.h
+++ b/Source/WebKit/UIProcess/WebContextMenuProxy.h
@@ -41,7 +41,7 @@ public:
 
     virtual void show() = 0;
 
-    virtual void showContextMenuWithItems(const Vector<Ref<WebContextMenuItem>>&) = 0;
+    virtual void showContextMenuWithItems(Vector<Ref<WebContextMenuItem>>&&) = 0;
 
 protected:
     WebContextMenuProxy(ContextMenuContextData&&, const UserData&);

diff --git a/Source/WebKit/UIProcess/gtk/WebContextMenuProxyGtk.cpp b/Source/WebKit/UIProcess/gtk/WebContextMenuProxyGtk.cpp
index 39738fb..3bb3bc9 100644 (file)
--- a/Source/WebKit/UIProcess/gtk/WebContextMenuProxyGtk.cpp
+++ b/Source/WebKit/UIProcess/gtk/WebContextMenuProxyGtk.cpp
@@ -149,14 +149,11 @@ void WebContextMenuProxyGtk::show()
             proposedAPIItems.append(WebContextMenuItem::create(item));
     }
 
-    Vector<Ref<WebContextMenuItem>> clientItems;
-    bool useProposedItems = true;
-
-    if (m_page->contextMenuClient().getContextMenuFromProposedMenu(*m_page, proposedAPIItems, clientItems, m_context.webHitTestResultData(), m_page->process().transformHandlesToObjects(m_userData.object()).get()))
-        useProposedItems = false;
-
-    const Vector<Ref<WebContextMenuItem>>& items = useProposedItems ? proposedAPIItems : clientItems;
+    m_page->contextMenuClient().getContextMenuFromProposedMenu(*m_page, WTFMove(proposedAPIItems), WebContextMenuListenerProxy::create(this).get(), m_context.webHitTestResultData(), m_page->process().transformHandlesToObjects(m_userData.object()).get());
+}
 
+void WebContextMenuProxyGtk::showContextMenuWithItems(Vector<Ref<WebContextMenuItem>>&& items)
+{
     if (!items.isEmpty())
         populate(items);
 
@@ -171,12 +168,7 @@ void WebContextMenuProxyGtk::show()
     NativeWebMouseEvent* mouseEvent = m_page->currentlyProcessedMouseDownEvent();
     const GdkEvent* event = mouseEvent ? mouseEvent->nativeEvent() : 0;
     gtk_menu_attach_to_widget(m_menu, GTK_WIDGET(m_webView), nullptr);
-    gtk_menu_popup(m_menu, nullptr, nullptr, reinterpret_cast<GtkMenuPositionFunc>(menuPositionFunction), this,
-                   event ? event->button.button : 3, event ? event->button.time : GDK_CURRENT_TIME);
-}
-
-void WebContextMenuProxyGtk::showContextMenuWithItems(const Vector<Ref<WebContextMenuItem>>&)
-{
+    gtk_menu_popup(m_menu, nullptr, nullptr, reinterpret_cast<GtkMenuPositionFunc>(menuPositionFunction), this, event ? event->button.button : 3, event ? event->button.time : GDK_CURRENT_TIME);
 }
 
 WebContextMenuProxyGtk::WebContextMenuProxyGtk(GtkWidget* webView, WebPageProxy& page, ContextMenuContextData&& context, const UserData& userData)

diff --git a/Source/WebKit/UIProcess/gtk/WebContextMenuProxyGtk.h b/Source/WebKit/UIProcess/gtk/WebContextMenuProxyGtk.h
index 84b988c..3cb4135 100644 (file)
--- a/Source/WebKit/UIProcess/gtk/WebContextMenuProxyGtk.h
+++ b/Source/WebKit/UIProcess/gtk/WebContextMenuProxyGtk.h
@@ -55,7 +55,7 @@ public:
 private:
     WebContextMenuProxyGtk(GtkWidget*, WebPageProxy&, ContextMenuContextData&&, const UserData&);
     void show() override;
-    void showContextMenuWithItems(const Vector<Ref<WebContextMenuItem>>&) override;
+    void showContextMenuWithItems(Vector<Ref<WebContextMenuItem>>&&) override;
     void append(GMenu*, const WebContextMenuItemGlib&);
     GRefPtr<GMenu> buildMenu(const Vector<WebContextMenuItemGlib>&);
     void populate(const Vector<Ref<WebContextMenuItem>>&);

diff --git a/Source/WebKit/UIProcess/mac/WebContextMenuProxyMac.h b/Source/WebKit/UIProcess/mac/WebContextMenuProxyMac.h
index d7deb6e..3fa606b 100644 (file)
--- a/Source/WebKit/UIProcess/mac/WebContextMenuProxyMac.h
+++ b/Source/WebKit/UIProcess/mac/WebContextMenuProxyMac.h
@@ -54,7 +54,7 @@ public:
     ~WebContextMenuProxyMac();
 
     void contextMenuItemSelected(const WebContextMenuItemData&);
-    void showContextMenuWithItems(const Vector<Ref<WebContextMenuItem>>&) override;
+    void showContextMenuWithItems(Vector<Ref<WebContextMenuItem>>&&) override;
 
 #if ENABLE(SERVICE_CONTROLS)
     void clearServicesMenu();

diff --git a/Source/WebKit/UIProcess/mac/WebContextMenuProxyMac.mm b/Source/WebKit/UIProcess/mac/WebContextMenuProxyMac.mm
index 792258c..9781574 100644 (file)
--- a/Source/WebKit/UIProcess/mac/WebContextMenuProxyMac.mm
+++ b/Source/WebKit/UIProcess/mac/WebContextMenuProxyMac.mm
@@ -451,8 +451,14 @@ RetainPtr<NSMenuItem> WebContextMenuProxyMac::createContextMenuItem(const WebCon
     }
 }
 
-void WebContextMenuProxyMac::showContextMenuWithItems(const Vector<Ref<WebContextMenuItem>>& items)
+void WebContextMenuProxyMac::showContextMenuWithItems(Vector<Ref<WebContextMenuItem>>&& items)
 {
+    if (m_page.contextMenuClient().showContextMenu(m_page, m_context.menuLocation(), items))
+        return;
+
+    if (items.isEmpty())
+        return;
+
     Vector<WebContextMenuItemData> data;
     data.reserveInitialCapacity(items.size());
     for (auto& item : items)
@@ -478,9 +484,6 @@ void WebContextMenuProxyMac::showContextMenu()
     for (auto& item : m_context.menuItems())
         proposedAPIItems.append(WebContextMenuItem::create(item));
 
-    Vector<Ref<WebContextMenuItem>> clientItems;
-    bool useProposedItems = true;
-
     if (m_contextMenuListener) {
         m_contextMenuListener->invalidate();
         m_contextMenuListener = nullptr;
@@ -488,22 +491,7 @@ void WebContextMenuProxyMac::showContextMenu()
 
     m_contextMenuListener = WebContextMenuListenerProxy::create(this);
 
-    if (m_page.contextMenuClient().getContextMenuFromProposedMenuAsync(m_page, proposedAPIItems, m_contextMenuListener.get(), m_context.webHitTestResultData(), m_page.process().transformHandlesToObjects(m_userData.object()).get()))
-        return;
-
-    // FIXME: Get rid of these two client calls once we don't need to support the C SPI.
-    if (m_page.contextMenuClient().getContextMenuFromProposedMenu(m_page, proposedAPIItems, clientItems, m_context.webHitTestResultData(), m_page.process().transformHandlesToObjects(m_userData.object()).get()))
-        useProposedItems = false;
-
-    if (m_page.contextMenuClient().showContextMenu(m_page, m_context.menuLocation(), useProposedItems ? proposedAPIItems : clientItems))
-        return;
-
-    auto&& items = WTFMove(useProposedItems ? proposedAPIItems : clientItems);
-    
-    if (items.isEmpty())
-        return;
-
-    showContextMenuWithItems(items);
+    m_page.contextMenuClient().getContextMenuFromProposedMenu(m_page, WTFMove(proposedAPIItems), *m_contextMenuListener, m_context.webHitTestResultData(), m_page.process().transformHandlesToObjects(m_userData.object()).get());
 }
 
 NSWindow *WebContextMenuProxyMac::window() const

diff --git a/Source/WebKit/UIProcess/wpe/WebContextMenuProxyWPE.h b/Source/WebKit/UIProcess/wpe/WebContextMenuProxyWPE.h
index 4986b66..8b2d3a3 100644 (file)
--- a/Source/WebKit/UIProcess/wpe/WebContextMenuProxyWPE.h
+++ b/Source/WebKit/UIProcess/wpe/WebContextMenuProxyWPE.h
@@ -36,7 +36,7 @@ public:
         return adoptRef(*new WebContextMenuProxyWPE(WTFMove(context), userData));
     }
 
-    void showContextMenuWithItems(const Vector<Ref<WebContextMenuItem>>&) final { }
+    void showContextMenuWithItems(Vector<Ref<WebContextMenuItem>>&&) final { }
     void show() final { };
 
 private: