Fix mis-classification of navigations when there is a pending page.

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 "base/bind.h"
 #include "base/command_line.h"
 #include "base/strings/stringprintf.h"
 #include "content/browser/frame_host/frame_navigation_entry.h"
 #include "content/browser/frame_host/frame_tree.h"
 #include "content/browser/frame_host/navigation_controller_impl.h"
@@ skipping 1007 matching lines @@
       net::URLRequest* request,
       content::ResourceContext* resource_context,
       content::AppCacheService* appcache_service,
       ResourceType resource_type,
       ScopedVector<content::ResourceThrottle>* throttles) override {
     DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
     throttles->push_back(new HttpThrottle);
   }
 };
 
-}  // namespace
-
-IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest,
-                       NavigationTypeClassification_InPageWhilePending) {
+// Loads |start_url|, then loads |stalled_url| which stalls. While the page is
+// stalled, an in-page navigation happens. Make sure that all the navigations
+// are properly classified.
+void DoReplaceStateWhilePending(Shell* shell,
+                                const GURL& start_url,
+                                const GURL& stalled_url) {
   NavigationControllerImpl& controller =
       static_cast<NavigationControllerImpl&>(
-          shell()->web_contents()->GetController());
+          shell->web_contents()->GetController());
 
   FrameTreeNode* root =
-      static_cast<WebContentsImpl*>(shell()->web_contents())->
+      static_cast<WebContentsImpl*>(shell->web_contents())->
           GetFrameTree()->root();
 
-  // Start with a normal page.
-  GURL url1(embedded_test_server()->GetURL(
-      "/navigation_controller/simple_page_1.html"));
-  EXPECT_TRUE(NavigateToURL(shell(), url1));
+  // Start with one page.
+  EXPECT_TRUE(NavigateToURL(shell, start_url));
 
   // Have the user decide to go to a different page which is very slow.
   StallDelegate stall_delegate;
   ResourceDispatcherHost::Get()->SetDelegate(&stall_delegate);
-  GURL url2(embedded_test_server()->GetURL(
-      "/navigation_controller/simple_page_2.html"));
-  controller.LoadURL(url2, Referrer(), ui::PAGE_TRANSITION_LINK, std::string());
+  controller.LoadURL(
+      stalled_url, Referrer(), ui::PAGE_TRANSITION_LINK, std::string());
 
   // That should be the pending entry.
   NavigationEntryImpl* entry = controller.GetPendingEntry();
   ASSERT_NE(nullptr, entry);
-  EXPECT_EQ(url2, entry->GetURL());
+  EXPECT_EQ(stalled_url, entry->GetURL());
 
   {
     // Now the existing page uses history.replaceState().
     FrameNavigateParamsCapturer capturer(root);
     capturer.set_wait_for_load(false);
     EXPECT_TRUE(content::ExecuteScript(root->current_frame_host(),
                                        "history.replaceState({}, '', 'x')"));
     capturer.Wait();
 
     // The fact that there was a pending entry shouldn't interfere with the
     // classification.
     EXPECT_EQ(NAVIGATION_TYPE_IN_PAGE, capturer.details().type);
   }
 
   ResourceDispatcherHost::Get()->SetDelegate(nullptr);
 }
 
+}  // namespace
+
+IN_PROC_BROWSER_TEST_F(
+    NavigationControllerBrowserTest,
+    NavigationTypeClassification_InPageWhileDifferentPending) {
+  GURL url1(embedded_test_server()->GetURL(
+      "/navigation_controller/simple_page_1.html"));
+  GURL url2(embedded_test_server()->GetURL(
+      "/navigation_controller/simple_page_2.html"));
+  DoReplaceStateWhilePending(shell(), url1, url2);
+}
+
+IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest,
+                       NavigationTypeClassification_InPageWhileSamePending) {
+  GURL url(embedded_test_server()->GetURL(
+      "/navigation_controller/simple_page_1.html"));
+  DoReplaceStateWhilePending(shell(), url, url);

[Charlie Reis, 2015/05/01 19:40:48]

Nice.

Can we add a test that does a replaceState to url2, to catch the bug I
mentioned?

[Avi (use Gerrit), 2015/05/01 20:30:57]

Done.

+}
+
 // Ensure the renderer process does not get confused about the current entry
 // due to subframes and replaced entries.  See https://crbug.com/480201.
 IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest,
                        PreventSpoofFromSubframeAndReplace) {
   // Start at an initial URL.
   GURL url1(embedded_test_server()->GetURL(
       "/navigation_controller/simple_page_1.html"));
   NavigateToURL(shell(), url1);
 
   // Now go to a page with a real iframe.
@@ skipping 81 matching lines @@
 
     // Make sure the URL is correct for both the entry and the main frame, and
     // that the process hasn't been killed for showing a spoof.
     EXPECT_TRUE(root->current_frame_host()->IsRenderFrameLive());
     EXPECT_EQ(url3, shell()->web_contents()->GetLastCommittedURL());
     EXPECT_EQ(url3, root->current_url());
   }
 }
 
 }  // namespace content