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 37 matching lines @@
 #include "net/base/net_errors.h"
 #include "url/gurl.h"
 #include "url/url_constants.h"
 
 namespace content {
 
 namespace {
 
 FrameMsg_Navigate_Type::Value GetNavigationType(
     BrowserContext* browser_context,
+    const FrameTreeNode* frame_tree_node,
     const NavigationEntryImpl& entry,
-    ReloadType reload_type) {
+    ReloadType reload_type,
+    bool is_same_document_history_load) {
   switch (reload_type) {
     case ReloadType::NORMAL:
       return FrameMsg_Navigate_Type::RELOAD;
     case ReloadType::MAIN_RESOURCE:
       return FrameMsg_Navigate_Type::RELOAD_MAIN_RESOURCE;
     case ReloadType::BYPASSING_CACHE:
     case ReloadType::DISABLE_LOFI_MODE:
       return FrameMsg_Navigate_Type::RELOAD_BYPASSING_CACHE;
     case ReloadType::ORIGINAL_REQUEST_URL:
       return FrameMsg_Navigate_Type::RELOAD_ORIGINAL_REQUEST_URL;
     case ReloadType::NONE:
       break;  // Fall through to rest of function.
   }
 
   // |RenderViewImpl::PopulateStateFromPendingNavigationParams| differentiates
   // between |RESTORE_WITH_POST| and |RESTORE|.
   if (entry.restore_type() == RestoreType::LAST_SESSION_EXITED_CLEANLY) {
     if (entry.GetHasPostData())
       return FrameMsg_Navigate_Type::RESTORE_WITH_POST;
     return FrameMsg_Navigate_Type::RESTORE;
   }
 
+  // A normal load of the current URL should be treated as a reload.
+  if (frame_tree_node->current_url() == entry.GetURL() &&

[clamy, 2016/10/12 11:28:32]

Sanity check: this is only called for browser-initiated navigations in
PlzNavigate. What currently happens if for whatever reason we have a
renderer-initiated navigation to the current url (in PlzNavigate & in the
current architecture)? Does it even start? If so, is it classified as same page?

+      entry.GetHistoryURLForDataURL().is_empty() && !entry.GetHasPostData() &&
+      !is_same_document_history_load) {
+    return FrameMsg_Navigate_Type::RELOAD_MAIN_RESOURCE;

[Charlie Reis, 2016/10/15 00:00:49]

I'm curious, does this list of conditions correspond to something in Blink that
would have converted it to a reload?  I'm a bit nervous about regressions if we
just came up with it from scratch, but maybe we can convince ourselves it's ok.

+  }
+
   return FrameMsg_Navigate_Type::NORMAL;
 }
 
 }  // namespace
 
 struct NavigatorImpl::NavigationMetricsData {
   NavigationMetricsData(base::TimeTicks start_time,
                         GURL url,
                         RestoreType restore_type)
       : start_time_(start_time), url_(url) {
@@ skipping 292 matching lines @@
     // which began in the chosen RenderFrameHost, since the request has already
     // been issued.  In that case, simply resume the response.
     bool is_transfer_to_same =
         is_transfer &&
         entry.transferred_global_request_id().child_id ==
             dest_render_frame_host->GetProcess()->GetID();
     if (!is_transfer_to_same) {
       navigation_data_.reset(new NavigationMetricsData(
           navigation_start, dest_url, entry.restore_type()));
       // Create the navigation parameters.
-      FrameMsg_Navigate_Type::Value navigation_type = GetNavigationType(
-          controller_->GetBrowserContext(), entry, reload_type);
+      FrameMsg_Navigate_Type::Value navigation_type =
+          GetNavigationType(controller_->GetBrowserContext(), frame_tree_node,
+                            entry, reload_type, is_same_document_history_load);
       dest_render_frame_host->Navigate(
           entry.ConstructCommonNavigationParams(
               frame_entry, post_body, dest_url, dest_referrer, navigation_type,
               lofi_state, navigation_start),
           entry.ConstructStartNavigationParams(),
           entry.ConstructRequestNavigationParams(
               frame_entry, is_same_document_history_load,
               is_history_navigation_in_new_child,
               entry.GetSubframeUniqueNames(frame_tree_node),
               frame_tree_node->has_committed_real_load(),
@@ skipping 684 matching lines @@
                                       bool is_history_navigation_in_new_child,
                                       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);
+      GetNavigationType(controller_->GetBrowserContext(), frame_tree_node,
+                        entry, reload_type, is_same_document_history_load);
   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