Layout Test fast/events/beforeunload-dom-manipulation-crash.html is crashing
authorpvollan@apple.com <pvollan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 29 Jan 2018 16:02:17 +0000 (16:02 +0000)
committerpvollan@apple.com <pvollan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 29 Jan 2018 16:02:17 +0000 (16:02 +0000)
https://bugs.webkit.org/show_bug.cgi?id=181204
<rdar://problem/36256274>

Reviewed by Ryosuke Niwa.

Source/WebCore:

When a frame element is moved in the DOM tree during the execution of a beforeunload handler,
the frame will be detached when removed from its previous position in the DOM tree. When being
detached, an attempt will also be made to stop the load by calling FrameLoader::stopAllLoaders().
However, this method will return early when executed in a beforeunload handler, since navigation
is not allowed then. The end result is a detached frame which will continue to load, and hitting
asserts in DocumentLoader::dataReceived(), and DocumentLoader::notifyFinished(). It should be
possible to stop a frame load, even when executing a beforeunload handler.

No new tests. Covered by the existing test fast/events/beforeunload-dom-manipulation-crash.html.

* loader/FrameLoader.cpp:
(WebCore::FrameLoader::isStopLoadingAllowed const):
(WebCore::FrameLoader::stopAllLoaders):
* loader/FrameLoader.h:

Tools:

Implement 'testRunner.forceImmediateCompletion()' for WK1.

* DumpRenderTree/TestRunner.cpp:
(forceImmediateCompletionCallback):
(TestRunner::staticFunctions):

LayoutTests:

* fast/events/beforeunload-dom-manipulation-crash.html: Make it clear that the
frame element is a child of the 'del' element.
* fast/events/beforeunload-dom-manipulation-crash-expected.html:
* platform/mac-wk1/TestExpectations: Unskip test.

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

LayoutTests/ChangeLog
LayoutTests/fast/events/beforeunload-dom-manipulation-crash-expected.txt
LayoutTests/fast/events/beforeunload-dom-manipulation-crash.html
LayoutTests/platform/mac-wk1/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/html/HTMLFrameOwnerElement.cpp
Source/WebCore/loader/FrameLoader.cpp
Source/WebCore/loader/FrameLoader.h
Tools/ChangeLog
Tools/DumpRenderTree/TestRunner.cpp

index 4e835ce..d954375 100644 (file)
@@ -1,5 +1,18 @@
 2018-01-29  Per Arne Vollan  <pvollan@apple.com>
 
+        Layout Test fast/events/beforeunload-dom-manipulation-crash.html is crashing
+        https://bugs.webkit.org/show_bug.cgi?id=181204
+        <rdar://problem/36256274>
+
+        Reviewed by Ryosuke Niwa.
+
+        * fast/events/beforeunload-dom-manipulation-crash.html: Make it clear that the
+        frame element is a child of the 'del' element.
+        * fast/events/beforeunload-dom-manipulation-crash-expected.html:
+        * platform/mac-wk1/TestExpectations: Unskip test.
+
+2018-01-29  Per Arne Vollan  <pvollan@apple.com>
+
         Mark js/dom/array-with-double-assign.html as a failure on Windows.
         https://bugs.webkit.org/show_bug.cgi?id=182239
 
index 8714ac4..a4fd9d8 100644 (file)
@@ -23,5 +23,6 @@ function nextStep() {
 <body onload="runTest()">
     <p>This test passes if it does not crash.</p>
     <del id="del">
-    <iframe id="iframe"></iframe>
+        <iframe id="iframe"></iframe>
+    </del>
 </body>
index 4aeb0f9..c262d4b 100644 (file)
@@ -453,8 +453,6 @@ webkit.org/b/176920 imported/w3c/web-platform-tests/streams/piping/error-propaga
 
 webkit.org/b/175886 svg/animations/smil-leak-element-instances.svg [ Pass Failure ]
 
-webkit.org/b/177020 fast/events/beforeunload-dom-manipulation-crash.html [ Skip ]
-
 # <rdar://problem/29201698> DumpRenderTree crashed in com.apple.CoreGraphics: CGDataProviderCopyData + 377
 [ HighSierra+ ] fast/canvas/webgl/tex-image-and-sub-image-2d-with-potentially-subsampled-image.html [ Crash ]
 
index 864d820..8984611 100644 (file)
@@ -1,3 +1,26 @@
+2018-01-29  Per Arne Vollan  <pvollan@apple.com>
+
+        Layout Test fast/events/beforeunload-dom-manipulation-crash.html is crashing
+        https://bugs.webkit.org/show_bug.cgi?id=181204
+        <rdar://problem/36256274>
+
+        Reviewed by Ryosuke Niwa.
+
+        When a frame element is moved in the DOM tree during the execution of a beforeunload handler,
+        the frame will be detached when removed from its previous position in the DOM tree. When being
+        detached, an attempt will also be made to stop the load by calling FrameLoader::stopAllLoaders().
+        However, this method will return early when executed in a beforeunload handler, since navigation
+        is not allowed then. The end result is a detached frame which will continue to load, and hitting
+        asserts in DocumentLoader::dataReceived(), and DocumentLoader::notifyFinished(). It should be
+        possible to stop a frame load, even when executing a beforeunload handler.
+
+        No new tests. Covered by the existing test fast/events/beforeunload-dom-manipulation-crash.html.
+
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::isStopLoadingAllowed const):
+        (WebCore::FrameLoader::stopAllLoaders):
+        * loader/FrameLoader.h:
+
 2018-01-29  Miguel Gomez  <magomez@igalia.com>
 
         [CoordnatedGraphics] A child layer of a semitransparent layer isn't clipped properly
