Title: [243972] releases/WebKitGTK/webkit-2.24
Revision
243972
Author
carlo...@webkit.org
Date
2019-04-08 03:13:59 -0700 (Mon, 08 Apr 2019)

Log Message

Merge r242788 - [WPE][GTK] Load events may occur in unexpected order when JS redirects page before subresource load finishes
https://bugs.webkit.org/show_bug.cgi?id=194131

Source/WebKit:

Reviewed by Michael Catanzaro.

Ensure we emit the load-failed and load-changed with finished event when there's still an ongoing load when a
new provisional load strarts. Previous load fails with cancelled error.

* UIProcess/API/glib/WebKitWebView.cpp:
(webkitWebViewWillStartLoad): Call webkitWebViewLoadFailed() if current page load state is not finished.
* UIProcess/API/glib/WebKitWebViewPrivate.h:
* UIProcess/API/gtk/PageClientImpl.cpp:
(WebKit::PageClientImpl::didStartProvisionalLoadForMainFrame): Call webkitWebViewWillStartLoad().
* UIProcess/API/wpe/APIViewClient.h:
(API::ViewClient::willStartLoad): Add willStartLoad() to API::ViewClient
* UIProcess/API/wpe/PageClientImpl.cpp:
(WebKit::PageClientImpl::didStartProvisionalLoadForMainFrame): Call WPEView::willStartLoad().
* UIProcess/API/wpe/PageClientImpl.h:
* UIProcess/API/wpe/WPEView.cpp:
(WKWPE::View::willStartLoad): Call API::ViewClient::willStartLoad().
* UIProcess/API/wpe/WPEView.h:
* UIProcess/PageLoadState.h:
(WebKit::PageLoadState::isProvisional const):
(WebKit::PageLoadState::isCommitted const):
(WebKit::PageLoadState::isFinished const):

Tools:

Patch by Michael Catanzaro <mcatanz...@igalia.com> on 2019-03-12
Reviewed by Michael Catanzaro.

* TestWebKitAPI/Tests/WebKitGLib/TestLoaderClient.cpp:
(uriChanged):
(testUnfinishedSubresourceLoad):
(serverCallback):
(beforeAll):

Modified Paths

Diff

Modified: releases/WebKitGTK/webkit-2.24/Source/WebKit/ChangeLog (243971 => 243972)


--- releases/WebKitGTK/webkit-2.24/Source/WebKit/ChangeLog	2019-04-08 09:09:01 UTC (rev 243971)
+++ releases/WebKitGTK/webkit-2.24/Source/WebKit/ChangeLog	2019-04-08 10:13:59 UTC (rev 243972)
@@ -1,3 +1,31 @@
+2019-03-12  Carlos Garcia Campos  <cgar...@igalia.com>
+
+        [WPE][GTK] Load events may occur in unexpected order when JS redirects page before subresource load finishes
+        https://bugs.webkit.org/show_bug.cgi?id=194131
+
+        Reviewed by Michael Catanzaro.
+
+        Ensure we emit the load-failed and load-changed with finished event when there's still an ongoing load when a
+        new provisional load strarts. Previous load fails with cancelled error.
+
+        * UIProcess/API/glib/WebKitWebView.cpp:
+        (webkitWebViewWillStartLoad): Call webkitWebViewLoadFailed() if current page load state is not finished.
+        * UIProcess/API/glib/WebKitWebViewPrivate.h:
+        * UIProcess/API/gtk/PageClientImpl.cpp:
+        (WebKit::PageClientImpl::didStartProvisionalLoadForMainFrame): Call webkitWebViewWillStartLoad().
+        * UIProcess/API/wpe/APIViewClient.h:
+        (API::ViewClient::willStartLoad): Add willStartLoad() to API::ViewClient
+        * UIProcess/API/wpe/PageClientImpl.cpp:
+        (WebKit::PageClientImpl::didStartProvisionalLoadForMainFrame): Call WPEView::willStartLoad().
+        * UIProcess/API/wpe/PageClientImpl.h:
+        * UIProcess/API/wpe/WPEView.cpp:
+        (WKWPE::View::willStartLoad): Call API::ViewClient::willStartLoad().
+        * UIProcess/API/wpe/WPEView.h:
+        * UIProcess/PageLoadState.h:
+        (WebKit::PageLoadState::isProvisional const):
+        (WebKit::PageLoadState::isCommitted const):
+        (WebKit::PageLoadState::isFinished const):
+
 2019-03-26  Philippe Normand  <pnorm...@igalia.com>
 
         [WPE][Qt] Uninitialized racy ViewBackend

