Create NavigationRequest from LoadURLParams

This CL allows to create the NavigationRequest directly from
LoadURLParams for new navigations.

Bug: 803859
Change-Id: Ic0f3f368ade86fe57bb549ae6c1d5711f1b58d0e
Reviewed-on: https://chromium-review.googlesource.com/c/1097407
Commit-Queue: Camille Lamy <clamy@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599176}

content/browser/frame_host/navigation_controller_impl.h

@@ -232,6 +232,13 @@ class CONTENT_EXPORT NavigationControllerImpl : public NavigationController {
   // navigation failed due to an SSL error.
   void SetPendingNavigationSSLError(bool error);
 
+// Returns true if the string corresponds to a valid data URL, false
+// otherwise.
+#if defined(OS_ANDROID)
+  static bool ValidateDataURLAsString(
+      const scoped_refptr<const base::RefCountedString>& data_url_as_string);
+#endif
+
  private:
   friend class RestoreHelper;
 
@@ -279,24 +286,47 @@ class CONTENT_EXPORT NavigationControllerImpl : public NavigationController {
 
   // Creates and returns a NavigationEntry based on |load_params| for a
   // navigation in |node|.
+  // |override_user_agent|, |should_replace_current_entry| and
+  // |has_user_gesture| will override the values from |load_params|. The same
+  // values should be passed to CreateNavigationRequestFromLoadParams.
   std::unique_ptr<NavigationEntryImpl> CreateNavigationEntryFromLoadParams(
       FrameTreeNode* node,
-      const LoadURLParams& load_params);
+      const LoadURLParams& load_params,
+      bool override_user_agent,
+      bool should_replace_current_entry,
+      bool has_user_gesture);
 
-  // Creates and returns a NavigationRequest based on the provided parameters.
+  // Creates and returns a NavigationRequest based on |load_params| for a
+  // new navigation in |node|.
   // Will return nullptr if the parameters are invalid and the navigation cannot
   // start.
-  std::unique_ptr<NavigationRequest> CreateNavigationRequest(
+  // |override_user_agent|, |should_replace_current_entry| and
+  // |has_user_gesture| will override the values from |load_params|. The same
+  // values should be passed to CreateNavigationEntryFromLoadParams.
+  // TODO(clamy): Remove the dependency on NavigationEntry and
+  // FrameNavigationEntry.
+  std::unique_ptr<NavigationRequest> CreateNavigationRequestFromLoadParams(
+      FrameTreeNode* node,
+      const LoadURLParams& load_params,
+      bool override_user_agent,
+      bool should_replace_current_entry,
+      bool has_user_gesture,
+      ReloadType reload_type,
+      const NavigationEntryImpl& entry,
+      FrameNavigationEntry* frame_entry);
+
+  // Creates and returns a NavigationRequest for a navigation to |entry|. Will
+  // return nullptr if the parameters are invalid and the navigation cannot
+  // start.
+  // TODO(clamy): Ensure this is only called for navigations to existing
+  // NavigationEntries.
+  std::unique_ptr<NavigationRequest> CreateNavigationRequestFromEntry(
       FrameTreeNode* frame_tree_node,
       const NavigationEntryImpl& entry,
       FrameNavigationEntry* frame_entry,
       ReloadType reload_type,
       bool is_same_document_history_load,
-      bool is_history_navigation_in_new_child,
-      const scoped_refptr<network::ResourceRequestBody>& post_body,
-      std::unique_ptr<NavigationUIData> navigation_ui_data,
-      base::TimeTicks input_start,
-      WasActivatedOption was_activated);
+      bool is_history_navigation_in_new_child);
 
   // Returns whether there is a pending NavigationEntry whose unique ID matches
   // the given NavigationHandle's pending_nav_entry_id.

content/browser/frame_host/navigation_entry_impl.cc

@@ -20,6 +20,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
 #include "components/url_formatter/url_formatter.h"
+#include "content/browser/frame_host/navigation_controller_impl.h"
 #include "content/common/content_constants_internal.h"
 #include "content/common/navigation_params.h"
 #include "content/common/page_state_serialization.h"
@@ -732,18 +733,8 @@ RequestNavigationParams NavigationEntryImpl::ConstructRequestNavigationParams(
       intended_as_new_entry, pending_offset_to_send, current_offset_to_send,
       current_length_to_send, IsViewSourceMode(), should_clear_history_list());
 #if defined(OS_ANDROID)
-  if (GetDataURLAsString() &&
-      GetDataURLAsString()->size() <= kMaxLengthOfDataURLString) {
-    // The number of characters that is enough for validating a data: URI.  From
-    // the GURL's POV, the only important part here is scheme, it doesn't check
-    // the actual content. Thus we can take only the prefix of the url, to avoid
-    // unneeded copying of a potentially long string.
-    const size_t kDataUriPrefixMaxLen = 64;
-    GURL data_url(std::string(
-        GetDataURLAsString()->front_as<char>(),
-        std::min(GetDataURLAsString()->size(), kDataUriPrefixMaxLen)));
-    if (data_url.is_valid() && data_url.SchemeIs(url::kDataScheme))
-      request_params.data_url_as_string = GetDataURLAsString()->data();
+  if (NavigationControllerImpl::ValidateDataURLAsString(GetDataURLAsString())) {
+    request_params.data_url_as_string = GetDataURLAsString()->data();
   }
 #endif
   return request_params;

content/browser/frame_host/navigation_request.cc

@@ -281,39 +281,17 @@ bool ShouldPropagateUserActivation(const url::Origin& previous_origin,
 // static
 std::unique_ptr<NavigationRequest> NavigationRequest::CreateBrowserInitiated(
     FrameTreeNode* frame_tree_node,
-    const GURL& dest_url,
-    const Referrer& dest_referrer,
+    const CommonNavigationParams& common_params,
+    const RequestNavigationParams& request_params,
+    bool browser_initiated,
+    const std::string& extra_headers,
     const FrameNavigationEntry& frame_entry,
     const NavigationEntryImpl& entry,
-    FrameMsg_Navigate_Type::Value navigation_type,
-    PreviewsState previews_state,
-    bool is_same_document_history_load,
-    bool is_history_navigation_in_new_child,
     const scoped_refptr<network::ResourceRequestBody>& post_body,
-    base::TimeTicks navigation_start,
-    NavigationControllerImpl* controller,
-    std::unique_ptr<NavigationUIData> navigation_ui_data,
-    base::TimeTicks input_start,
-    WasActivatedOption was_activated) {
-  // A form submission happens either because the navigation is a
-  // renderer-initiated form submission that took the OpenURL path or a
-  // back/forward/reload navigation the does a form resubmission.
-  scoped_refptr<network::ResourceRequestBody> request_body;
-  std::string post_content_type;
-  if (post_body) {
-    // Standard form submission from the renderer.
-    request_body = post_body;
-  } else if (frame_entry.method() == "POST") {
-    // Form resubmission during a back/forward/reload navigation.
-    request_body = frame_entry.GetPostData(&post_content_type);
-    // Might have a LF at end.
-    post_content_type =
-        base::TrimWhitespaceASCII(post_content_type, base::TRIM_ALL)
-            .as_string();
-  }
+    std::unique_ptr<NavigationUIData> navigation_ui_data) {
   // TODO(arthursonzogni): Form submission with the "GET" method is possible.
   // This is not currently handled here.
-  bool is_form_submission = !!request_body;
+  bool is_form_submission = !!post_body;
 
   base::Optional<url::Origin> initiator =
       frame_tree_node->IsMainFrame()
@@ -321,31 +299,10 @@ std::unique_ptr<NavigationRequest> NavigationRequest::CreateBrowserInitiated(
           : base::Optional<url::Origin>(
                 frame_tree_node->frame_tree()->root()->current_origin());
 
-  // While the navigation was started via the LoadURL path it may have come from
-  // the renderer in the first place as part of OpenURL.
-  bool browser_initiated = !entry.is_renderer_initiated();
-
-  CommonNavigationParams common_params = entry.ConstructCommonNavigationParams(
-      frame_entry, request_body, dest_url, dest_referrer, navigation_type,
-      previews_state, navigation_start, input_start);
-
-  RequestNavigationParams request_params =
-      entry.ConstructRequestNavigationParams(
-          frame_entry, common_params.url, common_params.method,
-          is_history_navigation_in_new_child,
-          entry.GetSubframeUniqueNames(frame_tree_node),
-          controller->GetPendingEntryIndex() == -1,
-          controller->GetIndexOfEntry(&entry),
-          controller->GetLastCommittedEntryIndex(),
-          controller->GetEntryCount());
-  request_params.post_content_type = post_content_type;
-  request_params.was_activated = was_activated;
-
   std::unique_ptr<NavigationRequest> navigation_request(new NavigationRequest(
       frame_tree_node, common_params,
       mojom::BeginNavigationParams::New(
-          entry.extra_headers(), net::LOAD_NORMAL,
-          false /* skip_service_worker */,
+          extra_headers, net::LOAD_NORMAL, false /* skip_service_worker */,
           blink::mojom::RequestContextType::LOCATION,
           blink::WebMixedContentContextType::kBlockable, is_form_submission,
           GURL() /* searchable_form_url */,

content/browser/frame_host/navigation_request.h

@@ -32,7 +32,6 @@ namespace content {
 
 class FrameNavigationEntry;
 class FrameTreeNode;
-class NavigationControllerImpl;
 class NavigationHandleImpl;
 class NavigationURLLoader;
 class NavigationData;
@@ -80,22 +79,20 @@ class CONTENT_EXPORT NavigationRequest : public NavigationURLLoaderDelegate {
   };
 
   // Creates a request for a browser-intiated navigation.
+  // Note: this is sometimes called for renderer-initiated navigations going
+  // through the OpenURL path. |browser_initiated| should be false in that case.
+  // TODO(clamy): Rename this function and consider merging it with
+  // CreateRendererInitiated.
   static std::unique_ptr<NavigationRequest> CreateBrowserInitiated(
       FrameTreeNode* frame_tree_node,
-      const GURL& dest_url,
-      const Referrer& dest_referrer,
+      const CommonNavigationParams& common_params,
+      const RequestNavigationParams& request_params,
+      bool browser_initiated,
+      const std::string& extra_headers,
       const FrameNavigationEntry& frame_entry,
       const NavigationEntryImpl& entry,
-      FrameMsg_Navigate_Type::Value navigation_type,
-      PreviewsState previews_state,
-      bool is_same_document_history_load,
-      bool is_history_navigation_in_new_child,
       const scoped_refptr<network::ResourceRequestBody>& post_body,
-      base::TimeTicks navigation_start,
-      NavigationControllerImpl* controller,
-      std::unique_ptr<NavigationUIData> navigation_ui_data,
-      base::TimeTicks input_start,
-      WasActivatedOption was_activated);
+      std::unique_ptr<NavigationUIData> navigation_ui_data);
 
   // Creates a request for a renderer-intiated navigation.
   // Note: |body| is sent to the IO thread when calling BeginNavigation, and

content/browser/frame_host/navigator_impl.cc

@@ -324,6 +324,9 @@ void NavigatorImpl::Navigate(std::unique_ptr<NavigationRequest> request,
                              ReloadType reload_type,
                              RestoreType restore_type) {
   TRACE_EVENT0("browser,navigation", "NavigatorImpl::Navigate");
+  TRACE_EVENT_INSTANT_WITH_TIMESTAMP0(
+      "navigation,rail", "NavigationTiming navigationStart",
+      TRACE_EVENT_SCOPE_GLOBAL, request->common_params().navigation_start);
 
   const GURL& dest_url = request->common_params().url;
   FrameTreeNode* frame_tree_node = request->frame_tree_node();

content/browser/frame_host/render_frame_host_manager_unittest.cc

@@ -436,6 +436,8 @@ class RenderFrameHostManagerTest : public RenderViewHostImplTestHarness {
                                        const NavigationEntryImpl& entry) {
     // Tests currently only navigate using main frame FrameNavigationEntries.
     FrameNavigationEntry* frame_entry = entry.root_node()->frame_entry.get();
+    FrameTreeNode* frame_tree_node =
+        manager->current_frame_host()->frame_tree_node();
     NavigationControllerImpl* controller =
         static_cast<NavigationControllerImpl*>(manager->current_frame_host()
                                                    ->frame_tree_node()
@@ -445,13 +447,37 @@ class RenderFrameHostManagerTest : public RenderViewHostImplTestHarness {
         entry.restore_type() == RestoreType::NONE
             ? FrameMsg_Navigate_Type::DIFFERENT_DOCUMENT
             : FrameMsg_Navigate_Type::RESTORE;
+    scoped_refptr<network::ResourceRequestBody> request_body;
+    std::string post_content_type;
+    if (frame_entry->method() == "POST") {
+      request_body = frame_entry->GetPostData(&post_content_type);
+      // Might have a LF at end.
+      post_content_type =
+          base::TrimWhitespaceASCII(post_content_type, base::TRIM_ALL)
+              .as_string();
+    }
+
+    CommonNavigationParams common_params =
+        entry.ConstructCommonNavigationParams(
+            *frame_entry, request_body, frame_entry->url(),
+            frame_entry->referrer(), navigate_type, PREVIEWS_UNSPECIFIED,
+            base::TimeTicks::Now(), base::TimeTicks::Now());
+    RequestNavigationParams request_params =
+        entry.ConstructRequestNavigationParams(
+            *frame_entry, common_params.url, common_params.method, false,
+            entry.GetSubframeUniqueNames(frame_tree_node),
+            controller->GetPendingEntryIndex() ==
+                -1 /* intended_as_new_entry */,
+            controller->GetIndexOfEntry(&entry),
+            controller->GetLastCommittedEntryIndex(),
+            controller->GetEntryCount());
+    request_params.post_content_type = post_content_type;
+
     std::unique_ptr<NavigationRequest> navigation_request =
         NavigationRequest::CreateBrowserInitiated(
-            manager->frame_tree_node_, frame_entry->url(),
-            frame_entry->referrer(), *frame_entry, entry, navigate_type,
-            PREVIEWS_UNSPECIFIED, false, false, nullptr, base::TimeTicks::Now(),
-            controller, nullptr, base::TimeTicks(),
-            WasActivatedOption::kUnknown);
+            frame_tree_node, common_params, request_params,
+            !entry.is_renderer_initiated(), entry.extra_headers(), *frame_entry,
+            entry, request_body, nullptr /* navigation_ui_data */);
 
     // Simulates request creation that triggers the 1st internal call to
     // GetFrameHostForNavigation.
@@ -2818,14 +2844,27 @@ TEST_F(RenderFrameHostManagerTestWithBrowserSideNavigation,
       ui::PAGE_TRANSITION_TYPED, false /* is_renderer_init */,
       nullptr /* blob_url_loader_factory */);
   FrameNavigationEntry* frame_entry = entry.root_node()->frame_entry.get();
+  FrameTreeNode* frame_tree_node =
+      manager->current_frame_host()->frame_tree_node();
+  CommonNavigationParams common_params = entry.ConstructCommonNavigationParams(
+      *frame_entry, nullptr, frame_entry->url(), frame_entry->referrer(),
+      FrameMsg_Navigate_Type::DIFFERENT_DOCUMENT, PREVIEWS_UNSPECIFIED,
+      base::TimeTicks::Now(), base::TimeTicks::Now());
+  RequestNavigationParams request_params =
+      entry.ConstructRequestNavigationParams(
+          *frame_entry, common_params.url, common_params.method, false,
+          entry.GetSubframeUniqueNames(frame_tree_node),
+          controller().GetPendingEntryIndex() == -1 /* intended_as_new_entry */,
+          static_cast<NavigationControllerImpl&>(controller())
+              .GetIndexOfEntry(&entry),
+          controller().GetLastCommittedEntryIndex(),
+          controller().GetEntryCount());
+
   std::unique_ptr<NavigationRequest> navigation_request =
       NavigationRequest::CreateBrowserInitiated(
-          contents()->GetFrameTree()->root(), frame_entry->url(),
-          frame_entry->referrer(), *frame_entry, entry,
-          FrameMsg_Navigate_Type::DIFFERENT_DOCUMENT, PREVIEWS_UNSPECIFIED,
-          false, false, nullptr, base::TimeTicks::Now(),
-          static_cast<NavigationControllerImpl*>(&controller()), nullptr,
-          base::TimeTicks(), WasActivatedOption::kUnknown);
+          frame_tree_node, common_params, request_params,
+          !entry.is_renderer_initiated(), entry.extra_headers(), *frame_entry,
+          entry, nullptr /* request_body */, nullptr /* navigation_ui_data */);
   manager->DidCreateNavigationRequest(navigation_request.get());
 
   // As the initial RenderFrame was not live, the new RenderFrameHost should be
@@ -2881,14 +2920,27 @@ TEST_F(RenderFrameHostManagerTestWithBrowserSideNavigation,
       ui::PAGE_TRANSITION_TYPED, false /* is_renderer_init */,
       nullptr /* blob_url_loader_factory */);
   FrameNavigationEntry* frame_entry = entry.root_node()->frame_entry.get();
+  FrameTreeNode* frame_tree_node =
+      manager->current_frame_host()->frame_tree_node();
+  CommonNavigationParams common_params = entry.ConstructCommonNavigationParams(
+      *frame_entry, nullptr, frame_entry->url(), frame_entry->referrer(),
+      FrameMsg_Navigate_Type::DIFFERENT_DOCUMENT, PREVIEWS_UNSPECIFIED,
+      base::TimeTicks::Now(), base::TimeTicks::Now());
+  RequestNavigationParams request_params =
+      entry.ConstructRequestNavigationParams(
+          *frame_entry, common_params.url, common_params.method, false,
+          entry.GetSubframeUniqueNames(frame_tree_node),
+          controller().GetPendingEntryIndex() == -1 /* intended_as_new_entry */,
+          static_cast<NavigationControllerImpl&>(controller())
+              .GetIndexOfEntry(&entry),
+          controller().GetLastCommittedEntryIndex(),
+          controller().GetEntryCount());
+
   std::unique_ptr<NavigationRequest> navigation_request =
       NavigationRequest::CreateBrowserInitiated(
-          contents()->GetFrameTree()->root(), frame_entry->url(),
-          frame_entry->referrer(), *frame_entry, entry,
-          FrameMsg_Navigate_Type::DIFFERENT_DOCUMENT, PREVIEWS_UNSPECIFIED,
-          false, false, nullptr, base::TimeTicks::Now(),
-          static_cast<NavigationControllerImpl*>(&controller()), nullptr,
-          base::TimeTicks(), WasActivatedOption::kUnknown);
+          frame_tree_node, common_params, request_params,
+          !entry.is_renderer_initiated(), entry.extra_headers(), *frame_entry,
+          entry, nullptr /* request_body */, nullptr /* navigation_ui_data */);
   manager->DidCreateNavigationRequest(navigation_request.get());
 
   // The current WebUI should still be in place and the pending WebUI should be
@@ -2941,14 +2993,27 @@ TEST_F(RenderFrameHostManagerTestWithBrowserSideNavigation,
       ui::PAGE_TRANSITION_TYPED, false /* is_renderer_init */,
       nullptr /* blob_url_loader_factory */);
   FrameNavigationEntry* frame_entry = entry.root_node()->frame_entry.get();
+  FrameTreeNode* frame_tree_node =
+      manager->current_frame_host()->frame_tree_node();
+  CommonNavigationParams common_params = entry.ConstructCommonNavigationParams(
+      *frame_entry, nullptr, frame_entry->url(), frame_entry->referrer(),
+      FrameMsg_Navigate_Type::DIFFERENT_DOCUMENT, PREVIEWS_UNSPECIFIED,
+      base::TimeTicks::Now(), base::TimeTicks::Now());
+  RequestNavigationParams request_params =
+      entry.ConstructRequestNavigationParams(
+          *frame_entry, common_params.url, common_params.method, false,
+          entry.GetSubframeUniqueNames(frame_tree_node),
+          controller().GetPendingEntryIndex() == -1 /* intended_as_new_entry */,
+          static_cast<NavigationControllerImpl&>(controller())
+              .GetIndexOfEntry(&entry),
+          controller().GetLastCommittedEntryIndex(),
+          controller().GetEntryCount());
+
   std::unique_ptr<NavigationRequest> navigation_request =
       NavigationRequest::CreateBrowserInitiated(
-          contents()->GetFrameTree()->root(), frame_entry->url(),
-          frame_entry->referrer(), *frame_entry, entry,
-          FrameMsg_Navigate_Type::DIFFERENT_DOCUMENT, PREVIEWS_UNSPECIFIED,
-          false, false, nullptr, base::TimeTicks::Now(),
-          static_cast<NavigationControllerImpl*>(&controller()), nullptr,
-          base::TimeTicks(), WasActivatedOption::kUnknown);
+          frame_tree_node, common_params, request_params,
+          !entry.is_renderer_initiated(), entry.extra_headers(), *frame_entry,
+          entry, nullptr /* request_body */, nullptr /* navigation_ui_data */);
   manager->DidCreateNavigationRequest(navigation_request.get());
 
   // The current WebUI should still be in place and there should be a new