Changeset 190717 in webkit

Timestamp:

Oct 8, 2015 3:06:41 AM (9 years ago)

Author:

Carlos Garcia Campos

Message:

[GTK] Stop using a nested main loop for popup menus
​https://bugs.webkit.org/show_bug.cgi?id=149920

Reviewed by Sergio Villar Senin.

Source/WebKit2:

WebPageProxy used to expect the popup menus to run in a nested
main loop and invalidated the menu right after showing it. But
this is no longer the case, so there's no reason to keep using
the nested main loop.

• UIProcess/gtk/WebPopupMenuProxyGtk.cpp:

(WebKit::WebPopupMenuProxyGtk::~WebPopupMenuProxyGtk):
(WebKit::WebPopupMenuProxyGtk::cancelTracking):
(WebKit::WebPopupMenuProxyGtk::menuItemActivated):
(WebKit::WebPopupMenuProxyGtk::WebPopupMenuProxyGtk): Deleted.
(WebKit::WebPopupMenuProxyGtk::showPopupMenu): Deleted.
(WebKit::WebPopupMenuProxyGtk::shutdownRunLoop): Deleted.
(WebKit::WebPopupMenuProxyGtk::menuUnmapped): Deleted.

• UIProcess/gtk/WebPopupMenuProxyGtk.h:

(WebKit::WebPopupMenuProxyGtk::setActiveItem): Deleted.

LayoutTests:

Unskip platform/gtk/fast/forms/menulist-typeahead-find.html that
was timing out because of the nested main loop.

• platform/gtk/TestExpectations:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/platform/gtk/TestExpectations (modified) (1 diff)
• Source/WebKit2/ChangeLog (modified) (1 diff)
• Source/WebKit2/UIProcess/gtk/WebPopupMenuProxyGtk.cpp (modified) (6 diffs)
• Source/WebKit2/UIProcess/gtk/WebPopupMenuProxyGtk.h (modified) (3 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2015-10-08  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [GTK] Stop using a nested main loop for popup menus
+        https://bugs.webkit.org/show_bug.cgi?id=149920
+
+        Reviewed by Sergio Villar Senin.
+
+        Unskip platform/gtk/fast/forms/menulist-typeahead-find.html that
+        was timing out because of the nested main loop.
+
+        * platform/gtk/TestExpectations:
+
 2015-10-08  Carlos Garcia Campos  <cgarcia@igalia.com>
 

trunk/LayoutTests/platform/gtk/TestExpectations

@@ -1587 +1587 @@
 Bug(GTK) http/tests/security/storage-blocking-strengthened-private-browsing-plugin.html [ Failure ]
 Bug(GTK) platform/gtk/fast/events/event-sender-metakey.html [ Failure ]
-Bug(GTK) platform/gtk/fast/forms/menulist-typeahead-find.html [ Failure ]
 Bug(GTK) plugins/keyboard-events.html [ Failure ]
 Bug(GTK) plugins/netscape-plugin-setwindow-size-2.html [ Failure ]

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2015-10-08  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [GTK] Stop using a nested main loop for popup menus
+        https://bugs.webkit.org/show_bug.cgi?id=149920
+
+        Reviewed by Sergio Villar Senin.
+
+        WebPageProxy used to expect the popup menus to run in a nested
+        main loop and invalidated the menu right after showing it. But
+        this is no longer the case, so there's no reason to keep using
+        the nested main loop.
+
+        * UIProcess/gtk/WebPopupMenuProxyGtk.cpp:
+        (WebKit::WebPopupMenuProxyGtk::~WebPopupMenuProxyGtk):
+        (WebKit::WebPopupMenuProxyGtk::cancelTracking):
+        (WebKit::WebPopupMenuProxyGtk::menuItemActivated):
+        (WebKit::WebPopupMenuProxyGtk::WebPopupMenuProxyGtk): Deleted.
+        (WebKit::WebPopupMenuProxyGtk::showPopupMenu): Deleted.
+        (WebKit::WebPopupMenuProxyGtk::shutdownRunLoop): Deleted.
+        (WebKit::WebPopupMenuProxyGtk::menuUnmapped): Deleted.
+        * UIProcess/gtk/WebPopupMenuProxyGtk.h:
+        (WebKit::WebPopupMenuProxyGtk::setActiveItem): Deleted.
+
 2015-10-07  Anders Carlsson  <andersca@apple.com>
 

trunk/Source/WebKit2/UIProcess/gtk/WebPopupMenuProxyGtk.cpp

@@ -43 +43 @@
     , m_webView(webView)
     , m_popup(gtk_menu_new())
-    , m_activeItem(-1)
     , m_previousKeyEventCharacter(0)
     , m_previousKeyEventTimestamp(0)
@@ -53 +52 @@
 WebPopupMenuProxyGtk::~WebPopupMenuProxyGtk()
 {
-    if (m_popup) {
-        g_signal_handlers_disconnect_matched(m_popup, G_SIGNAL_MATCH_DATA, 0, 0, 0, 0, this);
-        hidePopupMenu();
-        gtk_widget_destroy(m_popup);
-    }
+    cancelTracking();
 }
 
@@ -128 +123 @@
     }
 
