PlzNavigate: track pending commits

content/browser/frame_host/render_frame_host_manager.h

 // Copyright 2013 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_RENDER_FRAME_HOST_MANAGER_H_
 #define CONTENT_BROWSER_FRAME_HOST_RENDER_FRAME_HOST_MANAGER_H_
 
 #include <list>
 
 #include "base/basictypes.h"
@@ skipping 371 matching lines @@
 
   // PlzNavigate
   // Notifies the RFHM that a navigation has begun so that it can speculatively
   // create a new RenderFrameHost (and potentially a new process) if needed.
   void BeginNavigation(const NavigationRequest& request);
 
   // PlzNavigate
   // 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.
   void CleanUpNavigation();
 
   // PlzNavigate
   // Clears the speculative members, returning the RenderFrameHost to the caller
   // for disposal.
   scoped_ptr<RenderFrameHostImpl> UnsetSpeculativeRenderFrameHost();
 
@@ skipping 209 matching lines @@
 
   // For use in creating RenderFrameHosts.
   FrameTreeNode* frame_tree_node_;
 
   // Our delegate, not owned by us. Guaranteed non-NULL.
   Delegate* delegate_;
 
   // 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
   // all the RenderFrameHosts that we create.
   RenderFrameHostDelegate* render_frame_delegate_;
   RenderViewHostDelegate* render_view_delegate_;
   RenderWidgetHostDelegate* render_widget_delegate_;
 
   // Our RenderFrameHost and its associated Web UI (if any, will be NULL for
   // non-WebUI pages). This object is responsible for all communication with
@@ skipping 59 matching lines @@
   // Note: PlzNavigate only uses speculative RenderFrameHost and WebUI, not
   // the pending ones.
   scoped_ptr<RenderFrameHostImpl> speculative_render_frame_host_;
   scoped_ptr<WebUIImpl> speculative_web_ui_;
 
   // PlzNavigate
   // If true at navigation commit time the current WebUI will be kept instead of
   // 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);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_FRAME_HOST_RENDER_FRAME_HOST_MANAGER_H_