PlzNav: Fix NavigationControllerBrowserTest.EnsureSamePageNavigationUpdatesFrameNaviga…

content/browser/frame_host/navigator_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.
 
 #include "content/browser/frame_host/navigator_impl.h"
 
 #include <utility>
 
 #include "base/logging.h"
 #include "base/metrics/histogram_macros.h"
@@ skipping 1080 matching lines @@
                                       base::TimeTicks navigation_start) {
   CHECK(IsBrowserSideNavigationEnabled());
   DCHECK(frame_tree_node);
 
   // This value must be set here because creating a NavigationRequest might
   // change the renderer live/non-live status and change this result.
   bool should_dispatch_beforeunload =
       frame_tree_node->current_frame_host()->ShouldDispatchBeforeUnload();
   FrameMsg_Navigate_Type::Value navigation_type =
       GetNavigationType(controller_->GetBrowserContext(), entry, reload_type);
+  if (navigation_type == FrameMsg_Navigate_Type::NORMAL &&

[clamy, 2016/10/10 11:48:02]

nit: add a comment explaining that a normal load of the current url should be
treated as a reload?

[scottmg, 2016/10/11 19:52:32]

Done.

+      frame_tree_node->current_url() == entry.GetURL() &&
+      !is_same_document_history_load) {
+    navigation_type = FrameMsg_Navigate_Type::RELOAD_MAIN_RESOURCE;

[nasko, 2016/10/10 22:04:22]

I think we are best doing this for both current navigation code and PlzNavigate.
I'd rather not have the two paths behave differently and the plus of doing it
for current navigation is that we can eliminate the SAME_PAGE navigation
classification.

[scottmg, 2016/10/11 19:52:32]

I tried this, but Android seems to have some legacy behaviour in webview as
tested by LoadDataWithBaseUrlTest#testHistoryUrl that fails after changing this
for everywhere.

I'm looking into the pain that is Android now, but assuming that's fixable, I
won't attempt the removal of SAME_PAGE in this CL but rather defer it to
another.

(Based on Charlie's calendar it looks like he likely won't get to this until
next week.)

+  }
   std::unique_ptr<NavigationRequest> scoped_request =
       NavigationRequest::CreateBrowserInitiated(
           frame_tree_node, dest_url, dest_referrer, frame_entry, entry,
           navigation_type, lofi_state, is_same_document_history_load,
           is_history_navigation_in_new_child, navigation_start, controller_);
   NavigationRequest* navigation_request = scoped_request.get();
 
   // Navigation to a javascript URL is not a "real" navigation so there is no
   // need to create a NavigationHandle. The navigation commits immediately and
   // the NavigationRequest is not assigned to the FrameTreeNode as navigating to
@@ skipping 118 matching lines @@
     if (navigation_handle)
       navigation_handle->update_entry_id_for_transfer(entry->GetUniqueID());
 
     controller_->SetPendingEntry(std::move(entry));
     if (delegate_)
       delegate_->NotifyChangedNavigationState(content::INVALIDATE_TYPE_URL);
   }
 }
 
 }  // namespace content