Title: [285089] trunk
Revision
285089
Author
wenson_hs...@apple.com
Date
2021-10-30 20:19:15 -0700 (Sat, 30 Oct 2021)

Log Message

Layer tree should not be stuck in frozen state after explicitly stopping a page load
https://bugs.webkit.org/show_bug.cgi?id=232532
rdar://84522357

Reviewed by Tim Horton.

Source/WebCore:

Add WEBCORE_EXPORT to a method. See WebKit/ChangeLog for more details.

Test: http/tests/navigation/unfreeze-layer-tree-after-stopping-load.html

* loader/FrameLoader.h:

Source/WebKit:

>From diagnostic logs gathered in rdar://84522357, it's apparently possible for the layer tree to be stuck in a
state where it's indefinitely frozen after the user explicitly stops the page load (e.g. by tapping on the "x"
button in Safari). Specifically, these logs indicate that the `PageTransition` layer tree freeze reason may
persist even after the user has canceled the load, leading to situations where content may be visible but
unresponsive.

>From code inspection, this may happen for multiple reasons, one of which is that a media element has incremented
the load event delay count, which in turn prevents us from transitioning to completed state inside
`FrameLoader::checkLoadCompleteForThisFrame()`. It's unknown whether this (in particular) is the cause of
unresponsiveness observed by the reporter of rdar://84522357, but we can at least use this in order to come up
with a layout test; to mitigate all instances where the `PageTransition` reason may persist after the load has
been stopped, we make `WebPage::stopLoading()` complete the page transition (thereby lifting the freeze reason)
if needed.

* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::stopLoading):

Tools:

Add support for `TestRunner.stopLoading()`, which immediately stops the page load, emulating what would normally
happen if the user taps or clicks the "x" button to cancel loading in Safari. See the layout test for more
details.

* DumpRenderTree/TestRunner.cpp:
(stopLoadingCallback):
(TestRunner::staticFunctions):
* DumpRenderTree/TestRunner.h:
* DumpRenderTree/mac/TestRunnerMac.mm:
(TestRunner::stopLoading):
* DumpRenderTree/win/TestRunnerWin.cpp:
(TestRunner::stopLoading):
* WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl:
* WebKitTestRunner/InjectedBundle/TestRunner.cpp:
(WTR::TestRunner::stopLoading):
* WebKitTestRunner/InjectedBundle/TestRunner.h:

LayoutTests:

Add a test that verifies that a `requestAnimationFrame()` callback fires even after stopping page load. This
test fails without the fix in this patch, since the layer tree ends up in a permanently frozen state.

* http/tests/navigation/unfreeze-layer-tree-after-stopping-load-expected.txt: Added.
* http/tests/navigation/unfreeze-layer-tree-after-stopping-load.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (285088 => 285089)


--- trunk/LayoutTests/ChangeLog	2021-10-31 02:37:24 UTC (rev 285088)
+++ trunk/LayoutTests/ChangeLog	2021-10-31 03:19:15 UTC (rev 285089)
@@ -1,3 +1,17 @@
+2021-10-30  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        Layer tree should not be stuck in frozen state after explicitly stopping a page load
+        https://bugs.webkit.org/show_bug.cgi?id=232532
+        rdar://84522357
+
+        Reviewed by Tim Horton.
+
+        Add a test that verifies that a `requestAnimationFrame()` callback fires even after stopping page load. This
+        test fails without the fix in this patch, since the layer tree ends up in a permanently frozen state.
+
+        * http/tests/navigation/unfreeze-layer-tree-after-stopping-load-expected.txt: Added.
+        * http/tests/navigation/unfreeze-layer-tree-after-stopping-load.html: Added.
+
 2021-10-30  Myles C. Maxfield  <mmaxfi...@apple.com>
 
         [GPU Process] Small ImageBuffers cause the web process to crash

Added: trunk/LayoutTests/http/tests/navigation/unfreeze-layer-tree-after-stopping-load-expected.txt (0 => 285089)


--- trunk/LayoutTests/http/tests/navigation/unfreeze-layer-tree-after-stopping-load-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/http/tests/navigation/unfreeze-layer-tree-after-stopping-load-expected.txt	2021-10-31 03:19:15 UTC (rev 285089)
@@ -0,0 +1 @@
+This test requires WebKitTestRunner/DumpRenderTree.

