- Revision
- 248591
- Author
- [email protected]
- Date
- 2019-08-12 22:18:11 -0700 (Mon, 12 Aug 2019)
Log Message
FrameLoader::open can execute scritps via style recalc in Frame::setDocument
https://bugs.webkit.org/show_bug.cgi?id=200377
Reviewed by Antti Koivisto.
Source/WebCore:
Fixed the bug that FrameLoader::open can execute arbitrary author scripts via post style update callbacks
by adding PostResolutionCallbackDisabler, WidgetHierarchyUpdatesSuspensionScope, and NavigationDisabler
to CachedFrameBase::restore and FrameLoader::open.
This ensures all frames are restored from the page cache before any of them would start running scripts.
Test: fast/frames/restoring-page-cache-should-not-run-scripts-via-style-update.html
* history/CachedFrame.cpp:
(WebCore::CachedFrameBase::restore):
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::open):
* page/FrameViewLayoutContext.cpp:
(WebCore::FrameViewLayoutContext::layout): Fixed the debug assertion. The layout of a document may be
updated while we're preparing to put a page into the page cache.
* rendering/RenderLayerCompositor.cpp:
(WebCore::RenderLayerCompositor::updateCompositingLayers): Ditto.
LayoutTests:
Added a regression test.
* fast/frames/restoring-page-cache-should-not-run-scripts-via-style-update-expected.txt: Added.
* fast/frames/restoring-page-cache-should-not-run-scripts-via-style-update.html: Added.
* platform/win/TestExpectations: Skip the newly added test.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (248590 => 248591)
--- trunk/LayoutTests/ChangeLog 2019-08-13 03:56:37 UTC (rev 248590)
+++ trunk/LayoutTests/ChangeLog 2019-08-13 05:18:11 UTC (rev 248591)
@@ -1,3 +1,16 @@
+2019-08-12 Ryosuke Niwa <[email protected]>
+
+ FrameLoader::open can execute scritps via style recalc in Frame::setDocument
+ https://bugs.webkit.org/show_bug.cgi?id=200377
+
+ Reviewed by Antti Koivisto.
+
+ Added a regression test.
+
+ * fast/frames/restoring-page-cache-should-not-run-scripts-via-style-update-expected.txt: Added.
+ * fast/frames/restoring-page-cache-should-not-run-scripts-via-style-update.html: Added.
+ * platform/win/TestExpectations: Skip the newly added test.
+
2019-08-12 Daniel Bates <[email protected]>
Add a test to ensure that we dispatch keydown and keyup events when multiple keys are pressed at the same time
Added: trunk/LayoutTests/fast/frames/restoring-page-cache-should-not-run-scripts-via-style-update-expected.txt (0 => 248591)
--- trunk/LayoutTests/fast/frames/restoring-page-cache-should-not-run-scripts-via-style-update-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/frames/restoring-page-cache-should-not-run-scripts-via-style-update-expected.txt 2019-08-13 05:18:11 UTC (rev 248591)
@@ -0,0 +1,3 @@
+This tests that pageshow event is fired before the object element loads when a document in the page cache is restored.
+
+PASS
Added: trunk/LayoutTests/fast/frames/restoring-page-cache-should-not-run-scripts-via-style-update.html (0 => 248591)
--- trunk/LayoutTests/fast/frames/restoring-page-cache-should-not-run-scripts-via-style-update.html (rev 0)
+++ trunk/LayoutTests/fast/frames/restoring-page-cache-should-not-run-scripts-via-style-update.html 2019-08-13 05:18:11 UTC (rev 248591)
@@ -0,0 +1,102 @@
+<!DOCTYPE html>
+<html>
+<body>
+<p>This tests that pageshow event is fired before the object element loads when a document in the page cache is restored.</p>
+<div id="result"></div>
+<script>
+
+if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.waitUntilDone();
+ testRunner.setCanOpenWindows();
+ testRunner.overridePreference("WebKitUsesPageCachePreferenceKey", 1);
+ internals.settings.setPageCacheSupportsPlugins(true);
+}
+
+let newWindow;
+function start() {
+ result.textContent = 'Running...';
+ newWindow = window.open(URL.createObjectURL(newPage));
+}
+
+const newPage = new Blob([`<!DOCTYPE html>
+<html>
+<body>
+<p>This is new page.</p>
+<iframe id="iframe2"></iframe>
+<iframe id="iframe1"></iframe>
+<script>
+window.pageShowed = false;
+
+iframe2._onload_ = () => {
+ opener.postMessage({step: 'opened'}, '*');
+}
+_onmessage_ = () => {
+ opener.postMessage({step: 'ready'}, '*');
+}
+
+iframe1.contentDocument.body.innerHTML = '<span>hello</span>';
+iframe2.src = ""
+
+</scr` + `ipt>
+</body>
+</html>`], {'type': 'text/html'});
+
+window.subframePage = new Blob([`<!DOCTYPE html>
+<html>
+<body>
+<object id="object1"></object>
+<script>
+window.addEventListener('pagehide', () => {
+ const object1 = window.object1;
+ object1.remove();
+ document.body.appendChild(object1);
+ object1.addEventListener('beforeload', () => {
+ top.opener.postMessage({step: 'check', pageShowed: !!top.iframe1.contentDocument.body.firstChild.offsetHeight}, '*');
+ }, {once: true});
+});
+</sc` + `ript>
+</body>
+</html>`], {'type': 'text/html'});
+
+const secondPage = new Blob([`<!DOCTYPE html>
+<html>
+<body _onload_="opener.postMessage({step: 'navigated'}, '*')">
+<p>second page.</p>
+</body>
+</html>`], {'type': 'text/html'});
+
+let isLastStep = false;
+_onmessage_ = (event) => {
+ if (isLastStep && event.data.step != 'check')
+ return;
+ switch (event.data.step) {
+ case 'opened':
+ newWindow.postMessage('getready', '*');
+ break;
+ case 'ready':
+ newWindow.location = URL.createObjectURL(secondPage);
+ break;
+ case 'navigated':
+ isLastStep = true;
+ newWindow.history.back();
+ break;
+ case 'check':
+ result.textContent = event.data.pageShowed ? 'PASS' : 'FAIL';
+ newWindow.close();
+ if (window.testRunner)
+ testRunner.notifyDone();
+ break;
+ }
+}
+
+if (window.testRunner)
+ window._onload_ = start;
+else
+ document.write('<button _onclick_="start()">Start</button>');
+
+setTimeout(() => testRunner.notifyDone(), 3000);
+
+</script>
+</body>
+</html>
Modified: trunk/LayoutTests/platform/win/TestExpectations (248590 => 248591)
--- trunk/LayoutTests/platform/win/TestExpectations 2019-08-13 03:56:37 UTC (rev 248590)
+++ trunk/LayoutTests/platform/win/TestExpectations 2019-08-13 05:18:11 UTC (rev 248591)
@@ -2299,6 +2299,7 @@
fast/frames/restoring-page-cache-should-not-run-scripts.html [ Skip ]
http/tests/security/mixedContent/blob-url-in-iframe.html [ Skip ]
http/tests/security/contentSecurityPolicy/navigate-self-to-blob.html [ Skip ]
+fast/frames/restoring-page-cache-should-not-run-scripts-via-style-update.html [ Skip ]
# Clear Key not implemented
http/tests/media/clearkey/clear-key-hls-aes128.html [ Skip ] # Timeout
Modified: trunk/Source/WebCore/ChangeLog (248590 => 248591)
--- trunk/Source/WebCore/ChangeLog 2019-08-13 03:56:37 UTC (rev 248590)
+++ trunk/Source/WebCore/ChangeLog 2019-08-13 05:18:11 UTC (rev 248591)
@@ -1,3 +1,28 @@
+2019-08-12 Ryosuke Niwa <[email protected]>
+
+ FrameLoader::open can execute scritps via style recalc in Frame::setDocument
+ https://bugs.webkit.org/show_bug.cgi?id=200377
+
+ Reviewed by Antti Koivisto.
+
+ Fixed the bug that FrameLoader::open can execute arbitrary author scripts via post style update callbacks
+ by adding PostResolutionCallbackDisabler, WidgetHierarchyUpdatesSuspensionScope, and NavigationDisabler
+ to CachedFrameBase::restore and FrameLoader::open.
+
+ This ensures all frames are restored from the page cache before any of them would start running scripts.
+
+ Test: fast/frames/restoring-page-cache-should-not-run-scripts-via-style-update.html
+
+ * history/CachedFrame.cpp:
+ (WebCore::CachedFrameBase::restore):
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::open):
+ * page/FrameViewLayoutContext.cpp:
+ (WebCore::FrameViewLayoutContext::layout): Fixed the debug assertion. The layout of a document may be
+ updated while we're preparing to put a page into the page cache.
+ * rendering/RenderLayerCompositor.cpp:
+ (WebCore::RenderLayerCompositor::updateCompositingLayers): Ditto.
+
2019-08-12 Sam Weinig <[email protected]>
Replace multiparameter overloads of append() in StringBuilder as a first step toward standardizinging on the flexibleAppend() implementation
Modified: trunk/Source/WebCore/history/CachedFrame.cpp (248590 => 248591)
--- trunk/Source/WebCore/history/CachedFrame.cpp 2019-08-13 03:56:37 UTC (rev 248590)
+++ trunk/Source/WebCore/history/CachedFrame.cpp 2019-08-13 05:18:11 UTC (rev 248591)
@@ -39,12 +39,15 @@
#include "FrameLoaderClient.h"
#include "FrameView.h"
#include "Logging.h"
+#include "NavigationDisabler.h"
#include "Page.h"
#include "PageCache.h"
+#include "RenderWidget.h"
#include "RuntimeEnabledFeatures.h"
#include "SVGDocumentExtensions.h"
#include "ScriptController.h"
#include "SerializedScriptValue.h"
+#include "StyleTreeResolver.h"
#include <wtf/RefCountedLeakCounter.h>
#include <wtf/text/CString.h>
@@ -93,44 +96,50 @@
if (m_isMainFrame)
m_view->setParentVisible(true);
- Frame& frame = m_view->frame();
- m_cachedFrameScriptData->restore(frame);
+ auto frame = makeRef(m_view->frame());
+ {
+ Style::PostResolutionCallbackDisabler disabler(*m_document);
+ WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
+ NavigationDisabler disableNavigation { nullptr }; // Disable navigation globally.
- if (m_document->svgExtensions())
- m_document->accessSVGExtensions().unpauseAnimations();
+ m_cachedFrameScriptData->restore(frame.get());
- m_document->resume(ReasonForSuspension::PageCache);
+ if (m_document->svgExtensions())
+ m_document->accessSVGExtensions().unpauseAnimations();
- // It is necessary to update any platform script objects after restoring the
- // cached page.
- frame.script().updatePlatformScriptObjects();
+ m_document->resume(ReasonForSuspension::PageCache);
- frame.loader().client().didRestoreFromPageCache();
+ // It is necessary to update any platform script objects after restoring the
+ // cached page.
+ frame->script().updatePlatformScriptObjects();
- pruneDetachedChildFrames();
+ frame->loader().client().didRestoreFromPageCache();
- // Reconstruct the FrameTree. And open the child CachedFrames in their respective FrameLoaders.
- for (auto& childFrame : m_childFrames) {
- ASSERT(childFrame->view()->frame().page());
- frame.tree().appendChild(childFrame->view()->frame());
- childFrame->open();
- ASSERT_WITH_SECURITY_IMPLICATION(m_document == frame.document());
+ pruneDetachedChildFrames();
+
+ // Reconstruct the FrameTree. And open the child CachedFrames in their respective FrameLoaders.
+ for (auto& childFrame : m_childFrames) {
+ ASSERT(childFrame->view()->frame().page());
+ frame->tree().appendChild(childFrame->view()->frame());
+ childFrame->open();
+ ASSERT_WITH_SECURITY_IMPLICATION(m_document == frame->document());
+ }
}
#if PLATFORM(IOS_FAMILY)
if (m_isMainFrame) {
- frame.loader().client().didRestoreFrameHierarchyForCachedFrame();
+ frame->loader().client().didRestoreFrameHierarchyForCachedFrame();
if (DOMWindow* domWindow = m_document->domWindow()) {
// FIXME: Add SCROLL_LISTENER to the list of event types on Document, and use m_document->hasListenerType(). See <rdar://problem/9615482>.
// FIXME: Can use Document::hasListenerType() now.
- if (domWindow->scrollEventListenerCount() && frame.page())
- frame.page()->chrome().client().setNeedsScrollNotifications(frame, true);
+ if (domWindow->scrollEventListenerCount() && frame->page())
+ frame->page()->chrome().client().setNeedsScrollNotifications(frame, true);
}
}
#endif
- frame.view()->didRestoreFromPageCache();
+ frame->view()->didRestoreFromPageCache();
}
CachedFrame::CachedFrame(Frame& frame)
Modified: trunk/Source/WebCore/loader/FrameLoader.cpp (248590 => 248591)
--- trunk/Source/WebCore/loader/FrameLoader.cpp 2019-08-13 03:56:37 UTC (rev 248590)
+++ trunk/Source/WebCore/loader/FrameLoader.cpp 2019-08-13 05:18:11 UTC (rev 248591)
@@ -118,6 +118,7 @@
#include "SerializedScriptValue.h"
#include "Settings.h"
#include "ShouldTreatAsContinuingLoad.h"
+#include "StyleTreeResolver.h"
#include "SubframeLoader.h"
#include "SubresourceLoader.h"
#include "TextResourceDecoder.h"
@@ -2297,11 +2298,10 @@
url.setPath("/");
started();
- Document* document = cachedFrame.document();
- ASSERT(document);
+ auto document = makeRef(*cachedFrame.document());
ASSERT(document->domWindow());
- clear(document, true, true, cachedFrame.isMainFrame());
+ clear(document.ptr(), true, true, cachedFrame.isMainFrame());
document->attachToCachedFrame(cachedFrame);
document->setPageCacheState(Document::NotInPageCache);
@@ -2324,14 +2324,16 @@
if (previousViewFrameRect)
view->setFrameRect(previousViewFrameRect.value());
- {
- // Setting the document builds the render tree and runs post style resolution callbacks that can do anything,
- // including loading a child frame before its been re-attached to the frame tree as part of this restore.
- // For example, the HTML object element may load its content into a frame in a post style resolution callback.
- NavigationDisabler disableNavigation { &m_frame };
- m_frame.setDocument(document);
- }
+ // Setting the document builds the render tree and runs post style resolution callbacks that can do anything,
+ // including loading a child frame before its been re-attached to the frame tree as part of this restore.
+ // For example, the HTML object element may load its content into a frame in a post style resolution callback.
+ Style::PostResolutionCallbackDisabler disabler(document.get());
+ WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
+ NavigationDisabler disableNavigation { &m_frame };
+
+ m_frame.setDocument(document.copyRef());
+
document->domWindow()->resumeFromPageCache();
updateFirstPartyForCookies();
Modified: trunk/Source/WebCore/page/FrameViewLayoutContext.cpp (248590 => 248591)
--- trunk/Source/WebCore/page/FrameViewLayoutContext.cpp 2019-08-13 03:56:37 UTC (rev 248590)
+++ trunk/Source/WebCore/page/FrameViewLayoutContext.cpp 2019-08-13 05:18:11 UTC (rev 248591)
@@ -145,7 +145,8 @@
ASSERT(!view().isPainting());
ASSERT(frame().view() == &view());
ASSERT(frame().document());
- ASSERT(frame().document()->pageCacheState() == Document::NotInPageCache);
+ ASSERT(frame().document()->pageCacheState() == Document::NotInPageCache
+ || frame().document()->pageCacheState() == Document::AboutToEnterPageCache);
if (!canPerformLayout()) {
LOG(Layout, " is not allowed, bailing");
return;
Modified: trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp (248590 => 248591)
--- trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp 2019-08-13 03:56:37 UTC (rev 248590)
+++ trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp 2019-08-13 05:18:11 UTC (rev 248591)
@@ -691,7 +691,8 @@
m_updateCompositingLayersTimer.stop();
- ASSERT(m_renderView.document().pageCacheState() == Document::NotInPageCache);
+ ASSERT(m_renderView.document().pageCacheState() == Document::NotInPageCache
+ || m_renderView.document().pageCacheState() == Document::AboutToEnterPageCache);
// Compositing layers will be updated in Document::setVisualUpdatesAllowed(bool) if suppressed here.
if (!m_renderView.document().visualUpdatesAllowed())
