PlzNavigate: track pending commits

content/browser/frame_host/render_frame_host_manager.h

Index: content/browser/frame_host/render_frame_host_manager.h
diff --git a/content/browser/frame_host/render_frame_host_manager.h b/content/browser/frame_host/render_frame_host_manager.h
index 89eeef69f9d549dc85a644eb8021a57176ce91d2..f8d3716e8fb6e5601afa2f480469cec643a3779a 100644
--- a/content/browser/frame_host/render_frame_host_manager.h
+++ b/content/browser/frame_host/render_frame_host_manager.h
@@ -389,9 +389,11 @@ class CONTENT_EXPORT RenderFrameHostManager : public NotificationObserver {
   // Called (possibly several times) during a navigation to select or create an
   // appropriate RenderFrameHost for the provided URL. The returned pointer will
   // be for the current or the speculative RenderFrameHost and the instance is
-  // owned by this manager.
+  // owned by this manager. |is_for_commit| is true if the RenderFrameHostImpl
+  // is assured to commit a navigation and is no longer speculative.

[clamy, 2015/04/02 12:52:35]

I reintroduced the |is_for_commit| boolean, as it is not possible to set
pending_commit_current_frame_ to false after a call to
GetFrameHostForNavigation. While a commit in the speculative RFH will have been
canceled by destroying the RFH, this is not the case for a pending commit in the
current RFH.

[Charlie Reis, 2015/04/03 06:02:59]

I don't see how that makes this boolean necessary, though.  See my comments in
the .cc file.

   RenderFrameHostImpl* GetFrameHostForNavigation(
-      const NavigationRequest& request);
+      const NavigationRequest& request,
+      bool is_for_commit);
 
   // PlzNavigate
   // Clean up any state for any ongoing navigation.
@@ -621,8 +623,7 @@ class CONTENT_EXPORT RenderFrameHostManager : public NotificationObserver {
   // Whether a navigation requiring different RenderFrameHosts is pending. This
   // is either for cross-site requests or when required for the process type
   // (like WebUI).
-  // PlzNavigate: |cross_navigation_pending_| is not used for browser-side
-  // navigation.
+  // PlzNavigate: not used.

[Charlie Reis, 2015/04/03 06:02:59]

Great!  I'll plan to remove it after your CL lands.

   bool cross_navigation_pending_;
 
   // Implemented by the owner of this class.  These delegates are installed into
@@ -702,6 +703,14 @@ class CONTENT_EXPORT RenderFrameHostManager : public NotificationObserver {
   // creating a new one.
   bool should_reuse_web_ui_;
 
+  // PlzNavigate
+  // Indicates that a commit is pending in the current/speculative
+  // RenderFrameHost. This happens after the response for a request has been
+  // received in the network stack and a RenderFrameHost has been chosen to
+  // handle it, but before the renderer committed the provisional load,

[Charlie Reis, 2015/04/03 06:02:59]

nit: has committed
nit: end with period.

+  bool pending_commit_current_frame_;

[clamy, 2015/04/02 12:52:35]

With some sufficiently bad interleaving of requests it is possible to have
commits pending in both the current and speculative RFH, hence the use of two
different booleans.

+  bool pending_commit_speculative_frame_;
+
   base::WeakPtrFactory<RenderFrameHostManager> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(RenderFrameHostManager);