Keep lazy background page alive while there are pending network requests or

chrome/browser/extensions/extension_process_manager.cc

Index: chrome/browser/extensions/extension_process_manager.cc
diff --git a/chrome/browser/extensions/extension_process_manager.cc b/chrome/browser/extensions/extension_process_manager.cc
index f249cf4985e073a1d6ea880d38e74952dceec7ff..561a93c938432feac1821a8156d80cdf9b0479cb 100644
--- a/chrome/browser/extensions/extension_process_manager.cc
+++ b/chrome/browser/extensions/extension_process_manager.cc
@@ -123,6 +123,8 @@ ExtensionProcessManager::ExtensionProcessManager(Profile* profile)
                  content::Source<Profile>(profile));
   registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_HOST_VIEW_SHOULD_CLOSE,
                  content::Source<Profile>(profile));
+  registrar_.Add(this, content::NOTIFICATION_WEB_CONTENTS_CONNECTED,
+                 content::NotificationService::AllSources());
   registrar_.Add(this, content::NOTIFICATION_APP_TERMINATING,
                  content::NotificationService::AllSources());
 }
@@ -263,7 +265,6 @@ ExtensionHost* ExtensionProcessManager::GetBackgroundHostForExtension(
       return host;
   }
   return NULL;
-
 }
 
 std::set<RenderViewHost*>
@@ -277,10 +278,10 @@ std::set<RenderViewHost*>
     return result;
 
   // Gather up all the views for that site.
