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

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 7a89b26c1d1e5a179453f9ccdeabc4965d0ca207..28e8152c7f244be76402a02abbdbd6824b35543c 100644
--- a/content/browser/frame_host/navigation_controller_impl_browsertest.cc
+++ b/content/browser/frame_host/navigation_controller_impl_browsertest.cc
@@ -1070,4 +1070,95 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest,
   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