PlzNavigate: implement process reuse for ServiceWorkers

content/browser/frame_host/navigation_handle_impl.h

 // Copyright 2015 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_HANDLE_IMPL_H_
 #define CONTENT_BROWSER_FRAME_HOST_NAVIGATION_HANDLE_IMPL_H_
 
 #include "content/public/browser/navigation_handle.h"
 
 #include <stddef.h>
@@ skipping 362 matching lines @@
 
   CSPDisposition should_check_main_world_csp() const {
     return should_check_main_world_csp_;
   }
 
   const SourceLocation& source_location() const { return source_location_; }
   void set_source_location(const SourceLocation& source_location) {
     source_location_ = source_location;
   }
 
+  // PlzNavigate
+  // The ID of the RenderProcessHost we expect the navigation to commit in. This
+  // is used to inform the RenderProcessHost to expect a navigation to the url
+  // we're navigating to. If |should_update_on_redirects| is true, we will
+  // update the RenderProcessHost when encountering redirects, otherwise we will
+  // just inform it to stop expecting a navigation.

[Charlie Reis, 2017/05/15 03:41:52]

The should_update_on_redirects parameter seems kind of indirect to me, and I'm
not sure if it's correct.  If I understand correctly, it's meant to indicate
whether we'll do a process swap for a redirect?

We're using may_transfer_ for that, which (from our discussion on
https://codereview.chromium.org/2768813002/) is really about whether it's
from_begin_navigation(), right?  But that's not a good predictor of whether
there will be a process swap.  First, BeginNavigation cases can still swap
processes on redirect, such as link clicks on URLs that redirect to the Chrome
Web Store.  Second, the process swap logic is fairly complex and can't be boiled
down to just whether it's cross-site.

I think this means we don't want to have to guess whether a speculative RFH has
changed by listening to redirects.  Instead, we should have a more explicit
notification that the speculative RFH has changed, and use that to update the
RPH state.  Maybe even something closer to that code should be the place where
we tell RPH, rather than NavigationHandleImpl?

[clamy, 2017/05/16 14:50:45]

I have removed this part of the patch. For now, I'd prefer to keep the tracking
of navigations in NavigationhandleImpl as this is where the lifetime of
navigations is better tracked.

+  void SetSpeculativeProcessID(int render_process_host_id,
+                               bool should_update_on_redirects);
+
  private:
   friend class NavigationHandleImplTest;
 
   NavigationHandleImpl(const GURL& url,
                        const std::vector<GURL>& redirect_chain,
                        FrameTreeNode* frame_tree_node,
                        bool is_renderer_initiated,
                        bool is_same_page,
                        const base::TimeTicks& navigation_start,
                        int pending_nav_entry_id,
@@ skipping 161 matching lines @@
   CSPDisposition should_check_main_world_csp_;
 
   // Whether or not the navigation results from the submission of a form.
   bool is_form_submission_;
 
   // PlzNavigate
   // Information about the JavaScript that started the navigation. For
   // navigations initiated by Javascript.
   SourceLocation source_location_;
 
+  // PlzNavigate
+  // Used to inform a RenderProcessHost that we expect this navigation to commit
+  // in it.
+  int speculative_render_process_host_id_;
+
+  // PlzNavigate
+  // Whether we should inform the RenderProcessHost to expect a new navigation
+  // when this navigation is redirected.
+  bool should_inform_process_on_redirects_;
+
   base::WeakPtrFactory<NavigationHandleImpl> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(NavigationHandleImpl);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_FRAME_HOST_NAVIGATION_HANDLE_IMPL_H_