Classify navigations without page id in parallel to the existing classifier.

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 1052 matching lines @@
     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 {
+
+class FailureWatcher : public WebContentsObserver {
+ public:
+  // Observes failure for the specified |node|.
+  explicit FailureWatcher(FrameTreeNode* node)
+      : WebContentsObserver(
+            node->current_frame_host()->delegate()->GetAsWebContents()),
+        frame_tree_node_id_(node->frame_tree_node_id()),
+        message_loop_runner_(new MessageLoopRunner) {}
+
+  void Wait() {
+    message_loop_runner_->Run();
+  }
+
+ private:
+  void DidFailLoad(RenderFrameHost* render_frame_host,
+                   const GURL& validated_url,
+                   int error_code,
+                   const base::string16& error_description) override {
+    LOG(ERROR) << "FailureWatcher::DidFailLoad";

[Charlie Reis, 2015/04/24 22:02:02]

nit: Let's remove the LOG statements.

[Avi (use Gerrit), 2015/04/24 22:17:15]

Gah, part of my debugging via lots of log statement strategy. Done.

+    RenderFrameHostImpl* rfh =
+        static_cast<RenderFrameHostImpl*>(render_frame_host);
+    if (rfh->frame_tree_node()->frame_tree_node_id() != frame_tree_node_id_)
+      return;
+
+    message_loop_runner_->Quit();
+  }
+
+  void DidFailProvisionalLoad(
+      RenderFrameHost* render_frame_host,
+      const GURL& validated_url,
+      int error_code,
+      const base::string16& error_description) override {
+    LOG(ERROR) << "FailureWatcher::DidFailProvisionalLoad";
+    RenderFrameHostImpl* rfh =
+        static_cast<RenderFrameHostImpl*>(render_frame_host);
+    if (rfh->frame_tree_node()->frame_tree_node_id() != frame_tree_node_id_)
+      return;
+
+    message_loop_runner_->Quit();
+  }
+
+  // The id of the FrameTreeNode whose navigations to observe.
+  int frame_tree_node_id_;
+
+  // The MessageLoopRunner used to spin the message loop.
+  scoped_refptr<MessageLoopRunner> message_loop_runner_;
+};
+
+}  // namespace
+
+IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest,
+                       StopCausesFailureDespiteJavaScriptURL) {
+  NavigationControllerImpl& controller =
+      static_cast<NavigationControllerImpl&>(
+          shell()->web_contents()->GetController());
+
+  FrameTreeNode* root =
+      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));
+
+  // 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());
+
+  // That should be the pending entry.
+  NavigationEntryImpl* entry = controller.GetPendingEntry();
+  ASSERT_NE(nullptr, entry);
+  EXPECT_EQ(url2, entry->GetURL());
+
+  // Loading a JavaScript URL shouldn't affect the ability to stop.
+  {
+    FailureWatcher watcher(root);
+    GURL js("javascript:(function(){})()");
+    controller.LoadURL(js, Referrer(), ui::PAGE_TRANSITION_LINK, std::string());
+    shell()->web_contents()->Stop();

[Charlie Reis, 2015/04/24 22:02:02]

Maybe we can add an EXPECT that the pending entry is gone before the Stop call?

(It's fine to change that if we later add support for multiple pending entries,
but it's useful documentation for our current behavior in the meantime.  For
example, it makes it clear why this case is tricky.)

[Avi (use Gerrit), 2015/04/24 22:17:15]

Done.

+    watcher.Wait();
+  }
+
+  ResourceDispatcherHost::Get()->SetDelegate(nullptr);
+}
+
 }  // namespace content