- Revision
- 227948
- Author
- [email protected]
- Date
- 2018-01-31 20:18:38 -0800 (Wed, 31 Jan 2018)
Log Message
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.
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.
* history/PageCache.cpp:
(WebCore::PageCache::addIfCacheable): Fix a failing API test by allowing scripts to be executed
under the PageCache::prune method.
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::isStopLoadingAllowed const):
(WebCore::FrameLoader::stopAllLoaders):
* loader/FrameLoader.h:
* svg/graphics/SVGImage.cpp:
(WebCore::SVGImage::~SVGImage): Disable scripts disallowed assertions in this scope, since it is
safe in this context.
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.
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (227947 => 227948)
--- trunk/LayoutTests/ChangeLog 2018-02-01 01:56:52 UTC (rev 227947)
+++ trunk/LayoutTests/ChangeLog 2018-02-01 04:18:38 UTC (rev 227948)
@@ -1,3 +1,16 @@
+2018-01-31 Per Arne Vollan <[email protected]>
+
+ 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-31 Javier Fernandez <[email protected]>
inline-block baseline not computed correctly for vertical-lr
Modified: trunk/LayoutTests/fast/events/beforeunload-dom-manipulation-crash-expected.txt (227947 => 227948)
--- trunk/LayoutTests/fast/events/beforeunload-dom-manipulation-crash-expected.txt 2018-02-01 01:56:52 UTC (rev 227947)
+++ trunk/LayoutTests/fast/events/beforeunload-dom-manipulation-crash-expected.txt 2018-02-01 04:18:38 UTC (rev 227948)
@@ -1 +1,3 @@
This test passes if it does not crash.
+
+
Modified: trunk/LayoutTests/fast/events/beforeunload-dom-manipulation-crash.html (227947 => 227948)
--- trunk/LayoutTests/fast/events/beforeunload-dom-manipulation-crash.html 2018-02-01 01:56:52 UTC (rev 227947)
+++ trunk/LayoutTests/fast/events/beforeunload-dom-manipulation-crash.html 2018-02-01 04:18:38 UTC (rev 227948)
@@ -23,5 +23,6 @@
<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>
Modified: trunk/LayoutTests/platform/mac-wk1/TestExpectations (227947 => 227948)
--- trunk/LayoutTests/platform/mac-wk1/TestExpectations 2018-02-01 01:56:52 UTC (rev 227947)
+++ trunk/LayoutTests/platform/mac-wk1/TestExpectations 2018-02-01 04:18:38 UTC (rev 227948)
@@ -453,8 +453,6 @@
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 ]
Modified: trunk/Source/WebCore/ChangeLog (227947 => 227948)
--- trunk/Source/WebCore/ChangeLog 2018-02-01 01:56:52 UTC (rev 227947)
+++ trunk/Source/WebCore/ChangeLog 2018-02-01 04:18:38 UTC (rev 227948)
@@ -1,3 +1,32 @@
+2018-01-31 Per Arne Vollan <[email protected]>
+
+ 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.
+
+ * history/PageCache.cpp:
+ (WebCore::PageCache::addIfCacheable): Fix a failing API test by allowing scripts to be executed
+ under the PageCache::prune method.
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::isStopLoadingAllowed const):
+ (WebCore::FrameLoader::stopAllLoaders):
+ * loader/FrameLoader.h:
+ * svg/graphics/SVGImage.cpp:
+ (WebCore::SVGImage::~SVGImage): Disable scripts disallowed assertions in this scope, since it is
+ safe in this context.
+
2018-01-31 Javier Fernandez <[email protected]>
inline-block baseline not computed correctly for vertical-lr
Modified: trunk/Source/WebCore/history/PageCache.cpp (227947 => 227948)
--- trunk/Source/WebCore/history/PageCache.cpp 2018-02-01 01:56:52 UTC (rev 227947)
+++ trunk/Source/WebCore/history/PageCache.cpp 2018-02-01 04:18:38 UTC (rev 227948)
@@ -431,13 +431,14 @@
setPageCacheState(*page, Document::InPageCache);
- // Make sure we no longer fire any JS events past this point.
- ScriptDisallowedScope::InMainThread scriptDisallowedScope;
+ {
+ // Make sure we don't fire any JS events in this scope.
+ ScriptDisallowedScope::InMainThread scriptDisallowedScope;
- item.m_cachedPage = std::make_unique<CachedPage>(*page);
- item.m_pruningReason = PruningReason::None;
- m_items.add(&item);
-
+ item.m_cachedPage = std::make_unique<CachedPage>(*page);
+ item.m_pruningReason = PruningReason::None;
+ m_items.add(&item);
+ }
prune(PruningReason::ReachedMaxSize);
}
Modified: trunk/Source/WebCore/loader/FrameLoader.cpp (227947 => 227948)
--- trunk/Source/WebCore/loader/FrameLoader.cpp 2018-02-01 01:56:52 UTC (rev 227947)
+++ trunk/Source/WebCore/loader/FrameLoader.cpp 2018-02-01 04:18:38 UTC (rev 227948)
@@ -1254,6 +1254,11 @@
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,13 +1672,16 @@
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.
if (m_inStopAllLoaders)
return;
-
+
+ // This method might dispatch events.
+ RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(ScriptDisallowedScope::InMainThread::isScriptAllowed());
+
// Calling stopLoading() on the provisional document loader can blow away
// the frame from underneath.
Ref<Frame> protect(m_frame);
Modified: trunk/Source/WebCore/loader/FrameLoader.h (227947 => 227948)
--- trunk/Source/WebCore/loader/FrameLoader.h 2018-02-01 01:56:52 UTC (rev 227947)
+++ trunk/Source/WebCore/loader/FrameLoader.h 2018-02-01 04:18:38 UTC (rev 227948)
@@ -389,6 +389,7 @@
void dispatchGlobalObjectAvailableInAllWorlds();
bool isNavigationAllowed() const;
+ bool isStopLoadingAllowed() const;
Frame& m_frame;
FrameLoaderClient& m_client;
Modified: trunk/Source/WebCore/svg/graphics/SVGImage.cpp (227947 => 227948)
--- trunk/Source/WebCore/svg/graphics/SVGImage.cpp 2018-02-01 01:56:52 UTC (rev 227947)
+++ trunk/Source/WebCore/svg/graphics/SVGImage.cpp 2018-02-01 04:18:38 UTC (rev 227948)
@@ -76,6 +76,7 @@
SVGImage::~SVGImage()
{
if (m_page) {
+ ScriptDisallowedScope::DisableAssertionsInScope disabledScope;
// Store m_page in a local variable, clearing m_page, so that SVGImageChromeClient knows we're destructed.
std::unique_ptr<Page> currentPage = WTFMove(m_page);
currentPage->mainFrame().loader().frameDetached(); // Break both the loader and view references to the frame
Modified: trunk/Tools/ChangeLog (227947 => 227948)
--- trunk/Tools/ChangeLog 2018-02-01 01:56:52 UTC (rev 227947)
+++ trunk/Tools/ChangeLog 2018-02-01 04:18:38 UTC (rev 227948)
@@ -1,3 +1,17 @@
+2018-01-31 Per Arne Vollan <[email protected]>
+
+ 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-31 Alex Christensen <[email protected]>
Unreviewed, rolling out r227942.
Modified: trunk/Tools/DumpRenderTree/TestRunner.cpp (227947 => 227948)
--- trunk/Tools/DumpRenderTree/TestRunner.cpp 2018-02-01 01:56:52 UTC (rev 227947)
+++ trunk/Tools/DumpRenderTree/TestRunner.cpp 2018-02-01 04:18:38 UTC (rev 227948)
@@ -1986,6 +1986,13 @@
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 @@
{ "imageCountInGeneralPasteboard", imageCountInGeneralPasteboardCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
{ "setSpellCheckerLoggingEnabled", setSpellCheckerLoggingEnabledCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
{ "setOpenPanelFiles", setOpenPanelFilesCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
+ { "forceImmediateCompletion", forceImmediateCompletionCallback, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontDelete },
{ 0, 0, 0 }
};