Added: trunk/LayoutTests/http/tests/navigation/unfreeze-layer-tree-after-stopping-load.html (0 => 285089)


--- trunk/LayoutTests/http/tests/navigation/unfreeze-layer-tree-after-stopping-load.html	                        (rev 0)
+++ trunk/LayoutTests/http/tests/navigation/unfreeze-layer-tree-after-stopping-load.html	2021-10-31 03:19:15 UTC (rev 285089)
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<body>
+This test requires WebKitTestRunner/DumpRenderTree.
+<video width="1px" height="1px" src=""
+<script>
+testRunner.dumpAsText();
+testRunner.waitUntilDone();
+
+document.querySelector("video").play().catch(() => { });
+
+if (testRunner.stopLoading)
+    testRunner.stopLoading();
+
+requestAnimationFrame(() => testRunner.forceImmediateCompletion());
+</script>
+</body>
\ No newline at end of file

Modified: trunk/Source/WebCore/ChangeLog (285088 => 285089)


--- trunk/Source/WebCore/ChangeLog	2021-10-31 02:37:24 UTC (rev 285088)
+++ trunk/Source/WebCore/ChangeLog	2021-10-31 03:19:15 UTC (rev 285089)
@@ -1,3 +1,17 @@
+2021-10-30  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        Layer tree should not be stuck in frozen state after explicitly stopping a page load
+        https://bugs.webkit.org/show_bug.cgi?id=232532
+        rdar://84522357
+
+        Reviewed by Tim Horton.
+
+        Add WEBCORE_EXPORT to a method. See WebKit/ChangeLog for more details.
+
+        Test: http/tests/navigation/unfreeze-layer-tree-after-stopping-load.html
+
+        * loader/FrameLoader.h:
+
 2021-10-30  Chris Dumez  <cdu...@apple.com>
 
         [ BigSur wk2 Debug arm64 ] webaudio/AudioBufferSource/audiobuffersource-playbackrate.html is a flaky crash

Modified: trunk/Source/WebCore/loader/FrameLoader.h (285088 => 285089)


--- trunk/Source/WebCore/loader/FrameLoader.h	2021-10-31 02:37:24 UTC (rev 285088)
+++ trunk/Source/WebCore/loader/FrameLoader.h	2021-10-31 03:19:15 UTC (rev 285089)
@@ -298,7 +298,7 @@
 
     const URL& previousURL() const { return m_previousURL; }
 
-    void completePageTransitionIfNeeded();
+    WEBCORE_EXPORT void completePageTransitionIfNeeded();
 
     void setOverrideCachePolicyForTesting(ResourceRequestCachePolicy policy) { m_overrideCachePolicyForTesting = policy; }
     void setOverrideResourceLoadPriorityForTesting(ResourceLoadPriority priority) { m_overrideResourceLoadPriorityForTesting = priority; }

Modified: trunk/Source/WebKit/ChangeLog (285088 => 285089)


--- trunk/Source/WebKit/ChangeLog	2021-10-31 02:37:24 UTC (rev 285088)
+++ trunk/Source/WebKit/ChangeLog	2021-10-31 03:19:15 UTC (rev 285089)
@@ -1,3 +1,28 @@
+2021-10-30  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        Layer tree should not be stuck in frozen state after explicitly stopping a page load
+        https://bugs.webkit.org/show_bug.cgi?id=232532
+        rdar://84522357
+
+        Reviewed by Tim Horton.
+
+        From diagnostic logs gathered in rdar://84522357, it's apparently possible for the layer tree to be stuck in a
+        state where it's indefinitely frozen after the user explicitly stops the page load (e.g. by tapping on the "x"
+        button in Safari). Specifically, these logs indicate that the `PageTransition` layer tree freeze reason may
+        persist even after the user has canceled the load, leading to situations where content may be visible but
+        unresponsive.
+
+        From code inspection, this may happen for multiple reasons, one of which is that a media element has incremented
+        the load event delay count, which in turn prevents us from transitioning to completed state inside
+        `FrameLoader::checkLoadCompleteForThisFrame()`. It's unknown whether this (in particular) is the cause of
+        unresponsiveness observed by the reporter of rdar://84522357, but we can at least use this in order to come up
+        with a layout test; to mitigate all instances where the `PageTransition` reason may persist after the load has
+        been stopped, we make `WebPage::stopLoading()` complete the page transition (thereby lifting the freeze reason)
+        if needed.
+
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::stopLoading):
+
 2021-10-30  Myles C. Maxfield  <mmaxfi...@apple.com>
 
         Web process shouldn't crash if ImageBuffer::ensureBackendCreated() fails

Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp (285088 => 285089)


