OOPIF: Replicate dynamic window.name updates.

content/browser/site_per_process_browsertest.cc

Index: content/browser/site_per_process_browsertest.cc
diff --git a/content/browser/site_per_process_browsertest.cc b/content/browser/site_per_process_browsertest.cc
index f2a76b93138774dec32d1229c2dd83e4e48cae69..2f14b66f75132d642433f8b3f3f2eb9d29a82bd4 100644
--- a/content/browser/site_per_process_browsertest.cc
+++ b/content/browser/site_per_process_browsertest.cc
@@ -1185,6 +1185,65 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, WindowNameReplication) {
   EXPECT_EQ(result, "3-1-name");
 }
 
+// Verify that dynamic updates to a frame's window.name propagate to the
+// frame's proxies, so that the latest frame names can be used in frame

[nasko, 2015/03/05 18:09:28]

nit: s/used in frame navigations/used in navigations/

[alexmos, 2015/03/09 18:47:58]

Done.

+// navigations.
+IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, DynamicWindowName) {
+  GURL main_url(embedded_test_server()->GetURL("/frame_tree/2-4.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();
+

[nasko, 2015/03/05 18:09:28]

nit: no need for empty line

[alexmos, 2015/03/09 18:47:58]

Done.

+  TestNavigationObserver observer(shell()->web_contents());
+
+  // Load cross-site page into iframe.
+  GURL frame_url =
+      embedded_test_server()->GetURL("foo.com", "/frame_tree/3-1.html");
+  NavigateFrameToURL(root->child_at(0), frame_url);
+  EXPECT_TRUE(observer.last_navigation_succeeded());
+  EXPECT_EQ(frame_url, observer.last_navigation_url());
+
+  // Browser process should know the child frame's original window.name
+  // specified in the iframe element.
+  EXPECT_EQ(root->child_at(0)->frame_name(), "3-1-name");
+
+  // Update the child frame's window.name.
+  EXPECT_TRUE(ExecuteScript(root->child_at(0)->current_frame_host(),
+                            "window.domAutomationController.send("
+                            "window.name = 'updated-name');"));
+
+  // The change should propagate to the browser process.
+  EXPECT_EQ(root->child_at(0)->frame_name(), "updated-name");

[nasko, 2015/03/05 18:09:27]

What guarantees that the name has been propagated to the browser process between
ExecuteScript and this line? It looks like this could be timing dependent and
cause the test to be flaky.

[alexmos, 2015/03/09 18:47:58]

If I understand correctly, the ExecuteScript in 1214 will block until the JS
string finishes executing and returns a value (ignored here), by waiting for a
FrameHostMsg_DomOperationResponse.  On the renderer side, script execution
synchronously triggers a FrameHostMsg_DidChangeName, and then, after the
execution finishes, a FrameHostMsg_DomOperationResponse. So, assuming message
order is preserved, we should have received and processed
FrameHostMsg_DidChangeName before FrameHostMsg_DomOperationResponse, right?  I
used a similar approach in the DynamicSandboxFlags tests, and also ran the test
a bunch of times as a sanity check.

[nasko, 2015/03/09 21:43:40]

Acknowledged.

+
+  // The proxy in the parent process should also receive the updated name.
+  // Check that it can reference the child frame by its new name.
+  bool success = false;
+  EXPECT_TRUE(
+      ExecuteScriptAndExtractBool(shell()->web_contents(),
+                                  "window.domAutomationController.send("
+                                  "frames['updated-name'] == frames[0]);",
+                                  &success));
+  EXPECT_TRUE(success);
+
+  // Issue a renderer-initiated navigation from the root frame to the child
+  // frame using the frame's name. Make sure correct frame is navigated.
+  //
+  // TODO(alexmos): When blink::createWindow is refactored to handle
+  // RemoteFrames, this should also be tested via window.open(url, frame_name)
+  // and a more complicated frame hierarchy.
+  TestFrameNavigationObserver frame_observer(root->child_at(0));
+  EXPECT_TRUE(ExecuteScript(shell()->web_contents(),
+                            "window.domAutomationController.send("
+                            "frames['updated-name'].location.href = "
+                            "'/cross-site/foo.com/title1.html');"));

[nasko, 2015/03/05 18:09:28]

Why not use the result of embedded_test_server()->GetURL("foo.com",
"/title1.html") here instead of using the cross-site redirector?

[alexmos, 2015/03/09 18:47:58]

Done.

+  frame_observer.Wait();
+  EXPECT_EQ(embedded_test_server()->GetURL("foo.com", "/title1.html"),
+            root->child_at(0)->current_url());
+}
+
 // TODO(lfg): Merge the test below with NavigateRemoteFrame test.
 // TODO(lfg): Disabled because this triggers http://crbug.com/433012, and since
 // the renderer process crashes, it causes the title watcher to never return.