-    gulong unmapHandler = g_signal_connect(m_popup, "unmap", G_CALLBACK(menuUnmapped), this);
-
     const GdkEvent* event = m_client->currentlyProcessedMouseDownEvent() ? m_client->currentlyProcessedMouseDownEvent()->nativeEvent() : nullptr;
     gtk_menu_popup_for_device(GTK_MENU(m_popup), event ? gdk_event_get_device(event) : nullptr, nullptr, nullptr,
@@ -150 +143 @@
        return;
     }
-
-    // WebPageProxy expects the menu to run in a nested run loop, since it invalidates the
-    // menu right after calling WebPopupMenuProxy::showPopupMenu().
-    m_runLoop = adoptGRef(g_main_loop_new(nullptr, FALSE));
-
-// This is to suppress warnings about gdk_threads_leave and gdk_threads_enter.
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-    gdk_threads_leave();
-    g_main_loop_run(m_runLoop.get());
-    gdk_threads_enter();
-#pragma GCC diagnostic pop
-
-    m_runLoop.clear();
-
-    g_signal_handler_disconnect(m_popup, unmapHandler);
-
-    if (!m_client)
-        return;
-
-    m_client->valueChangedForPopupMenu(this, m_activeItem);
 }
 
@@ -177 +149 @@
     gtk_menu_popdown(GTK_MENU(m_popup));
     resetTypeAheadFindState();
+}
+
+void WebPopupMenuProxyGtk::cancelTracking()
+{
+    if (!m_popup)
+        return;
+
+    g_signal_handlers_disconnect_matched(m_popup, G_SIGNAL_MATCH_DATA, 0, 0, nullptr, nullptr, this);
+    hidePopupMenu();
+    gtk_widget_destroy(m_popup);
+    m_popup = nullptr;
 }
 
@@ -257 +240 @@
 }
 
-void WebPopupMenuProxyGtk::shutdownRunLoop()
-{
-    if (g_main_loop_is_running(m_runLoop.get()))
-        g_main_loop_quit(m_runLoop.get());
-}
-
 void WebPopupMenuProxyGtk::menuItemActivated(GtkAction* action, WebPopupMenuProxyGtk* popupMenu)
 {
-    popupMenu->setActiveItem(GPOINTER_TO_INT(g_object_get_data(G_OBJECT(action), "popup-menu-action-index")));
-    popupMenu->shutdownRunLoop();
-}
-
-void WebPopupMenuProxyGtk::menuUnmapped(GtkWidget*, WebPopupMenuProxyGtk* popupMenu)
-{
-    popupMenu->shutdownRunLoop();
+    if (popupMenu->m_client)
+        popupMenu->m_client->valueChangedForPopupMenu(popupMenu, GPOINTER_TO_INT(g_object_get_data(G_OBJECT(action), "popup-menu-action-index")));
 }
 

trunk/Source/WebKit2/UIProcess/gtk/WebPopupMenuProxyGtk.h

@@ -44 +44 @@
     ~WebPopupMenuProxyGtk();
 
-    virtual void showPopupMenu(const WebCore::IntRect&, WebCore::TextDirection, double pageScaleFactor, const Vector<WebPopupItem>&, const PlatformPopupMenuData&, int32_t selectedIndex);
-    virtual void hidePopupMenu();
+    virtual void showPopupMenu(const WebCore::IntRect&, WebCore::TextDirection, double pageScaleFactor, const Vector<WebPopupItem>&, const PlatformPopupMenuData&, int32_t selectedIndex) override;
+    virtual void hidePopupMenu() override;
+    virtual void cancelTracking() override;
 
 private:
     WebPopupMenuProxyGtk(GtkWidget*, WebPopupMenuProxy::Client*);
-    void shutdownRunLoop();
-    void setActiveItem(int activeItem) { m_activeItem = activeItem; }
+
     void setCurrentlySelectedMenuItem(GtkWidget* item) { m_currentlySelectedMenuItem = item; }
     GtkAction* createGtkActionForMenuItem(const WebPopupItem&, int itemIndex);
@@ -59 +59 @@
 
     static void menuItemActivated(GtkAction*, WebPopupMenuProxyGtk*);
-    static void menuUnmapped(GtkWidget*, WebPopupMenuProxyGtk*);
     static void selectItemCallback(GtkWidget*, WebPopupMenuProxyGtk*);
     static gboolean keyPressEventCallback(GtkWidget*, GdkEventKey*, WebPopupMenuProxyGtk*);
@@ -65 +64 @@
     GtkWidget* m_webView;
     GtkWidget* m_popup;
-    int m_activeItem;
-    GRefPtr<GMainLoop> m_runLoop;
 
     // Typeahead find.