Title: [249890] trunk
Revision
249890
Author
carlo...@webkit.org
Date
2019-09-16 00:49:10 -0700 (Mon, 16 Sep 2019)

Log Message

REGRESSION(r249142): [GTK] Epiphany delayed page loads continue indefinitely
https://bugs.webkit.org/show_bug.cgi?id=201544

Reviewed by Michael Catanzaro.

Source/WebKit:

WebPageProxy::loadAlternateHTML() is an exception, because it's an API request but always sets the navigationID
to 0. We always want to reset the pending API request URL when alternate HTML load starts.

* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::didStartProvisionalLoadForFrameShared): Check also that it's an API alternate HTML load
to reset the pending API request URL.

Tools:

Add new test cases.

* TestWebKitAPI/Tests/WebKitGLib/TestLoaderClient.cpp:
(testWebViewActiveURI):
(testWebViewIsLoading):
* TestWebKitAPI/glib/WebKitGLib/LoadTrackingTest.cpp:
(loadChangedCallback):
(LoadTrackingTest::loadAlternateHTML):
(LoadTrackingTest::reset):
* TestWebKitAPI/glib/WebKitGLib/LoadTrackingTest.h:

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (249889 => 249890)


--- trunk/Source/WebKit/ChangeLog	2019-09-16 05:51:49 UTC (rev 249889)
+++ trunk/Source/WebKit/ChangeLog	2019-09-16 07:49:10 UTC (rev 249890)
@@ -1,3 +1,17 @@
+2019-09-16  Carlos Garcia Campos  <cgar...@igalia.com>
+
+        REGRESSION(r249142): [GTK] Epiphany delayed page loads continue indefinitely
+        https://bugs.webkit.org/show_bug.cgi?id=201544
+
+        Reviewed by Michael Catanzaro.
+
+        WebPageProxy::loadAlternateHTML() is an exception, because it's an API request but always sets the navigationID
+        to 0. We always want to reset the pending API request URL when alternate HTML load starts.
+
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::didStartProvisionalLoadForFrameShared): Check also that it's an API alternate HTML load
+        to reset the pending API request URL.
+
 2019-09-15  David Kilzer  <ddkil...@apple.com>
 
         REGRESSION (r248592): Leak of CFDictionaryRef in WebKit::NetworkRTCProvider::proxyInfoFromSession()

Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (249889 => 249890)


--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp	2019-09-16 05:51:49 UTC (rev 249889)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp	2019-09-16 07:49:10 UTC (rev 249890)
@@ -4047,8 +4047,8 @@
     RELEASE_LOG_IF_ALLOWED(Loading, "didStartProvisionalLoadForFrame: frameID = %" PRIu64, frameID.toUInt64());
 
     auto transaction = m_pageLoadState.transaction();
-
-    if (navigation)
+    bool fromAlternateHTMLAPI = !unreachableURL.isEmpty() && unreachableURL == m_pageLoadState.pendingAPIRequestURL();
+    if (navigation || fromAlternateHTMLAPI)
         m_pageLoadState.clearPendingAPIRequest(transaction);
 
     if (frame->isMainFrame()) {

Modified: trunk/Tools/ChangeLog (249889 => 249890)


--- trunk/Tools/ChangeLog	2019-09-16 05:51:49 UTC (rev 249889)
+++ trunk/Tools/ChangeLog	2019-09-16 07:49:10 UTC (rev 249890)
@@ -1,3 +1,21 @@
+2019-09-16  Carlos Garcia Campos  <cgar...@igalia.com>
+
+        REGRESSION(r249142): [GTK] Epiphany delayed page loads continue indefinitely
+        https://bugs.webkit.org/show_bug.cgi?id=201544
+
+        Reviewed by Michael Catanzaro.
+
+        Add new test cases.
+
+        * TestWebKitAPI/Tests/WebKitGLib/TestLoaderClient.cpp:
+        (testWebViewActiveURI):
+        (testWebViewIsLoading):
+        * TestWebKitAPI/glib/WebKitGLib/LoadTrackingTest.cpp:
+        (loadChangedCallback):
+        (LoadTrackingTest::loadAlternateHTML):
+        (LoadTrackingTest::reset):
+        * TestWebKitAPI/glib/WebKitGLib/LoadTrackingTest.h:
+
 2019-09-15  David Kilzer  <ddkil...@apple.com>
 
         block-spammers should obtain credentials the same way as webkit-patch

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitGLib/TestLoaderClient.cpp (249889 => 249890)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitGLib/TestLoaderClient.cpp	2019-09-16 05:51:49 UTC (rev 249889)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitGLib/TestLoaderClient.cpp	2019-09-16 07:49:10 UTC (rev 249890)
@@ -292,6 +292,13 @@
         LoadTrackingTest::loadURI(uri);
     }
 
