Fix cross-process location.replace for main frames and subframes.

content/browser/frame_host/navigation_controller_impl_browsertest.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/frame_host/navigation_controller_impl.h"
 
 #include <stdint.h>
 #include <utility>
 
 #include "base/bind.h"
@@ skipping 191 matching lines @@
   NavigateToURL(shell(), main_url);
   FrameTreeNode* root = static_cast<WebContentsImpl*>(shell()->web_contents())
                             ->GetFrameTree()
                             ->root();
   ASSERT_EQ(1U, root->child_count());
   ASSERT_NE(nullptr, root->child_at(0));
 
   // The main frame's nav_entry_id should match the last committed entry.
   int unique_id = controller.GetLastCommittedEntry()->GetUniqueID();
   EXPECT_EQ(unique_id, root->current_frame_host()->nav_entry_id());
+  EXPECT_EQ(1, controller.GetEntryCount());
 
   // The about:blank iframe should have inherited the same nav_entry_id.
   EXPECT_EQ(unique_id, root->child_at(0)->current_frame_host()->nav_entry_id());
 
   // Use NavigateFrameToURL to go cross-site in the subframe.
   GURL foo_url(embedded_test_server()->GetURL(
       "foo.com", "/navigation_controller/simple_page_1.html"));
   NavigateFrameToURL(root->child_at(0), foo_url);
   EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
+  EXPECT_EQ(1, controller.GetEntryCount());
 
-  // The unique ID should have stayed the same for the auto-subframe navigation,
-  // since the new page replaces the initial about:blank page in the subframe.
-  EXPECT_EQ(unique_id, controller.GetLastCommittedEntry()->GetUniqueID());
-  EXPECT_EQ(unique_id, root->current_frame_host()->nav_entry_id());
-  EXPECT_EQ(unique_id, root->child_at(0)->current_frame_host()->nav_entry_id());
+  // The unique ID should have updated for the new subframe navigation, even
+  // though new page replaces the initial about:blank page in the subframe.
+  int unique_id2 = controller.GetLastCommittedEntry()->GetUniqueID();
+  EXPECT_NE(unique_id, unique_id2);
+  EXPECT_EQ(unique_id2, root->current_frame_host()->nav_entry_id());
+  EXPECT_EQ(unique_id2,
+            root->child_at(0)->current_frame_host()->nav_entry_id());
 
   // Navigating in the subframe again should create a new entry.
   GURL foo_url2(embedded_test_server()->GetURL(
       "foo.com", "/navigation_controller/simple_page_2.html"));
   NavigateFrameToURL(root->child_at(0), foo_url2);
   EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
-  int unique_id2 = controller.GetLastCommittedEntry()->GetUniqueID();
-  EXPECT_NE(unique_id, unique_id2);
+  EXPECT_EQ(2, controller.GetEntryCount());
+  int unique_id3 = controller.GetLastCommittedEntry()->GetUniqueID();
+  EXPECT_NE(unique_id2, unique_id3);
 
   // The unique ID should have updated for the current RenderFrameHost in both
   // frames, not just the subframe.
-  EXPECT_EQ(unique_id2, root->current_frame_host()->nav_entry_id());
-  EXPECT_EQ(unique_id2,
+  EXPECT_EQ(unique_id3, root->current_frame_host()->nav_entry_id());
+  EXPECT_EQ(unique_id3,
             root->child_at(0)->current_frame_host()->nav_entry_id());
 }
 
 // This test used to make sure that a scheme used to prevent spoofs didn't ever
 // interfere with navigations. We switched to a different scheme, so now this is
 // just a test to make sure we can still navigate once we prune the history
 // list.
 IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest,
                        DontIgnoreBackAfterNavEntryLimit) {
   NavigationController& controller =
@@ skipping 377 matching lines @@
     FrameNavigateParamsCapturer capturer(root);
     NavigateFrameToURL(root, error_url);
     capturer.Wait();
     EXPECT_EQ(NAVIGATION_TYPE_NEW_PAGE, capturer.details().type);
     NavigationEntry* entry = controller.GetLastCommittedEntry();
     EXPECT_EQ(PAGE_TYPE_ERROR, entry->GetPageType());
     EXPECT_EQ(2, controller.GetEntryCount());
   }
 
   // Navigate again to the page that fails to load. It must result in an error
