Merge sync and async code paths for getting context menus
authorachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 16 Jan 2018 21:58:34 +0000 (21:58 +0000)
committerachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 16 Jan 2018 21:58:34 +0000 (21:58 +0000)
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

12 files changed:
Source/WebKit/CMakeLists.txt
Source/WebKit/ChangeLog
Source/WebKit/PlatformMac.cmake
Source/WebKit/UIProcess/API/APIContextMenuClient.h
Source/WebKit/UIProcess/API/C/WKPage.cpp
Source/WebKit/UIProcess/API/glib/WebKitContextMenuClient.cpp
Source/WebKit/UIProcess/WebContextMenuProxy.h
Source/WebKit/UIProcess/gtk/WebContextMenuProxyGtk.cpp
Source/WebKit/UIProcess/gtk/WebContextMenuProxyGtk.h
Source/WebKit/UIProcess/mac/WebContextMenuProxyMac.h
Source/WebKit/UIProcess/mac/WebContextMenuProxyMac.mm
Source/WebKit/UIProcess/wpe/WebContextMenuProxyWPE.h

index cee99edad2a841e9e00698de784b82196c6c9cdd..b17f854c001eac808560af5fbb9d5c811075ba01 100644 (file)
@@ -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
index 14c15ab7dd568fd632ec00348693981fb46f8aa1..bcbd1f0c72925a476c020a551dfbe06642c8c4c7 100644 (file)
@@ -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
index 9ab6c4a5c830d7d2fc3951fdcd092d05d089a628..78896c57b756fd14be55d48b05c1e00e7a6dcc3f 100644 (file)
@@ -168,7 +168,6 @@ list(APPEND WebKit_SOURCES
 
     UIProcess/HighPerformanceGraphicsUsageSampler.cpp
     UIProcess/PerActivityStateCPUUsageSampler.cpp
-    UIProcess/WebContextMenuListenerProxy.cpp
     UIProcess/WebResourceLoadStatisticsStore.cpp
     UIProcess/WebResourceLoadStatisticsTelemetry.cpp
 
index ba1e36b1f8834d8c874e5899b970e73eafc83e4c..2fe2494a2cc619b1cf5d99ed5b2c0e43017436fa 100644 (file)
@@ -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; }
index 7669d7bcac9e0decaff01506a78a897a0add2b58..af2bded7398e35f1af9e82e07b868d842c57467a 100644 (file)
@@ -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
index bfea5f96ad87160ff8f28b44f6a62265634809b5..97e9256017737b7665acc8eb2f358eb2ec5900b9 100644 (file)
@@ -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;
index e68912cb8948ca11c0588d9b4cf388950d229833..3f05f4395c08656ee60c1a467c6fe809115a0210 100644 (file)
@@ -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&);
index 39738fb8a79408113eb0a100448820b8a48ba423..3bb3bc9931389e03a6bbc5eab05be635292e889f 100644 (file)
@@ -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)
index 84b988cb79c8d40fd2f861a794396fd99c9bf8d0..3cb41352ea6fceb4468034e36c66a45daa7b4c22 100644 (file)
@@ -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>>&);
index d7deb6e6341e5135b8a973f9eeb2344ea0ffa5fe..3fa606b2096516cd31555ed5aef38ef63b32f457 100644 (file)
@@ -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();
index 792258c778181764476f7c68a878422d7669c931..97815749bddcb093881e88be71767074ae6f8c73 100644 (file)
@@ -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
index 4986b66d301180cc3fe82c6ac935a234fbd1b329..8b2d3a36ce4ec14b638a7789938ef0b14a36eab1 100644 (file)
@@ -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: