PlzNavigate: Avoid duplicate SiteInstance creation during navigation.

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..a048ee962d72ae504b1813674d3150bcebd84781 100644
--- a/content/browser/frame_host/render_frame_host_manager.h
+++ b/content/browser/frame_host/render_frame_host_manager.h
@@ -423,6 +423,35 @@ class CONTENT_EXPORT RenderFrameHostManager : public NotificationObserver {
   FRIEND_TEST_ALL_PREFIXES(CrossProcessFrameTreeBrowserTest,
                            CreateCrossProcessSubframeProxies);
 
+  // PlzNavigate
+  enum RenderFrameHostToUse {
+    INVALID,
+    REUSE_CURRENT,
+    NEW_SPECULATIVE,
+    REUSE_SPECULATIVE,
+  };
+
+  // Stores information regarding a SiteInstance targeted at a specific URL. It
+  // can point to an existing one or store the details needed to create a new
+  // one.

[Charlie Reis, 2015/03/31 06:18:19]

Sounds good.  Can you also mention that the purpose of the struct is to allow
checks and comparisons without necessarily creating a new SiteInstance?

[carlosk, 2015/03/31 16:38:19]

Done.

+  struct SiteInstanceDescriptor {
+    explicit SiteInstanceDescriptor(content::SiteInstance* site_instance)
+        : existing_site_instance(site_instance),
+          new_is_related_to_current(false) {}
+
+    SiteInstanceDescriptor(GURL site_url,
+                           bool related_to_current,
+                           BrowserContext* browser_context);
+
+    // Set with an existing SiteInstance to be reused.
+    content::SiteInstance* existing_site_instance;
+    // In case |existing_site_instance| is null, specify a new site URL.

[Charlie Reis, 2015/03/31 06:18:19]

nit: Blank lines between these members.

[carlosk, 2015/03/31 16:38:19]

Done.

+    GURL new_site_url;
+    // In case |existing_site_instance| is null, specify if the new site should
+    // be created in new BrowsingInstance or not.
+    bool new_is_related_to_current;
+  };
+
   // Used with FrameTree::ForEach to erase RenderFrameProxyHosts from a
   // FrameTreeNode's RenderFrameHostManager.
   static bool ClearProxiesInSiteInstance(int32 site_instance_id,
@@ -471,29 +500,39 @@ class CONTENT_EXPORT RenderFrameHostManager : public NotificationObserver {
   SiteInstance* GetSiteInstanceForNavigation(const GURL& dest_url,
                                              SiteInstance* source_instance,
                                              SiteInstance* dest_instance,
+                                             SiteInstance* candidate_instance,
                                              ui::PageTransition transition,
                                              bool dest_is_restore,
                                              bool dest_is_view_source_mode);
 
-  // Returns an appropriate SiteInstance object for the given |dest_url|,
-  // possibly reusing the current SiteInstance.  If --process-per-tab is used,
-  // this is only called when ShouldSwapBrowsingInstancesForNavigation returns
-  // true. |source_instance| is the SiteInstance of the frame that initiated the
-  // navigation. |current_instance| is the SiteInstance of the frame that is
-  // currently navigating. |dest_instance|, is a predetermined SiteInstance
-  // that'll be used if not null.
+  // Returns a descriptor of the appropriate SiteInstance object for the given
+  // |dest_url|, possibly reusing the current, source or destination
+  // SiteInstance.

[Charlie Reis, 2015/03/31 06:18:19]

Also mention that the SiteInstance can be obtained from the descriptor using
ConvertToSiteInstance.

[carlosk, 2015/03/31 16:38:20]

Done.

+  //
+  // |source_instance| is the SiteInstance of the frame that initiated the
+  // navigation. |dest_instance| is a predetermined SiteInstance that will be

[Charlie Reis, 2015/03/31 06:18:19]

Please do not reorder these sentences, so that they continue to match the order
of the source/current/dest parameters.

[carlosk, 2015/03/31 16:38:20]

Reordered them as the original file. 

It made sense before as I had changed the signature to not passing the current
one. Hence the "Internally" that is now removed.

+  // used if not null. Internally, |current_instance| is the SiteInstance of the
+  // frame that is currently navigating.
   // For example, if you have a parent frame A, which has a child frame B, and
   // A is trying to change the src attribute of B, this will cause a navigation
   // where the source SiteInstance is A and B is the current SiteInstance.
+  //
   // This is a helper function for GetSiteInstanceForNavigation.
-  SiteInstance* GetSiteInstanceForURL(const GURL& dest_url,
-                                      SiteInstance* source_instance,
-                                      SiteInstance* current_instance,
-                                      SiteInstance* dest_instance,
-                                      ui::PageTransition transition,
-                                      bool dest_is_restore,
-                                      bool dest_is_view_source_mode,
-                                      bool force_browsing_instance_swap);
+  SiteInstanceDescriptor DetermineSiteInstanceForURL(
+      const GURL& dest_url,
+      SiteInstance* source_instance,
+      SiteInstance* current_instance,
+      SiteInstance* dest_instance,
+      ui::PageTransition transition,
+      bool dest_is_restore,
+      bool dest_is_view_source_mode,
+      bool force_browsing_instance_swap);
+
+  // Converts a SiteInstanceDescriptor to the actual SiteInstance it describes.
+  // If a |candidate_instance| is provided (is not nullptr) and it matches the
+  // description, it is returned verbatim.

[Charlie Reis, 2015/03/31 06:18:19]

s/verbatim/as is/

[carlosk, 2015/03/31 16:38:19]

Done.

+  SiteInstance* ConvertToSiteInstance(const SiteInstanceDescriptor& descriptor,
+                                      SiteInstance* candidate_instance);
 
   // Determines the appropriate url to use as the current url for SiteInstance
   // selection.