-  // page, the EXISTING_PAGE navigation type, and no addition to the history
-  // list. We do not use SAME_PAGE here; that case only differs in that it
-  // clears the pending entry, and there is no pending entry after a load
-  // failure.
+  // page, the NEW_PAGE navigation type with did_replace_entry, and no addition
+  // to the history list. We do not use SAME_PAGE here; that case differs in
+  // that it clears the pending entry, and there is no pending entry after a
+  // load failure.
   {
     FrameNavigateParamsCapturer capturer(root);
     NavigateFrameToURL(root, error_url);
     capturer.Wait();
-    EXPECT_EQ(NAVIGATION_TYPE_EXISTING_PAGE, capturer.details().type);
+    EXPECT_EQ(NAVIGATION_TYPE_NEW_PAGE, capturer.details().type);
+    EXPECT_TRUE(capturer.details().did_replace_entry);
     NavigationEntry* entry = controller.GetLastCommittedEntry();
     EXPECT_EQ(PAGE_TYPE_ERROR, entry->GetPageType());
     EXPECT_EQ(2, controller.GetEntryCount());
   }
 
   // Make a new entry ...
   NavigateToURL(shell(), GURL(url::kAboutBlankURL));
   EXPECT_EQ(3, controller.GetEntryCount());
 
-  // ... and replace it with a failed load. (Note that when you set the
-  // should_replace_current_entry flag, the navigation is classified as NEW_PAGE
-  // because that is a classification of the renderer's behavior, and the flag
-  // is a browser-side flag.)
+  // ... and replace it with a failed load. (Note that NavigateToURLAndReplace
+  // sets the should_replace_current_entry flag, which propagates from the
+  // browser to the renderer and back.)
   {
     FrameNavigateParamsCapturer capturer(root);
     NavigateToURLAndReplace(shell(), error_url);
     capturer.Wait();
     EXPECT_EQ(NAVIGATION_TYPE_NEW_PAGE, capturer.details().type);
+    EXPECT_TRUE(capturer.details().did_replace_entry);
     NavigationEntry* entry = controller.GetLastCommittedEntry();
     EXPECT_EQ(PAGE_TYPE_ERROR, entry->GetPageType());
     EXPECT_EQ(3, controller.GetEntryCount());
   }
 
   // Make a new web ui page to force a process swap ...
   GURL web_ui_page(std::string(kChromeUIScheme) + "://" +
                    std::string(kChromeUIGpuHost));
   NavigateToURL(shell(), web_ui_page);
   EXPECT_EQ(4, controller.GetEntryCount());
 
-  // ... and replace it with a failed load. (It is NEW_PAGE for the reason noted
-  // above.)
+  // ... and replace it with a failed load.
   {
     FrameNavigateParamsCapturer capturer(root);
     NavigateToURLAndReplace(shell(), error_url);
     capturer.Wait();
     EXPECT_EQ(NAVIGATION_TYPE_NEW_PAGE, capturer.details().type);
+    EXPECT_TRUE(capturer.details().did_replace_entry);
     NavigationEntry* entry = controller.GetLastCommittedEntry();
     EXPECT_EQ(PAGE_TYPE_ERROR, entry->GetPageType());
     EXPECT_EQ(4, controller.GetEntryCount());
   }
 }
 
 // Various tests for navigation type classifications. TODO(avi): It's rather
 // bogus that the same info is in two different enums; http://crbug.com/453555.
 
 // Verify that navigations for NAVIGATION_TYPE_NEW_PAGE are correctly
@@ skipping 61 matching lines @@
     FrameNavigateParamsCapturer capturer(root);
     std::string script =
         "history.pushState({}, 'page 1', 'simple_page_1.html')";
     EXPECT_TRUE(content::ExecuteScript(root->current_frame_host(), script));
     capturer.Wait();
     EXPECT_EQ(ui::PAGE_TRANSITION_LINK | ui::PAGE_TRANSITION_CLIENT_REDIRECT,
               capturer.params().transition);
     EXPECT_EQ(NAVIGATION_TYPE_NEW_PAGE, capturer.details().type);
     EXPECT_TRUE(capturer.details().is_in_page);
   }
