Initial PlzNavigate RDH-side logic.

content/browser/frame_host/render_frame_host_manager_unittest.cc

Index: content/browser/frame_host/render_frame_host_manager_unittest.cc
diff --git a/content/browser/frame_host/render_frame_host_manager_unittest.cc b/content/browser/frame_host/render_frame_host_manager_unittest.cc
index 00164a12e268614e7c62621e4e2276ecc5f3ade7..7cc71c1f22748b525551970623da18c108927c1b 100644
--- a/content/browser/frame_host/render_frame_host_manager_unittest.cc
+++ b/content/browser/frame_host/render_frame_host_manager_unittest.cc
@@ -7,13 +7,14 @@
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
 #include "content/browser/frame_host/cross_site_transferring_request.h"
-#include "content/browser/frame_host/navigation_before_commit_info.h"
 #include "content/browser/frame_host/navigation_controller_impl.h"
 #include "content/browser/frame_host/navigation_entry_impl.h"
 #include "content/browser/frame_host/navigation_request.h"
 #include "content/browser/frame_host/navigator.h"
 #include "content/browser/frame_host/navigator_impl.h"
 #include "content/browser/frame_host/render_frame_host_manager.h"
+#include "content/browser/loader/navigation_url_loader.h"
+#include "content/browser/loader/navigation_url_loader_factory.h"
 #include "content/browser/site_instance_impl.h"
 #include "content/browser/webui/web_ui_controller_factory_registry.h"
 #include "content/common/view_messages.h"
@@ -23,6 +24,7 @@
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_widget_host_iterator.h"
+#include "content/public/browser/stream_handle.h"
 #include "content/public/browser/web_contents_delegate.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/browser/web_ui_controller.h"
@@ -30,6 +32,7 @@
 #include "content/public/common/content_switches.h"
 #include "content/public/common/javascript_message_type.h"
 #include "content/public/common/page_transition_types.h"
+#include "content/public/common/resource_response.h"
 #include "content/public/common/url_constants.h"
 #include "content/public/common/url_utils.h"
 #include "content/public/test/mock_render_process_host.h"
@@ -253,6 +256,56 @@ class FrameLifetimeConsistencyChecker : public WebContentsObserver {
   std::set<std::pair<int, int> > deleted_routes_;
 };
 
+class TestNavigationURLLoader : public NavigationURLLoader {
+ public:
+  TestNavigationURLLoader(BrowserContext* browser_context,
+                          int64 frame_tree_node_id,
+                          const NavigationRequestInfo& request_info,
+                          ResourceRequestBody* request_body,
+                          NavigationURLLoader::Delegate* delegate)
+      : delegate_(delegate) {
+  }
+
+  virtual ~TestNavigationURLLoader() {
+    // Record the number of times a loader was canceled before ResponseStarted
+    // or RequestFailed was returned.
+    if (delegate_)
+      cancel_count_++;
+  }
+
+  // NavigationURLLoader implementation.
+  virtual void FollowRedirect() OVERRIDE { }
+
+  void CallOnResponseStarted(ResourceResponse* response,
+                             scoped_ptr<StreamHandle> body) {
+    delegate_->OnResponseStarted(response, body.Pass());
+    delegate_ = NULL;
+  }
+
+  static int cancel_count() { return cancel_count_; }
+
+ private:
+  static int cancel_count_;
+  NavigationURLLoader::Delegate* delegate_;
+};
+
+int TestNavigationURLLoader::cancel_count_ = 0;
+
+class TestNavigationURLLoaderFactory : public NavigationURLLoaderFactory {
+ public:
+  // NavigationURLLoaderFactory implementation.
+  virtual NavigationURLLoader* CreateLoader(
+      BrowserContext* browser_context,
+      int64 frame_tree_node_id,
+      const NavigationRequestInfo& request_info,
+      ResourceRequestBody* request_body,
+      NavigationURLLoader::Delegate* delegate) OVERRIDE {
+    return new TestNavigationURLLoader(
+        browser_context, frame_tree_node_id, request_info, request_body,
+        delegate);
+  }
+};
+
 }  // namespace
 
 class RenderFrameHostManagerTest
