Remove dependency on NavigationEntry to get a SiteInstance

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 92c31fbb1212f89f1899702fd742fe1f3572eba0..1f7b979300300076e2518b5a2c52dad144eb953b 100644
--- a/content/browser/frame_host/render_frame_host_manager.h
+++ b/content/browser/frame_host/render_frame_host_manager.h
@@ -9,6 +9,7 @@
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
+#include "content/browser/frame_host/navigation_entry_impl.h"
 #include "content/browser/renderer_host/render_view_host_delegate.h"
 #include "content/browser/site_instance_impl.h"
 #include "content/common/content_export.h"
@@ -386,15 +387,17 @@ class CONTENT_EXPORT RenderFrameHostManager : public NotificationObserver {
   // switch.  Can be overridden in unit tests.
   bool ShouldTransitionCrossSite();
 
-  // Returns true if for the navigation from |current_entry| to |new_entry|,
+  // Returns true if for the navigation from |current_entry| to |new_url|,
   // a new SiteInstance and BrowsingInstance should be created (even if we are
   // in a process model that doesn't usually swap).  This forces a process swap
   // and severs script connections with existing tabs.  Cases where this can
   // happen include transitions between WebUI and regular web pages.
-  // Either of the entries may be NULL.
+  // |current_entry| and |new_site_instance| may be null.
   bool ShouldSwapBrowsingInstancesForNavigation(
       const NavigationEntry* current_entry,

[Charlie Reis, 2014/08/14 06:28:09]

Looks like we can eliminate |current_entry| from this method as well, which
would make it a more consistent set of parameters (even if we can't yet remove
it from UpdateStateForNavigate). 

[clamy, 2014/08/14 10:02:59]

Done.

-      const NavigationEntryImpl* new_entry) const;
+      SiteInstance* new_site_instance,
+      const GURL& new_url,

[Charlie Reis, 2014/08/14 06:28:09]

It's very important that this is the result of GetEffectiveURL, and not just the
entry's URL (for hosted apps).  Let's call it |new_effective_url| and document
the requirement.

[clamy, 2014/08/14 10:02:59]

Done.

+      bool new_is_view_source_mode) const;
 
   // Returns true if it is safe to reuse the current WebUI when navigating from
   // |current_entry| to |new_entry|.
@@ -402,14 +405,18 @@ class CONTENT_EXPORT RenderFrameHostManager : public NotificationObserver {
       const NavigationEntry* current_entry,
       const NavigationEntryImpl* new_entry) const;
 
-  // Returns an appropriate SiteInstance object for the given NavigationEntry,
+  // 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.
-  SiteInstance* GetSiteInstanceForEntry(
-      const NavigationEntryImpl& entry,
+  // true. |navigation_instance| will be used if it is not null.
+  SiteInstance* GetSiteInstanceForURL(
+      const GURL& dest_url,
+      SiteInstance* navigation_instance,

[Charlie Reis, 2014/08/14 06:28:09]

We should be clearer about which parameters are for the destination, by grouping
them together and giving them a common prefix:

dest_url, dest_site_instance, dest_transition, dest_is_restore,
dest_is_view_source_mode, current_instance, force_browsing_instance_swap

[clamy, 2014/08/14 10:02:59]

Done.

       SiteInstance* current_instance,
-      bool force_browsing_instance_swap);
+      PageTransition page_transition,
+      NavigationEntryImpl::RestoreType restore_type,

[Charlie Reis, 2014/08/14 06:28:09]

We can make this a bool |dest_is_restore| to avoid having to include
navigation_entry_impl.h in this header file.

[clamy, 2014/08/14 10:02:59]

Done.

+      bool force_browsing_instance_swap,
+      bool is_view_source_mode);
 
   // Creates a RenderFrameHost and corresponding RenderViewHost if necessary.
   scoped_ptr<RenderFrameHostImpl> CreateRenderFrameHost(SiteInstance* instance,