+
+  // Now test NEW_PAGE with replacement.  First, go back so that we can ensure
+  // location.replace leaves the forward history.
+  {
+    FrameNavigateParamsCapturer capturer(root);
+    shell()->web_contents()->GetController().GoBack();
+    capturer.Wait();
+    EXPECT_TRUE(shell()->web_contents()->GetController().CanGoForward());
+  }
+
+  {
+    // location.replace().
+    FrameNavigateParamsCapturer capturer(root);
+    GURL frame_url(embedded_test_server()->GetURL(
+        "/navigation_controller/simple_page_1.html"));
+    std::string script = "location.replace('" + frame_url.spec() + "')";
+    EXPECT_TRUE(content::ExecuteScript(root->current_frame_host(), script));
+    capturer.Wait();
+    EXPECT_EQ(ui::PAGE_TRANSITION_LINK | ui::PAGE_TRANSITION_CLIENT_REDIRECT,
+              capturer.params().transition);
+    // Unlike replaceState, this replaces the existing entry with a new one.
+    EXPECT_EQ(NAVIGATION_TYPE_NEW_PAGE, capturer.details().type);
+    EXPECT_TRUE(capturer.details().did_replace_entry);
+    EXPECT_FALSE(capturer.details().is_in_page);
+
+    // Make sure the forward history did not get pruned.
+    EXPECT_TRUE(shell()->web_contents()->GetController().CanGoForward());
+  }
+
+  {
+    // Cross-site location.replace().
+    FrameNavigateParamsCapturer capturer(root);
+    GURL frame_url(embedded_test_server()->GetURL(
+        "foo.com", "/navigation_controller/simple_page_1.html"));
+    std::string script = "location.replace('" + frame_url.spec() + "')";
+    EXPECT_TRUE(content::ExecuteScript(root->current_frame_host(), script));
+    capturer.Wait();
+    EXPECT_EQ(ui::PAGE_TRANSITION_LINK | ui::PAGE_TRANSITION_CLIENT_REDIRECT,
+              capturer.params().transition);
+    // Unlike replaceState, this replaces the existing entry with a new one.
+    EXPECT_EQ(NAVIGATION_TYPE_NEW_PAGE, capturer.details().type);
+    EXPECT_TRUE(capturer.details().did_replace_entry);
+    EXPECT_FALSE(capturer.details().is_in_page);
+
+    // Make sure the forward history did not get pruned.
+    EXPECT_TRUE(shell()->web_contents()->GetController().CanGoForward());
+  }
 }
 
 // Verify that navigations for NAVIGATION_TYPE_EXISTING_PAGE are correctly
 // classified.
 IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest,
                        NavigationTypeClassification_ExistingPage) {
   GURL url1(embedded_test_server()->GetURL(
       "/navigation_controller/simple_page_1.html"));
   NavigateToURL(shell(), url1);
   GURL url2(embedded_test_server()->GetURL(
@@ skipping 101 matching lines @@
     FrameNavigateParamsCapturer capturer(root);
     EXPECT_TRUE(content::ExecuteScript(root->current_frame_host(),
                                        "location.reload()"));
     capturer.Wait();
     EXPECT_EQ(ui::PAGE_TRANSITION_LINK | ui::PAGE_TRANSITION_CLIENT_REDIRECT,
               capturer.params().transition);
     EXPECT_EQ(NAVIGATION_TYPE_EXISTING_PAGE, capturer.details().type);
     EXPECT_FALSE(capturer.details().is_in_page);
   }
 
-  {
-    // location.replace().
-    FrameNavigateParamsCapturer capturer(root);
-    GURL frame_url(embedded_test_server()->GetURL(
-        "/navigation_controller/simple_page_1.html"));
-    std::string script = "location.replace('" + frame_url.spec() + "')";
-    EXPECT_TRUE(content::ExecuteScript(root->current_frame_host(), script));
-    capturer.Wait();
-    EXPECT_EQ(ui::PAGE_TRANSITION_LINK | ui::PAGE_TRANSITION_CLIENT_REDIRECT,
-              capturer.params().transition);
-    EXPECT_EQ(NAVIGATION_TYPE_EXISTING_PAGE, capturer.details().type);
-    EXPECT_FALSE(capturer.details().is_in_page);
-  }
-
   // Now, various in-page navigations.
 
   {
     // history.replaceState().
     FrameNavigateParamsCapturer capturer(root);
     std::string script =
         "history.replaceState({}, 'page 2', 'simple_page_2.html')";
     EXPECT_TRUE(content::ExecuteScript(root->current_frame_host(), script));
     capturer.Wait();
     EXPECT_EQ(ui::PAGE_TRANSITION_LINK | ui::PAGE_TRANSITION_CLIENT_REDIRECT,
               capturer.params().transition);
+    // Unlike location.replace, this updates the existing entry.
     EXPECT_EQ(NAVIGATION_TYPE_EXISTING_PAGE, capturer.details().type);
+    EXPECT_TRUE(capturer.details().did_replace_entry);
     EXPECT_TRUE(capturer.details().is_in_page);
   }
 
   // Back and forward across a fragment navigation.
 
   GURL url_links(embedded_test_server()->GetURL(
       "/navigation_controller/page_with_links.html"));
   NavigateToURL(shell(), url_links);
   std::string script = "document.getElementById('fraglink').click()";
   EXPECT_TRUE(content::ExecuteScript(root->current_frame_host(), script));
@@ skipping 118 matching lines @@
   ASSERT_EQ(1U, root->child_count());
   ASSERT_NE(nullptr, root->child_at(0));
 
   {
     // Initial load.
     LoadCommittedCapturer capturer(root->child_at(0));
     GURL frame_url(embedded_test_server()->GetURL(
         "/navigation_controller/simple_page_1.html"));
     NavigateFrameToURL(root->child_at(0), frame_url);
     capturer.Wait();
-    EXPECT_EQ(ui::PAGE_TRANSITION_AUTO_SUBFRAME, capturer.transition_type());
+    // TODO(creis): This doesn't seem right.
+    EXPECT_EQ(ui::PAGE_TRANSITION_MANUAL_SUBFRAME, capturer.transition_type());
   }
 
   {
     // Simple load.
     FrameNavigateParamsCapturer capturer(root->child_at(0));
     GURL frame_url(embedded_test_server()->GetURL(
         "/navigation_controller/simple_page_2.html"));
     NavigateFrameToURL(root->child_at(0), frame_url);
     capturer.Wait();
     EXPECT_EQ(ui::PAGE_TRANSITION_MANUAL_SUBFRAME,
@@ skipping 50 matching lines @@
         "/navigation_controller/simple_page_1.html"));
     std::string script = "location.assign('" + frame_url.spec() + "')";
     EXPECT_TRUE(content::ExecuteScript(root->child_at(0)->current_frame_host(),
                                        script));
     capturer.Wait();
     EXPECT_EQ(ui::PAGE_TRANSITION_MANUAL_SUBFRAME,
               capturer.params().transition);
     EXPECT_EQ(NAVIGATION_TYPE_NEW_SUBFRAME, capturer.details().type);
   }
 
+  // Go back so that we can ensure location.replace leaves the forward history.
+  {
+    FrameNavigateParamsCapturer capturer(root->child_at(0));
+    shell()->web_contents()->GetController().GoBack();
+    capturer.Wait();
+    EXPECT_TRUE(shell()->web_contents()->GetController().CanGoForward());
+  }
+
   {
     // location.replace().
-    LoadCommittedCapturer capturer(root->child_at(0));
+    FrameNavigateParamsCapturer capturer(root->child_at(0));
     GURL frame_url(embedded_test_server()->GetURL(
         "/navigation_controller/simple_page_2.html"));
     std::string script = "location.replace('" + frame_url.spec() + "')";
     EXPECT_TRUE(content::ExecuteScript(root->child_at(0)->current_frame_host(),
                                        script));
     capturer.Wait();
-    EXPECT_EQ(ui::PAGE_TRANSITION_AUTO_SUBFRAME, capturer.transition_type());
+    // Unlike replaceState, this replaces the existing entry with a new one.
+    EXPECT_EQ(ui::PAGE_TRANSITION_MANUAL_SUBFRAME,
+              capturer.params().transition);
+    EXPECT_EQ(NAVIGATION_TYPE_NEW_SUBFRAME, capturer.details().type);
+    EXPECT_TRUE(capturer.details().did_replace_entry);
+
+    // Make sure the forward history did not get pruned.
+    EXPECT_TRUE(shell()->web_contents()->GetController().CanGoForward());
   }
 
   {
+    // Cross-site location.replace().
+    FrameNavigateParamsCapturer capturer(root->child_at(0));
+    GURL frame_url(embedded_test_server()->GetURL(
+        "foo.com", "/navigation_controller/simple_page_2.html"));
+    std::string script = "location.replace('" + frame_url.spec() + "')";
+    EXPECT_TRUE(content::ExecuteScript(root->child_at(0)->current_frame_host(),
+                                       script));
+    capturer.Wait();
+    // Unlike replaceState, this replaces the existing entry with a new one.
+    EXPECT_EQ(ui::PAGE_TRANSITION_MANUAL_SUBFRAME,
+              capturer.params().transition);
+    EXPECT_EQ(NAVIGATION_TYPE_NEW_SUBFRAME, capturer.details().type);
+    EXPECT_TRUE(capturer.details().did_replace_entry);
+
+    // Make sure the forward history did not get pruned.
+    EXPECT_TRUE(shell()->web_contents()->GetController().CanGoForward());
+  }
+
+  {
     // history.pushState().
     FrameNavigateParamsCapturer capturer(root->child_at(0));
     std::string script =
         "history.pushState({}, 'page 1', 'simple_page_1.html')";
     EXPECT_TRUE(content::ExecuteScript(root->child_at(0)->current_frame_host(),
                                        script));
     capturer.Wait();
     EXPECT_EQ(ui::PAGE_TRANSITION_MANUAL_SUBFRAME,
               capturer.params().transition);
     EXPECT_EQ(NAVIGATION_TYPE_NEW_SUBFRAME, capturer.details().type);
   }
 
   {
     // history.replaceState().
     LoadCommittedCapturer capturer(root->child_at(0));
     std::string script =
         "history.replaceState({}, 'page 2', 'simple_page_2.html')";
     EXPECT_TRUE(content::ExecuteScript(root->child_at(0)->current_frame_host(),
                                        script));
     capturer.Wait();
+    // Unlike location.replace, this updates the existing entry.
     EXPECT_EQ(ui::PAGE_TRANSITION_AUTO_SUBFRAME, capturer.transition_type());
   }
 
   {
     // Reload.
     LoadCommittedCapturer capturer(root->child_at(0));
     EXPECT_TRUE(content::ExecuteScript(root->child_at(0)->current_frame_host(),
                                        "location.reload()"));
     capturer.Wait();
     EXPECT_EQ(ui::PAGE_TRANSITION_AUTO_SUBFRAME, capturer.transition_type());
@@ skipping 34 matching lines @@
     capturer.Wait();
 
     std::vector<FrameNavigateParams> params = capturer.all_params();
     std::vector<LoadCommittedDetails> details = capturer.all_details();
     ASSERT_EQ(2U, params.size());
     ASSERT_EQ(2U, details.size());
     EXPECT_EQ(ui::PAGE_TRANSITION_LINK, params[0].transition);
     EXPECT_EQ(NAVIGATION_TYPE_NEW_PAGE, details[0].type);
     EXPECT_EQ(ui::PAGE_TRANSITION_LINK | ui::PAGE_TRANSITION_CLIENT_REDIRECT,
               params[1].transition);
-    EXPECT_EQ(NAVIGATION_TYPE_EXISTING_PAGE, details[1].type);
+    EXPECT_EQ(NAVIGATION_TYPE_NEW_PAGE, details[1].type);
+    EXPECT_TRUE(details[1].did_replace_entry);
   }
 }
 
 // Verify that the LoadCommittedDetails::is_in_page value is properly set for
 // non-IN_PAGE navigations. (It's tested for IN_PAGE navigations with the
 // NavigationTypeClassification_InPage test.)
 IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest,
                        LoadCommittedDetails_IsInPage) {
   GURL links_url(embedded_test_server()->GetURL(
       "/navigation_controller/page_with_links.html"));
@@ skipping 162 matching lines @@
     ASSERT_EQ(2U, entry->root_node()->children.size());
     FrameNavigationEntry* frame_entry =
         entry->root_node()->children[1]->frame_entry.get();
     EXPECT_EQ(about_blank_url, frame_entry->url());
   } else {
     // There are no subframe FrameNavigationEntries by default.
     EXPECT_EQ(0U, entry->root_node()->children.size());
   }
   EXPECT_FALSE(root->child_at(1)->has_committed_real_load());
 