--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp	2021-10-31 02:37:24 UTC (rev 285088)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp	2021-10-31 03:19:15 UTC (rev 285089)
@@ -1818,7 +1818,10 @@
         return;
 
     SendStopResponsivenessTimer stopper;
-    m_page->userInputBridge().stopLoadingFrame(*m_mainFrame->coreFrame());
+
+    Ref coreFrame = *m_mainFrame->coreFrame();
+    m_page->userInputBridge().stopLoadingFrame(coreFrame.get());
+    coreFrame->loader().completePageTransitionIfNeeded();
 }
 
 bool WebPage::defersLoading() const

Modified: trunk/Tools/ChangeLog (285088 => 285089)


--- trunk/Tools/ChangeLog	2021-10-31 02:37:24 UTC (rev 285088)
+++ trunk/Tools/ChangeLog	2021-10-31 03:19:15 UTC (rev 285089)
@@ -1,3 +1,28 @@
+2021-10-30  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        Layer tree should not be stuck in frozen state after explicitly stopping a page load
+        https://bugs.webkit.org/show_bug.cgi?id=232532
+        rdar://84522357
+
+        Reviewed by Tim Horton.
+
+        Add support for `TestRunner.stopLoading()`, which immediately stops the page load, emulating what would normally
+        happen if the user taps or clicks the "x" button to cancel loading in Safari. See the layout test for more
+        details.
+
+        * DumpRenderTree/TestRunner.cpp:
+        (stopLoadingCallback):
+        (TestRunner::staticFunctions):
+        * DumpRenderTree/TestRunner.h:
+        * DumpRenderTree/mac/TestRunnerMac.mm:
+        (TestRunner::stopLoading):
+        * DumpRenderTree/win/TestRunnerWin.cpp:
+        (TestRunner::stopLoading):
+        * WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl:
+        * WebKitTestRunner/InjectedBundle/TestRunner.cpp:
+        (WTR::TestRunner::stopLoading):
+        * WebKitTestRunner/InjectedBundle/TestRunner.h:
+
 2021-10-30  Chris Dumez  <cdu...@apple.com>
 
         Improve error handling in sendWithAsyncReply()

Modified: trunk/Tools/DumpRenderTree/TestRunner.cpp (285088 => 285089)


--- trunk/Tools/DumpRenderTree/TestRunner.cpp	2021-10-31 02:37:24 UTC (rev 285088)
+++ trunk/Tools/DumpRenderTree/TestRunner.cpp	2021-10-31 03:19:15 UTC (rev 285089)
@@ -1802,6 +1802,13 @@
     return JSValueMakeUndefined(context);
 }
 
