Attachment 368465 Details for Bug 197233 – Patch

[patch] Patch

bug-197233-20190429222129.patch (text/plain), 13.44 KB, created by Alice Mikhaylenko on 2019-04-29 10:21:31 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Alice Mikhaylenko

Created: 2019-04-29 10:21:31 PDT

Size: 13.44 KB

patch

obsolete

>Subversion Revision: 244732
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 17580430691d9fbd10fd7fe8f4361bd7bf5de868..c5d35cd4b4e8d99ce88c49744df8be4a9862439c 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -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 NOBODY (OOPS!).
>+
>+        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  Commit Queue  <commit-queue@webkit.org>
> 
>         Unreviewed, rolling out r244648.
>diff --git a/Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp b/Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp
>index af10d684c1d03b5d684c0b1bde96a84dd7ba17e0..887f7b31c0e6d3aed12fc103d490ea5ea0b37973 100644
>--- a/Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp
>+++ b/Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp
>@@ -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)
>diff --git a/Source/WebKit/UIProcess/API/gtk/PageClientImpl.cpp b/Source/WebKit/UIProcess/API/gtk/PageClientImpl.cpp
>index a0adb134a448e5822f08719139117fd27be11205..2c98f20c2f81340813a80074cf62d9c623d70938 100644
>--- a/Source/WebKit/UIProcess/API/gtk/PageClientImpl.cpp
>+++ b/Source/WebKit/UIProcess/API/gtk/PageClientImpl.cpp
>@@ -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;
>     }
> 
>diff --git a/Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp b/Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp
>index 816f0fa769f2bed40068bf6e75b17e91bfac7593..10ea4b22cd6d3fd8025c77a231546ea1dc329c32 100644
>--- a/Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp
>+++ b/Source/WebKit/UIProcess/API/gtk/WebKitWebViewBase.cpp
>@@ -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();
> }
>diff --git a/Source/WebKit/UIProcess/API/gtk/WebKitWebViewBasePrivate.h b/Source/WebKit/UIProcess/API/gtk/WebKitWebViewBasePrivate.h
>index 1360285bfa236b4c35c7444f259e1698f4a0451f..54fe01e5c309a29489a8de92262dc897c71695aa 100644
>--- a/Source/WebKit/UIProcess/API/gtk/WebKitWebViewBasePrivate.h
>+++ b/Source/WebKit/UIProcess/API/gtk/WebKitWebViewBasePrivate.h
>@@ -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*);

Actions: View | Formatted Diff | Diff

Attachments on bug 197233: 368125 | 368465