PlzNav: Fix NavigationControllerBrowserTest.EnsureSamePageNavigationUpdatesFrameNaviga…

content/browser/frame_host/navigation_request.cc

 // Copyright 2014 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_request.h"
 
 #include <utility>
 
 #include "content/browser/devtools/render_frame_devtools_agent_host.h"
 #include "content/browser/frame_host/frame_tree.h"
@@ skipping 365 matching lines @@
     common_params_.lofi_state = LOFI_ON;
   else
     common_params_.lofi_state = LOFI_OFF;
 
   // Select an appropriate renderer to commit the navigation.
   RenderFrameHostImpl* render_frame_host =
       frame_tree_node_->render_manager()->GetFrameHostForNavigation(*this);
   NavigatorImpl::CheckWebUIRendererDoesNotDisplayNormalURL(render_frame_host,
                                                            common_params_.url);
 
+  if (render_frame_host->GetLastCommittedURL() ==
+          navigation_handle_->original_url() &&
+      common_params_.navigation_type == FrameMsg_Navigate_Type::NORMAL &&
+      navigation_handle_->WasServerRedirect() &&

[clamy, 2016/10/05 12:47:04]

Is the redirect part needed? Otherwise, when we create the nav request, we could
just determine that we're trying to navigate to the same url we navigated to,
and that this is like a reload. This way, we don't need to do a comparison of
the original url.

[nasko, 2016/10/05 22:07:09]

Yeah, I'm not sure why we have the redirect check here. Actually, having a
redirect when loading an URL shouldn't cause us to reload the original, should
it?

+      ui::PageTransitionCoreTypeIs(navigation_handle_->GetPageTransition(),
+                                   ui::PAGE_TRANSITION_RELOAD)) {
+    common_params_.navigation_type =
+        FrameMsg_Navigate_Type::RELOAD_MAIN_RESOURCE;

[nasko, 2016/10/05 22:07:09]

I'm a bit afraid of such change. I do want us to do overall, as I'd like to
eliminate the SAME_PAGE classification, however in my naive CL for doing that,
there are lots of failures. https://codereview.chromium.org/2246363003/

+  }
+
   // For renderer-initiated navigations that are set to commit in a different
   // renderer, allow the embedder to cancel the transfer.
   if (!browser_initiated_ &&
       render_frame_host != frame_tree_node_->current_frame_host() &&
       !frame_tree_node_->navigator()->GetDelegate()->ShouldTransferNavigation(
           frame_tree_node_->IsMainFrame())) {
     frame_tree_node_->ResetNavigationRequest(false);
     return;
   }
 
@@ skipping 155 matching lines @@
 
   TransferNavigationHandleOwnership(render_frame_host);
   render_frame_host->CommitNavigation(response_.get(), std::move(body_),
                                       common_params_, request_params_,
                                       is_view_source_);
 
   frame_tree_node_->ResetNavigationRequest(true);
 }
 
 }  // namespace content