OOPIF: Fix willSendRequest in A-B-A nested case.

content/browser/frame_host/navigation_controller_impl_browsertest.cc

Index: content/browser/frame_host/navigation_controller_impl_browsertest.cc
diff --git a/content/browser/frame_host/navigation_controller_impl_browsertest.cc b/content/browser/frame_host/navigation_controller_impl_browsertest.cc
index d676d4504475ba7db098c54adf46a622294a95ec..1a6426a319f1a466568bfaea29de966e70d3caf6 100644
--- a/content/browser/frame_host/navigation_controller_impl_browsertest.cc
+++ b/content/browser/frame_host/navigation_controller_impl_browsertest.cc
@@ -1361,8 +1361,69 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest,
     EXPECT_EQ(0U, entry->root_node()->children.size());
   }
 
-  // TODO(creis): Add tests for another subframe on B, and for a subframe on A
-  // within it.  Both are currently broken.
+  // 4. Create a third iframe on the same site as the second.  This ensures that
+  // the commit type is correct even when the subframe process already exists.
+  {
+    LoadCommittedCapturer capturer(shell()->web_contents());
+    std::string script = "var iframe = document.createElement('iframe');"
+                         "iframe.src = '" + foo_url.spec() + "';"
+                         "document.body.appendChild(iframe);";
+    EXPECT_TRUE(content::ExecuteScript(root->current_frame_host(), script));
+    capturer.Wait();
+    EXPECT_EQ(ui::PAGE_TRANSITION_AUTO_SUBFRAME, capturer.transition_type());
+  }
+
+  // The last committed NavigationEntry shouldn't have changed.
+  EXPECT_EQ(1, controller.GetEntryCount());
+  entry = controller.GetLastCommittedEntry();
+  EXPECT_EQ(main_url, entry->GetURL());
+  root_entry = entry->root_node()->frame_entry.get();
+  EXPECT_EQ(main_url, root_entry->url());
+
+  // Verify subframe entries if we're in --site-per-process mode.
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kSitePerProcess)) {
+    // The entry should now have 3 subframe FrameNavigationEntries.
+    ASSERT_EQ(3U, entry->root_node()->children.size());
+    FrameNavigationEntry* frame_entry =
+        entry->root_node()->children[2]->frame_entry.get();
+    EXPECT_EQ(foo_url, frame_entry->url());
+  } else {
+    // There are no subframe FrameNavigationEntries by default.
+    EXPECT_EQ(0U, entry->root_node()->children.size());
+  }
+
+  // 5. Create a nested iframe on the original site (A-B-A).
+  {
+    LoadCommittedCapturer capturer(shell()->web_contents());

[Avi (use Gerrit), 2015/07/08 18:49:23]

Why a LoadCommittedCapturer? A LoadCommittedCapturer is strictly weaker than a
FrameNavigateParamsCapturer, as it only captures the params, not the details,
but a LoadCommittedCapturer is useful when we only have a load commit but not a
navigation.

I can understand its use above for iframe creation, but for a load like this, we
should capture the full navigation and test for the classification as well as
the transition type.

[Charlie Reis, 2015/07/08 20:27:03]

I don't understand how this one is different than those above.  These are all
cases where we create a new frame and wait for AUTO_SUBFRAME, which I thought we
couldn't use FrameNavigateParamsCapturer for.

[Avi (use Gerrit), 2015/07/08 20:36:15]

OK. That's right.

+    std::string script = "var iframe = document.createElement('iframe');"
+                         "iframe.src = '" + frame_url.spec() + "';"
+                         "document.body.appendChild(iframe);";
+    FrameTreeNode* child = root->child_at(2);
+    EXPECT_TRUE(content::ExecuteScript(child->current_frame_host(), script));
+    capturer.Wait();
+    EXPECT_EQ(ui::PAGE_TRANSITION_AUTO_SUBFRAME, capturer.transition_type());
+  }
+
+  // The last committed NavigationEntry shouldn't have changed.
+  EXPECT_EQ(1, controller.GetEntryCount());
+  entry = controller.GetLastCommittedEntry();
+  EXPECT_EQ(main_url, entry->GetURL());
+  root_entry = entry->root_node()->frame_entry.get();
+  EXPECT_EQ(main_url, root_entry->url());
+
+  // Verify subframe entries if we're in --site-per-process mode.
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kSitePerProcess)) {
+    // There should be a corresponding FrameNavigationEntry.
+    ASSERT_EQ(1U, entry->root_node()->children[2]->children.size());
+    FrameNavigationEntry* frame_entry =
+        entry->root_node()->children[2]->children[0]->frame_entry.get();
+    EXPECT_EQ(frame_url, frame_entry->url());
+  } else {
+    // There are no subframe FrameNavigationEntries by default.
+    EXPECT_EQ(0U, entry->root_node()->children.size());
+  }
 
   // Check the end result of the frame tree.
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
@@ -1371,8 +1432,10 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest,
     EXPECT_EQ(
         " Site A ------------ proxies for B\n"
         "   |--Site A ------- proxies for B\n"
+        "   |--Site B ------- proxies for A\n"
+        "   |    +--Site B -- proxies for A\n"
         "   +--Site B ------- proxies for A\n"
-        "        +--Site B -- proxies for A\n"
+        "        +--Site A -- proxies for B\n"
         "Where A = http://127.0.0.1/\n"
         "      B = http://foo.com/",
         visualizer.DepictFrameTree(root));