Commit c8a7cccf authored by Balazs Engedy's avatar Balazs Engedy Committed by Commit Bot

Let `about:blank#ref` also be classified as initial empty document.

Before this CL, content::FrameTreeNode used to classify only the literal
`about:blank` as the URL corresponding to the initial empty document.

However, navigations from the initial empty document to URLs such as
`about:blank#ref` are same-document and therefore re-use the initial
empty document, so committing any such same-document navigations should
not be considered "real loads" by FrameTreeNode (given they are not
considered "real loads" by Blink).

This CL changes FrameTreeNode::SetCurrentURL() to use !IsAboutBlank() to
determine whether the first real load is currently taking place in a
frame, that is, whether it is a first time a URL is being committed that
is not the initial empty document (modulo an optional #fragment).

Bug: 821022, 729021
Change-Id: I040a11f58bf27174e0c450377f8cd81a7abeac70
Reviewed-on: https://chromium-review.googlesource.com/958925Reviewed-by: default avatarCharlie Reis <creis@chromium.org>
Commit-Queue: Balazs Engedy <engedy@chromium.org>
Commit-Queue: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542645}
parent 242f4a44
......@@ -313,7 +313,7 @@ void FrameTreeNode::SetOriginalOpener(FrameTreeNode* opener) {
}
void FrameTreeNode::SetCurrentURL(const GURL& url) {
if (!has_committed_real_load_ && url != url::kAboutBlankURL)
if (!has_committed_real_load_ && !url.IsAboutBlank())
has_committed_real_load_ = true;
current_frame_host()->SetLastCommittedUrl(url);
blame_context_.TakeSnapshot();
......
......@@ -25,6 +25,7 @@
#include "content/public/test/browser_test_utils.h"
#include "content/public/test/content_browser_test.h"
#include "content/public/test/content_browser_test_utils.h"
#include "content/public/test/test_frame_navigation_observer.h"
#include "content/public/test/test_navigation_observer.h"
#include "content/public/test/test_utils.h"
#include "content/shell/browser/shell.h"
......@@ -1229,4 +1230,73 @@ IN_PROC_BROWSER_TEST_F(RenderFrameHostImplBrowserTest,
}
}
// Regression test for https://crbug.com/821022.
//
// Test the edge case of the above, namely, where the following commits take
// place in a subframe embedded into a document at `http://foo.com/`:
//
// 1) the initial empty document (`about:blank`)
// 2) `about:blank#ref`
// 3) `http://foo.com`
//
// Here, (2) should classify as a same-document navigation, and (3) should be
// considered the first real load. Because the first real load is same-origin
// with the initial empty document, the latter's `window` global object
// asssociated with the initial empty document is re-used for document
// corresponding to the first real committed load.
IN_PROC_BROWSER_TEST_F(
RenderFrameHostImplBrowserTest,
InterfaceProviderRequestNotPresentForFirstRealLoadAfterAboutBlankWithRef) {
const GURL kMainFrameURL(embedded_test_server()->GetURL("/title1.html"));
const GURL kSubframeURLTwo("about:blank#ref");
const GURL kSubframeURLThree(embedded_test_server()->GetURL("/title2.html"));
const auto kNavigateToOneThenTwoScript = base::StringPrintf(
"var f = document.createElement(\"iframe\");"
"f.src=\"%s\"; "
"document.body.append(f);",
kSubframeURLTwo.spec().c_str());
const auto kNavigateToThreeScript =
base::StringPrintf("f.src=\"%s\";", kSubframeURLThree.spec().c_str());
ASSERT_TRUE(NavigateToURL(shell(), kMainFrameURL));
// Trigger navigation (1) by creating a new subframe, and then trigger
// navigation (2) by setting it's `src` attribute before adding it to the DOM.
//
// We must set 'src` before adding the iframe element to the DOM, otherwise it
// will load `about:blank` as the first real load instead of
// |kSubframeURLTwo|. See: https://crbug.com/778318.
//
// Note that the child frame will first cycle through loading the initial
// empty document regardless of when/how/if the `src` attribute is set.
ASSERT_TRUE(ExecuteScript(shell(), kNavigateToOneThenTwoScript));
WaitForLoadStop(shell()->web_contents());
FrameTreeNode* root = static_cast<WebContentsImpl*>(shell()->web_contents())
->GetFrameTree()
->root();
ASSERT_EQ(1u, root->child_count());
FrameTreeNode* child = root->child_at(0u);
EXPECT_FALSE(child->has_committed_real_load());
EXPECT_EQ(kSubframeURLTwo, child->current_url());
EXPECT_EQ(url::Origin::Create(kMainFrameURL), child->current_origin());
// Set the `src` attribute again to trigger navigation (3).
TestFrameNavigationObserver commit_observer(child->current_frame_host());
ScopedFakeInterfaceProviderRequestInjector injector(shell()->web_contents());
injector.set_fake_request_for_next_commit(nullptr);
ASSERT_TRUE(ExecuteScript(shell(), kNavigateToThreeScript));
commit_observer.WaitForCommit();
EXPECT_FALSE(injector.original_request_of_last_commit().is_pending());
EXPECT_TRUE(child->has_committed_real_load());
EXPECT_EQ(kSubframeURLThree, child->current_url());
EXPECT_EQ(url::Origin::Create(kMainFrameURL), child->current_origin());
}
} // namespace content
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment