From 4a298cdf4f4bb2ae55d07f3f948e007b905bfb94 Mon Sep 17 00:00:00 2001 From: zhangxin11112342 Date: Thu, 18 Apr 2024 10:07:25 +0800 Subject: [PATCH] =?UTF-8?q?=E4=BF=AE=E5=A4=8DCVE-2024-1671?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: zhangxin11112342 --- content/browser/navigation_browsertest.cc | 81 ++++++ .../browser/renderer_host/frame_tree_node.cc | 6 + .../browser/renderer_host/frame_tree_node.h | 50 +++- .../navigation_controller_impl_browsertest.cc | 268 +++++++++++++++++- .../renderer_host/navigation_request.cc | 19 ++ .../renderer_host/render_frame_host_impl.cc | 12 +- .../renderer_host/render_frame_host_impl.h | 34 +-- .../renderer_host/render_frame_host_owner.h | 4 + 8 files changed, 434 insertions(+), 40 deletions(-) diff --git a/content/browser/navigation_browsertest.cc b/content/browser/navigation_browsertest.cc index 73e897d82c..14cd8ac17d 100644 --- a/content/browser/navigation_browsertest.cc +++ b/content/browser/navigation_browsertest.cc @@ -97,6 +97,7 @@ #include "third_party/blink/public/mojom/frame/sudden_termination_disabler_type.mojom-shared.h" #include "ui/base/page_transition_types.h" #include "url/gurl.h" +#include "url/url_constants.h" #include "url/url_util.h" namespace content { @@ -2168,6 +2169,86 @@ IN_PROC_BROWSER_TEST_F(NavigationBrowserTest, BlockedSrcDocRendererInitiated) { } } +// Ensure that about:srcdoc navigations get their origin and base URL from their +// parent frame (since that's where the content comes from) and not from the +// initiator of the navigation (like about:blank cases). See also the +// NavigateGrandchildToAboutBlank test. See https://crbug.com/1515381. +IN_PROC_BROWSER_TEST_F(NavigationBrowserTest, GrandchildToAboutSrcdoc_BaseUrl) { + GURL url_a = embedded_test_server()->GetURL( + "a.com", "/frame_tree/page_with_one_frame.html"); + EXPECT_TRUE(NavigateToURL(shell(), url_a)); + FrameTreeNode* subframe = main_frame()->child_at(0); + + // Navigate the subframe to a cross-site URL with a srcdoc subframe. + GURL url_b = embedded_test_server()->GetURL( + "b.com", "/frame_tree/page_with_srcdoc_frame.html"); + TestNavigationObserver observer(web_contents()); + EXPECT_TRUE(ExecJs(subframe, JsReplace("location.href = $1", url_b))); + observer.Wait(); + EXPECT_EQ(url_b, subframe->current_frame_host()->GetLastCommittedURL()); + FrameTreeNode* grandchild = subframe->child_at(0); + EXPECT_EQ("hello", + EvalJs(grandchild, "document.body.innerHTML").ExtractString()); + + // From the main frame, navigate the grandchild frame to about:srcdoc. + TestNavigationObserver srcdoc_observer(web_contents()); + // TODO(https://crbug.com/1169736): It shouldn't be possible to navigate to + // about:srcdoc by executing location.href = "about:srcdoc". + EXPECT_TRUE( + ExecJs(main_frame(), "frames[0][0].location.href = 'about:srcdoc';")); + srcdoc_observer.Wait(); + + // The content comes from the parent frame and not the initiator. + EXPECT_EQ("hello", + EvalJs(grandchild, "document.body.innerHTML").ExtractString()); + + // The origin and base URI should be inherited from the parent frame and not + // the initiator, unlike navigations to about:blank. + EXPECT_EQ(GURL(url::kAboutSrcdocURL), + grandchild->current_frame_host()->GetLastCommittedURL()); + EXPECT_EQ(url::Origin::Create(url_b), + grandchild->current_frame_host()->GetLastCommittedOrigin()); + EXPECT_EQ(url_b, EvalJs(grandchild, "document.baseURI").ExtractString()); +} + +// Ensure that about:blank navigations get their origin and base URL from the +// initiator of the navigation, and not from their parent frame (like +// about:srcdoc cases). See also the NavigateGrandchildToAboutSrcdoc test. +IN_PROC_BROWSER_TEST_F(NavigationBrowserTest, GrandchildToAboutBlank_BaseUrl) { + GURL url_a = embedded_test_server()->GetURL( + "a.com", "/frame_tree/page_with_one_frame.html"); + EXPECT_TRUE(NavigateToURL(shell(), url_a)); + FrameTreeNode* subframe = main_frame()->child_at(0); + + // Navigate the subframe to a cross-site URL with a srcdoc subframe. + GURL url_b = embedded_test_server()->GetURL( + "b.com", "/frame_tree/page_with_srcdoc_frame.html"); + TestNavigationObserver observer(web_contents()); + EXPECT_TRUE(ExecJs(subframe, JsReplace("location.href = $1", url_b))); + observer.Wait(); + EXPECT_EQ(url_b, subframe->current_frame_host()->GetLastCommittedURL()); + FrameTreeNode* grandchild = subframe->child_at(0); + EXPECT_EQ("hello", + EvalJs(grandchild, "document.body.innerHTML").ExtractString()); + + // From the main frame, navigate the grandchild frame to about:blank. + TestNavigationObserver srcdoc_observer(web_contents()); + EXPECT_TRUE( + ExecJs(main_frame(), "frames[0][0].location.href = 'about:blank';")); + srcdoc_observer.Wait(); + + // There is no content when navigating to about:blank. + EXPECT_EQ("", EvalJs(grandchild, "document.body.innerHTML").ExtractString()); + + // The origin and base URI should be inherited from the initiator of the + // navigation and not the parent frame, unlike navigations to about:srcdoc. + EXPECT_EQ(GURL(url::kAboutBlankURL), + grandchild->current_frame_host()->GetLastCommittedURL()); + EXPECT_EQ(url::Origin::Create(url_a), + grandchild->current_frame_host()->GetLastCommittedOrigin()); + EXPECT_EQ(url_a, EvalJs(grandchild, "document.baseURI").ExtractString()); +} + // Test renderer initiated navigations to about:srcdoc are routed through the // browser process. It means RenderFrameHostImpl::BeginNavigation() is called. IN_PROC_BROWSER_TEST_F(NavigationBrowserTest, AboutSrcDocUsesBeginNavigation) { diff --git a/content/browser/renderer_host/frame_tree_node.cc b/content/browser/renderer_host/frame_tree_node.cc index 23cfc295ff..94bf816840 100644 --- a/content/browser/renderer_host/frame_tree_node.cc +++ b/content/browser/renderer_host/frame_tree_node.cc @@ -834,6 +834,12 @@ void FrameTreeNode::DidConsumeHistoryUserActivation() { } } +void FrameTreeNode::DidOpenDocumentInputStream() { + // document.open causes the document to lose its "initial empty document" + // status. + set_not_on_initial_empty_document(); +} + void FrameTreeNode::PruneChildFrameNavigationEntries( NavigationEntryImpl* entry) { for (size_t i = 0; i < current_frame_host()->child_count(); ++i) { diff --git a/content/browser/renderer_host/frame_tree_node.h b/content/browser/renderer_host/frame_tree_node.h index cc6eceeffe..4963a3ef4b 100644 --- a/content/browser/renderer_host/frame_tree_node.h +++ b/content/browser/renderer_host/frame_tree_node.h @@ -40,7 +40,6 @@ namespace content { class NavigationRequest; -class RenderFrameHostImpl; class NavigationEntryImpl; class FrameTree; @@ -184,14 +183,19 @@ class CONTENT_EXPORT FrameTreeNode : public RenderFrameHostOwner { return current_frame_host()->GetLastCommittedURL(); } - // Note that the current RenderFrameHost might not exist yet when calling this - // during FrameTreeNode initialization. In this case the FrameTreeNode must be - // on the initial empty document. Refer RFHI::is_initial_empty_document for a - // more details. + // Moves this frame out of the initial empty document state, which is a + // one-way change for FrameTreeNode (i.e., it cannot go back into the initial + // empty document state). + void set_not_on_initial_empty_document() { + is_on_initial_empty_document_ = false; + } + // Returns false if the frame has committed a document that is not the initial + // empty document, or if the current document's input stream has been opened + // with document.open(), causing the document to lose its "initial empty + // document" status. For more details, see the definition of + // `is_on_initial_empty_document_`. bool is_on_initial_empty_document() const { - return current_frame_host() - ? current_frame_host()->is_initial_empty_document() - : true; + return is_on_initial_empty_document_; } // Returns whether the frame's owner element in the parent document is @@ -652,9 +656,8 @@ class CONTENT_EXPORT FrameTreeNode : public RenderFrameHostOwner { bool UpdateUserActivationState( blink::mojom::UserActivationUpdateType update_type, blink::mojom::UserActivationNotificationType notification_type) override; - void DidConsumeHistoryUserActivation() override; - + void DidOpenDocumentInputStream() override; std::unique_ptr CreateNavigationRequestForSynchronousRendererCommit( RenderFrameHostImpl* render_frame_host, @@ -785,6 +788,33 @@ class CONTENT_EXPORT FrameTreeNode : public RenderFrameHostOwner { // stores the srcdoc_attribute's value for re-use in history navigations. std::string srcdoc_value_; + // Whether this frame is still on the initial about:blank document or the + // synchronously committed about:blank document committed at frame creation, + // and its "initial empty document"-ness is still true. + // This will be false if either of these has happened: + // - The current RenderFrameHost commits a cross-document navigation that is + // not the synchronously committed about:blank document per: + // https://html.spec.whatwg.org/multipage/browsers.html#creating-browsing-contexts:is-initial-about:blank + // - The document's input stream has been opened with document.open(), per + // https://html.spec.whatwg.org/multipage/dynamic-markup-insertion.html#opening-the-input-stream:is-initial-about:blank + // NOTE: we treat both the "initial about:blank document" and the + // "synchronously committed about:blank document" as the initial empty + // document. In the future, we plan to remove the synchronous about:blank + // commit so that this state will only be true if the frame is on the + // "initial about:blank document". See also: + // - https://github.com/whatwg/html/issues/6863 + // - https://crbug.com/1215096 + // + // Note that cross-document navigations update this state at + // DidCommitNavigation() time. Thus, this is still true when a cross-document + // navigation from an initial empty document is in the pending-commit window, + // after sending the CommitNavigation IPC but before receiving + // DidCommitNavigation(). This is in contrast to + // has_committed_any_navigation(), which is updated in CommitNavigation(). + // TODO(alexmos): Consider updating this at CommitNavigation() time as well to + // match the has_committed_any_navigation() behavior. + bool is_on_initial_empty_document_ = true; + // Whether the frame's owner element in the parent document is collapsed. bool is_collapsed_ = false; diff --git a/content/browser/renderer_host/navigation_controller_impl_browsertest.cc b/content/browser/renderer_host/navigation_controller_impl_browsertest.cc index a813ad4862..fa2a91280a 100644 --- a/content/browser/renderer_host/navigation_controller_impl_browsertest.cc +++ b/content/browser/renderer_host/navigation_controller_impl_browsertest.cc @@ -30,6 +30,7 @@ #include "content/browser/renderer_host/frame_navigation_entry.h" #include "content/browser/renderer_host/frame_tree.h" #include "content/browser/renderer_host/navigation_controller_impl.h" +#include "content/browser/renderer_host/navigation_type.h" #include "content/browser/renderer_host/navigation_entry_impl.h" #include "content/browser/renderer_host/navigation_entry_restore_context_impl.h" #include "content/browser/renderer_host/navigation_request.h" @@ -85,6 +86,7 @@ #include "third_party/blink/public/mojom/frame/frame.mojom-test-utils.h" #include "third_party/blink/public/mojom/frame/user_activation_update_types.mojom.h" #include "ui/display/display_util.h" +#include "url/url_constants.h" namespace content { namespace { @@ -4816,6 +4818,42 @@ IN_PROC_BROWSER_TEST_P(InitialEmptyDocNavigationControllerBrowserTest, } } +// Ensure the FrameTreeNode's initial empty document status does not become true +// again after it becomes false, even temporarily during a navigation after an +// early RenderFrameHost swap after a renderer crash. +// TODO(https://crbug.com/1072817): This test may become meaningless if we stop +// doing early RFH swaps for crashed frames. +IN_PROC_BROWSER_TEST_P(NavigationControllerBrowserTest, + InitialEmptyDocumentStatusAfterCrash) { + FrameTreeNode* root = static_cast(shell()->web_contents()) + ->GetPrimaryFrameTree() + .root(); + NavigationControllerImpl& controller = static_cast( + shell()->web_contents()->GetController()); + EXPECT_TRUE(root->is_on_initial_empty_document()); + + // Navigating to a real URL should leave the initial empty document. + GURL url_1(embedded_test_server()->GetURL("/title1.html")); + EXPECT_TRUE(NavigateToURL(shell(), url_1)); + EXPECT_FALSE(root->is_on_initial_empty_document()); + + // Crash the renderer. + CrashTab(shell()->web_contents()); + + // When starting a new navigation, the early RFH swap should not cause the + // FrameTreeNode to think it is back on the initial empty document, even + // temporarily before the new navigation commits. + GURL url_2(embedded_test_server()->GetURL("/title2.html")); + TestNavigationManager navigation_manager(shell()->web_contents(), url_2); + controller.LoadURL(url_2, Referrer(), ui::PAGE_TRANSITION_LINK, + std::string()); + EXPECT_TRUE(navigation_manager.WaitForRequestStart()); + EXPECT_FALSE(root->is_on_initial_empty_document()); + + ASSERT_TRUE(navigation_manager.WaitForNavigationFinished()); + EXPECT_FALSE(root->is_on_initial_empty_document()); +} + // Test a same-document navigation in a new window's initial empty document to // ensure it is classified correctly. IN_PROC_BROWSER_TEST_P(NavigationControllerBrowserTest, @@ -11295,7 +11333,7 @@ IN_PROC_BROWSER_TEST_P(NavigationControllerBrowserTest, // Ensure the renderer process does not get killed if the main frame URL's path // changes when going back in a subframe, since this was possible after // a replaceState in the main frame (thanks to https://crbug.com/373041). -// See https:///crbug.com/486916. +// See https://crbug.com/486916. IN_PROC_BROWSER_TEST_P(NavigationControllerBrowserTest, SubframeBackFromReplaceState) { // Start at a page with a real iframe. @@ -11434,6 +11472,234 @@ IN_PROC_BROWSER_TEST_P(NavigationControllerBrowserTest, cloned_previous_entry->root_node()->frame_entry->url()); } +// Test that location.replace in a subframe correctly replaces a shared +// FrameNavigationEntry in the current NavigationEntry rather than updating the +// shared FrameNavigationEntry across multiple NavigationEntries. +// See https://crbug.com/1515381. +IN_PROC_BROWSER_TEST_P(NavigationControllerBrowserTest, + SubframeBackFromSubframeLocationReplace) { + // It is safe to obtain the root frame tree node here, as it doesn't change. + FrameTreeNode* root = static_cast(shell()->web_contents()) + ->GetPrimaryFrameTree() + .root(); + NavigationControllerImpl& controller = static_cast( + shell()->web_contents()->GetController()); + + // 1. Navigate to A(B(srcdoc_B)). + GURL main_url( + embedded_test_server()->GetURL("a.com", "/page_with_blank_iframe.html")); + GURL middle_frame_url(embedded_test_server()->GetURL( + "b.com", "/frame_tree/page_with_srcdoc_frame.html")); + EXPECT_TRUE(NavigateToURL(shell(), main_url)); + ASSERT_EQ(1U, root->child_count()); + FrameTreeNode* child = root->child_at(0); + { + FrameNavigateParamsCapturer capturer(child); + EXPECT_TRUE(NavigateToURLFromRenderer(child, middle_frame_url)); + capturer.Wait(); + EXPECT_EQ(NAVIGATION_TYPE_AUTO_SUBFRAME, capturer.navigation_type()); + } + // Verify the subframe URLs and srcdoc origin. + ASSERT_EQ(1U, child->child_count()); + FrameTreeNode* grandchild = child->child_at(0); + EXPECT_EQ(middle_frame_url, + child->current_frame_host()->GetLastCommittedURL()); + EXPECT_EQ(GURL(url::kAboutSrcdocURL), + grandchild->current_frame_host()->GetLastCommittedURL()); + EXPECT_EQ(url::Origin::Create(middle_frame_url), + grandchild->current_frame_host()->GetLastCommittedOrigin()); + NavigationEntryImpl* entry1 = controller.GetLastCommittedEntry(); + + // 2. Navigate innermost frame to about:blank_A, from the main frame. + { + FrameNavigateParamsCapturer capturer(grandchild); + EXPECT_TRUE(ExecJs(root, "frames[0][0].location = 'about:blank';")); + capturer.Wait(); + EXPECT_EQ(NAVIGATION_TYPE_NEW_SUBFRAME, capturer.navigation_type()); + } + EXPECT_EQ(GURL(url::kAboutBlankURL), + grandchild->current_frame_host()->GetLastCommittedURL()); + EXPECT_EQ(url::Origin::Create(main_url), + grandchild->current_frame_host()->GetLastCommittedOrigin()); + // The middle frame's FrameNavigationEntry should be shared across the two + // NavigationEntries. + EXPECT_EQ(2, controller.GetEntryCount()); + NavigationEntryImpl* entry2 = controller.GetLastCommittedEntry(); + EXPECT_NE(entry1, entry2); + EXPECT_EQ(entry1->GetFrameEntry(child), entry2->GetFrameEntry(child)); + EXPECT_NE(entry1->GetFrameEntry(grandchild), + entry2->GetFrameEntry(grandchild)); + + // 3. Location.replace in middle frame, to A(A(about:blank_A)). + { + FrameNavigateParamsCapturer capturer(child); + EXPECT_TRUE( + ExecJs(root, JsReplace("frames[0].location.replace($1)", main_url))); + capturer.Wait(); + EXPECT_TRUE(capturer.did_replace_entry()); + EXPECT_EQ(NAVIGATION_TYPE_AUTO_SUBFRAME, capturer.navigation_type()); + } + // There's a new grandchild FrameTreeNode at this point. + grandchild = child->child_at(0); + EXPECT_EQ(GURL(url::kAboutBlankURL), + grandchild->current_frame_host()->GetLastCommittedURL()); + EXPECT_EQ(url::Origin::Create(main_url), + grandchild->current_frame_host()->GetLastCommittedOrigin()); + // The FrameNavigationEntry in the middle frame should no longer be shared. + EXPECT_NE(entry1->GetFrameEntry(child), entry2->GetFrameEntry(child)); + + // 4. Go back. + { + TestNavigationObserver observer(shell()->web_contents()); + EXPECT_TRUE(ExecJs(root, "history.back()")); + observer.Wait(); + } + + // There's a new grandchild FrameTreeNode again at this point. + grandchild = child->child_at(0); + + // Ensure the middle and innermost frames both navigated. + EXPECT_EQ(middle_frame_url, + child->current_frame_host()->GetLastCommittedURL()); + EXPECT_EQ(GURL(url::kAboutSrcdocURL), + grandchild->current_frame_host()->GetLastCommittedURL()); + + // Ensure the innermost frame ends up back in B's origin and process. + EXPECT_EQ(url::Origin::Create(middle_frame_url), + grandchild->current_frame_host()->GetLastCommittedOrigin()); + EXPECT_EQ(child->current_frame_host()->GetProcess(), + grandchild->current_frame_host()->GetProcess()); + if (AreAllSitesIsolatedForTesting()) { + EXPECT_NE(root->current_frame_host()->GetProcess(), + grandchild->current_frame_host()->GetProcess()); + } + + // Make sure the renderer processes have not been killed. + EXPECT_TRUE(root->current_frame_host()->IsRenderFrameLive()); + EXPECT_TRUE(child->current_frame_host()->IsRenderFrameLive()); +} + +// Similar to SubframeBackFromSubframeLocationReplace test, but covers a tricky +// early RFH swap case if the renderer process crashes before location.replace. +// See https://crbug.com/1515381. +IN_PROC_BROWSER_TEST_P(NavigationControllerBrowserTest, + SubframeBackFromSubframeLocationReplaceAfterCrash) { + // This tests something that can only happen with out of process iframes. + if (!AreAllSitesIsolatedForTesting()) { + return; + } + + // It is safe to obtain the root frame tree node here, as it doesn't change. + FrameTreeNode* root = static_cast(shell()->web_contents()) + ->GetPrimaryFrameTree() + .root(); + NavigationControllerImpl& controller = static_cast( + shell()->web_contents()->GetController()); + + // 1. Navigate to A(B(srcdoc_B)). + GURL main_url( + embedded_test_server()->GetURL("a.com", "/page_with_blank_iframe.html")); + GURL middle_frame_url(embedded_test_server()->GetURL( + "b.com", "/frame_tree/page_with_srcdoc_frame.html")); + EXPECT_TRUE(NavigateToURL(shell(), main_url)); + ASSERT_EQ(1U, root->child_count()); + FrameTreeNode* child = root->child_at(0); + { + FrameNavigateParamsCapturer capturer(child); + EXPECT_TRUE(NavigateToURLFromRenderer(child, middle_frame_url)); + capturer.Wait(); + EXPECT_EQ(NAVIGATION_TYPE_AUTO_SUBFRAME, capturer.navigation_type()); + } + // Verify the subframe URLs and srcdoc origin. + ASSERT_EQ(1U, child->child_count()); + FrameTreeNode* grandchild = child->child_at(0); + EXPECT_EQ(middle_frame_url, + child->current_frame_host()->GetLastCommittedURL()); + EXPECT_EQ(GURL(url::kAboutSrcdocURL), + grandchild->current_frame_host()->GetLastCommittedURL()); + EXPECT_EQ(url::Origin::Create(middle_frame_url), + grandchild->current_frame_host()->GetLastCommittedOrigin()); + NavigationEntryImpl* entry1 = controller.GetLastCommittedEntry(); + + // 2. Navigate innermost frame to about:blank_A, from the main frame. + { + FrameNavigateParamsCapturer capturer(grandchild); + EXPECT_TRUE(ExecJs(root, "frames[0][0].location = 'about:blank';")); + capturer.Wait(); + EXPECT_EQ(NAVIGATION_TYPE_NEW_SUBFRAME, capturer.navigation_type()); + } + EXPECT_EQ(GURL(url::kAboutBlankURL), + grandchild->current_frame_host()->GetLastCommittedURL()); + EXPECT_EQ(url::Origin::Create(main_url), + grandchild->current_frame_host()->GetLastCommittedOrigin()); + // The middle frame's FrameNavigationEntry should be shared across the two + // NavigationEntries. + EXPECT_EQ(2, controller.GetEntryCount()); + NavigationEntryImpl* entry2 = controller.GetLastCommittedEntry(); + EXPECT_NE(entry1, entry2); + EXPECT_EQ(entry1->GetFrameEntry(child), entry2->GetFrameEntry(child)); + EXPECT_NE(entry1->GetFrameEntry(grandchild), + entry2->GetFrameEntry(grandchild)); + + // Simulate a renderer process crash in the child frame, so that the next + // navigation in this frame will trigger an early RFH swap. + // TODO(https://crbug.com/1072817): This test may become unnecessary if we + // stop doing early RFH swaps for crashed renderer processes. + RenderProcessHost* process_b = child->current_frame_host()->GetProcess(); + RenderProcessHostWatcher crash_observer( + process_b, RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT); + EXPECT_TRUE(process_b->Shutdown(RESULT_CODE_KILLED)); + crash_observer.Wait(); + + // 3. Location.replace in middle frame, to A(A(about:blank_A)). + { + FrameNavigateParamsCapturer capturer(child); + EXPECT_TRUE( + ExecJs(root, JsReplace("frames[0].location.replace($1)", main_url))); + capturer.Wait(); + EXPECT_TRUE(capturer.did_replace_entry()); + EXPECT_EQ(NAVIGATION_TYPE_AUTO_SUBFRAME, capturer.navigation_type()); + } + // There's a new grandchild FrameTreeNode at this point. + grandchild = child->child_at(0); + EXPECT_EQ(GURL(url::kAboutBlankURL), + grandchild->current_frame_host()->GetLastCommittedURL()); + EXPECT_EQ(url::Origin::Create(main_url), + grandchild->current_frame_host()->GetLastCommittedOrigin()); + // The FrameNavigationEntry in the middle frame should no longer be shared. + EXPECT_NE(entry1->GetFrameEntry(child), entry2->GetFrameEntry(child)); + + // 4. Go back. + { + TestNavigationObserver observer(shell()->web_contents()); + EXPECT_TRUE(ExecJs(root, "history.back()")); + observer.Wait(); + } + + // There's a new grandchild FrameTreeNode again at this point. + grandchild = child->child_at(0); + + // Ensure the middle and innermost frames both navigated. + EXPECT_EQ(middle_frame_url, + child->current_frame_host()->GetLastCommittedURL()); + EXPECT_EQ(GURL(url::kAboutSrcdocURL), + grandchild->current_frame_host()->GetLastCommittedURL()); + + // Ensure the innermost frame ends up back in B's origin and process. + EXPECT_EQ(url::Origin::Create(middle_frame_url), + grandchild->current_frame_host()->GetLastCommittedOrigin()); + EXPECT_EQ(child->current_frame_host()->GetProcess(), + grandchild->current_frame_host()->GetProcess()); + if (AreAllSitesIsolatedForTesting()) { + EXPECT_NE(root->current_frame_host()->GetProcess(), + grandchild->current_frame_host()->GetProcess()); + } + + // Make sure the renderer processes have not been killed. + EXPECT_TRUE(root->current_frame_host()->IsRenderFrameLive()); + EXPECT_TRUE(child->current_frame_host()->IsRenderFrameLive()); +} + // Test that shared FrameNavigationEntries with opaque initiator origins can be // restored in future sessions, even if the internal nonce is not preserved. // See https://crbug.com/1354634. diff --git a/content/browser/renderer_host/navigation_request.cc b/content/browser/renderer_host/navigation_request.cc index 5342ef7d86..ccc62010dc 100644 --- a/content/browser/renderer_host/navigation_request.cc +++ b/content/browser/renderer_host/navigation_request.cc @@ -5538,11 +5538,30 @@ void NavigationRequest::CommitNavigation() { : GetOriginToCommit().value(); // TODO(crbug.com/979296): Consider changing this code to copy an origin // instead of creating one from a URL which lacks opacity information. + url::Origin origin_to_commit = GetOriginToCommit().value(); isolation_info_for_subresources_ = GetRenderFrameHost()->ComputeIsolationInfoForSubresourcesForPendingCommit( origin_to_commit, is_credentialless(), ComputeFencedFrameNonce()); DCHECK(!isolation_info_for_subresources_.IsEmpty()); + // If this is a srcdoc document, the content comes from the parent frame, so + // the origin must be the parent and not the initiator. In this case, do not + // inherit the base URI from the initiator if the origins do not agree + // (accounting for the case that the chosen origin might be opaque with a + // precursor of the parent's origin, in a sandboxed case). There should also + // not be an initiator base URL if there is no initiator origin, such as in a + // browser-initiated navigation. + if (GetURL().IsAboutSrcdoc() && + (!common_params().initiator_origin || + origin_to_commit.GetTupleOrPrecursorTupleIfOpaque() != + common_params() + .initiator_origin->GetTupleOrPrecursorTupleIfOpaque())) { + // TODO(https://crbug.com/1169736): Make this unreachable by blocking + // cross-origin about:srcdoc navigations. Then enforce that the chosen + // origin for srcdoc cases agrees with the parent frame's origin. + common_params_->initiator_base_url = absl::nullopt; + } + // TODO(https://crbug.com/888079): The storage key's origin is ignored at the // moment. We will be able to use it once the browser can compute the origin // to commit. diff --git a/content/browser/renderer_host/render_frame_host_impl.cc b/content/browser/renderer_host/render_frame_host_impl.cc index b2182a0871..8fbba3ef4e 100644 --- a/content/browser/renderer_host/render_frame_host_impl.cc +++ b/content/browser/renderer_host/render_frame_host_impl.cc @@ -3836,7 +3836,7 @@ void RenderFrameHostImpl::DidNavigate( if (!navigation_request->IsSameDocument() && (!navigation_request->is_synchronous_renderer_commit() || !navigation_request->GetURL().IsAboutBlank())) { - SetNotInitialEmptyDocument(); + navigation_request->frame_tree_node()->set_not_on_initial_empty_document(); } // For uuid-in-package: resources served from WebBundles, use the Bundle's @@ -4802,7 +4802,11 @@ void RenderFrameHostImpl::DidOpenDocumentInputStream(const GURL& url) { GURL filtered_url(url); GetProcess()->FilterURL(/*empty_allowed=*/false, &filtered_url); renderer_url_info_.last_document_url = filtered_url; - DidOpenDocumentInputStream(); + // `owner_` could be null if we get this message asynchronously from the + // renderer in pending deletion state. + if (owner_) { + owner_->DidOpenDocumentInputStream(); + } } RenderWidgetHostImpl* RenderFrameHostImpl::GetRenderWidgetHost() { @@ -9523,6 +9527,10 @@ void RenderFrameHostImpl::SendAllPendingBeaconsOnNavigation() { } } +bool RenderFrameHostImpl::is_initial_empty_document() const { + return frame_tree_node_->is_on_initial_empty_document(); +} + uint32_t RenderFrameHostImpl::FindSuddenTerminationHandlers(bool same_origin) { uint32_t navigation_termination = 0; // Search this frame and subframes for sudden termination disablers diff --git a/content/browser/renderer_host/render_frame_host_impl.h b/content/browser/renderer_host/render_frame_host_impl.h index d3355eaffa..5488cda8cb 100644 --- a/content/browser/renderer_host/render_frame_host_impl.h +++ b/content/browser/renderer_host/render_frame_host_impl.h @@ -2843,19 +2843,17 @@ class CONTENT_EXPORT RenderFrameHostImpl // document. void SendAllPendingBeaconsOnNavigation(); - // Sets `is_initial_empty_document_` to false. - void SetNotInitialEmptyDocument() { is_initial_empty_document_ = false; } - // Returns false if this document not the initial empty document, or if the // current document's input stream has been opened with document.open(), // causing the document to lose its "initial empty document" status. For more - // details, see the definition of `is_initial_empty_document_`. - bool is_initial_empty_document() const { return is_initial_empty_document_; } + // details, see the definition of `FrameTreeNode::is_initial_empty_document_`. + // This is implemented in the .cc file to avoid a circular dependency on + // frame_tree_node.h. + // TODO(https://crbug.com/1517371): Remove this function. Callers within RFHI + // should migrate to another way of getting this information such as having a + // caller pass it or through `owner_`. + bool is_initial_empty_document() const; - // Sets `is_initial_empty_document_` to - // false. Must only be called after the current document's input stream has - // been opened with document.open(). - void DidOpenDocumentInputStream() { is_initial_empty_document_ = false; } enum class FencedFrameStatus { kNotNestedInFencedFrame, @@ -4960,24 +4958,6 @@ class CONTENT_EXPORT RenderFrameHostImpl // nested within a fenced frame. const FencedFrameStatus fenced_frame_status_; - // Whether this document is the initial about:blank document or the - // synchronously committed about:blank document committed at frame creation, - // and its "initial empty document"-ness is still true. - // This will be false if either of these has happened: - // - The RenderFrameHost had committed a cross-document navigation that is - // not the synchronously committed about:blank document per: - // https://html.spec.whatwg.org/multipage/browsers.html#creating-browsing-contexts:is-initial-about:blank - // - The document's input stream has been opened with document.open(), per - // https://html.spec.whatwg.org/multipage/dynamic-markup-insertion.html#opening-the-input-stream:is-initial-about:blank - // NOTE: we treat both the "initial about:blank document" and the - // "synchronously committed about:blank document" as the initial empty - // document. In the future, we plan to remove the synchronous about:blank - // commit so that this state will only be true if the frame is on the - // "initial about:blank document". See also: - // - https://github.com/whatwg/html/issues/6863 - // - https://crbug.com/1215096 - bool is_initial_empty_document_ = true; - // Testing callback run in DidStopLoading() regardless of loading state. This // is useful for tests that need to detect when newly created frames finish // loading about:blank. diff --git a/content/browser/renderer_host/render_frame_host_owner.h b/content/browser/renderer_host/render_frame_host_owner.h index 623c381b0f..3f2452d898 100644 --- a/content/browser/renderer_host/render_frame_host_owner.h +++ b/content/browser/renderer_host/render_frame_host_owner.h @@ -89,6 +89,10 @@ class RenderFrameHostOwner { // has been consumed, in response to an event in the renderer process. virtual void DidConsumeHistoryUserActivation() = 0; + // Called when document.open occurs, which causes the frame to no longer be in + // an initial empty document state. + virtual void DidOpenDocumentInputStream() = 0; + // Creates a NavigationRequest for a synchronous navigation that has // committed in the renderer process. Those are: // - same-document renderer-initiated navigations. -- Gitee