-  // 3. A real same-site navigation in the nested iframe should be AUTO.
+  // 3. A real same-site navigation in the nested iframe should be MANUAL with
+  // replacement.
   GURL frame_url(embedded_test_server()->GetURL(
       "/navigation_controller/simple_page_1.html"));
   {
     LoadCommittedCapturer capturer(root->child_at(0)->child_at(0));
     std::string script = "var frames = document.getElementsByTagName('iframe');"
                          "frames[0].src = '" + frame_url.spec() + "';";
     EXPECT_TRUE(content::ExecuteScript(root->child_at(0)->current_frame_host(),
                                        script));
     capturer.Wait();
-    EXPECT_EQ(ui::PAGE_TRANSITION_AUTO_SUBFRAME, capturer.transition_type());
+    EXPECT_EQ(ui::PAGE_TRANSITION_MANUAL_SUBFRAME, capturer.transition_type());
   }
 
   // Check last committed NavigationEntry.  It should have replaced the previous
-  // frame entry in the original NavigationEntry.
+  // NavigationEntry.
   EXPECT_EQ(1, controller.GetEntryCount());
-  EXPECT_EQ(entry, controller.GetLastCommittedEntry());
+  NavigationEntryImpl* entry2 = controller.GetLastCommittedEntry();
+  EXPECT_NE(entry, entry2);
 
   // Verify subframe entries if they're enabled (e.g. in --site-per-process).
   if (SiteIsolationPolicy::UseSubframeNavigationEntries()) {
     // The entry should still have one nested subframe FrameNavigationEntry.
-    ASSERT_EQ(1U, entry->root_node()->children[0]->children.size());
+    ASSERT_EQ(1U, entry2->root_node()->children[0]->children.size());
     FrameNavigationEntry* frame_entry =
-        entry->root_node()->children[0]->children[0]->frame_entry.get();
+        entry2->root_node()->children[0]->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());
+    EXPECT_EQ(0U, entry2->root_node()->children.size());
   }
   EXPECT_FALSE(root->child_at(0)->has_committed_real_load());
   EXPECT_TRUE(root->child_at(0)->child_at(0)->has_committed_real_load());
   EXPECT_FALSE(root->child_at(1)->has_committed_real_load());
 