@@ -262,9 +315,13 @@ class RenderFrameHostManagerTest
     RenderViewHostImplTestHarness::SetUp();
     WebUIControllerFactory::RegisterFactory(&factory_);
     lifetime_checker_.reset(new FrameLifetimeConsistencyChecker(contents()));
+    loader_factory_.reset(new TestNavigationURLLoaderFactory);
+    NavigationURLLoader::SetFactoryForTesting(loader_factory_.get());
   }
 
   virtual void TearDown() OVERRIDE {
+    NavigationURLLoader::SetFactoryForTesting(NULL);
+    loader_factory_.reset();
     lifetime_checker_.reset();
     RenderViewHostImplTestHarness::TearDown();
     WebUIControllerFactory::UnregisterFactoryForTesting(&factory_);
@@ -383,6 +440,11 @@ class RenderFrameHostManagerTest
     return manager->navigation_request_for_testing();
   }
 
+  TestNavigationURLLoader* GetLoaderForNavigationRequest(
+      NavigationRequest* request) const {
+    return static_cast<TestNavigationURLLoader*>(request->loader_for_testing());
+  }
+
   void EnableBrowserSideNavigation() {
     CommandLine::ForCurrentProcess()->AppendSwitch(
         switches::kEnableBrowserSideNavigation);
@@ -390,6 +452,7 @@ class RenderFrameHostManagerTest
  private:
   RenderFrameHostManagerTestWebUIControllerFactory factory_;
   scoped_ptr<FrameLifetimeConsistencyChecker> lifetime_checker_;
+  scoped_ptr<TestNavigationURLLoaderFactory> loader_factory_;
 };
 
 // Tests that when you navigate from a chrome:// url to another page, and
