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 398 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) {
+  // PlzNavigate: Entries are not discarded following the reception of
+  // DidFailProvisionalLoad. They are discarded in DidFailNavigation, when we
+  // still have the NavigationRequest. Since every change of the
+  // NavigationEntry would cause the NavigationRequest to go away, the right
+  // pending NavigationEntry will be discarded in DidFailNavigation.
+  if (IsBrowserSideNavigationEnabled()) {
+    SUCCEED() << "PlzNavigate: test not applicable.";

[Charlie Reis, 2016/04/11 21:10:57]

I'm a bit nervous removing coverage for this without an equivalent PlzNavigate
test, but I'll take your word for it.  :)

[clamy, 2016/04/12 14:08:01]

I've made a few changes that enable the test to test something a bit similar on
PlzNavigate (what if we start a new navigation in the middle of committing an
error page, which would send a DidFailProvisionalLoad after we created a pending
entry for the new navigation).

+    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 4797 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