Modified: releases/WebKitGTK/webkit-2.24/Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp (243971 => 243972)


--- releases/WebKitGTK/webkit-2.24/Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp	2019-04-08 09:09:01 UTC (rev 243971)
+++ releases/WebKitGTK/webkit-2.24/Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp	2019-04-08 10:13:59 UTC (rev 243972)
@@ -424,6 +424,11 @@
         }
     }
 
+    void willStartLoad(WKWPE::View&) override
+    {
+        webkitWebViewWillStartLoad(m_webView);
+    }
+
     WebKitWebView* m_webView;
 };
 #endif
@@ -2070,6 +2075,18 @@
     return getPage(webView);
 }
 
+void webkitWebViewWillStartLoad(WebKitWebView* webView)
+{
+    // This is called before NavigationClient::didStartProvisionalNavigation(), the page load state hasn't been committed yet.
+    auto& pageLoadState = getPage(webView).pageLoadState();
+    if (pageLoadState.isFinished())
+        return;
+
+    GUniquePtr<GError> error(g_error_new_literal(WEBKIT_NETWORK_ERROR, WEBKIT_NETWORK_ERROR_CANCELLED, _("Load request cancelled")));
+    webkitWebViewLoadFailed(webView, pageLoadState.isProvisional() ? WEBKIT_LOAD_STARTED : WEBKIT_LOAD_COMMITTED,
+        webView->priv->activeURI.data(), error.get());
+}
+
 void webkitWebViewLoadChanged(WebKitWebView* webView, WebKitLoadEvent loadEvent)
 {
     WebKitWebViewPrivate* priv = webView->priv;

Modified: releases/WebKitGTK/webkit-2.24/Source/WebKit/UIProcess/API/glib/WebKitWebViewPrivate.h (243971 => 243972)


--- releases/WebKitGTK/webkit-2.24/Source/WebKit/UIProcess/API/glib/WebKitWebViewPrivate.h	2019-04-08 09:09:01 UTC (rev 243971)
+++ releases/WebKitGTK/webkit-2.24/Source/WebKit/UIProcess/API/glib/WebKitWebViewPrivate.h	2019-04-08 10:13:59 UTC (rev 243972)
@@ -41,6 +41,7 @@
 
 void webkitWebViewCreatePage(WebKitWebView*, Ref<API::PageConfiguration>&&);
 WebKit::WebPageProxy& webkitWebViewGetPage(WebKitWebView*);
+void webkitWebViewWillStartLoad(WebKitWebView*);
 void webkitWebViewLoadChanged(WebKitWebView*, WebKitLoadEvent);
 void webkitWebViewLoadFailed(WebKitWebView*, WebKitLoadEvent, const char* failingURI, GError*);
 void webkitWebViewLoadFailedWithTLSErrors(WebKitWebView*, const char* failingURI, GError*, GTlsCertificateFlags, GTlsCertificate*);

Modified: releases/WebKitGTK/webkit-2.24/Source/WebKit/UIProcess/API/gtk/PageClientImpl.cpp (243971 => 243972)


--- releases/WebKitGTK/webkit-2.24/Source/WebKit/UIProcess/API/gtk/PageClientImpl.cpp	2019-04-08 09:09:01 UTC (rev 243971)
+++ releases/WebKitGTK/webkit-2.24/Source/WebKit/UIProcess/API/gtk/PageClientImpl.cpp	2019-04-08 10:13:59 UTC (rev 243972)
@@ -461,6 +461,9 @@
 
 void PageClientImpl::didStartProvisionalLoadForMainFrame()
 {
+    if (WEBKIT_IS_WEB_VIEW(m_viewWidget))
+        webkitWebViewWillStartLoad(WEBKIT_WEB_VIEW(m_viewWidget));
+
     webkitWebViewBaseDidStartProvisionalLoadForMainFrame(WEBKIT_WEB_VIEW_BASE(m_viewWidget));
 }
 

Modified: releases/WebKitGTK/webkit-2.24/Source/WebKit/UIProcess/API/wpe/APIViewClient.h (243971 => 243972)


--- releases/WebKitGTK/webkit-2.24/Source/WebKit/UIProcess/API/wpe/APIViewClient.h	2019-04-08 09:09:01 UTC (rev 243971)
+++ releases/WebKitGTK/webkit-2.24/Source/WebKit/UIProcess/API/wpe/APIViewClient.h	2019-04-08 10:13:59 UTC (rev 243972)
@@ -43,6 +43,7 @@
 
     virtual void frameDisplayed(WKWPE::View&) { }
     virtual void handleDownloadRequest(WKWPE::View&, WebKit::DownloadProxy&) { }
+    virtual void willStartLoad(WKWPE::View&) { }
 };
 
 } // namespace API

