Close Lazy Background Page after event dispatch

BUG=81752
TEST=

Review URL: http://codereview.chromium.org/8230035

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@106573 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 196 insertions, 30 deletions

diff --git a/chrome/browser/extensions/alert_apitest.cc b/chrome/browser/extensions/alert_apitest.cc
index 24fe043..fe026b6 100644
--- a/chrome/browser/extensions/alert_apitest.cc
+++ b/chrome/browser/extensions/alert_apitest.cc
@@ -9,6 +9,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/app_modal_dialogs/app_modal_dialog.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/common/extensions/extension.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/browser/renderer_host/render_view_host.h"
 
@@ -17,7 +18,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, AlertBasic) {
 
   const Extension* extension = GetSingleLoadedExtension();
   ExtensionHost* host = browser()->profile()->GetExtensionProcessManager()->
-      GetBackgroundHostForExtension(extension);
+      GetBackgroundHostForExtension(extension->id());
   ASSERT_TRUE(host);
   host->render_view_host()->ExecuteJavascriptInWebFrame(string16(),
       ASCIIToUTF16("alert('This should not crash.');"));
diff --git a/chrome/browser/extensions/browser_action_apitest.cc b/chrome/browser/extensions/browser_action_apitest.cc
index bf6fbf1..39b6f9c 100644
--- a/chrome/browser/extensions/browser_action_apitest.cc
+++ b/chrome/browser/extensions/browser_action_apitest.cc
@@ -392,7 +392,7 @@ IN_PROC_BROWSER_TEST_F(BrowserActionApiTest, DISABLED_CloseBackgroundPage) {
   // There is a background page and a browser action with no badge text.
   ExtensionProcessManager* manager =
       browser()->profile()->GetExtensionProcessManager();
-  ASSERT_TRUE(manager->GetBackgroundHostForExtension(extension));
+  ASSERT_TRUE(manager->GetBackgroundHostForExtension(extension->id()));
   ExtensionAction* action = extension->browser_action();
   ASSERT_EQ("", action->GetBadgeText(ExtensionAction::kDefaultTabId));
 
@@ -409,6 +409,6 @@ IN_PROC_BROWSER_TEST_F(BrowserActionApiTest, DISABLED_CloseBackgroundPage) {
   // so we wait for the notification before checking that it's really gone
   // and the badge text has been set.
   host_destroyed_observer.Wait();
-  ASSERT_FALSE(manager->GetBackgroundHostForExtension(extension));
+  ASSERT_FALSE(manager->GetBackgroundHostForExtension(extension->id()));
   ASSERT_EQ("X", action->GetBadgeText(ExtensionAction::kDefaultTabId));
 }
diff --git a/chrome/browser/extensions/extension_crash_recovery_browsertest.cc b/chrome/browser/extensions/extension_crash_recovery_browsertest.cc
index a4cf4a9..3898d87 100644
--- a/chrome/browser/extensions/extension_crash_recovery_browsertest.cc
+++ b/chrome/browser/extensions/extension_crash_recovery_browsertest.cc
@@ -67,8 +67,8 @@ class ExtensionCrashRecoveryTest : public ExtensionBrowserTest {
         GetExtensionService()->extensions()->at(index);
     ASSERT_TRUE(extension);
     std::string extension_id(extension->id());
-    ExtensionHost* extension_host =
-        GetExtensionProcessManager()->GetBackgroundHostForExtension(extension);
+    ExtensionHost* extension_host = GetExtensionProcessManager()->
+        GetBackgroundHostForExtension(extension_id);
     ASSERT_TRUE(extension_host);
 
     RenderProcessHost* extension_rph =
@@ -76,8 +76,8 @@ class ExtensionCrashRecoveryTest : public ExtensionBrowserTest {
     base::KillProcess(extension_rph->GetHandle(), content::RESULT_CODE_KILLED,
                       false);
     ASSERT_TRUE(WaitForExtensionCrash(extension_id));
-    ASSERT_FALSE(
-        GetExtensionProcessManager()->GetBackgroundHostForExtension(extension));
+    ASSERT_FALSE(GetExtensionProcessManager()->
+                 GetBackgroundHostForExtension(extension_id));
   }
 
   void CheckExtensionConsistency(size_t index) {
@@ -85,8 +85,8 @@ class ExtensionCrashRecoveryTest : public ExtensionBrowserTest {
     const Extension* extension =
         GetExtensionService()->extensions()->at(index);
     ASSERT_TRUE(extension);
-    ExtensionHost* extension_host =
-        GetExtensionProcessManager()->GetBackgroundHostForExtension(extension);
+    ExtensionHost* extension_host = GetExtensionProcessManager()->
+        GetBackgroundHostForExtension(extension->id());
     ASSERT_TRUE(extension_host);
     ASSERT_TRUE(GetExtensionProcessManager()->HasExtensionHost(extension_host));
     ASSERT_TRUE(extension_host->IsRenderViewLive());
diff --git a/chrome/browser/extensions/extension_event_router.cc b/chrome/browser/extensions/extension_event_router.cc
index 74dd20a..8f18685 100644
--- a/chrome/browser/extensions/extension_event_router.cc
+++ b/chrome/browser/extensions/extension_event_router.cc
@@ -217,7 +217,7 @@ bool ExtensionEventRouter::CanDispatchEventNow(
       GetExtensionById(extension_id, false);  // exclude disabled extensions
   if (extension && extension->background_url().is_valid()) {
     ExtensionProcessManager* pm = profile_->GetExtensionProcessManager();
-    if (!pm->GetBackgroundHostForExtension(extension)) {
+    if (!pm->GetBackgroundHostForExtension(extension_id)) {
       pm->CreateBackgroundHost(extension, extension->background_url());
       return false;
     }
diff --git a/chrome/browser/extensions/extension_management_browsertest.cc b/chrome/browser/extensions/extension_management_browsertest.cc
index 1bae8ee..feddcdc 100644
--- a/chrome/browser/extensions/extension_management_browsertest.cc
+++ b/chrome/browser/extensions/extension_management_browsertest.cc
@@ -40,7 +40,8 @@ class ExtensionManagementTest : public ExtensionBrowserTest {
     // sync with the Extension.
     ExtensionProcessManager* manager = browser()->profile()->
         GetExtensionProcessManager();
-    ExtensionHost* ext_host = manager->GetBackgroundHostForExtension(extension);
+    ExtensionHost* ext_host =
+        manager->GetBackgroundHostForExtension(extension->id());
     EXPECT_TRUE(ext_host);
     if (!ext_host)
       return false;
@@ -205,26 +206,26 @@ IN_PROC_BROWSER_TEST_F(ExtensionManagementTest, DisableEnable) {
   const size_t size_before = service->extensions()->size();
 
   // Load an extension, expect the background page to be available.
+  std::string extension_id = "bjafgdebaacbbbecmhlhpofkepfkgcpa";
   ASSERT_TRUE(LoadExtension(
       test_data_dir_.AppendASCII("good").AppendASCII("Extensions")
-                    .AppendASCII("bjafgdebaacbbbecmhlhpofkepfkgcpa")
+                    .AppendASCII(extension_id)
                     .AppendASCII("1.0")));
   ASSERT_EQ(size_before + 1, service->extensions()->size());
   EXPECT_EQ(0u, service->disabled_extensions()->size());
-  const Extension* extension = service->extensions()->at(size_before);
-  EXPECT_TRUE(manager->GetBackgroundHostForExtension(extension));
+  EXPECT_TRUE(manager->GetBackgroundHostForExtension(extension_id));
 
   // After disabling, the background page should go away.
-  service->DisableExtension("bjafgdebaacbbbecmhlhpofkepfkgcpa");
+  service->DisableExtension(extension_id);
   EXPECT_EQ(size_before, service->extensions()->size());
   EXPECT_EQ(1u, service->disabled_extensions()->size());
-  EXPECT_FALSE(manager->GetBackgroundHostForExtension(extension));
+  EXPECT_FALSE(manager->GetBackgroundHostForExtension(extension_id));
 
   // And bring it back.
-  service->EnableExtension("bjafgdebaacbbbecmhlhpofkepfkgcpa");
+  service->EnableExtension(extension_id);
   EXPECT_EQ(size_before + 1, service->extensions()->size());
   EXPECT_EQ(0u, service->disabled_extensions()->size());
-  EXPECT_TRUE(manager->GetBackgroundHostForExtension(extension));
+  EXPECT_TRUE(manager->GetBackgroundHostForExtension(extension_id));
 }
 
 // Used for testing notifications sent during extension updates.
diff --git a/chrome/browser/extensions/extension_process_manager.cc b/chrome/browser/extensions/extension_process_manager.cc
index df68f78..c2a61d8 100644
--- a/chrome/browser/extensions/extension_process_manager.cc
+++ b/chrome/browser/extensions/extension_process_manager.cc
@@ -191,7 +191,7 @@ void ExtensionProcessManager::CreateBackgroundHost(
     return;
 
   // Don't create multiple background hosts for an extension.
-  if (GetBackgroundHostForExtension(extension))
+  if (GetBackgroundHostForExtension(extension->id()))
     return;
 
   ExtensionHost* host =
@@ -227,14 +227,15 @@ void ExtensionProcessManager::OpenOptionsPage(const Extension* extension,
 }
 
 ExtensionHost* ExtensionProcessManager::GetBackgroundHostForExtension(
-    const Extension* extension) {
+    const std::string& extension_id) {
   for (ExtensionHostSet::iterator iter = background_hosts_.begin();
        iter != background_hosts_.end(); ++iter) {
     ExtensionHost* host = *iter;
-    if (host->extension() == extension)
+    if (host->extension_id() == extension_id)
       return host;
   }
   return NULL;
+
 }
 
 std::set<RenderViewHost*>
@@ -417,6 +418,27 @@ bool ExtensionProcessManager::HasExtensionHost(ExtensionHost* host) const {
   return all_hosts_.find(host) != all_hosts_.end();
 }
 
+void ExtensionProcessManager::OnExtensionIdle(const std::string& extension_id) {
+  ExtensionHost* host = GetBackgroundHostForExtension(extension_id);
+  if (host && !HasVisibleViews(extension_id))
+    CloseBackgroundHost(host);
+}
+
+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()->site().host() == extension_id &&
+        host->delegate()->GetRenderViewType() !=
+        chrome::VIEW_TYPE_EXTENSION_BACKGROUND_PAGE) {
+      return true;
+    }
+  }
+  return false;
+}
+
 void ExtensionProcessManager::Observe(
     int type,
     const content::NotificationSource& source,
@@ -447,9 +469,7 @@ void ExtensionProcessManager::Observe(
            iter != background_hosts_.end(); ++iter) {
         ExtensionHost* host = *iter;
         if (host->extension_id() == extension->id()) {
-          delete host;
-          // |host| should deregister itself from our structures.
-          DCHECK(background_hosts_.find(host) == background_hosts_.end());
+          CloseBackgroundHost(host);
           break;
         }
       }
@@ -473,9 +493,7 @@ void ExtensionProcessManager::Observe(
       ExtensionHost* host = content::Details<ExtensionHost>(details).ptr();
       if (host->extension_host_type() ==
           chrome::VIEW_TYPE_EXTENSION_BACKGROUND_PAGE) {
-        delete host;
-        // |host| should deregister itself from our structures.
-        CHECK(background_hosts_.find(host) == background_hosts_.end());
+        CloseBackgroundHost(host);
       }
       break;
     }
@@ -506,6 +524,14 @@ void ExtensionProcessManager::OnExtensionHostCreated(ExtensionHost* host,
       content::Details<ExtensionHost>(host));
 }
 
+void ExtensionProcessManager::CloseBackgroundHost(ExtensionHost* host) {
+  CHECK(host->extension_host_type() ==
+        chrome::VIEW_TYPE_EXTENSION_BACKGROUND_PAGE);
+  delete host;
+  // |host| should deregister itself from our structures.
+  CHECK(background_hosts_.find(host) == background_hosts_.end());
+}
+
 void ExtensionProcessManager::CloseBackgroundHosts() {
   for (ExtensionHostSet::iterator iter = background_hosts_.begin();
        iter != background_hosts_.end(); ) {
diff --git a/chrome/browser/extensions/extension_process_manager.h b/chrome/browser/extensions/extension_process_manager.h
index 6ecd784..41d0bbb 100644
--- a/chrome/browser/extensions/extension_process_manager.h
+++ b/chrome/browser/extensions/extension_process_manager.h
@@ -65,7 +65,7 @@ class ExtensionProcessManager : public content::NotificationObserver {
 
   // Gets the ExtensionHost for the background page for an extension, or NULL if
   // the extension isn't running or doesn't have a background page.
-  ExtensionHost* GetBackgroundHostForExtension(const Extension* extension);
+  ExtensionHost* GetBackgroundHostForExtension(const std::string& extension_id);
 
   // Returns the SiteInstance that the given URL belongs to.
   virtual SiteInstance* GetSiteInstanceForURL(const GURL& url);
@@ -107,6 +107,10 @@ class ExtensionProcessManager : public content::NotificationObserver {
   // Returns true if |host| is managed by this process manager.
   bool HasExtensionHost(ExtensionHost* host) const;
 
+  // Called when the render reports that the extension is idle (only if
+  // lazy background pages are enabled).
+  void OnExtensionIdle(const std::string& extension_id);
+
   typedef std::set<ExtensionHost*> ExtensionHostSet;
   typedef ExtensionHostSet::const_iterator const_iterator;
   const_iterator begin() const { return all_hosts_.begin(); }
@@ -158,6 +162,13 @@ class ExtensionProcessManager : public content::NotificationObserver {
   typedef std::set<RenderViewHost*> RenderViewHostSet;
   RenderViewHostSet all_extension_views_;
 
+ private:
+  // Close the given |host| iff it's a background page.
+  void CloseBackgroundHost(ExtensionHost* host);
+
+  // Excludes background page.
+  bool HasVisibleViews(const std::string& extension_id);
+
   DISALLOW_COPY_AND_ASSIGN(ExtensionProcessManager);
 };
 
diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc
index 2e990da..7c71eb1 100644
--- a/chrome/browser/extensions/extension_service.cc
+++ b/chrome/browser/extensions/extension_service.cc
@@ -838,8 +838,7 @@ void ExtensionService::ReloadExtension(const std::string& extension_id) {
     // the inspector and hang onto a cookie for it, so that we can reattach
     // later.
     ExtensionProcessManager* manager = profile_->GetExtensionProcessManager();
-    ExtensionHost* host = manager->GetBackgroundHostForExtension(
-        current_extension);
+    ExtensionHost* host = manager->GetBackgroundHostForExtension(extension_id);
     if (host) {
       // Look for an open inspector for the background page.
       int devtools_cookie = DevToolsManager::GetInstance()->DetachClientHost(
diff --git a/chrome/browser/extensions/lazy_background_page_apitest.cc b/chrome/browser/extensions/lazy_background_page_apitest.cc
new file mode 100644
index 0000000..db3e7c5
--- /dev/null
+++ b/chrome/browser/extensions/lazy_background_page_apitest.cc
@@ -0,0 +1,128 @@
+// Copyright (c) 2011 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.
+
+#include "base/command_line.h"
+#include "base/file_path.h"
+#include "chrome/browser/extensions/browser_action_test_util.h"
+#include "chrome/browser/extensions/extension_apitest.h"
+#include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_window.h"
+#include "chrome/browser/ui/omnibox/location_bar.h"
+#include "chrome/common/chrome_switches.h"
+#include "chrome/common/extensions/extension.h"
+#include "chrome/test/base/ui_test_utils.h"
+#include "content/browser/tab_contents/tab_contents.h"
+#include "content/public/browser/notification_service.h"
+
+#include "googleurl/src/gurl.h"
+
+class LazyBackgroundPageApiTest : public ExtensionApiTest {
+public:
+  void SetUpCommandLine(CommandLine* command_line) {
+    ExtensionApiTest::SetUpCommandLine(command_line);
+    command_line->AppendSwitch(switches::kEnableLazyBackgroundPages);
+  }
+};
+
+IN_PROC_BROWSER_TEST_F(LazyBackgroundPageApiTest, BrowserActionCreateTab) {
+  ASSERT_TRUE(CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kEnableLazyBackgroundPages));
+
+  FilePath extdir = test_data_dir_.AppendASCII("lazy_background_page").
+      AppendASCII("browser_action_create_tab");
+  ASSERT_TRUE(LoadExtension(extdir));
+
+  // Lazy Background Page doesn't exist yet.
+  ExtensionProcessManager* pm =
+      browser()->profile()->GetExtensionProcessManager();
+  EXPECT_FALSE(pm->GetBackgroundHostForExtension(last_loaded_extension_id_));
+  int num_tabs_before = browser()->tab_count();
+
+  // Observe background page being created and closed after
+  // the browser action is clicked.
+  ui_test_utils::WindowedNotificationObserver bg_pg_created(
+      chrome::NOTIFICATION_EXTENSION_BACKGROUND_PAGE_READY,
+      content::NotificationService::AllSources());
+  ui_test_utils::WindowedNotificationObserver bg_pg_closed(
+      chrome::NOTIFICATION_EXTENSION_HOST_DESTROYED,
+      content::NotificationService::AllSources());
+  BrowserActionTestUtil(browser()).Press(0);
+  bg_pg_created.Wait();
+  bg_pg_closed.Wait();
+
+  // Background page created a new tab before it closed.
+  EXPECT_FALSE(pm->GetBackgroundHostForExtension(last_loaded_extension_id_));
+  EXPECT_EQ(num_tabs_before + 1, browser()->tab_count());
+  EXPECT_EQ("chrome://extensions/",
+            browser()->GetSelectedTabContents()->GetURL().spec());
+}
+
+IN_PROC_BROWSER_TEST_F(LazyBackgroundPageApiTest,
+                       BrowserActionCreateTabAfterCallback) {
+  ASSERT_TRUE(CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kEnableLazyBackgroundPages));
+
+  FilePath extdir = test_data_dir_.AppendASCII("lazy_background_page").
+      AppendASCII("browser_action_with_callback");
+  ASSERT_TRUE(LoadExtension(extdir));
+
+  // Lazy Background Page doesn't exist yet.
+  ExtensionProcessManager* pm =
+      browser()->profile()->GetExtensionProcessManager();
+  EXPECT_FALSE(pm->GetBackgroundHostForExtension(last_loaded_extension_id_));
+  int num_tabs_before = browser()->tab_count();
+
+  // Observe background page being created and closed after
+  // the browser action is clicked.
+  ui_test_utils::WindowedNotificationObserver bg_pg_created(
+      chrome::NOTIFICATION_EXTENSION_BACKGROUND_PAGE_READY,
+      content::NotificationService::AllSources());
+  ui_test_utils::WindowedNotificationObserver bg_pg_closed(
+      chrome::NOTIFICATION_EXTENSION_HOST_DESTROYED,
+      content::NotificationService::AllSources());
+  BrowserActionTestUtil(browser()).Press(0);
+  bg_pg_created.Wait();
+  bg_pg_closed.Wait();
+
+  // Background page is closed before it created a new tab.
+  // TODO(tessamac): Implement! Close background page after callback.
+  EXPECT_FALSE(pm->GetBackgroundHostForExtension(last_loaded_extension_id_));
+  EXPECT_EQ(num_tabs_before, browser()->tab_count());
+}
+
+IN_PROC_BROWSER_TEST_F(LazyBackgroundPageApiTest,
+                       BroadcastEvent) {
+  ASSERT_TRUE(CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kEnableLazyBackgroundPages));
+
+  FilePath extdir = test_data_dir_.AppendASCII("lazy_background_page").
+      AppendASCII("broadcast_event");
+  ASSERT_TRUE(LoadExtension(extdir));
+
+  // Lazy Background Page doesn't exist yet.
+  ExtensionProcessManager* pm =
+      browser()->profile()->GetExtensionProcessManager();
+  EXPECT_FALSE(pm->GetBackgroundHostForExtension(last_loaded_extension_id_));
+  int num_page_actions = browser()->window()->GetLocationBar()->
+      GetLocationBarForTesting()->PageActionVisibleCount();
+
+  // Open a tab to a URL that will trigger the page action to show.
+  // stegosaurus.html doesn't actually exist but that doesn't seem to matter.
+  GURL stego_url = GURL("stegosaurus.html");
+  ui_test_utils::NavigateToURL(browser(), stego_url);
+
+  // New page action is never shown because background page is never created.
+  // TODO(tessamac): Implement! Broadcast events (like tab updates) should
+  //                 cause lazy background pages to be created.
+  EXPECT_FALSE(pm->GetBackgroundHostForExtension(last_loaded_extension_id_));
+  EXPECT_EQ(num_page_actions,   // should be + 1
+            browser()->window()->GetLocationBar()->
+            GetLocationBarForTesting()->PageActionVisibleCount());
+}
+
+// TODO: background page with timer.
+// TODO: background page that interacts with popup.
+// TODO: background page with menu.