+static JSValueRef stopLoadingCallback(JSContextRef context, JSObjectRef function, JSObjectRef thisObject, size_t argumentCount, const JSValueRef arguments[], JSValueRef* exception)
+{
+    TestRunner* controller = static_cast<TestRunner*>(JSObjectGetPrivate(thisObject));
+    controller->stopLoading();
+    return JSValueMakeUndefined(context);
+}
+
 static JSValueRef forceImmediateCompletionCallback(JSContextRef context, JSObjectRef function, JSObjectRef thisObject, size_t argumentCount, const JSValueRef arguments[], JSValueRef* exception)
 {
     TestRunner* controller = static_cast<TestRunner*>(JSObjectGetPrivate(thisObject));
@@ -2113,6 +2120,7 @@
         { "setSpellCheckerLoggingEnabled", setSpellCheckerLoggingEnabledCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
         { "setOpenPanelFiles", setOpenPanelFilesCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
         { "setOpenPanelFilesMediaIcon", SetOpenPanelFilesMediaIconCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
+        { "stopLoading", stopLoadingCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
         { "forceImmediateCompletion", forceImmediateCompletionCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
 #if PLATFORM(IOS_FAMILY)
         { "setPagePaused", setPagePausedCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },

Modified: trunk/Tools/DumpRenderTree/TestRunner.h (285088 => 285089)


--- trunk/Tools/DumpRenderTree/TestRunner.h	2021-10-31 02:37:24 UTC (rev 285088)
+++ trunk/Tools/DumpRenderTree/TestRunner.h	2021-10-31 03:19:15 UTC (rev 285089)
@@ -76,6 +76,7 @@
     void execCommand(JSStringRef name, JSStringRef value);
     bool findString(JSContextRef, JSStringRef, JSObjectRef optionsArray);
     void forceImmediateCompletion();
+    void stopLoading();
     void goBack();
     JSValueRef originsWithApplicationCache(JSContextRef);
     long long applicationCacheDiskUsageForOrigin(JSStringRef name);

Modified: trunk/Tools/DumpRenderTree/mac/TestRunnerMac.mm (285088 => 285089)


--- trunk/Tools/DumpRenderTree/mac/TestRunnerMac.mm	2021-10-31 02:37:24 UTC (rev 285088)
+++ trunk/Tools/DumpRenderTree/mac/TestRunnerMac.mm	2021-10-31 03:19:15 UTC (rev 285089)
@@ -282,6 +282,11 @@
         fprintf(stderr, "TestRunner::notifyDone() called unexpectedly.");
 }
 
+void TestRunner::stopLoading()
+{
+    [mainFrame.webView stopLoading:nil];
+}
+
 void TestRunner::forceImmediateCompletion()
 {
     if (m_waitToDump) {

Modified: trunk/Tools/DumpRenderTree/win/TestRunnerWin.cpp (285088 => 285089)


--- trunk/Tools/DumpRenderTree/win/TestRunnerWin.cpp	2021-10-31 02:37:24 UTC (rev 285088)
+++ trunk/Tools/DumpRenderTree/win/TestRunnerWin.cpp	2021-10-31 03:19:15 UTC (rev 285089)
@@ -109,6 +109,11 @@
     applicationCache->deleteAllApplicationCaches();
 }
 
+void TestRunner::stopLoading()
+{
+    // FIXME: Not implemented.
+}
+
 long long TestRunner::applicationCacheDiskUsageForOrigin(JSStringRef url)
 {
     COMPtr<IWebSecurityOrigin2> origin;

Modified: trunk/Tools/WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl (285088 => 285089)


--- trunk/Tools/WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl	2021-10-31 02:37:24 UTC (rev 285088)
+++ trunk/Tools/WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl	2021-10-31 03:19:15 UTC (rev 285089)
@@ -94,6 +94,7 @@
     undefined setNavigationGesturesEnabled(boolean value);
     undefined setIgnoresViewportScaleLimits(boolean value);
     undefined setShouldDownloadUndisplayableMIMETypes(boolean value);
+    undefined stopLoading();
 
     // Special DOM functions.
     undefined clearBackForwardList();

Modified: trunk/Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp (285088 => 285089)


--- trunk/Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp	2021-10-31 02:37:24 UTC (rev 285088)
+++ trunk/Tools/WebKitTestRunner/InjectedBundle/TestRunner.cpp	2021-10-31 03:19:15 UTC (rev 285089)
@@ -968,6 +968,11 @@
     InjectedBundle::singleton().queueLoadHTMLString(toWK(content).get(), baseURLWK.get(), unreachableURLWK.get());
 }
 
+void TestRunner::stopLoading()
+{
+    WKBundlePageStopLoading(page());
+}
+
 void TestRunner::queueReload()
 {
     InjectedBundle::singleton().queueReload();

Modified: trunk/Tools/WebKitTestRunner/InjectedBundle/TestRunner.h (285088 => 285089)


--- trunk/Tools/WebKitTestRunner/InjectedBundle/TestRunner.h	2021-10-31 02:37:24 UTC (rev 285088)
+++ trunk/Tools/WebKitTestRunner/InjectedBundle/TestRunner.h	2021-10-31 03:19:15 UTC (rev 285089)
@@ -338,6 +338,7 @@
     void setIgnoresViewportScaleLimits(bool);
     void setShouldDownloadUndisplayableMIMETypes(bool);
     void setShouldAllowDeviceOrientationAndMotionAccess(bool);
+    void stopLoading();
 
     bool didCancelClientRedirect() const { return m_didCancelClientRedirect; }
     void setDidCancelClientRedirect(bool value) { m_didCancelClientRedirect = value; }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to