Modified: releases/WebKitGTK/webkit-2.24/Source/WebKit/UIProcess/API/wpe/PageClientImpl.cpp (243971 => 243972)


--- releases/WebKitGTK/webkit-2.24/Source/WebKit/UIProcess/API/wpe/PageClientImpl.cpp	2019-04-08 09:09:01 UTC (rev 243971)
+++ releases/WebKitGTK/webkit-2.24/Source/WebKit/UIProcess/API/wpe/PageClientImpl.cpp	2019-04-08 10:13:59 UTC (rev 243972)
@@ -291,6 +291,11 @@
 {
 }
 
+void PageClientImpl::didStartProvisionalLoadForMainFrame()
+{
+    m_view.willStartLoad();
+}
+
 void PageClientImpl::didFirstVisuallyNonEmptyLayoutForMainFrame()
 {
 }

Modified: releases/WebKitGTK/webkit-2.24/Source/WebKit/UIProcess/API/wpe/PageClientImpl.h (243971 => 243972)


--- releases/WebKitGTK/webkit-2.24/Source/WebKit/UIProcess/API/wpe/PageClientImpl.h	2019-04-08 09:09:01 UTC (rev 243971)
+++ releases/WebKitGTK/webkit-2.24/Source/WebKit/UIProcess/API/wpe/PageClientImpl.h	2019-04-08 10:13:59 UTC (rev 243972)
@@ -112,6 +112,7 @@
     void willRecordNavigationSnapshot(WebBackForwardListItem&) override;
     void didRemoveNavigationGestureSnapshot() override;
 
+    void didStartProvisionalLoadForMainFrame() override;
     void didFirstVisuallyNonEmptyLayoutForMainFrame() override;
     void didFinishLoadForMainFrame() override;
     void didFailLoadForMainFrame() override;

Modified: releases/WebKitGTK/webkit-2.24/Source/WebKit/UIProcess/API/wpe/WPEView.cpp (243971 => 243972)


--- releases/WebKitGTK/webkit-2.24/Source/WebKit/UIProcess/API/wpe/WPEView.cpp	2019-04-08 09:09:01 UTC (rev 243971)
+++ releases/WebKitGTK/webkit-2.24/Source/WebKit/UIProcess/API/wpe/WPEView.cpp	2019-04-08 10:13:59 UTC (rev 243972)
@@ -171,6 +171,11 @@
     m_client->handleDownloadRequest(*this, download);
 }
 