index 41f9db9..9e64c76 100644 (file)
@@ -80,6 +80,8 @@ void HTMLFrameOwnerElement::disconnectContentFrame()
     // see if this behavior is really needed as Gecko does not allow this.
     if (RefPtr<Frame> frame = contentFrame()) {
         Ref<Frame> protect(*frame);
+        // FrameLoader::frameDetached() might dispatch an unload event.
+        ASSERT(ScriptDisallowedScope::InMainThread::isScriptAllowed());
         frame->loader().frameDetached();
         frame->disconnectOwnerElement();
     }
index 77b2435..056877b 100644 (file)
@@ -1254,6 +1254,11 @@ bool FrameLoader::isNavigationAllowed() const
     return m_pageDismissalEventBeingDispatched == PageDismissalType::None && NavigationDisabler::isNavigationAllowed(m_frame);
 }
 
+bool FrameLoader::isStopLoadingAllowed() const
+{
+    return m_pageDismissalEventBeingDispatched == PageDismissalType::None;
+}
+
 struct SharedBool : public RefCounted<SharedBool> {
     bool value { false };
 };
@@ -1667,7 +1672,7 @@ void FrameLoader::reload(OptionSet<ReloadOption> options)
 void FrameLoader::stopAllLoaders(ClearProvisionalItemPolicy clearProvisionalItemPolicy)
 {
     ASSERT(!m_frame.document() || m_frame.document()->pageCacheState() != Document::InPageCache);
-    if (!isNavigationAllowed())
+    if (!isStopLoadingAllowed())
         return;
 
     // If this method is called from within this method, infinite recursion can occur (3442218). Avoid this.
index a6c9819..ede5dcd 100644 (file)
@@ -389,6 +389,7 @@ private:
     void dispatchGlobalObjectAvailableInAllWorlds();
 
     bool isNavigationAllowed() const;
+    bool isStopLoadingAllowed() const;
 
     Frame& m_frame;
     FrameLoaderClient& m_client;
index d1da426..75cf2ea 100644 (file)
@@ -1,3 +1,17 @@
+2018-01-29  Per Arne Vollan  <pvollan@apple.com>
+
+        Layout Test fast/events/beforeunload-dom-manipulation-crash.html is crashing
+        https://bugs.webkit.org/show_bug.cgi?id=181204
+        <rdar://problem/36256274>
+
+        Reviewed by Ryosuke Niwa.
+
+        Implement 'testRunner.forceImmediateCompletion()' for WK1.
+
+        * DumpRenderTree/TestRunner.cpp:
+        (forceImmediateCompletionCallback):
+        (TestRunner::staticFunctions):
+
 2018-01-29  Frederic Wang  <fwang@igalia.com>
 
         Unreviewed, add myself to some watch lists.
index 3828990..40afd10 100644 (file)
@@ -1986,6 +1986,13 @@ static JSValueRef simulateWebNotificationClickCallback(JSContextRef context, JSO
     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));
+    controller->forceImmediateCompletion();
+    return JSValueMakeUndefined(context);
+}
+
 static JSValueRef failNextNewCodeBlock(JSContextRef context, JSObjectRef function, JSObjectRef thisObject, size_t argumentCount, const JSValueRef arguments[], JSValueRef* exception)
 {
     if (argumentCount < 1)
@@ -2242,6 +2249,7 @@ JSStaticFunction* TestRunner::staticFunctions()
         { "imageCountInGeneralPasteboard", imageCountInGeneralPasteboardCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
         { "setSpellCheckerLoggingEnabled", setSpellCheckerLoggingEnabledCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
         { "setOpenPanelFiles", setOpenPanelFilesCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
+        { "forceImmediateCompletion", forceImmediateCompletionCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
         { 0, 0, 0 }
     };