-  for (RenderViewHostSet::iterator view = all_extension_views_.begin();
+  for (ExtensionRenderViews::iterator view = all_extension_views_.begin();
        view != all_extension_views_.end(); ++view) {
-    if ((*view)->site_instance() == site_instance)
-      result.insert(*view);
+    if (view->first->site_instance() == site_instance)
+      result.insert(view->first);
   }
 
   return result;
@@ -289,16 +290,53 @@ std::set<RenderViewHost*>
 void ExtensionProcessManager::RegisterRenderViewHost(
     RenderViewHost* render_view_host,
     const Extension* extension) {
-  all_extension_views_.insert(render_view_host);
+  all_extension_views_[render_view_host] = content::VIEW_TYPE_INVALID;
 }
 
 void ExtensionProcessManager::UnregisterRenderViewHost(
     RenderViewHost* render_view_host) {
-  all_extension_views_.erase(render_view_host);
+  ExtensionRenderViews::iterator view =
+      all_extension_views_.find(render_view_host);
+  if (view == all_extension_views_.end())
+    return;
+
+  content::ViewType view_type = view->second;
+  all_extension_views_.erase(view);
+
+  // Keepalive count, balanced in UpdateRegisteredRenderView.
+  if (view_type != content::VIEW_TYPE_INVALID &&
+      view_type != chrome::VIEW_TYPE_EXTENSION_BACKGROUND_PAGE) {
+    const Extension* extension =
+        GetProfile()->GetExtensionService()->extensions()->GetByID(
+            render_view_host->site_instance()->GetSite().host());

[Yoyo Zhou, 2012/03/02 03:10:46]

Maybe a TODO: it seems like this could be a helper function to get an Extension
from a RenderViewHost. I think I've seen other places it could be used.

[Matt Perry, 2012/03/02 20:25:21]

Added a local helper.

+    if (extension)
+      DecrementLazyKeepaliveCount(extension);
+  }
 }
 
-SiteInstance* ExtensionProcessManager::GetSiteInstanceForURL(
-    const GURL& url) {
+void ExtensionProcessManager::UpdateRegisteredRenderView(
+    RenderViewHost* render_view_host) {
+  ExtensionRenderViews::iterator view =
+      all_extension_views_.find(render_view_host);
+  if (view == all_extension_views_.end())
+    return;
+
+  view->second = render_view_host->delegate()->GetRenderViewType();
+
+  // Keep the lazy background page alive as long as any non-background-page
+  // extension views are visible. Keepalive count balanced in
+  // UnregisterRenderViewHost.
+  if (view->second != content::VIEW_TYPE_INVALID &&
+      view->second != chrome::VIEW_TYPE_EXTENSION_BACKGROUND_PAGE) {
+    const Extension* extension =
+        GetProfile()->GetExtensionService()->extensions()->GetByID(
+            render_view_host->site_instance()->GetSite().host());
+    if (extension)
+      IncrementLazyKeepaliveCount(extension);
+  }
+}
+
+SiteInstance* ExtensionProcessManager::GetSiteInstanceForURL(const GURL& url) {
   return site_instance_->GetRelatedSiteInstance(url);
 }
 
@@ -318,7 +356,6 @@ int ExtensionProcessManager::IncrementLazyKeepaliveCount(
   if (extension->background_page_persists())
     return 0;
 
-  // TODO(mpcomplete): Handle visible views changing.
   int& count = ::GetLazyKeepaliveCount(GetProfile(), extension);
   if (++count == 1)
     OnLazyBackgroundPageActive(extension->id());
@@ -342,7 +379,7 @@ int ExtensionProcessManager::DecrementLazyKeepaliveCount(
 void ExtensionProcessManager::OnLazyBackgroundPageIdle(
     const std::string& extension_id) {
   ExtensionHost* host = GetBackgroundHostForExtension(extension_id);
-  if (host && !HasVisibleViews(extension_id))
+  if (host)
     host->SendShouldClose();
 }
 
@@ -360,19 +397,20 @@ void ExtensionProcessManager::OnShouldCloseAck(
     host->OnShouldCloseAck(sequence_id);
 }
 
-bool ExtensionProcessManager::HasVisibleViews(const std::string& extension_id) {
-  const std::set<RenderViewHost*>& views =
-      GetRenderViewHostsForExtension(extension_id);
-  for (std::set<RenderViewHost*>::const_iterator it = views.begin();
-       it != views.end(); ++it) {
-    const RenderViewHost* host = *it;
-    if (host->site_instance()->GetSite().host() == extension_id &&
-        host->delegate()->GetRenderViewType() !=
-        chrome::VIEW_TYPE_EXTENSION_BACKGROUND_PAGE) {
-      return true;
-    }
-  }
-  return false;
+void ExtensionProcessManager::OnNetworkRequestStarted(
+    RenderViewHost* render_view_host) {
+  ExtensionHost* host = GetBackgroundHostForExtension(
+      render_view_host->site_instance()->GetSite().host());
+  if (host)
+    IncrementLazyKeepaliveCount(host->extension());
+}
+
+void ExtensionProcessManager::OnNetworkRequestDone(
+    RenderViewHost* render_view_host) {
+  ExtensionHost* host = GetBackgroundHostForExtension(
+      render_view_host->site_instance()->GetSite().host());
+  if (host)
+    DecrementLazyKeepaliveCount(host->extension());
 }
 
 void ExtensionProcessManager::Observe(
@@ -428,6 +466,13 @@ void ExtensionProcessManager::Observe(
       break;
     }
 
+    case content::NOTIFICATION_WEB_CONTENTS_CONNECTED: {

[Yoyo Zhou, 2012/03/02 03:10:46]

Naively, it seems like this should be balanced with WEB_CONTENTS_DISCONNECTED.

[Matt Perry, 2012/03/02 20:25:21]

No need - UnregisterRenderViewHost cleans up our RenderViewHost list. All this
notification does is give us a convenient time after RegisterRenderViewHost
where the view_type is available (view_type never changes).

+      content::WebContents* contents =
+          content::Source<content::WebContents>(source).ptr();
+      UpdateRegisteredRenderView(contents->GetRenderViewHost());
+      break;
+    }
+
     case content::NOTIFICATION_APP_TERMINATING: {
       // Close background hosts when the last browser is closed so that they
       // have time to shutdown various objects on different threads. Our