+void View::willStartLoad()
+{
+    m_client->willStartLoad(*this);
+}
+
 void View::setSize(const WebCore::IntSize& size)
 {
     m_size = size;

Modified: releases/WebKitGTK/webkit-2.24/Source/WebKit/UIProcess/API/wpe/WPEView.h (243971 => 243972)


--- releases/WebKitGTK/webkit-2.24/Source/WebKit/UIProcess/API/wpe/WPEView.h	2019-04-08 09:09:01 UTC (rev 243971)
+++ releases/WebKitGTK/webkit-2.24/Source/WebKit/UIProcess/API/wpe/WPEView.h	2019-04-08 10:13:59 UTC (rev 243972)
@@ -59,6 +59,7 @@
     void setClient(std::unique_ptr<API::ViewClient>&&);
     void frameDisplayed();
     void handleDownloadRequest(WebKit::DownloadProxy&);
+    void willStartLoad();
 
     WebKit::WebPageProxy& page() { return *m_pageProxy; }
 

Modified: releases/WebKitGTK/webkit-2.24/Source/WebKit/UIProcess/PageLoadState.h (243971 => 243972)


--- releases/WebKitGTK/webkit-2.24/Source/WebKit/UIProcess/PageLoadState.h	2019-04-08 09:09:01 UTC (rev 243971)
+++ releases/WebKitGTK/webkit-2.24/Source/WebKit/UIProcess/PageLoadState.h	2019-04-08 10:13:59 UTC (rev 243972)
@@ -121,6 +121,9 @@
     void reset(const Transaction::Token&);
 
     bool isLoading() const;
+    bool isProvisional() const { return m_committedState.state == State::Provisional; }
+    bool isCommitted() const { return m_committedState.state == State::Committed; }
+    bool isFinished() const { return m_committedState.state == State::Finished; }
 
     const String& provisionalURL() const { return m_committedState.provisionalURL; }
     const String& url() const { return m_committedState.url; }

Modified: releases/WebKitGTK/webkit-2.24/Tools/ChangeLog (243971 => 243972)


--- releases/WebKitGTK/webkit-2.24/Tools/ChangeLog	2019-04-08 09:09:01 UTC (rev 243971)
+++ releases/WebKitGTK/webkit-2.24/Tools/ChangeLog	2019-04-08 10:13:59 UTC (rev 243972)
@@ -1,3 +1,16 @@
+2019-03-12  Michael Catanzaro  <mcatanz...@igalia.com>
+
+        [WPE][GTK] Load events may occur in unexpected order when JS redirects page before subresource load finishes
+        https://bugs.webkit.org/show_bug.cgi?id=194131
+
+        Reviewed by Michael Catanzaro.
+
+        * TestWebKitAPI/Tests/WebKitGLib/TestLoaderClient.cpp:
+        (uriChanged):
+        (testUnfinishedSubresourceLoad):
+        (serverCallback):
+        (beforeAll):
+
 2019-03-18  Adrian Perez de Castro  <ape...@igalia.com>
 
         [WPE] Tarballs generated with “make dist” cannot build documentation

Modified: releases/WebKitGTK/webkit-2.24/Tools/TestWebKitAPI/Tests/WebKitGLib/TestLoaderClient.cpp (243971 => 243972)


--- releases/WebKitGTK/webkit-2.24/Tools/TestWebKitAPI/Tests/WebKitGLib/TestLoaderClient.cpp	2019-04-08 09:09:01 UTC (rev 243971)
+++ releases/WebKitGTK/webkit-2.24/Tools/TestWebKitAPI/Tests/WebKitGLib/TestLoaderClient.cpp	2019-04-08 10:13:59 UTC (rev 243972)
@@ -240,6 +240,34 @@
     test->waitUntilFinished();
 }
 
