Start a renderer-initiated navigation before commit in RendererInitiatedPendingEntries

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 "base/basictypes.h"
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/files/file_util.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/stl_util.h"
@@ skipping 3233 matching lines @@
   NavigationControllerImpl& controller = controller_impl();
   Navigator* navigator =
       contents()->GetFrameTree()->root()->navigator();
 
   const GURL url1("nonexistent:12121");
   const GURL url1_fixed("http://nonexistent:12121/");
   const GURL url2("http://foo");
 
   // We create pending entries for renderer-initiated navigations so that we
   // can show them in new tabs when it is safe.
+  main_test_rfh()->SendRendererInitiatedNavigationRequest(url1, false);
+  main_test_rfh()->PrepareForCommit();

[Charlie Reis, 2015/05/28 17:02:27]

These two lines are only needed for PlzNavigate, right?  There's no interactions
with the browser process before DidStartProvisionalLoad by default, and the
BeforeUnload_ACK in PrepareForCommit isn't used for renderer-initiated
navigations.

[nasko, 2015/05/28 17:16:44]

While they are fairly PlzNavigate specific, in my unit tests CL I've added
initialization of the RenderFrame as the first step in
SendRendererInitiatedNavigationRequest. I could manually do it, but it seemed
better to write it in a way that is compatible with PlzNavigate style
navigation.

I could drop the PrepareForCommit. In general I have been aiming for more
streamlined flow of calls that are consistent, logical and don't force you to
know all the intricate details of each call.

[Charlie Reis, 2015/05/28 17:40:19]

Ok.
Hmm, there's a tradeoff here.  We're already running into problems where the
unit tests don't match reality, and this introduces another case
(BeforeUnload_ACK).  I'm ok with it in this case, though, since we need to be
able to handle that message and it does make writing the tests easier.

   navigator->DidStartProvisionalLoad(main_test_rfh(), url1);
 
   // Simulate what happens if a BrowserURLHandler rewrites the URL, causing
   // the virtual URL to differ from the URL.
   controller.GetPendingEntry()->SetURL(url1_fixed);
   controller.GetPendingEntry()->SetVirtualURL(url1);
 
   EXPECT_EQ(url1_fixed, controller.GetPendingEntry()->GetURL());
   EXPECT_EQ(url1, controller.GetPendingEntry()->GetVirtualURL());
   EXPECT_TRUE(controller.GetPendingEntry()->is_renderer_initiated());
@@ skipping 1630 matching lines @@
   {
     LoadCommittedDetails details;
     controller_impl().RendererDidNavigate(main_test_rfh(), params, &details);
     EXPECT_EQ(PAGE_TYPE_ERROR,
               controller_impl().GetLastCommittedEntry()->GetPageType());
     EXPECT_EQ(NAVIGATION_TYPE_IN_PAGE, details.type);
   }
 }
 
 }  // namespace content