Pass origin information for remote frame creation.

content/browser/frame_host/frame_tree_browsertest.cc

Index: content/browser/frame_host/frame_tree_browsertest.cc
diff --git a/content/browser/frame_host/frame_tree_browsertest.cc b/content/browser/frame_host/frame_tree_browsertest.cc
index 9f16066cd38752be333d5af39867ef7d4618d346..f4dcbe83100b37988983e6f6dbe15b718fbad1c3 100644
--- a/content/browser/frame_host/frame_tree_browsertest.cc
+++ b/content/browser/frame_host/frame_tree_browsertest.cc
@@ -199,6 +199,43 @@ IN_PROC_BROWSER_TEST_F(FrameTreeBrowserTest, IsRenderFrameLive) {
   EXPECT_TRUE(root->child_at(0)->current_frame_host()->IsRenderFrameLive());
 }
 
+// Ensure that origins are correctly set on navigations.
+IN_PROC_BROWSER_TEST_F(FrameTreeBrowserTest, OriginSetOnNavigation) {
+  host_resolver()->AddRule("*", "127.0.0.1");

[nasko, 2014/11/20 17:22:54]

Let's use the EmbeddedTestServer in all new tests. The tests in
site_per_process_browsertest.cc use it if you want to look up an example.

[ncarter (slow), 2014/11/20 20:50:45]

The three second version:
 - test_server() becomes embedded_test_server()
 - Start() becomes InitializeAndWaitUntilReady()
 - URLs that were "files/whatever" become "/whatever" (there's no /files
servelet -- the files are just reachable at URL paths relative to the test root
dir)

[alexmos, 2014/11/20 21:09:10]

As we discussed in person, I can do that in a separate CL for all the tests in
this file.  That way we won't randomize this CL and can centralize the
EmbeddedTestServer initialization.

[nasko, 2014/11/20 21:21:21]

Acknowledged.

+  ASSERT_TRUE(test_server()->Start());
+  GURL main_url(test_server()->GetURL("files/frame_tree/top.html"));
+  EXPECT_TRUE(NavigateToURL(shell(), main_url));
+
+  // It is safe to obtain the root frame tree node here, as it doesn't change.
+  FrameTreeNode* root = static_cast<WebContentsImpl*>(shell()->web_contents())
+                            ->GetFrameTree()->root();
+
+  // Extra '/' is added because the replicated origin is serialized in RFC 6454
+  // format, which dictates no trailing '/', whereas GURL::GetOrigin does put a
+  // '/' at the end.
+  EXPECT_EQ(root->current_replication_state().origin.string() + '/',
+            main_url.GetOrigin().spec());
+
+  GURL frame_url(test_server()->GetURL("files/title1.html"));
+  NavigateFrameToURL(root->child_at(0), frame_url);
+
+  EXPECT_EQ(
+      root->child_at(0)->current_replication_state().origin.string() + '/',
+      frame_url.GetOrigin().spec());
+
+  GURL data_url("data:text/html,foo");
+  EXPECT_TRUE(NavigateToURL(shell(), data_url));

[nasko, 2014/11/20 17:22:54]

Why not navigate another iframe on the page instead of the top-level? It will
give us a bit more variety, as lots of tests already navigate the top-level
frame.

[alexmos, 2014/11/20 21:09:10]

Interesting -- doing that made the test crash when running without
--site-per-process, while passing just fine with --site-per-process.  So I think
your suspicion about NavigateFrameToURL misbehaving in regular mode is correct. 
So I'm leaving this test as is, and instead I've added the
navigate-frame-to-data-url test to the OriginSetOnCrossProcessNavigations below,
which only runs with --site-per-process.  For the record, the crash was this
CHECK(render_view_host):
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/fr...

+
+  // Navigating to a data URL should set a unique origin.  This is represented
+  // as "null" per RFC 6454.
+  EXPECT_EQ(root->current_replication_state().origin.string(), "null");
+
+  // Re-navigating to a normal URL should update the origin.
+  EXPECT_TRUE(NavigateToURL(shell(), main_url));
+  EXPECT_EQ(root->current_replication_state().origin.string() + '/',
+            main_url.GetOrigin().spec());
+}
+
 class CrossProcessFrameTreeBrowserTest : public ContentBrowserTest {
  public:
   CrossProcessFrameTreeBrowserTest() {}
@@ -277,4 +314,43 @@ IN_PROC_BROWSER_TEST_F(CrossProcessFrameTreeBrowserTest,
   EXPECT_TRUE(root->child_at(0)->current_frame_host()->IsRenderFrameLive());
 }
 
+IN_PROC_BROWSER_TEST_F(CrossProcessFrameTreeBrowserTest,
+                       OriginSetOnCrossProcessNavigations) {
+  host_resolver()->AddRule("*", "127.0.0.1");

[nasko, 2014/11/20 17:22:54]

Same as the other tests, let's use EmbeddedTestServer.

[alexmos, 2014/11/20 21:09:10]

See above -- will do the whole file in a separate CL.

+  ASSERT_TRUE(test_server()->Start());
+  GURL main_url(test_server()->GetURL("files/site_per_process_main.html"));
+  EXPECT_TRUE(NavigateToURL(shell(), main_url));
+
+  // It is safe to obtain the root frame tree node here, as it doesn't change.
+  FrameTreeNode* root = static_cast<WebContentsImpl*>(shell()->web_contents())
+                            ->GetFrameTree()->root();
+
+  EXPECT_EQ(root->current_replication_state().origin.string() + '/',
+            main_url.GetOrigin().spec());
+
+  // First frame is an about:blank frame.  Check that its origin is correctly
+  // inherited from the parent.
+  EXPECT_EQ(
+      root->child_at(0)->current_replication_state().origin.string() + '/',
+      main_url.GetOrigin().spec());
+
+  // These must stay in scope with replace_host.
+  GURL::Replacements replace_host;

[nasko, 2014/11/20 17:22:54]

Ah, you also skip all that boilerplate code and just use GetURL(host, path) ;).

[alexmos, 2014/11/20 21:09:10]

test_server() doesn't support this, right?  If so, how about I do that cleanup
in the CL above, as this file has a few more places using this?

[nasko, 2014/11/20 21:21:21]

Yes, the API is on the EmbeddedTestServer. This was a comment that you lose
complexity if you use it.

+  std::string foo_com("foo.com");
+
+  // Load cross-site page into the first frame.
+  GURL cross_site_url(test_server()->GetURL("files/title2.html"));
+  replace_host.SetHostStr(foo_com);
+  cross_site_url = cross_site_url.ReplaceComponents(replace_host);
+  NavigateFrameToURL(root->child_at(0), cross_site_url);
+
+  EXPECT_EQ(
+      root->child_at(0)->current_replication_state().origin.string() + '/',
+      cross_site_url.GetOrigin().spec());
+
+  // The root's origin shouldn't have changed.
+  EXPECT_EQ(root->current_replication_state().origin.string() + '/',
+            main_url.GetOrigin().spec());
+}
+
 }  // namespace content