PlzNav: Fix NavigationControllerBrowserTest.EnsureSamePageNavigationUpdatesFrameNaviga…

content/browser/frame_host/navigation_controller_impl.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.
 
 /*
  * Copyright (C) 2006, 2007, 2008, 2009 Apple Inc. All rights reserved.
  * Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies)
  * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved.
  *     (http://www.torchmobile.com/)
  *
@@ skipping 1771 matching lines @@
 
   // If an interstitial page is showing, the previous renderer is blocked and
   // cannot make new requests.  Unblock (and disable) it to allow this
   // navigation to succeed.  The interstitial will stay visible until the
   // resulting DidNavigate.
   if (delegate_->GetInterstitialPage()) {
     static_cast<InterstitialPageImpl*>(delegate_->GetInterstitialPage())->
         CancelForNavigation();
   }
 
+  // Note that we don't want to convert to a reload for history navigations, so

[Charlie Reis, 2016/11/01 21:18:40]

nit: It's not obvious what this block is doing at first glance, so I'd recommend
starting with something like "Convert enter-in-the-omnibox to a reload, since
that's how Blink treats it anyway."

[scottmg, 2016/11/01 22:14:30]

Done.

+  // this must be above the retrieval of the pending_entry_ below when
+  // pending_entry_index_ is used.
+  if (reload_type == ReloadType::NONE && GetLastCommittedEntry() &&
+      pending_entry_ && pending_entry_->frame_tree_node_id() == -1 &&
+      pending_entry_->GetURL() == GetLastCommittedEntry()->GetURL() &&
+      !pending_entry_->GetHasPostData() &&
+      // This check is required for Android WebView loadDataWithBaseURL.
+      GetLastCommittedEntry()->GetVirtualURL() ==
+          pending_entry_->GetVirtualURL()) {

[Charlie Reis, 2016/11/01 21:18:40]

Just as a sanity check, I patched this in and tried it on chrome://settings,
which seems to do the right thing (both GetURL and GetVirtualURL match between
pending and last committed, though not each other).

[scottmg, 2016/11/01 22:14:30]

Thanks for checking!

+    reload_type = ReloadType::MAIN_RESOURCE;
+  }
+
   // For session history navigations only the pending_entry_index_ is set.
   if (!pending_entry_) {
     CHECK_NE(pending_entry_index_, -1);
     pending_entry_ = entries_[pending_entry_index_].get();
   }
 
   // Any renderer-side debug URLs or javascript: URLs should be ignored if the
   // renderer process is not live, unless it is the initial navigation of the
   // tab.
   if (IsRendererDebugURL(pending_entry_->GetURL())) {
@@ skipping 282 matching lines @@
     }
   }
 }
 
 void NavigationControllerImpl::SetGetTimestampCallbackForTest(
     const base::Callback<base::Time()>& get_timestamp_callback) {
   get_timestamp_callback_ = get_timestamp_callback;
 }
 
 }  // namespace content