+    void loadURIAndRedirectOnCommitted(const char* uri, const char* redirectURI)
+    {
+        reset();
+        m_uriToLoadOnCommitted = redirectURI;
+        LoadTrackingTest::loadURI(uri);
+    }
+
     void provisionalLoadStarted()
     {
         m_currentURIList[Provisional] = m_currentURI;
@@ -305,6 +312,11 @@
     void loadCommitted()
     {
         m_currentURIList[Commited] = m_currentURI;
+        if (!m_uriToLoadOnCommitted.isNull()) {
+            m_estimatedProgress = 0;
+            m_activeURI = m_uriToLoadOnCommitted;
+            webkit_web_view_load_uri(m_webView, m_uriToLoadOnCommitted.data());
+        }
     }
 
     void loadFinished()
@@ -311,6 +323,8 @@
     {
         m_currentURIList[Finished] = m_currentURI;
         LoadTrackingTest::loadFinished();
+        if (!m_uriToLoadOnCommitted.isNull())
+            m_uriToLoadOnCommitted = { };
     }
 
     void checkURIAtState(State state, const char* path)
@@ -324,12 +338,14 @@
 private:
     void reset()
     {
-        m_currentURI = CString();
+        m_currentURI = { };
+        m_uriToLoadOnCommitted = { };
         m_currentURIList.clear();
         m_currentURIList.grow(m_currentURIList.capacity());
     }
 
     CString m_currentURI;
+    CString m_uriToLoadOnCommitted;
     Vector<CString, 4> m_currentURIList;
 };
 
@@ -415,6 +431,19 @@
     test->checkURIAtState(ViewURITrackingTest::State::ProvisionalAfterRedirect, "/normal-change-request");
     test->checkURIAtState(ViewURITrackingTest::State::Commited, "/request-changed-on-redirect");
     test->checkURIAtState(ViewURITrackingTest::State::Finished, "/request-changed-on-redirect");
