PlzNavigate: don't discard pending entry in DidFailProvisionalLoad

content/browser/frame_host/navigation_controller_impl_unittest.cc

 // Copyright 2013 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 <stddef.h>
 #include <stdint.h>
 
 #include <memory>
@@ skipping 27 matching lines @@
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents_delegate.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/common/browser_side_navigation_policy.h"
 #include "content/public/common/page_state.h"
 #include "content/public/common/page_type.h"
 #include "content/public/common/url_constants.h"
 #include "content/public/test/mock_render_process_host.h"
 #include "content/public/test/test_notification_tracker.h"
 #include "content/public/test/test_utils.h"
+#include "content/test/browser_side_navigation_test_utils.h"
 #include "content/test/test_render_frame_host.h"
 #include "content/test/test_render_view_host.h"
 #include "content/test/test_web_contents.h"
 #include "skia/ext/platform_canvas.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/WebKit/public/web/WebFrameOwnerProperties.h"
 #include "third_party/WebKit/public/web/WebSandboxFlags.h"
 
 using base::Time;
 
@@ skipping 251 matching lines @@
  private:
   void DidNavigateAnyFrame(RenderFrameHost* render_frame_host,
                            const LoadCommittedDetails& details,
                            const FrameNavigateParams& params) override {
     details_ = details;
   }
 
   LoadCommittedDetails details_;
 };
 
+// PlzNavigate
+// A NavigationControllerTest run with --enable-browser-side-navigation.
+class NavigationControllerTestWithBrowserSideNavigation
+    : public NavigationControllerTest {
+ public:
+  void SetUp() override {
+    EnableBrowserSideNavigation();
+    NavigationControllerTest::SetUp();
+  }
+};
+
 // -----------------------------------------------------------------------------
 
 TEST_F(NavigationControllerTest, Defaults) {
   NavigationControllerImpl& controller = controller_impl();
 
   EXPECT_FALSE(controller.GetPendingEntry());
   EXPECT_FALSE(controller.GetVisibleEntry());
   EXPECT_FALSE(controller.GetLastCommittedEntry());
   EXPECT_EQ(controller.GetPendingEntryIndex(), -1);
   EXPECT_EQ(controller.GetLastCommittedEntryIndex(), -1);
@@ skipping 80 matching lines @@
 // 1. Pending entry for A is created.
 // 2. DidStartProvisionalLoad message for A arrives.
 // 3. Pending entry for B is created.
 // 4. DidFailProvisionalLoad message for A arrives. The logic here discards.
 // 5. DidStartProvisionalLoad message for B arrives.
 //
 // At step (4), the pending entry for B is discarded, when A is the one that
 // is being aborted. This caused the last committed entry to be displayed in
 // the omnibox, which is the entry before A was created.
 TEST_F(NavigationControllerTest, DontDiscardWrongPendingEntry) {
+  if (IsBrowserSideNavigationEnabled()) {
+    // PlzNavigate: this exact order of events cannot happen in PlzNavigate. A
+    // similar issue with the wrong pending entry being discarded is tested in
+    // the PlzNavigate version of the test below.
+    SUCCEED() << "Test is not applicable with PlzNavigate.";
+    return;
+  }
+
   NavigationControllerImpl& controller = controller_impl();
   GURL initial_url("http://www.google.com");
   GURL url_1("http://foo.com");
   GURL url_2("http://foo2.com");
 
   // Navigate inititally. This is the url that could erroneously be the visible
   // entry when url_1 fails.
   NavigateAndCommit(initial_url);
 
   // Set the pending entry as url_1 and receive the DidStartProvisionalLoad
@@ skipping 12 matching lines @@
   EXPECT_EQ(url_2, controller.GetVisibleEntry()->GetURL());
   main_test_rfh()->SimulateNavigationError(url_1, net::ERR_ABORTED);
   EXPECT_EQ(url_2, controller.GetVisibleEntry()->GetURL());
 
   // Get the DidStartProvisionalLoad message for url_2.
   main_test_rfh()->SimulateNavigationStart(url_2);
 
   EXPECT_EQ(url_2, controller.GetVisibleEntry()->GetURL());
 }
 
+// PlzNavigate: tests a case similar to
+// NavigationControllerTest.DontDiscardWrongPendingEntry.
+// Tests hat receiving a DidFailProvisionalLoad from the renderer that is

[Charlie Reis, 2016/04/13 17:09:12]

nit: s/hat/that/

+// trying to commit an error page won't reset the pending entry of a navigation
+// that just started.
+TEST_F(NavigationControllerTestWithBrowserSideNavigation,
+       DontDiscardWrongPendingEntry) {
+  NavigationControllerImpl& controller = controller_impl();
+  GURL initial_url("http://www.google.com");
+  GURL url_1("http://google.com/foo");
+  GURL url_2("http://foo2.com");
+
+  // Navigate inititally. This is the url that could erroneously be the visible
+  // entry when url_1 fails.
+  NavigateAndCommit(initial_url);
+
+  // Set the pending entry as url_1 and receive the DidStartProvisionalLoad
+  // message, creating the NavigationHandle.
+  controller.LoadURL(url_1, Referrer(), ui::PAGE_TRANSITION_TYPED,
+                     std::string());
+  EXPECT_EQ(url_1, controller.GetVisibleEntry()->GetURL());
+  main_test_rfh()->SimulateNavigationStart(url_1);
+  EXPECT_EQ(url_1, controller.GetVisibleEntry()->GetURL());
+
+  // The navigation fails and needs to show an error page. This resets the
+  // pending entry.
+  main_test_rfh()->SimulateNavigationError(url_1, net::ERR_TIMED_OUT);
+  EXPECT_EQ(initial_url, controller.GetVisibleEntry()->GetURL());
+
+  // A navigation to url_2 starts, creating a pending navigation entry.
+  controller.LoadURL(url_2, Referrer(), ui::PAGE_TRANSITION_TYPED,
+                     std::string());
+  EXPECT_EQ(url_2, controller.GetVisibleEntry()->GetURL());
+
+  // The DidFailProvsionalLoad IPC is received from the current RFH that is
+  // committing an error page. This should not reset the pending entry for the
+  // new ongoing navigation.
+  FrameHostMsg_DidFailProvisionalLoadWithError_Params error;
+  error.error_code = net::ERR_TIMED_OUT;
+  error.url = url_1;
+  main_test_rfh()->OnMessageReceived(
+      FrameHostMsg_DidFailProvisionalLoadWithError(
+          main_test_rfh()->GetRoutingID(), error));
+  EXPECT_EQ(url_2, controller.GetVisibleEntry()->GetURL());
+}
+
 TEST_F(NavigationControllerTest, LoadURL) {
   NavigationControllerImpl& controller = controller_impl();
   TestNotificationTracker notifications;
   RegisterForAllNavNotifications(&notifications, &controller);
 
   const GURL url1("http://foo1");
   const GURL url2("http://foo2");
 
   controller.LoadURL(
       url1, Referrer(), ui::PAGE_TRANSITION_TYPED, std::string());
@@ skipping 4776 matching lines @@
   EXPECT_EQ(default_ssl_status.content_status,
             observer.details().ssl_status.content_status);
   EXPECT_EQ(
       0u,
       observer.details().ssl_status.signed_certificate_timestamp_ids.size());
 
   EXPECT_EQ(1, main_test_rfh()->GetProcess()->bad_msg_count());
 }
 
 }  // namespace content