[GTK] Back/forward gesture snapshot always times out
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 29 Apr 2019 19:08:18 +0000 (19:08 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 29 Apr 2019 19:08:18 +0000 (19:08 +0000)
https://bugs.webkit.org/show_bug.cgi?id=197233

Patch by Alexander Mikhaylenko <exalm7659@gmail.com> on 2019-04-29
Reviewed by Michael Catanzaro.

Delaying web process launch caused a regression where we create ViewGestureController when the
web process doesn't yet exist. The controller immediately tries to connect to it and fails,
and because of that never receives DidHitRenderTreeSizeThreshold() message, so navigation
snapshot always stays until timeout after performing the gesture.

To prevent this, create the controller in webkitWebViewBaseDidRelaunchWebProcess() instead of
webkitWebViewBaseCreateWebPage(). Additionally, since settings are now created earlier than
ViewGestureController, store the value of whether swipe gesture is enabled in WebKitWebViewBase
and immediately apply it when creating the controller.

Since there is now a point where controller is null, make webkitWebViewBaseViewGestureController()
return null and add null checks everywhere.

* UIProcess/API/glib/WebKitWebView.cpp:
(enableBackForwardNavigationGesturesChanged):
Move the logic into webkitWebViewBaseSetEnableBackForwardNavigationGesture().
* UIProcess/API/gtk/PageClientImpl.cpp:
(WebKit::PageClientImpl::wheelEventWasNotHandledByWebCore): Add a null check.
* UIProcess/API/gtk/WebKitWebViewBase.cpp:
(webkitWebViewBaseDraw): Ditto.
(webkitWebViewBaseScrollEvent): Ditto.
(webkitWebViewBaseSetEnableBackForwardNavigationGesture): Added. In addition to what was in
WebKitWebViewBase::enableBackForwardNavigationGesturesChanged(), store the value in a field
for the case ViewGestureController doesn't exist yet.
(webkitWebViewBaseViewGestureController): Return a pointer instead of reference.
(webkitWebViewBaseCreateWebPage): Stop creating ViewGestureController.
(webkitWebViewBaseDidRelaunchWebProcess): Move creating ViewGestureController here. Also
immediately call setSwipeGestureEnabled() with the stored value.
(webkitWebViewBaseDidStartProvisionalLoadForMainFrame): Add a null check.
(webkitWebViewBaseDidFirstVisuallyNonEmptyLayoutForMainFrame):Ditto.
(webkitWebViewBaseDidFinishLoadForMainFrame): Ditto.
(webkitWebViewBaseDidFailLoadForMainFrame): Ditto.
(webkitWebViewBaseDidSameDocumentNavigationForMainFrame): Ditto.
(webkitWebViewBaseDidRestoreScrollPosition): Ditto.
* UIProcess/API/gtk/WebKitWebViewBasePrivate.h:

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

Source/WebKit/ChangeLog
Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp
Source/WebKit/UIProcess/API/gtk/PageClientImpl.cpp
Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp
Source/WebKit/UIProcess/API/gtk/WebKitWebViewBasePrivate.h

index 44d4f09..174107f 100644 (file)
@@ -1,3 +1,46 @@
+2019-04-29  Alexander Mikhaylenko  <exalm7659@gmail.com>
+
+        [GTK] Back/forward gesture snapshot always times out
+        https://bugs.webkit.org/show_bug.cgi?id=197233
+
+        Reviewed by Michael Catanzaro.
+
+        Delaying web process launch caused a regression where we create ViewGestureController when the
+        web process doesn't yet exist. The controller immediately tries to connect to it and fails,
+        and because of that never receives DidHitRenderTreeSizeThreshold() message, so navigation
+        snapshot always stays until timeout after performing the gesture.
+
+        To prevent this, create the controller in webkitWebViewBaseDidRelaunchWebProcess() instead of
+        webkitWebViewBaseCreateWebPage(). Additionally, since settings are now created earlier than
+        ViewGestureController, store the value of whether swipe gesture is enabled in WebKitWebViewBase
+        and immediately apply it when creating the controller.
+
+        Since there is now a point where controller is null, make webkitWebViewBaseViewGestureController()
+        return null and add null checks everywhere.
+
+        * UIProcess/API/glib/WebKitWebView.cpp:
+        (enableBackForwardNavigationGesturesChanged):
+        Move the logic into webkitWebViewBaseSetEnableBackForwardNavigationGesture().
+        * UIProcess/API/gtk/PageClientImpl.cpp:
+        (WebKit::PageClientImpl::wheelEventWasNotHandledByWebCore): Add a null check.
+        * UIProcess/API/gtk/WebKitWebViewBase.cpp:
+        (webkitWebViewBaseDraw): Ditto.
+        (webkitWebViewBaseScrollEvent): Ditto.
+        (webkitWebViewBaseSetEnableBackForwardNavigationGesture): Added. In addition to what was in
+        WebKitWebViewBase::enableBackForwardNavigationGesturesChanged(), store the value in a field
+        for the case ViewGestureController doesn't exist yet.
+        (webkitWebViewBaseViewGestureController): Return a pointer instead of reference.
+        (webkitWebViewBaseCreateWebPage): Stop creating ViewGestureController.
+        (webkitWebViewBaseDidRelaunchWebProcess): Move creating ViewGestureController here. Also
+        immediately call setSwipeGestureEnabled() with the stored value.
+        (webkitWebViewBaseDidStartProvisionalLoadForMainFrame): Add a null check.
+        (webkitWebViewBaseDidFirstVisuallyNonEmptyLayoutForMainFrame):Ditto.
+        (webkitWebViewBaseDidFinishLoadForMainFrame): Ditto.
+        (webkitWebViewBaseDidFailLoadForMainFrame): Ditto.
+        (webkitWebViewBaseDidSameDocumentNavigationForMainFrame): Ditto.
+        (webkitWebViewBaseDidRestoreScrollPosition): Ditto.
+        * UIProcess/API/gtk/WebKitWebViewBasePrivate.h:
+
 2019-04-29  Chris Dumez  <cdumez@apple.com>
 
         User-facing strings should use curly quotes instead of straight
index af10d68..887f7b3 100644 (file)
@@ -514,11 +514,7 @@ static void userAgentChanged(WebKitSettings* settings, GParamSpec*, WebKitWebVie
 static void enableBackForwardNavigationGesturesChanged(WebKitSettings* settings, GParamSpec*, WebKitWebView* webView)
 {
     gboolean enable = webkit_settings_get_enable_back_forward_navigation_gestures(settings);
-
-    ViewGestureController& controller = webkitWebViewBaseViewGestureController(WEBKIT_WEB_VIEW_BASE(webView));
-    controller.setSwipeGestureEnabled(enable);
-
-    getPage(webView).setShouldRecordNavigationSnapshots(enable);
+    webkitWebViewBaseSetEnableBackForwardNavigationGesture(WEBKIT_WEB_VIEW_BASE(webView), enable);
 }
 
 static void webkitWebViewUpdateFavicon(WebKitWebView* webView, cairo_surface_t* favicon)
index a0adb13..2c98f20 100644 (file)
@@ -431,9 +431,9 @@ void PageClientImpl::doneWithTouchEvent(const NativeWebTouchEvent& event, bool w
 
 void PageClientImpl::wheelEventWasNotHandledByWebCore(const NativeWebWheelEvent& event)
 {
-    ViewGestureController& controller = webkitWebViewBaseViewGestureController(WEBKIT_WEB_VIEW_BASE(m_viewWidget));
-    if (controller.isSwipeGestureEnabled()) {
-        controller.wheelEventWasNotHandledByWebCore(&event.nativeEvent()->scroll);
+    ViewGestureController* controller = webkitWebViewBaseViewGestureController(WEBKIT_WEB_VIEW_BASE(m_viewWidget));
+    if (controller && controller->isSwipeGestureEnabled()) {
+        controller->wheelEventWasNotHandledByWebCore(&event.nativeEvent()->scroll);
         return;
     }
 
index 816f0fa..10ea4b2 100644 (file)
@@ -206,6 +206,7 @@ struct _WebKitWebViewBasePrivate {
     std::unique_ptr<GestureController> gestureController;
 #endif
     std::unique_ptr<ViewGestureController> viewGestureController;
+    bool isBackForwardNavigationGestureEnabled { false };
 };
 
 WEBKIT_DEFINE_TYPE(WebKitWebViewBase, webkit_web_view_base, GTK_TYPE_CONTAINER)
@@ -572,9 +573,9 @@ static gboolean webkitWebViewBaseDraw(GtkWidget* widget, cairo_t* cr)
     }
 
     if (showingNavigationSnapshot) {
-        ViewGestureController& controller = webkitWebViewBaseViewGestureController(webViewBase);
         RefPtr<cairo_pattern_t> group = adoptRef(cairo_pop_group(cr));
-        controller.draw(cr, group.get());
+        if (auto* controller = webkitWebViewBaseViewGestureController(webViewBase))
+            controller->draw(cr, group.get());
     }
 
     GTK_WIDGET_CLASS(webkit_web_view_base_parent_class)->draw(widget, cr);
@@ -889,8 +890,8 @@ static gboolean webkitWebViewBaseScrollEvent(GtkWidget* widget, GdkEventScroll*
         }
     }
 
-    ViewGestureController& controller = webkitWebViewBaseViewGestureController(webViewBase);
-    if (controller.isSwipeGestureEnabled() && controller.handleScrollWheelEvent(event))
+    ViewGestureController* controller = webkitWebViewBaseViewGestureController(webViewBase);
+    if (controller && controller->isSwipeGestureEnabled() && controller->handleScrollWheelEvent(event))
         return GDK_EVENT_STOP;
 
     webkitWebViewBaseHandleWheelEvent(webViewBase, reinterpret_cast<GdkEvent*>(event));
@@ -1189,9 +1190,21 @@ GestureController& webkitWebViewBaseGestureController(WebKitWebViewBase* webView
 }
 #endif
 
-ViewGestureController& webkitWebViewBaseViewGestureController(WebKitWebViewBase* webViewBase)
+void webkitWebViewBaseSetEnableBackForwardNavigationGesture(WebKitWebViewBase* webViewBase, bool enabled)
 {
-    return *webViewBase->priv->viewGestureController;
+    WebKitWebViewBasePrivate* priv = webViewBase->priv;
+
+    priv->isBackForwardNavigationGestureEnabled = enabled;
+
+    if (priv->pageProxy->hasRunningProcess())
+        webViewBase->priv->viewGestureController->setSwipeGestureEnabled(enabled);
+
+    priv->pageProxy->setShouldRecordNavigationSnapshots(enabled);
+}
+
+ViewGestureController* webkitWebViewBaseViewGestureController(WebKitWebViewBase* webViewBase)
+{
+    return webViewBase->priv->viewGestureController.get();
 }
 
 static gboolean webkitWebViewBaseQueryTooltip(GtkWidget* widget, gint /* x */, gint /* y */, gboolean keyboardMode, GtkTooltip* tooltip)
@@ -1420,8 +1433,6 @@ void webkitWebViewBaseCreateWebPage(WebKitWebViewBase* webkitWebViewBase, Ref<AP
     priv->pageProxy->setIntrinsicDeviceScaleFactor(gtk_widget_get_scale_factor(GTK_WIDGET(webkitWebViewBase)));
     g_signal_connect(webkitWebViewBase, "notify::scale-factor", G_CALLBACK(deviceScaleFactorChanged), nullptr);
 #endif
-
-    priv->viewGestureController = std::make_unique<WebKit::ViewGestureController>(*priv->pageProxy);
 }
 
 void webkitWebViewBaseSetTooltipText(WebKitWebViewBase* webViewBase, const char* tooltip)
@@ -1639,11 +1650,12 @@ void webkitWebViewBaseDidRelaunchWebProcess(WebKitWebViewBase* webkitWebViewBase
     // Queue a resize to ensure the new DrawingAreaProxy is resized.
     gtk_widget_queue_resize_no_redraw(GTK_WIDGET(webkitWebViewBase));
 
+    WebKitWebViewBasePrivate* priv = webkitWebViewBase->priv;
+
 #if PLATFORM(X11) && USE(TEXTURE_MAPPER_GL) && !USE(REDIRECTED_XCOMPOSITE_WINDOW)
     if (PlatformDisplay::sharedDisplay().type() != PlatformDisplay::Type::X11)
         return;
 
-    WebKitWebViewBasePrivate* priv = webkitWebViewBase->priv;
     auto* drawingArea = static_cast<DrawingAreaProxyCoordinatedGraphics*>(priv->pageProxy->drawingArea());
     ASSERT(drawingArea);
 
@@ -1655,6 +1667,9 @@ void webkitWebViewBaseDidRelaunchWebProcess(WebKitWebViewBase* webkitWebViewBase
 #else
     UNUSED_PARAM(webkitWebViewBase);
 #endif
+
+    priv->viewGestureController = std::make_unique<WebKit::ViewGestureController>(*priv->pageProxy);
+    priv->viewGestureController->setSwipeGestureEnabled(priv->isBackForwardNavigationGestureEnabled);
 }
 
 void webkitWebViewBasePageClosed(WebKitWebViewBase* webkitWebViewBase)
@@ -1700,36 +1715,42 @@ RefPtr<WebKit::ViewSnapshot> webkitWebViewBaseTakeViewSnapshot(WebKitWebViewBase
 
 void webkitWebViewBaseDidStartProvisionalLoadForMainFrame(WebKitWebViewBase* webkitWebViewBase)
 {
-    if (webkitWebViewBase->priv->viewGestureController->isSwipeGestureEnabled())
-        webkitWebViewBase->priv->viewGestureController->didStartProvisionalLoadForMainFrame();
+    ViewGestureController* controller = webkitWebViewBaseViewGestureController(webkitWebViewBase);
+    if (controller && controller->isSwipeGestureEnabled())
+        controller->didStartProvisionalLoadForMainFrame();
 }
 
 void webkitWebViewBaseDidFirstVisuallyNonEmptyLayoutForMainFrame(WebKitWebViewBase* webkitWebViewBase)
 {
-    if (webkitWebViewBase->priv->viewGestureController->isSwipeGestureEnabled())
-        webkitWebViewBase->priv->viewGestureController->didFirstVisuallyNonEmptyLayoutForMainFrame();
+    ViewGestureController* controller = webkitWebViewBaseViewGestureController(webkitWebViewBase);
+    if (controller && controller->isSwipeGestureEnabled())
+        controller->didFirstVisuallyNonEmptyLayoutForMainFrame();
 }
 
 void webkitWebViewBaseDidFinishLoadForMainFrame(WebKitWebViewBase* webkitWebViewBase)
 {
-    if (webkitWebViewBase->priv->viewGestureController->isSwipeGestureEnabled())
-        webkitWebViewBase->priv->viewGestureController->didFinishLoadForMainFrame();
+    ViewGestureController* controller = webkitWebViewBaseViewGestureController(webkitWebViewBase);
+    if (controller && controller->isSwipeGestureEnabled())
+        controller->didFinishLoadForMainFrame();
 }
 
 void webkitWebViewBaseDidFailLoadForMainFrame(WebKitWebViewBase* webkitWebViewBase)
 {
-    if (webkitWebViewBase->priv->viewGestureController->isSwipeGestureEnabled())
-        webkitWebViewBase->priv->viewGestureController->didFailLoadForMainFrame();
+    ViewGestureController* controller = webkitWebViewBaseViewGestureController(webkitWebViewBase);
+    if (controller && controller->isSwipeGestureEnabled())
+        controller->didFailLoadForMainFrame();
 }
 
 void webkitWebViewBaseDidSameDocumentNavigationForMainFrame(WebKitWebViewBase* webkitWebViewBase, SameDocumentNavigationType type)
 {
-    if (webkitWebViewBase->priv->viewGestureController->isSwipeGestureEnabled())
-        webkitWebViewBase->priv->viewGestureController->didSameDocumentNavigationForMainFrame(type);
+    ViewGestureController* controller = webkitWebViewBaseViewGestureController(webkitWebViewBase);
+    if (controller && controller->isSwipeGestureEnabled())
+        controller->didSameDocumentNavigationForMainFrame(type);
 }
 
 void webkitWebViewBaseDidRestoreScrollPosition(WebKitWebViewBase* webkitWebViewBase)
 {
-    if (webkitWebViewBase->priv->viewGestureController->isSwipeGestureEnabled())
+    ViewGestureController* controller = webkitWebViewBaseViewGestureController(webkitWebViewBase);
+    if (controller && controller->isSwipeGestureEnabled())
         webkitWebViewBase->priv->viewGestureController->didRestoreScrollPosition();
 }
index 1360285..54fe01e 100644 (file)
@@ -84,7 +84,8 @@ WebKit::GestureController& webkitWebViewBaseGestureController(WebKitWebViewBase*
 
 RefPtr<WebKit::ViewSnapshot> webkitWebViewBaseTakeViewSnapshot(WebKitWebViewBase*);
 
-WebKit::ViewGestureController& webkitWebViewBaseViewGestureController(WebKitWebViewBase*);
+void webkitWebViewBaseSetEnableBackForwardNavigationGesture(WebKitWebViewBase*, bool enabled);
+WebKit::ViewGestureController* webkitWebViewBaseViewGestureController(WebKitWebViewBase*);
 
 void webkitWebViewBaseDidStartProvisionalLoadForMainFrame(WebKitWebViewBase*);
 void webkitWebViewBaseDidFirstVisuallyNonEmptyLayoutForMainFrame(WebKitWebViewBase*);