@@ -1698,7 +1761,6 @@ TEST_F(RenderFrameHostManagerTest, BrowserSideNavigationBeginNavigation) {
   const GURL kUrl1("http://www.google.com/");
   const GURL kUrl2("http://www.chromium.org/");
   const GURL kUrl3("http://www.gmail.com/");
-  const int64 kFirstNavRequestID = 1;
 
   // TODO(clamy): we should be enabling browser side navigations here
   // when CommitNavigation is properly implemented.
@@ -1723,7 +1785,6 @@ TEST_F(RenderFrameHostManagerTest, BrowserSideNavigationBeginNavigation) {
       kUrl1, subframe_request->info().first_party_for_cookies);
   EXPECT_FALSE(subframe_request->info().is_main_frame);
   EXPECT_TRUE(subframe_request->info().parent_is_main_frame);
-  EXPECT_EQ(kFirstNavRequestID, subframe_request->navigation_request_id());
 
   // Simulate a BeginNavigation IPC on the main frame.
   contents()->GetMainFrame()->SendBeginNavigationWithURL(kUrl3);
@@ -1734,7 +1795,6 @@ TEST_F(RenderFrameHostManagerTest, BrowserSideNavigationBeginNavigation) {
   EXPECT_EQ(kUrl3, main_request->info().first_party_for_cookies);
   EXPECT_TRUE(main_request->info().is_main_frame);
   EXPECT_FALSE(main_request->info().parent_is_main_frame);
-  EXPECT_EQ(kFirstNavRequestID + 1, main_request->navigation_request_id());
 }
 
 // PlzNavigate: Test that RequestNavigation creates a NavigationRequest and that
@@ -1756,11 +1816,9 @@ TEST_F(RenderFrameHostManagerTest,
   RenderFrameHostImpl* rfh = main_test_rfh();
 
   // Now commit the same url.
-  NavigationBeforeCommitInfo commit_info;
-  commit_info.navigation_url = kUrl;
-  commit_info.navigation_request_id = main_request->navigation_request_id();
-  render_manager->CommitNavigation(commit_info);
-  main_request = GetNavigationRequestForRenderFrameManager(render_manager);
+  scoped_refptr<ResourceResponse> response(new ResourceResponse);
+  GetLoaderForNavigationRequest(main_request)->CallOnResponseStarted(
+      response.get(), scoped_ptr<StreamHandle>());
 
   // The main RFH should not have been changed, and the renderer should have
   // been initialized.
@@ -1792,22 +1850,17 @@ TEST_F(RenderFrameHostManagerTest,
       GetNavigationRequestForRenderFrameManager(render_manager);
   ASSERT_TRUE(main_request);
 
-  NavigationBeforeCommitInfo commit_info;
-  commit_info.navigation_url = kUrl2;
-  commit_info.navigation_request_id = main_request->navigation_request_id();
-  render_manager->CommitNavigation(commit_info);
+  scoped_refptr<ResourceResponse> response(new ResourceResponse);
+  GetLoaderForNavigationRequest(main_request)->CallOnResponseStarted(
+      response.get(), scoped_ptr<StreamHandle>());
   EXPECT_NE(main_test_rfh(), rfh);
   EXPECT_TRUE(main_test_rfh()->render_view_host()->IsRenderViewLive());
 }
 
-// PlzNavigate: Test that a navigation commit is ignored if another request has
-// been issued in the meantime.
-// TODO(carlosk): add checks to assert that the cancel call was sent to
-// ResourceDispatcherHost in the IO thread by extending
-// ResourceDispatcherHostDelegate (like in cross_site_transfer_browsertest.cc
-// and plugin_browsertest.cc).
+// PlzNavigate: Test that a navigation is cancelled if another request has been
+// issued in the meantime.
 TEST_F(RenderFrameHostManagerTest,
-       BrowserSideNavigationIgnoreStaleNavigationCommit) {
+       BrowserSideNavigationReplacePendingNavigation) {
   const GURL kUrl0("http://www.wikipedia.org/");
   const GURL kUrl0_site = SiteInstance::GetSiteForURL(browser_context(), kUrl0);
   const GURL kUrl1("http://www.chromium.org/");
@@ -1826,27 +1879,21 @@ TEST_F(RenderFrameHostManagerTest,
   NavigationRequest* request1 =
       GetNavigationRequestForRenderFrameManager(render_manager);
   ASSERT_TRUE(request1);
-  int64 request_id1 = request1->navigation_request_id();
+  int cancel_count = TestNavigationURLLoader::cancel_count();
 
   // Request navigation to the 2nd URL and gather more data.
   main_test_rfh()->SendBeginNavigationWithURL(kUrl2);
   NavigationRequest* request2 =
       GetNavigationRequestForRenderFrameManager(render_manager);
   ASSERT_TRUE(request2);
-  int64 request_id2 = request2->navigation_request_id();
-  EXPECT_NE(request_id1, request_id2);
-
-  // Confirms that a stale commit is ignored by the RHFM.
-  NavigationBeforeCommitInfo nbc_info;
-  nbc_info.navigation_url = kUrl1;
-  nbc_info.navigation_request_id = request_id1;
-  render_manager->CommitNavigation(nbc_info);
-  EXPECT_EQ(kUrl0_site, main_test_rfh()->GetSiteInstance()->GetSiteURL());
+
+  // Confirm that the first request got canceled.
+  EXPECT_EQ(cancel_count + 1, TestNavigationURLLoader::cancel_count());

[clamy, 2014/09/12 20:51:25]

We do not test anymore that a call to OnResponseStarted on the old
NavigationURLLoader will not lead to a commit in RenderFrameHostManager. This is
mainly what we were trying to test here (to avoid the Cancelation-Commit race
between the UI and the IO thread).

[davidben, 2014/09/19 18:30:49]

That's true. It got split over two tests. This tests that we cancel the loader
on the UI thread. The loader tests assert that loader cancel stops calls on the
delegate. I've added a test to test the race that may happen.

 
   // Confirms that a valid, request-matching commit is correctly processed.
-  nbc_info.navigation_url = kUrl2;
-  nbc_info.navigation_request_id = request_id2;
-  render_manager->CommitNavigation(nbc_info);
+  scoped_refptr<ResourceResponse> response(new ResourceResponse);
+  GetLoaderForNavigationRequest(request2)->CallOnResponseStarted(
+      response.get(), scoped_ptr<StreamHandle>());
   EXPECT_EQ(kUrl2_site, main_test_rfh()->GetSiteInstance()->GetSiteURL());
 }