Add should_replace_entry to NavigationControllerImpl::CreateNavigationEntry

Since set_should_replace_entry is called from almost all callers of
NavigationControllerImpl::CreateNavigationEntry, this CL adds it as a
a parameter to NavigationControllerImpl::CreateNavigationEntry instead.
Bug: 1015882

Change-Id: I5a0703bc32561178435d0b41f3ffeec6ae2edec0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2251603
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#781692}

content/browser/frame_host/navigation_controller_impl.cc

@@ -473,7 +473,8 @@ std::unique_ptr<NavigationEntry> NavigationController::CreateNavigationEntry(
   return NavigationControllerImpl::CreateNavigationEntry(
       url, referrer, std::move(initiator_origin),
       nullptr /* source_site_instance */, transition, is_renderer_initiated,
-      extra_headers, browser_context, std::move(blob_url_loader_factory));
+      extra_headers, browser_context, std::move(blob_url_loader_factory),
+      false /* should_replace_entry */);
 }
 
 // static
@@ -487,7 +488,8 @@ NavigationControllerImpl::CreateNavigationEntry(
     bool is_renderer_initiated,
     const std::string& extra_headers,
     BrowserContext* browser_context,
-    scoped_refptr<network::SharedURLLoaderFactory> blob_url_loader_factory) {
+    scoped_refptr<network::SharedURLLoaderFactory> blob_url_loader_factory,
+    bool should_replace_entry) {
   GURL url_to_load;
   GURL virtual_url;
   bool reverse_on_redirect = false;
@@ -509,6 +511,7 @@ NavigationControllerImpl::CreateNavigationEntry(
   entry->set_user_typed_url(virtual_url);
   entry->set_update_virtual_url_with_url(reverse_on_redirect);
   entry->set_extra_headers(extra_headers);
+  entry->set_should_replace_entry(should_replace_entry);
   return entry;
 }
 
@@ -2253,6 +2256,11 @@ void NavigationControllerImpl::NavigateFromFrameProxy(
 
   FrameTreeNode* node = render_frame_host->frame_tree_node();
 
+  // Don't allow an entry replacement if there is no entry to replace.
+  // http://crbug.com/457149
+  if (GetEntryCount() == 0)
+    should_replace_current_entry = false;
+
   // Create a NavigationEntry for the transfer, without making it the pending
   // entry. Subframe transfers should have a clone of the last committed entry
   // with a FrameNavigationEntry for the target frame. Main frame transfers
@@ -2278,7 +2286,7 @@ void NavigationControllerImpl::NavigateFromFrameProxy(
           GURL(url::kAboutBlankURL), referrer, initiator_origin,
           source_site_instance, page_transition, is_renderer_initiated,
           extra_headers, browser_context_,
-          nullptr /* blob_url_loader_factory */));
+          nullptr /* blob_url_loader_factory */, should_replace_current_entry));
     }
     entry->AddOrUpdateFrameEntry(
         node, -1, -1, nullptr,
@@ -2291,19 +2299,12 @@ void NavigationControllerImpl::NavigateFromFrameProxy(
     entry = NavigationEntryImpl::FromNavigationEntry(CreateNavigationEntry(
         url, referrer, initiator_origin, source_site_instance, page_transition,
         is_renderer_initiated, extra_headers, browser_context_,
-        blob_url_loader_factory));
+        blob_url_loader_factory, should_replace_current_entry));
     entry->root_node()->frame_entry->set_source_site_instance(
         static_cast<SiteInstanceImpl*>(source_site_instance));
     entry->root_node()->frame_entry->set_method(method);
   }
 
-  // Don't allow an entry replacement if there is no entry to replace.
-  // http://crbug.com/457149
-  if (GetEntryCount() == 0)
-    should_replace_current_entry = false;
-
-  entry->set_should_replace_entry(should_replace_current_entry);
-
   bool override_user_agent = false;
   if (GetLastCommittedEntry() &&
       GetLastCommittedEntry()->GetIsOverridingUserAgent()) {
@@ -3094,7 +3095,7 @@ NavigationControllerImpl::CreateNavigationEntryFromLoadParams(
           GURL(url::kAboutBlankURL), params.referrer, params.initiator_origin,
           params.source_site_instance.get(), params.transition_type,
           params.is_renderer_initiated, extra_headers_crlf, browser_context_,
-          blob_url_loader_factory));
+          blob_url_loader_factory, should_replace_current_entry));
     }
 
     entry->AddOrUpdateFrameEntry(
@@ -3109,7 +3110,7 @@ NavigationControllerImpl::CreateNavigationEntryFromLoadParams(
         params.url, params.referrer, params.initiator_origin,
         params.source_site_instance.get(), params.transition_type,
         params.is_renderer_initiated, extra_headers_crlf, browser_context_,
-        blob_url_loader_factory));
+        blob_url_loader_factory, should_replace_current_entry));
     entry->set_source_site_instance(
         static_cast<SiteInstanceImpl*>(params.source_site_instance.get()));
     entry->SetRedirectChain(params.redirect_chain);
@@ -3117,7 +3118,6 @@ NavigationControllerImpl::CreateNavigationEntryFromLoadParams(
 
   // Set the FTN ID (only used in non-site-per-process, for tests).
   entry->set_frame_tree_node_id(node->frame_tree_node_id());
-  entry->set_should_replace_entry(should_replace_current_entry);
   entry->set_should_clear_history_list(params.should_clear_history_list);
   entry->SetIsOverridingUserAgent(override_user_agent);
   entry->set_has_user_gesture(has_user_gesture);

content/browser/frame_host/navigation_controller_impl.h

@@ -315,8 +315,8 @@ class CONTENT_EXPORT NavigationControllerImpl : public NavigationController {
   // requests corresponding to the current pending entry.
   std::unique_ptr<PendingEntryRef> ReferencePendingEntry();
 
-  // Like NavigationController::CreateNavigationEntry, but takes an extra
-  // |source_site_instance| argument.
+  // Like NavigationController::CreateNavigationEntry, but takes extra arguments
+  // like |source_site_instance| and |should_replace_entry|.
   static std::unique_ptr<NavigationEntryImpl> CreateNavigationEntry(
       const GURL& url,
       Referrer referrer,
@@ -326,7 +326,8 @@ class CONTENT_EXPORT NavigationControllerImpl : public NavigationController {
       bool is_renderer_initiated,
       const std::string& extra_headers,
       BrowserContext* browser_context,
-      scoped_refptr<network::SharedURLLoaderFactory> blob_url_loader_factory);
+      scoped_refptr<network::SharedURLLoaderFactory> blob_url_loader_factory,
+      bool should_replace_entry);
 
  private:
   friend class RestoreHelper;

content/browser/frame_host/navigator.cc

@@ -834,9 +834,8 @@ Navigator::GetNavigationEntryForRendererInitiatedNavigation(
               ui::PAGE_TRANSITION_LINK, true /* is_renderer_initiated */,
               std::string() /* extra_headers */,
               controller_->GetBrowserContext(),
-              nullptr /* blob_url_loader_factory */));
-
-  entry->set_should_replace_entry(common_params.should_replace_current_entry);
+              nullptr /* blob_url_loader_factory */,
+              common_params.should_replace_current_entry));
 
   controller_->SetPendingEntry(std::move(entry));
   if (delegate_)