re-use WebUIs

content/browser/web_contents/render_view_host_manager.cc

Index: content/browser/web_contents/render_view_host_manager.cc
diff --git a/content/browser/web_contents/render_view_host_manager.cc b/content/browser/web_contents/render_view_host_manager.cc
index 95021bfbf2be75f0d7b09e1e628278fa3c28eea6..5e24bb148f467f0d9cce629246a2c30fad8bb992 100644
--- a/content/browser/web_contents/render_view_host_manager.cc
+++ b/content/browser/web_contents/render_view_host_manager.cc
@@ -45,6 +45,7 @@ RenderViewHostManager::RenderViewHostManager(
       render_view_delegate_(render_view_delegate),
       render_view_host_(NULL),
       pending_render_view_host_(NULL),
+      pending_web_ui_(NULL),
       interstitial_page_(NULL) {
 }
 
@@ -63,6 +64,8 @@ RenderViewHostManager::~RenderViewHostManager() {
        ++iter) {
     iter->second->Shutdown();
   }
+
+  FreePendingWebUI();
 }
 
 void RenderViewHostManager::Init(content::BrowserContext* browser_context,
@@ -206,7 +209,7 @@ void RenderViewHostManager::DidNavigateMainFrame(
     DCHECK(render_view_host == render_view_host_);
 
     // Even when there is no pending RVH, there may be a pending Web UI.
-    if (pending_web_ui_.get())
+    if (pending_web_ui_)
       CommitPending();
     return;
   }
@@ -406,6 +409,20 @@ bool RenderViewHostManager::ShouldSwapProcessesForNavigation(
   return false;
 }
 
+bool RenderViewHostManager::ShouldReuseWebUI(
+    const NavigationEntry* curr_entry,
+    const NavigationEntryImpl* new_entry) const {
+  NavigationControllerImpl& controller =
+      delegate_->GetControllerForRenderManager();
+  WebUIControllerFactory* factory =
+      content::GetContentClient()->browser()->GetWebUIControllerFactory();
+  return curr_entry && web_ui_.get() &&
+      (factory->GetWebUIType(controller.GetBrowserContext(),

[Dan Beam, 2012/04/21 02:53:07]

can you split this into multiple lines?

[Evan Stade, 2012/04/24 17:52:40]

not easily.

+                             curr_entry->GetURL()) ==
+       factory->GetWebUIType(controller.GetBrowserContext(),
+                             new_entry->GetURL()));
+}
+
 SiteInstance* RenderViewHostManager::GetSiteInstanceForEntry(
     const NavigationEntryImpl& entry,
     SiteInstance* curr_instance) {
@@ -581,7 +598,7 @@ bool RenderViewHostManager::InitRenderView(RenderViewHost* render_view_host,
                                            const NavigationEntryImpl& entry) {
   // If the pending navigation is to a WebUI, tell the RenderView about any
   // bindings it will need enabled.
-  if (pending_web_ui_.get())
+  if (pending_web_ui_)
     render_view_host->AllowBindings(pending_web_ui_->GetBindings());
 
   return delegate_->CreateRenderViewForRenderManager(render_view_host);
@@ -595,10 +612,9 @@ void RenderViewHostManager::CommitPending() {
   bool will_focus_location_bar = delegate_->FocusLocationBarByDefault();
 
   // Next commit the Web UI, if any.
-  web_ui_.swap(pending_web_ui_);
-  if (web_ui_.get() && pending_web_ui_.get() && !pending_render_view_host_)
-    web_ui_->GetController()->DidBecomeActiveForReusedRenderView();
-  pending_web_ui_.reset();
+  if (web_ui_.get() != pending_web_ui_)

[Dan Beam, 2012/04/21 02:53:07]

you don't really need .get() operators for scoped_ptr's IIRC, as they have a T*
operator

+    web_ui_.reset(pending_web_ui_);
+  pending_web_ui_ = NULL;
 
   // It's possible for the pending_render_view_host_ to be NULL when we aren't
   // crossing process boundaries. If so, we just needed to handle the Web UI
@@ -682,6 +698,7 @@ void RenderViewHostManager::CommitPending() {
   delegate_->NotifySwappedFromRenderManager();
 }
 

[Dan Beam, 2012/04/21 02:53:07]

why \n?

[Evan Stade, 2012/04/24 17:52:40]

Done.

+
 RenderViewHostImpl* RenderViewHostManager::UpdateRendererStateForNavigate(
     const NavigationEntryImpl& entry) {
   // If we are cross-navigating, then we want to get back to normal and navigate
@@ -692,14 +709,6 @@ RenderViewHostImpl* RenderViewHostManager::UpdateRendererStateForNavigate(
     cross_navigation_pending_ = false;
   }
 
-  // This will possibly create (set to NULL) a Web UI object for the pending
-  // page. We'll use this later to give the page special access. This must
-  // happen before the new renderer is created below so it will get bindings.
-  // It must also happen after the above conditional call to CancelPending(),
-  // otherwise CancelPending may clear the pending_web_ui_ and the page will
-  // not have it's bindings set appropriately.
-  pending_web_ui_.reset(delegate_->CreateWebUIForRenderManager(entry.GetURL()));
-
   // render_view_host_ will not be deleted before the end of this method, so we
   // don't have to worry about this SiteInstance's ref count dropping to zero.
   SiteInstance* curr_instance = render_view_host_->GetSiteInstance();
@@ -708,8 +717,9 @@ RenderViewHostImpl* RenderViewHostManager::UpdateRendererStateForNavigate(
   // Again, new_instance won't be deleted before the end of this method, so it
   // is safe to use a normal pointer here.
   SiteInstance* new_instance = curr_instance;
-  bool force_swap = ShouldSwapProcessesForNavigation(
-      delegate_->GetLastCommittedNavigationEntryForRenderManager(), &entry);
+  const content::NavigationEntry* curr_entry =
+      delegate_->GetLastCommittedNavigationEntryForRenderManager();
+  bool force_swap = ShouldSwapProcessesForNavigation(curr_entry, &entry);
   if (ShouldTransitionCrossSite() || force_swap)
     new_instance = GetSiteInstanceForEntry(entry, curr_instance);
 
@@ -717,6 +727,14 @@ RenderViewHostImpl* RenderViewHostManager::UpdateRendererStateForNavigate(
     // New SiteInstance.
     DCHECK(!cross_navigation_pending_);
 
+    // This will possibly create (set to NULL) a Web UI object for the pending
+    // page. We'll use this later to give the page special access. This must
+    // happen before the new renderer is created below so it will get bindings.
+    // It must also happen after the above conditional call to CancelPending(),
+    // otherwise CancelPending may clear the pending_web_ui_ and the page will
+    // not have it's bindings set appropriately.

[Dan Beam, 2012/04/21 02:53:07]

its

[Evan Stade, 2012/04/24 17:52:40]

Done.

+    pending_web_ui_ = delegate_->CreateWebUIForRenderManager(entry.GetURL());
+
     // Create a pending RVH and navigate it.
     bool success = CreatePendingRenderView(entry, new_instance);
     if (!success)
@@ -766,7 +784,12 @@ RenderViewHostImpl* RenderViewHostManager::UpdateRendererStateForNavigate(
 
     return pending_render_view_host_;
   } else {
-    if (pending_web_ui_.get() && render_view_host_->IsRenderViewLive())
+    if (ShouldReuseWebUI(curr_entry, &entry))
+      pending_web_ui_ = web_ui_.get();
+    else
+      pending_web_ui_ = delegate_->CreateWebUIForRenderManager(entry.GetURL());
+
+    if (pending_web_ui_ && render_view_host_->IsRenderViewLive())
       pending_web_ui_->GetController()->RenderViewReused(render_view_host_);
 
     // The renderer can exit view source mode when any error or cancellation
@@ -810,7 +833,7 @@ void RenderViewHostManager::CancelPending() {
     pending_render_view_host->Shutdown();
   }
 
-  pending_web_ui_.reset();
+  FreePendingWebUI();
 }
 
 void RenderViewHostManager::RenderViewDeleted(RenderViewHost* rvh) {
@@ -848,3 +871,10 @@ bool RenderViewHostManager::IsSwappedOut(RenderViewHost* rvh) {
   return swapped_out_hosts_.find(rvh->GetSiteInstance()->GetId()) !=
       swapped_out_hosts_.end();
 }
+
+void RenderViewHostManager::FreePendingWebUI() {
+  if (pending_web_ui_ && pending_web_ui_ != web_ui_.get())
+    delete pending_web_ui_;
+
+  pending_web_ui_ = NULL;
+}