+static void uriChanged(WebKitWebView* webView, GParamSpec*, LoadTrackingTest* test)
+{
+    const char* uri = webkit_web_view_get_uri(webView);
+    if (g_str_has_suffix(uri, "/normal"))
+        test->m_activeURI = uri;
+}
+
+static void testUnfinishedSubresourceLoad(LoadTrackingTest* test, gconstpointer)
+{
+    // Verify that LoadFinished occurs even if the next load starts before the
+    // previous load actually finishes.
+    test->loadURI(kServer->getURIForPath("/unfinished-subresource-load").data());
+    auto signalID = g_signal_connect(test->m_webView, "notify::uri", G_CALLBACK(uriChanged), test);
+    test->waitUntilLoadFinished();
+    test->waitUntilLoadFinished();
+    g_signal_handler_disconnect(test->m_webView, signalID);
+
+    Vector<LoadTrackingTest::LoadEvents>& events = test->m_loadEvents;
+    g_assert_cmpint(events.size(), ==, 7);
+    g_assert_cmpint(events[0], ==, LoadTrackingTest::ProvisionalLoadStarted);
+    g_assert_cmpint(events[1], ==, LoadTrackingTest::LoadCommitted);
+    g_assert_cmpint(events[2], ==, LoadTrackingTest::LoadFailed);
+    g_assert_cmpint(events[3], ==, LoadTrackingTest::LoadFinished);
+    g_assert_cmpint(events[4], ==, LoadTrackingTest::ProvisionalLoadStarted);
+    g_assert_cmpint(events[5], ==, LoadTrackingTest::LoadCommitted);
+    g_assert_cmpint(events[6], ==, LoadTrackingTest::LoadFinished);
+}
+
 class ViewURITrackingTest: public LoadTrackingTest {
 public:
     MAKE_GLIB_TEST_FIXTURE(ViewURITrackingTest);
@@ -590,6 +618,16 @@
         "Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!"
         "Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!Testing!</body></html>";
 
+    static const char* unfinishedSubresourceLoadResponseString = "<html><body>"
+        "<img src=""
+        "<script>"
+        "  function run() {"
+        "      location = '/normal';"
+        "  }"
+        "  setInterval(run(), 50);"
+        "</script>"
+        "</body></html>";
+
     if (message->method != SOUP_METHOD_GET) {
         soup_message_set_status(message, SOUP_STATUS_NOT_IMPLEMENTED);
         return;
@@ -625,6 +663,12 @@
     } else if (g_str_equal(path, "/redirect-to-data")) {
         soup_message_set_status(message, SOUP_STATUS_MOVED_PERMANENTLY);
         soup_message_headers_append(message->response_headers, "Location", "data:text/plain;charset=utf-8,data-uri");
+    } else if (g_str_equal(path, "/unfinished-subresource-load")) {
+        soup_message_body_append(message->response_body, SOUP_MEMORY_STATIC, unfinishedSubresourceLoadResponseString, strlen(unfinishedSubresourceLoadResponseString));
+    } else if (g_str_equal(path, "/stall")) {
+        // This request is never unpaused and stalls forever.
+        soup_server_pause_message(server, message);
+        return;
     } else
         soup_message_set_status(message, SOUP_STATUS_NOT_FOUND);
 
@@ -655,6 +699,7 @@
     LoadTrackingTest::add("WebKitWebView", "reload", testWebViewReload);
     LoadTrackingTest::add("WebKitWebView", "history-load", testWebViewHistoryLoad);
     LoadTwiceAndReloadTest::add("WebKitWebView", "load-twice-and-reload", testWebViewLoadTwiceAndReload);
+    LoadTrackingTest::add("WebKitWebView", "unfinished-subresource-load", testUnfinishedSubresourceLoad);
 
     // This test checks that web view notify::uri signal is correctly emitted
     // and the uri is already updated when loader client signals are emitted.
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to