-  // 4. A real cross-site navigation in the second iframe should be AUTO.
+  // 4. A real cross-site navigation in the second iframe should be MANUAL with
+  // replacement.
   GURL foo_url(embedded_test_server()->GetURL(
       "foo.com", "/navigation_controller/simple_page_2.html"));
   {
     LoadCommittedCapturer capturer(root->child_at(1));
     std::string script = "var frames = document.getElementsByTagName('iframe');"
                          "frames[1].src = '" + foo_url.spec() + "';";
     EXPECT_TRUE(content::ExecuteScript(root->current_frame_host(), script));
     capturer.Wait();
-    EXPECT_EQ(ui::PAGE_TRANSITION_AUTO_SUBFRAME, capturer.transition_type());
+    EXPECT_EQ(ui::PAGE_TRANSITION_MANUAL_SUBFRAME, capturer.transition_type());
   }
 
   // Check last committed NavigationEntry.
   EXPECT_EQ(1, controller.GetEntryCount());
-  EXPECT_EQ(entry, controller.GetLastCommittedEntry());
+  NavigationEntryImpl* entry3 = controller.GetLastCommittedEntry();
+  EXPECT_NE(entry2, entry3);
 
   // Verify subframe entries if they're enabled (e.g. in --site-per-process).
   if (SiteIsolationPolicy::UseSubframeNavigationEntries()) {
     // The entry should still have two subframe FrameNavigationEntries.
-    ASSERT_EQ(2U, entry->root_node()->children.size());
+    ASSERT_EQ(2U, entry3->root_node()->children.size());
     FrameNavigationEntry* frame_entry =
-        entry->root_node()->children[1]->frame_entry.get();
+        entry3->root_node()->children[1]->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());
+    EXPECT_EQ(0U, entry3->root_node()->children.size());
   }
   EXPECT_FALSE(root->child_at(0)->has_committed_real_load());
   EXPECT_TRUE(root->child_at(0)->child_at(0)->has_committed_real_load());
   EXPECT_TRUE(root->child_at(1)->has_committed_real_load());
 
   // 5. A new navigation to about:blank in the nested frame should count as a
   // real load, since that frame has already committed a real load and this is
   // not the initial blank page.
   {
     LoadCommittedCapturer capturer(root->child_at(0)->child_at(0));
     std::string script = "var frames = document.getElementsByTagName('iframe');"
                          "frames[0].src = 'about:blank';";
     EXPECT_TRUE(content::ExecuteScript(root->child_at(0)->current_frame_host(),
                                        script));
     capturer.Wait();
     EXPECT_EQ(ui::PAGE_TRANSITION_MANUAL_SUBFRAME, capturer.transition_type());
   }
 
   // This should have created a new NavigationEntry.
   EXPECT_EQ(2, controller.GetEntryCount());
