Attachment 346804 Details for Bug 188422 – Patch and layout test

[patch] Patch and layout test

bug-188422-20180808171448.patch (text/plain), 4.98 KB, created by Daniel Bates on 2018-08-08 17:14:48 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Daniel Bates

Created: 2018-08-08 17:14:48 PDT

Size: 4.98 KB

patch

obsolete

>Subversion Revision: 234703
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 385adbe0fe85485157420db21c6140a2e3dea5de..c79ea1784334bde42d555278c04216f9a960db53 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,24 @@
>+2018-08-08  Daniel Bates  <dabates@apple.com>
>+
>+        Disallow navigations when page cache updates the current document of the frame
>+        https://bugs.webkit.org/show_bug.cgi?id=188422
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Make use of NavigationDisabler to disallow navigations when associating the cached
>+        document back with its frame (i.e. calling Frame::setDocument()).
>+
>+        When we associate a cached document with its frame we will construct its render tree
>+        and run post style resolution callbacks that can do anything, including performing
>+        a frame load. Until page restoration is comnplete the frame tree is in a transient
>+        state that makes reasoning about it difficult and error prone. We should not allow
>+        navigations in this state.
>+
>+        Test: fast/history/go-back-to-object-subframe.html
>+
>+        * loader/FrameLoader.cpp:
>+        (WebCore::FrameLoader::open):
>+
> 2018-08-08  Eric Carlson  <eric.carlson@apple.com>
> 
>         Add LogArgument template for PlatformMediaSession::RemoteControlCommandType
>diff --git a/Source/WebCore/loader/FrameLoader.cpp b/Source/WebCore/loader/FrameLoader.cpp
>index de71b0c97006b1e85d328f9c501894383d8c2973..1845613c79c0ee497481db9a1e6e7e29a122744b 100644
>--- a/Source/WebCore/loader/FrameLoader.cpp
>+++ b/Source/WebCore/loader/FrameLoader.cpp
>@@ -2263,8 +2263,15 @@ void FrameLoader::open(CachedFrameBase& cachedFrame)
>     // Use the previous ScrollView's frame rect.
>     if (previousViewFrameRect)
>         view->setFrameRect(previousViewFrameRect.value());
>-    
>-    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.
>+        NavigationDisabler disableNavigation { &m_frame };
>+        m_frame.setDocument(document);
>+    }
>+
>     document->domWindow()->resumeFromDocumentSuspension();
> 
>     updateFirstPartyForCookies();
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index dbdba41392b35fb77887ffb0279670fb95e39ed7..5898c9548976c80ec6753ffa2a3ed1ea11ea7c93 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,18 @@
>+2018-08-08  Daniel Bates  <dabates@apple.com>
>+
>+        Disallow navigations when page cache updates the current document of the frame
>+        https://bugs.webkit.org/show_bug.cgi?id=188422
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add a test case that ensures that we do not hit the assertion ASSERT(ownerFrame || m_frame.isMainFrame())
>+        in FrameLoader::addExtraFieldsToRequest() when navigating back to a page that loads a nested
>+        page, whose URL contains a fragment, via an HTML object element. This assertion fails if
>+        navigations are allowed when restoring a page from the page cache.
>+
>+        * fast/history/go-back-to-object-subframe-expected.txt: Added.
>+        * fast/history/go-back-to-object-subframe.html: Added.
>+
> 2018-08-08  Joseph Pecoraro  <pecoraro@apple.com>
> 
>         Web Inspector: XHR content sometimes shows as error even though load succeeded
>diff --git a/LayoutTests/fast/history/go-back-to-object-subframe-expected.txt b/LayoutTests/fast/history/go-back-to-object-subframe-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..e66bdd30d76690de5ca02845412e6575da674745
>--- /dev/null
>+++ b/LayoutTests/fast/history/go-back-to-object-subframe-expected.txt
>@@ -0,0 +1,2 @@
>+PASS. You didn't crash.
>+
>diff --git a/LayoutTests/fast/history/go-back-to-object-subframe.html b/LayoutTests/fast/history/go-back-to-object-subframe.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..580a42f83e5769acfdff44a86382067c8427081d
>--- /dev/null
>+++ b/LayoutTests/fast/history/go-back-to-object-subframe.html
>@@ -0,0 +1,29 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<script>
>+if (window.testRunner) {
>+    testRunner.dumpAsText();
>+    testRunner.overridePreference("WebKitUsesPageCachePreferenceKey", 1);
>+    testRunner.waitUntilDone();
>+}
>+
>+function runTest(e)
>+{
>+    if (e.persisted) {
>+        if (window.testRunner)
>+            testRunner.notifyDone();
>+    } else {
>+        // Navigate using a timeout to make sure we generate a history entry that we can go back to.
>+        setTimeout(() => { location.href = "data:text/html,<script>history.back()</" + "script>"; }, 0);
>+    }
>+}
>+
>+window.onpageshow = runTest;
>+</script>
>+</head>
>+<body>
>+<div>PASS. You didn't crash.</div>
>+<object data="resources/subframe.html#dummy"  width="400" height="200" style="border: 1px solid black"></object>
>+</body>
>+</html>

Flags:

rniwa: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 188422: 346804