+
+    test->loadURIAndRedirectOnCommitted(kServer->getURIForPath("/normal").data(), kServer->getURIForPath("/headers").data());
+    test->waitUntilLoadFinished();
+    test->checkURIAtState(ViewURITrackingTest::State::Provisional, "/normal");
+    test->checkURIAtState(ViewURITrackingTest::State::ProvisionalAfterRedirect, nullptr);
+    test->checkURIAtState(ViewURITrackingTest::State::Commited, "/normal");
+    // Pending API request is always updated immedately.
+    test->checkURIAtState(ViewURITrackingTest::State::Finished, "/headers");
+    test->waitUntilLoadFinished();
+    test->checkURIAtState(ViewURITrackingTest::State::Provisional, "/headers");
+    test->checkURIAtState(ViewURITrackingTest::State::ProvisionalAfterRedirect, nullptr);
+    test->checkURIAtState(ViewURITrackingTest::State::Commited, "/headers");
+    test->checkURIAtState(ViewURITrackingTest::State::Finished, "/headers");
 }
 
 class ViewIsLoadingTest: public LoadTrackingTest {
@@ -454,23 +483,34 @@
 {
     test->loadURI(kServer->getURIForPath("/normal").data());
     test->waitUntilLoadFinished();
+    g_assert_false(webkit_web_view_is_loading(test->m_webView));
 
     test->reload();
     test->waitUntilLoadFinished();
+    g_assert_false(webkit_web_view_is_loading(test->m_webView));
 
     test->loadURI(kServer->getURIForPath("/error").data());
     test->waitUntilLoadFinished();
+    g_assert_false(webkit_web_view_is_loading(test->m_webView));
 
     test->loadURI(kServer->getURIForPath("/normal").data());
     test->waitUntilLoadFinished();
+    g_assert_false(webkit_web_view_is_loading(test->m_webView));
     test->loadURI(kServer->getURIForPath("/normal2").data());
     test->waitUntilLoadFinished();
+    g_assert_false(webkit_web_view_is_loading(test->m_webView));
 
     test->goBack();
     test->waitUntilLoadFinished();
+    g_assert_false(webkit_web_view_is_loading(test->m_webView));
 
     test->goForward();
     test->waitUntilLoadFinished();
+    g_assert_false(webkit_web_view_is_loading(test->m_webView));
+
+    test->loadAlternateHTML("<html><head><title>Title</title></head></html>", "file:///foo", nullptr);
+    test->waitUntilLoadFinished();
+    g_assert_false(webkit_web_view_is_loading(test->m_webView));
 }
 
 class WebPageURITest: public WebViewTest {

Modified: trunk/Tools/TestWebKitAPI/glib/WebKitGLib/LoadTrackingTest.cpp (249889 => 249890)


--- trunk/Tools/TestWebKitAPI/glib/WebKitGLib/LoadTrackingTest.cpp	2019-09-16 05:51:49 UTC (rev 249889)
+++ trunk/Tools/TestWebKitAPI/glib/WebKitGLib/LoadTrackingTest.cpp	2019-09-16 07:49:10 UTC (rev 249890)
@@ -31,6 +31,7 @@
     case WEBKIT_LOAD_REDIRECTED:
         g_assert_true(webkit_web_view_is_loading(webView));
         test->m_activeURI = webkit_web_view_get_uri(webView);
+        test->m_committedURI = test->m_activeURI;
         if (!test->m_redirectURI.isNull())
             g_assert_cmpstr(test->m_redirectURI.data(), ==, test->m_activeURI.data());
         test->provisionalLoadReceivedServerRedirect();
@@ -48,15 +49,15 @@
         break;
     }
     case WEBKIT_LOAD_FINISHED:
-        if (!test->m_loadFailed) {
-            g_assert_false(webkit_web_view_is_loading(webView));
+        if (!test->m_loadFailed)
             g_assert_cmpstr(test->m_activeURI.data(), ==, webkit_web_view_get_uri(webView));
-        } else if (!g_error_matches(test->m_error.get(), WEBKIT_NETWORK_ERROR, WEBKIT_NETWORK_ERROR_CANCELLED)) {
-            // When a new load is started before the previous one has finished, we receive the load-finished signal
-            // of the ongoing load while we already have a provisional URL for the new load. This is the only case
-            // where isloading is true when the load has finished.
+
+        // When a new load is started before the previous one has finished, we receive the load-finished signal
+        // of the ongoing load while we already have a provisional URL for the new load. This is the only case
+        // where isloading is true when the load has finished.
+        if (test->m_activeURI == test->m_committedURI)
             g_assert_false(webkit_web_view_is_loading(webView));
-        }
+
         test->loadFinished();
         break;
     default:
@@ -224,6 +225,12 @@
     WebViewTest::goForward();
 }
 
+void LoadTrackingTest::loadAlternateHTML(const char* html, const char* contentURI, const char* baseURI)
+{
+    reset();
+    WebViewTest::loadAlternateHTML(html, contentURI, baseURI);
+}
+
 void LoadTrackingTest::reset()
 {
     m_runLoadUntilCompletion = false;
@@ -231,4 +238,5 @@
     m_loadEvents.clear();
     m_estimatedProgress = 0;
     m_error.reset();
+    m_committedURI = { };
 }

Modified: trunk/Tools/TestWebKitAPI/glib/WebKitGLib/LoadTrackingTest.h (249889 => 249890)


--- trunk/Tools/TestWebKitAPI/glib/WebKitGLib/LoadTrackingTest.h	2019-09-16 05:51:49 UTC (rev 249889)
+++ trunk/Tools/TestWebKitAPI/glib/WebKitGLib/LoadTrackingTest.h	2019-09-16 07:49:10 UTC (rev 249890)
@@ -46,6 +46,7 @@
     void reload();
     void goBack();
     void goForward();
+    void loadAlternateHTML(const char* html, const char* contentURI, const char* baseURI);
     void reset();
 
     void setRedirectURI(const char* uri) { m_redirectURI = uri; }
@@ -65,4 +66,5 @@
     Vector<LoadEvents> m_loadEvents;
     float m_estimatedProgress;
     CString m_redirectURI;
+    CString m_committedURI;
 };
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to