PlzNavigate: Remove the RequestNavigation IPC

content/browser/frame_host/navigation_request.h

 // 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.
 
 #ifndef CONTENT_BROWSER_FRAME_HOST_NAVIGATION_REQUEST_H_
 #define CONTENT_BROWSER_FRAME_HOST_NAVIGATION_REQUEST_H_
 
 #include "base/basictypes.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
@@ skipping 34 matching lines @@
 
     // The response started on the IO thread and is ready to be committed. This
     // is one of the two final states for the request.
     RESPONSE_STARTED,
 
     // The request failed on the IO thread and an error page should be
     // displayed. This is one of the two final states for the request.
     FAILED,
   };
 
-  static scoped_ptr<NavigationRequest> Create(
-    FrameTreeNode* frame_tree_node,
-    const NavigationEntryImpl& entry,
-    FrameMsg_Navigate_Type::Value navigation_type,
-    base::TimeTicks navigation_start);
+  // Creates a request for a browser-intiated navigation.
+  static scoped_ptr<NavigationRequest> CreateBrowserInitiated(
+      FrameTreeNode* frame_tree_node,
+      const NavigationEntryImpl& entry,
+      FrameMsg_Navigate_Type::Value navigation_type,
+      base::TimeTicks navigation_start);
 
-  NavigationRequest(FrameTreeNode* frame_tree_node,
-                    const CommonNavigationParams& common_params,
-                    const CommitNavigationParams& commit_params,
-                    const NavigationEntryImpl* navitation_entry);
+  // Creates a request for a renderer-intiated navigation.
+  // Note: |body| is sent to the IO thread when calling BeginNavigation, and
+  // should no longer be manipulated afterwards on the UI thread.
+  static scoped_ptr<NavigationRequest> CreateRendererInitiated(
+      FrameTreeNode* frame_tree_node,
+      const CommonNavigationParams& common_params,
+      const BeginNavigationParams& begin_params,
+      scoped_refptr<ResourceRequestBody> body);
 
   ~NavigationRequest() override;
 
-  // Called on the UI thread by the RenderFrameHostManager which owns the
-  // NavigationRequest. Takes ownership of |info|. After calling this function,
-  // |body| can no longer be manipulated on the UI thread.
-  void BeginNavigation(scoped_ptr<NavigationRequestInfo> info,
-                       scoped_refptr<ResourceRequestBody> body);
+  // Called on the UI thread by the Navigator to start the navigation on the IO
+  // thread. After calling this function, |body_| can no longer be manipulated

[davidben, 2015/02/03 02:23:20]

body_ doesn't seem to exist. I think this sentence is probably redundant with
the one in CreateRendererInitiated() and we can say that, as soon as you call
that function, you can't touch it from your thread anymore.

(Maybe add a TODO to see if we can un-refcount ResourceRequestBody? I haven't
looked into how painful that would be, but this threading subtly does suggest
that unique ownership would be better.)

[clamy, 2015/02/03 16:17:09]

Done.

+  // on the UI thread.
+  void BeginNavigation();
 
   CommonNavigationParams& common_params() { return common_params_; }

[davidben, 2015/02/03 02:23:21]

I think this is no longer needed now that NavigatorImpl doesn't fill in the
referrer for this object.

[carlosk, 2015/02/03 16:06:02]

It's still needed because it's accessed in some places to get information like
the navigation URL. But it most probably can become a const/const call.

[clamy, 2015/02/03 16:17:09]

We already have a const accessor, so I removed the non-const one.

 
   const CommonNavigationParams& common_params() const { return common_params_; }
 
   const CommitNavigationParams& commit_params() const { return commit_params_; }
 
   NavigationURLLoader* loader_for_testing() const { return loader_.get(); }
 
   NavigationState state() const { return state_; }
 
   SiteInstanceImpl* source_site_instance() const {
     return source_site_instance_.get();
   }
 
   SiteInstanceImpl* dest_site_instance() const {
     return dest_site_instance_.get();
   }
 
   NavigationEntryImpl::RestoreType restore_type() const {
     return restore_type_;
   };
 
   bool is_view_source() const { return is_view_source_; };
 
   int bindings() const { return bindings_; };
 
+  bool browser_initiated() const { return browser_initiated_ ; }
+
   void SetWaitingForRendererResponse() {
     DCHECK(state_ == NOT_STARTED);
     state_ = WAITING_FOR_RENDERER_RESPONSE;
   }
 
  private:
+  NavigationRequest(FrameTreeNode* frame_tree_node,
+                    const CommonNavigationParams& common_params,
+                    const BeginNavigationParams& begin_params,
+                    const CommitNavigationParams& commit_params,
+                    scoped_refptr<ResourceRequestBody> body,
+                    bool browser_initiated,
+                    const NavigationEntryImpl* navitation_entry);
+
   // NavigationURLLoaderDelegate implementation.
   void OnRequestRedirected(
       const net::RedirectInfo& redirect_info,
       const scoped_refptr<ResourceResponse>& response) override;
   void OnResponseStarted(const scoped_refptr<ResourceResponse>& response,
                          scoped_ptr<StreamHandle> body) override;
   void OnRequestFailed(int net_error) override;
 
   FrameTreeNode* frame_tree_node_;
 
   // Initialized on creation of the NavigationRequest. Sent to the renderer when
   // the navigation is ready to commit.
   // Note: When the navigation is ready to commit, the url in |common_params|
   // will be set to the final navigation url, obtained after following all
   // redirects.
   CommonNavigationParams common_params_;

[carlosk, 2015/02/03 16:06:02]

I think this one could become const as well.

[clamy, 2015/02/03 16:17:09]

No, because redirects will modify the url there.

+  const BeginNavigationParams begin_params_;
   const CommitNavigationParams commit_params_;
+  const bool browser_initiated_;
 
   NavigationState state_;
 
+
+  // The parameters to send to the IO thread. |loader_| takes ownership of
+  // |info_| after calling BeginNavigation.
+  scoped_ptr<NavigationRequestInfo> info_;
+
   scoped_ptr<NavigationURLLoader> loader_;
 
   // These next items are used in browser-initiated navigations to store
   // information from the NavigationEntryImpl that is required after request
   // creation time.
   scoped_refptr<SiteInstanceImpl> source_site_instance_;
   scoped_refptr<SiteInstanceImpl> dest_site_instance_;
   NavigationEntryImpl::RestoreType restore_type_;
   bool is_view_source_;
   int bindings_;
 
   DISALLOW_COPY_AND_ASSIGN(NavigationRequest);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_FRAME_HOST_NAVIGATION_REQUEST_H_