-  EXPECT_NE(entry, controller.GetLastCommittedEntry());
-  NavigationEntryImpl* entry2 = controller.GetLastCommittedEntry();
+  NavigationEntryImpl* entry4 = controller.GetLastCommittedEntry();
+  EXPECT_NE(entry3, entry4);
 
   // Verify subframe entries if they're enabled (e.g. in --site-per-process).
   if (SiteIsolationPolicy::UseSubframeNavigationEntries()) {
-    ASSERT_EQ(2U, entry->root_node()->children.size());
+    ASSERT_EQ(2U, entry4->root_node()->children.size());
     FrameNavigationEntry* frame_entry =
-        entry2->root_node()->children[0]->children[0]->frame_entry.get();
+        entry4->root_node()->children[0]->children[0]->frame_entry.get();
     EXPECT_EQ(about_blank_url, frame_entry->url());
   } else {
     // There are no subframe FrameNavigationEntries by default.
-    EXPECT_EQ(0U, entry->root_node()->children.size());
+    EXPECT_EQ(0U, entry4->root_node()->children.size());
   }
   EXPECT_FALSE(root->child_at(0)->has_committed_real_load());
   EXPECT_TRUE(root->child_at(0)->child_at(0)->has_committed_real_load());
   EXPECT_TRUE(root->child_at(1)->has_committed_real_load());
 
   // Check the end result of the frame tree.
   if (AreAllSitesIsolatedForTesting()) {
     FrameTreeVisualizer visualizer;
     EXPECT_EQ(
         " Site A ------------ proxies for B\n"
@@ skipping 308 matching lines @@
     EXPECT_EQ(foo_url, entry3->root_node()->children[1]->frame_entry->url());
     ASSERT_EQ(1U, entry3->root_node()->children[1]->children.size());
     EXPECT_EQ(
         bar_url,
         entry3->root_node()->children[1]->children[0]->frame_entry->url());
   } else {
     // There are no subframe FrameNavigationEntries by default.
     EXPECT_EQ(0U, entry3->root_node()->children.size());
   }
 
-  // 6. Navigate the second subframe cross-site, clearing its existing subtree.
+  // 5. Navigate the second subframe cross-site, clearing its existing subtree.
   GURL baz_url(embedded_test_server()->GetURL(
       "baz.com", "/navigation_controller/simple_page_1.html"));
   {
     FrameNavigateParamsCapturer capturer(root->child_at(1));
     std::string script = "var frames = document.getElementsByTagName('iframe');"
                          "frames[1].src = '" + baz_url.spec() + "';";
     EXPECT_TRUE(content::ExecuteScript(root->current_frame_host(), script));
     capturer.Wait();
     EXPECT_EQ(ui::PAGE_TRANSITION_MANUAL_SUBFRAME,
               capturer.params().transition);
@@ skipping 50 matching lines @@
   ASSERT_NE(nullptr, root->child_at(0));
 
   // Navigate to a real page in the subframe, so that the next navigation will
   // be MANUAL_SUBFRAME.
   GURL subframe_url(embedded_test_server()->GetURL(
       "/navigation_controller/simple_page_1.html"));
   {
     LoadCommittedCapturer capturer(root->child_at(0));
     NavigateFrameToURL(root->child_at(0), subframe_url);
     capturer.Wait();
-    EXPECT_EQ(ui::PAGE_TRANSITION_AUTO_SUBFRAME, capturer.transition_type());
+    EXPECT_EQ(ui::PAGE_TRANSITION_MANUAL_SUBFRAME, capturer.transition_type());
   }
 
   // 2. In-page navigation in the main frame.
   std::string push_script = "history.pushState({}, 'page 2', 'page_2.html')";
   EXPECT_TRUE(content::ExecuteScript(root->current_frame_host(), push_script));
   EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
 
   // TODO(creis): Verify subframe entries.  https://crbug.com/522193.
 
   // 3. Add a nested subframe.
@@ skipping 1249 matching lines @@
 IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, ReloadOriginalRequest) {
   GURL original_url(embedded_test_server()->GetURL(
       "/navigation_controller/simple_page_1.html"));
   NavigateToURL(shell(), original_url);
   FrameTreeNode* root = static_cast<WebContentsImpl*>(shell()->web_contents())
                             ->GetFrameTree()
                             ->root();
   RenderProcessKilledObserver kill_observer(shell()->web_contents());
 
   // Redirect so that we can use ReloadOriginalRequest.
+  // TODO(creis): The test is failing because this replacement isn't preserving
+  // the redirect chain or original_request_url for a cross process transfer.
+  // I think it worked before because those values stuck around when we just
+  // updated the old NavEntry, but that's not a safe thing to do on a
+  // SiteInstance change.  Instead, we should propagate them during the transfer
+  // so that they show up in the new commit.
+  // We appear to be trying to propagate the redirect chain, but the request in
+  // the network stack doesn't have the earlier items in the chain.  Is this a
+  // bug or expected?  Does it apply to all client redirects, or just this test?
+  // Does it apply to server redirects?
   GURL redirect_url(embedded_test_server()->GetURL(
       "foo.com", "/navigation_controller/simple_page_1.html"));
   {
     std::string script = "location.replace('" + redirect_url.spec() + "');";
     FrameNavigateParamsCapturer capturer(root);
     EXPECT_TRUE(ExecuteScript(shell()->web_contents(), script));
     capturer.Wait();
     EXPECT_EQ(ui::PAGE_TRANSITION_LINK | ui::PAGE_TRANSITION_CLIENT_REDIRECT,
               capturer.params().transition);
-    EXPECT_EQ(NAVIGATION_TYPE_EXISTING_PAGE, capturer.details().type);
+    EXPECT_EQ(NAVIGATION_TYPE_NEW_PAGE, capturer.details().type);
   }
 
   // Modify an entry in the session history and reload the original request.
   {
     // We first send a replaceState() to the renderer, which will cause the
     // renderer to send back a DidCommitProvisionalLoad. Immediately after,
     // we send a ReloadOriginalRequest (which in this case is a different
     // origin) and will also cause the renderer to commit the frame. In the
     // end we verify that both navigations committed and that the URLs are
     // correct.
     std::string script = "history.replaceState({}, '', 'foo');";
     root->render_manager()
         ->current_frame_host()
         ->ExecuteJavaScriptWithUserGestureForTests(base::UTF8ToUTF16(script));
     EXPECT_FALSE(shell()->web_contents()->IsLoading());
     shell()->web_contents()->GetController().ReloadOriginalRequestURL(false);
     EXPECT_TRUE(shell()->web_contents()->IsLoading());
     EXPECT_EQ(redirect_url, shell()->web_contents()->GetLastCommittedURL());
 
     // Wait until there's no more navigations.
     GURL modified_url(embedded_test_server()->GetURL(
         "foo.com", "/navigation_controller/foo"));
     FrameNavigateParamsCapturer capturer(root);
     capturer.set_wait_for_load(false);
     capturer.set_navigations_remaining(2);
     capturer.Wait();
     EXPECT_EQ(2U, capturer.all_details().size());
+    // TODO(creis): This is failing in --site-per-process.
     EXPECT_EQ(modified_url, capturer.all_params()[0].url);
     EXPECT_EQ(original_url, capturer.all_params()[1].url);
     EXPECT_EQ(original_url, shell()->web_contents()->GetLastCommittedURL());
   }
 
   // Make sure the renderer is still alive.
   EXPECT_TRUE(
       ExecuteScript(shell()->web_contents(), "console.log('Success');"));
 }
 
 }  // namespace content