Persist lazy background event listeners to the Prefs file.

Also only start the lazy bg page on extension install (via dispatching the
onInstalled event).

BUG=81752
TEST=no

Review URL: https://chromiumcodereview.appspot.com/9383024

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

13 files changed, 236 insertions, 120 deletions

diff --git a/chrome/browser/extensions/component_loader_unittest.cc b/chrome/browser/extensions/component_loader_unittest.cc
index bf5bb49..c014fd1 100644
--- a/chrome/browser/extensions/component_loader_unittest.cc
+++ b/chrome/browser/extensions/component_loader_unittest.cc
@@ -28,10 +28,11 @@ class MockExtensionService : public TestExtensionService {
   MockExtensionService() : ready_(false), unloaded_count_(0) {
   }
 
-  virtual void AddExtension(const Extension* extension) OVERRIDE {
-    ASSERT_FALSE(extension_set_.Contains(extension->id()));
+  virtual bool AddExtension(const Extension* extension) OVERRIDE {
+    EXPECT_FALSE(extension_set_.Contains(extension->id()));
     // ExtensionService must become the owner of the extension object.
     extension_set_.Insert(extension);
+    return true;
   }
 
   virtual void UnloadExtension(
diff --git a/chrome/browser/extensions/extension_event_router.cc b/chrome/browser/extensions/extension_event_router.cc
index 8d0a8df..4c4b8f6 100644
--- a/chrome/browser/extensions/extension_event_router.cc
+++ b/chrome/browser/extensions/extension_event_router.cc
@@ -32,6 +32,7 @@ using extensions::ExtensionAPI;
 namespace {
 
 const char kDispatchEvent[] = "Event.dispatchJSON";
+const char kOnInstalledEvent[] = "experimental.extension.onInstalled";
 
 void NotifyEventListenerRemovedOnIOThread(
     void* profile,
@@ -43,15 +44,15 @@ void NotifyEventListenerRemovedOnIOThread(
 
 }  // namespace
 
-struct ExtensionEventRouter::EventListener {
+struct ExtensionEventRouter::ListenerProcess {
   content::RenderProcessHost* process;
   std::string extension_id;
 
-  EventListener(content::RenderProcessHost* process,
+  ListenerProcess(content::RenderProcessHost* process,
                 const std::string& extension_id)
       : process(process), extension_id(extension_id) {}
 
-  bool operator<(const EventListener& that) const {
+  bool operator<(const ListenerProcess& that) const {
     if (process < that.process)
       return true;
     if (process == that.process && extension_id < that.extension_id)
@@ -101,6 +102,12 @@ ExtensionEventRouter::ExtensionEventRouter(Profile* profile)
                  content::NotificationService::AllSources());
   registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_HOST_DID_STOP_LOADING,
                  content::Source<Profile>(profile_));
+  registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_LOADED,
+                 content::Source<Profile>(profile_));
+  registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_UNLOADED,
+                 content::Source<Profile>(profile_));
+  registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_INSTALLED,
+                 content::Source<Profile>(profile_));
   // TODO(tessamac): also get notified for background page crash/failure.
 }
 
@@ -110,7 +117,7 @@ void ExtensionEventRouter::AddEventListener(
     const std::string& event_name,
     content::RenderProcessHost* process,
     const std::string& extension_id) {
-  EventListener listener(process, extension_id);
+  ListenerProcess listener(process, extension_id);
   DCHECK_EQ(listeners_[event_name].count(listener), 0u) << event_name;
   listeners_[event_name].insert(listener);
 
@@ -128,7 +135,7 @@ void ExtensionEventRouter::RemoveEventListener(
     const std::string& event_name,
     content::RenderProcessHost* process,
     const std::string& extension_id) {
-  EventListener listener(process, extension_id);
+  ListenerProcess listener(process, extension_id);
   DCHECK_EQ(listeners_[event_name].count(listener), 1u) <<
       " PID=" << process->GetID() << " extension=" << extension_id <<
       " event=" << event_name;
@@ -155,31 +162,55 @@ void ExtensionEventRouter::RemoveEventListener(
 void ExtensionEventRouter::AddLazyEventListener(
     const std::string& event_name,
     const std::string& extension_id) {
-  EventListener lazy_listener(NULL, extension_id);
-  if (lazy_listeners_[event_name].count(lazy_listener) == 0)
-    lazy_listeners_[event_name].insert(lazy_listener);
+  ListenerProcess lazy_listener(NULL, extension_id);
+  bool is_new = lazy_listeners_[event_name].insert(lazy_listener).second;
+  if (is_new) {
+    ExtensionPrefs* prefs = profile_->GetExtensionService()->extension_prefs();
+    std::set<std::string> events = prefs->GetRegisteredEvents(extension_id);
+    bool prefs_is_new = events.insert(event_name).second;
+    if (prefs_is_new)
+      prefs->SetRegisteredEvents(extension_id, events);
+  }
 }
 
 void ExtensionEventRouter::RemoveLazyEventListener(
     const std::string& event_name,
     const std::string& extension_id) {
-  EventListener lazy_listener(NULL, extension_id);
-  lazy_listeners_[event_name].erase(lazy_listener);
+  ListenerProcess lazy_listener(NULL, extension_id);
+  bool did_exist = lazy_listeners_[event_name].erase(lazy_listener) > 0;
+  if (did_exist) {
+    ExtensionPrefs* prefs = profile_->GetExtensionService()->extension_prefs();
+    std::set<std::string> events = prefs->GetRegisteredEvents(extension_id);
+    bool prefs_did_exist = events.erase(event_name) > 0;
+    DCHECK(prefs_did_exist);
+    prefs->SetRegisteredEvents(extension_id, events);
+  }
 }
 
 bool ExtensionEventRouter::HasEventListener(const std::string& event_name) {
-  return (listeners_.find(event_name) != listeners_.end() &&
-          !listeners_[event_name].empty());
+  return (HasEventListenerImpl(listeners_, "", event_name) ||
+          HasEventListenerImpl(lazy_listeners_, "", event_name));
 }
 
 bool ExtensionEventRouter::ExtensionHasEventListener(
     const std::string& extension_id, const std::string& event_name) {
-  ListenerMap::iterator it = listeners_.find(event_name);
-  if (it == listeners_.end())
+  return (HasEventListenerImpl(listeners_, extension_id, event_name) ||
+          HasEventListenerImpl(lazy_listeners_, extension_id, event_name));
+}
+
+bool ExtensionEventRouter::HasEventListenerImpl(
+    const ListenerMap& listener_map,
+    const std::string& extension_id,
+    const std::string& event_name) {
+  ListenerMap::const_iterator it = listener_map.find(event_name);
+  if (it == listener_map.end())
     return false;
 
-  std::set<EventListener>& listeners = it->second;
-  for (std::set<EventListener>::iterator listener = listeners.begin();
+  const std::set<ListenerProcess>& listeners = it->second;
+  if (extension_id.empty())
+    return !listeners.empty();
+
+  for (std::set<ListenerProcess>::const_iterator listener = listeners.begin();
        listener != listeners.end(); ++listener) {
     if (listener->extension_id == extension_id)
       return true;
@@ -232,10 +263,8 @@ bool ExtensionEventRouter::CanDispatchEventNow(const Extension* extension) {
     // TODO(mpcomplete): this is incorrect. We need to check whether the page
     // has finished loading. If not, we can't dispatch the event (because the
     // listener hasn't been set up yet).
-    if (!pm->GetBackgroundHostForExtension(extension->id())) {
-      pm->CreateBackgroundHost(extension, extension->GetBackgroundURL());
+    if (!pm->GetBackgroundHostForExtension(extension->id()))
       return false;
-    }
   }
 
   return true;
@@ -263,11 +292,11 @@ void ExtensionEventRouter::DispatchEventImpl(
   if (it == listeners_.end())
     return;
 
-  std::set<EventListener>& listeners = it->second;
+  std::set<ListenerProcess>& listeners = it->second;
   ExtensionService* service = profile_->GetExtensionService();
 
   // Send the event only to renderers that are listening for it.
-  for (std::set<EventListener>::iterator listener = listeners.begin();
+  for (std::set<ListenerProcess>::iterator listener = listeners.begin();
        listener != listeners.end(); ++listener) {
     if (!extension_id.empty() && extension_id != listener->extension_id)
       continue;
@@ -317,13 +346,14 @@ void ExtensionEventRouter::LoadLazyBackgroundPagesForEvent(
     const std::string& extension_id,
     const linked_ptr<ExtensionEvent>& event) {
   ExtensionService* service = profile_->GetExtensionService();
+  ExtensionProcessManager* pm = profile_->GetExtensionProcessManager();
 
   ListenerMap::iterator it = lazy_listeners_.find(event->event_name);
   if (it == lazy_listeners_.end())
     return;
 
-  std::set<EventListener>& listeners = it->second;
-  for (std::set<EventListener>::iterator listener = listeners.begin();
+  std::set<ListenerProcess>& listeners = it->second;
+  for (std::set<ListenerProcess>::iterator listener = listeners.begin();
        listener != listeners.end(); ++listener) {
     if (!extension_id.empty() && extension_id != listener->extension_id)
       continue;
@@ -332,9 +362,8 @@ void ExtensionEventRouter::LoadLazyBackgroundPagesForEvent(
         listener->extension_id);
 
     if (extension && !CanDispatchEventNow(extension)) {
-      // TODO(mpcomplete): make sure Background Page notification doesn't
-      //                   happen before the event is added to the pending list.
       AppendEvent(extension->id(), event);
+      pm->CreateBackgroundHost(extension, extension->GetBackgroundURL());
     }
   }
 }
@@ -403,9 +432,10 @@ void ExtensionEventRouter::Observe(
       for (ListenerMap::iterator it = listeners_.begin();
            it != listeners_.end(); ) {
         ListenerMap::iterator current_it = it++;
-        for (std::set<EventListener>::iterator jt = current_it->second.begin();
+        for (std::set<ListenerProcess>::iterator jt =
+                 current_it->second.begin();
              jt != current_it->second.end(); ) {
-          std::set<EventListener>::iterator current_jt = jt++;
+          std::set<ListenerProcess>::iterator current_jt = jt++;
           if (current_jt->process == renderer) {
             RemoveEventListener(current_it->first,
                                 current_jt->process,
@@ -426,6 +456,41 @@ void ExtensionEventRouter::Observe(
       }
       break;
     }
+    case chrome::NOTIFICATION_EXTENSION_LOADED: {
+      // Add all registered lazy listeners to our cache.
+      const Extension* extension =
+          content::Details<const Extension>(details).ptr();
+      std::set<std::string> registered_events =
+          profile_->GetExtensionService()->extension_prefs()->
+              GetRegisteredEvents(extension->id());
+      ListenerProcess lazy_listener(NULL, extension->id());
+      for (std::set<std::string>::iterator it = registered_events.begin();
+           it != registered_events.end(); ++it) {
+        lazy_listeners_[*it].insert(lazy_listener);
+      }
+      break;
+    }
+    case chrome::NOTIFICATION_EXTENSION_UNLOADED: {
+      // Remove all registered lazy listeners from our cache.
+      UnloadedExtensionInfo* unloaded =
+          content::Details<UnloadedExtensionInfo>(details).ptr();
+      ListenerProcess lazy_listener(NULL, unloaded->extension->id());
+      for (ListenerMap::iterator it = lazy_listeners_.begin();
+           it != lazy_listeners_.end(); ++it) {
+        it->second.erase(lazy_listener);
+      }
+      break;
+    }
+    case chrome::NOTIFICATION_EXTENSION_INSTALLED: {
+      // Dispatch the onInstalled event.
+      const Extension* extension =
+          content::Details<const Extension>(details).ptr();
+      AddLazyEventListener(kOnInstalledEvent, extension->id());
+      DispatchEventToExtension(
+          extension->id(), kOnInstalledEvent, "[]", NULL, GURL());
+      break;
+    }
+
     // TODO(tessamac): if background page crashed/failed clear queue.
     default:
       NOTREACHED();
diff --git a/chrome/browser/extensions/extension_event_router.h b/chrome/browser/extensions/extension_event_router.h
index b142198..daef554 100644
--- a/chrome/browser/extensions/extension_event_router.h
+++ b/chrome/browser/extensions/extension_event_router.h
@@ -118,8 +118,8 @@ class ExtensionEventRouter : public content::NotificationObserver {
                          const linked_ptr<ExtensionEvent>& event,
                          bool was_pending);
 
-  // Ensures that all non-persistent background pages that are interested in the
-  // given event are loaded, and queues the event if the page is not ready yet.
+  // Ensures that all lazy background pages that are interested in the given
+  // event are loaded, and queues the event if the page is not ready yet.
   // If |extension_id| is non-empty, we load only that extension's page
   // (assuming it is interested in the event).
   void LoadLazyBackgroundPagesForEvent(
@@ -136,27 +136,37 @@ class ExtensionEventRouter : public content::NotificationObserver {
   void DispatchPendingEvents(const std::string& extension_id);
 
  private:
-  // An extension listening to an event.
-  struct EventListener;
+  // The extension and process that contains the event listener for a given
+  // event.
+  struct ListenerProcess;
+
+  // A map between an event name and a set of extensions that are listening
+  // to that event.
+  typedef std::map<std::string, std::set<ListenerProcess> > ListenerMap;
 
   virtual void Observe(int type,
                        const content::NotificationSource& source,
                        const content::NotificationDetails& details) OVERRIDE;
 
+  // Returns true if the given listener map contains a event listeners for
+  // the given event. If |extension_id| is non-empty, we also check that that
+  // extension is one of the listeners.
+  bool HasEventListenerImpl(const ListenerMap& listeners,
+                            const std::string& extension_id,
+                            const std::string& event_name);
+
   Profile* profile_;
 
   content::NotificationRegistrar registrar_;
 
   scoped_refptr<ExtensionDevToolsManager> extension_devtools_manager_;
 
-  // A map between an event name and a set of extensions that are listening
-  // to that event.
-  typedef std::map<std::string, std::set<EventListener> > ListenerMap;
+  // The list of active extension processes that are listening to events.
   ListenerMap listeners_;
 
-  // Keeps track of all the non-persistent background pages that are listening
-  // to events.
-  // TODO(mpcomplete): save to disk.
+  // The list of all the lazy (non-persistent) background pages that are
+  // listening to events. This is just a cache of the real list, which is
+  // stored on disk in the extension prefs.
   ListenerMap lazy_listeners_;
 
   // A map between an extension id and the queue of events pending
diff --git a/chrome/browser/extensions/extension_prefs.cc b/chrome/browser/extensions/extension_prefs.cc
index 121f405..63cbbec 100644
--- a/chrome/browser/extensions/extension_prefs.cc
+++ b/chrome/browser/extensions/extension_prefs.cc
@@ -157,6 +157,10 @@ const char kPrefContentSettings[] = "content_settings";
 // A preference that contains extension-set content settings.
 const char kPrefIncognitoContentSettings[] = "incognito_content_settings";
 
+// A list of event names that this extension has registered from its lazy
+// background page.
+const char kRegisteredEvents[] = "events";
+
 // Provider of write access to a dictionary storing extension prefs.
 class ScopedExtensionPrefUpdate : public DictionaryPrefUpdate {
  public:
@@ -894,6 +898,35 @@ void ExtensionPrefs::SetActivePermissions(
       extension_id, kPrefActivePermissions, permissions);
 }
 
+std::set<std::string> ExtensionPrefs::GetRegisteredEvents(
+    const std::string& extension_id) {
+  std::set<std::string> events;
+  const DictionaryValue* extension = GetExtensionPref(extension_id);
+  if (!extension)
+    return events;
+
+  ListValue* value = NULL;
+  if (!extension->GetList(kRegisteredEvents, &value))
+    return events;
+
+  for (size_t i = 0; i < value->GetSize(); ++i) {
+    std::string event;
+    if (value->GetString(i, &event))
+      events.insert(event);
+  }
+  return events;
+}
+
+void ExtensionPrefs::SetRegisteredEvents(
+    const std::string& extension_id, const std::set<std::string>& events) {
+  ListValue* value = new ListValue();
+  for (std::set<std::string>::const_iterator it = events.begin();
+       it != events.end(); ++it) {
+    value->Append(new StringValue(*it));
+  }
+  UpdateExtensionPref(extension_id, kRegisteredEvents, value);
+}
+
 bool ExtensionPrefs::IsIncognitoEnabled(const std::string& extension_id) {
   return ReadExtensionPrefBoolean(extension_id, kPrefIncognitoEnabled);
 }
diff --git a/chrome/browser/extensions/extension_prefs.h b/chrome/browser/extensions/extension_prefs.h
index 75d1e32..f5bb0834 100644
--- a/chrome/browser/extensions/extension_prefs.h
+++ b/chrome/browser/extensions/extension_prefs.h
@@ -236,6 +236,11 @@ class ExtensionPrefs : public ExtensionContentSettingsStore::Observer,
   void SetActivePermissions(const std::string& extension_id,
                             const ExtensionPermissionSet* permissions);
 
+  // Returns the list of events that the given extension has registered for.
+  std::set<std::string> GetRegisteredEvents(const std::string& extension_id);
+  void SetRegisteredEvents(const std::string& extension_id,
+                           const std::set<std::string>& events);
+
   // Returns true if the user enabled this extension to be loaded in incognito
   // mode.
   bool IsIncognitoEnabled(const std::string& extension_id);
diff --git a/chrome/browser/extensions/extension_process_manager.cc b/chrome/browser/extensions/extension_process_manager.cc
index f00ea56..9e824db 100644
--- a/chrome/browser/extensions/extension_process_manager.cc
+++ b/chrome/browser/extensions/extension_process_manager.cc
@@ -35,10 +35,6 @@ using content::OpenURLParams;
 using content::Referrer;
 using content::SiteInstance;
 
-namespace events {
-const char kOnInstalled[] = "experimental.extension.onInstalled";
-};  // namespace events
-
 namespace {
 
 // Incognito profiles use this process manager. It is mostly a shim that decides
@@ -71,14 +67,9 @@ class IncognitoExtensionProcessManager : public ExtensionProcessManager {
 
 static void CreateBackgroundHostForExtensionLoad(
     ExtensionProcessManager* manager, const Extension* extension) {
-  if (extension->has_background_page()) {
-    if (extension->background_page_persists()) {
-      manager->CreateBackgroundHost(extension, extension->GetBackgroundURL());
-    } else {
-      // TODO(mpcomplete): Only call this on install once we persist event
-      // registration. Also call this for regular background pages.
-      manager->DispatchExtensionInstalledEvent(extension);
-    }
+  if (extension->has_background_page() &&
+      extension->background_page_persists()) {
+    manager->CreateBackgroundHost(extension, extension->GetBackgroundURL());
   }
 }
 
@@ -418,14 +409,6 @@ void ExtensionProcessManager::CloseBackgroundHosts() {
   }
 }
 
-void ExtensionProcessManager::DispatchExtensionInstalledEvent(
-    const Extension* extension) {
-  ExtensionEventRouter* router = GetProfile()->GetExtensionEventRouter();
-  router->AddLazyEventListener(events::kOnInstalled, extension->id());
-  router->DispatchEventToExtension(
-      extension->id(), events::kOnInstalled, "[]", NULL, GURL());
-}
-
 //
 // IncognitoExtensionProcessManager
 //
diff --git a/chrome/browser/extensions/extension_process_manager.h b/chrome/browser/extensions/extension_process_manager.h
index 2c12779..41f7c60 100644
--- a/chrome/browser/extensions/extension_process_manager.h
+++ b/chrome/browser/extensions/extension_process_manager.h
@@ -93,10 +93,6 @@ class ExtensionProcessManager : public content::NotificationObserver {
   // lazy background pages are enabled).
   void OnExtensionIdle(const std::string& extension_id);
 
-  // Dispatch an event to the extension to let it know it was just installed.
-  // TODO(mpcomplete): Temporary until we implement persistent event registry.
-  void DispatchExtensionInstalledEvent(const Extension* extension);
-
   typedef std::set<ExtensionHost*> ExtensionHostSet;
   typedef ExtensionHostSet::const_iterator const_iterator;
   const_iterator begin() const { return all_hosts_.begin(); }
diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc
index 89abe7b..28a5363 100644
--- a/chrome/browser/extensions/extension_service.cc
+++ b/chrome/browser/extensions/extension_service.cc
@@ -2017,7 +2017,7 @@ void ExtensionService::OnLoadedInstalledExtensions() {
       content::NotificationService::NoDetails());
 }
 
-void ExtensionService::AddExtension(const Extension* extension) {
+bool ExtensionService::AddExtension(const Extension* extension) {
   // Ensure extension is deleted unless we transfer ownership.
   scoped_refptr<const Extension> scoped_extension(extension);
 
@@ -2028,7 +2028,7 @@ void ExtensionService::AddExtension(const Extension* extension) {
       !extension->is_theme() &&
       extension->location() != Extension::COMPONENT &&
       !Extension::IsExternalLocation(extension->location()))
-    return;
+    return false;
 
   SetBeingUpgraded(extension, false);
 
@@ -2057,7 +2057,9 @@ void ExtensionService::AddExtension(const Extension* extension) {
         content::Source<Profile>(profile_),
         content::Details<const Extension>(extension));
     SyncExtensionChangeIfNeeded(*extension);
-    return;
+    // Although the extension is disabled, we technically did succeed in adding
+    // it to the list of installed extensions.
+    return true;
   }
 
   // All apps that are displayed in the launcher are ordered by their ordinals
@@ -2068,6 +2070,8 @@ void ExtensionService::AddExtension(const Extension* extension) {
   extensions_.Insert(scoped_extension);
   SyncExtensionChangeIfNeeded(*extension);
   NotifyExtensionLoaded(extension);
+
+  return true;
 }
 
 void ExtensionService::InitializePermissions(const Extension* extension) {
@@ -2262,13 +2266,13 @@ void ExtensionService::OnExtensionInstalled(
   extension_prefs_->SetDelaysNetworkRequests(
       extension->id(), extension->ImplicitlyDelaysNetworkStartup());
 
-  content::NotificationService::current()->Notify(
-      chrome::NOTIFICATION_EXTENSION_INSTALLED,
-      content::Source<Profile>(profile_),
-      content::Details<const Extension>(extension));
-
   // Transfer ownership of |extension| to AddExtension.
-  AddExtension(scoped_extension);
+  if (AddExtension(scoped_extension)) {
+    content::NotificationService::current()->Notify(
+        chrome::NOTIFICATION_EXTENSION_INSTALLED,
+        content::Source<Profile>(profile_),
+        content::Details<const Extension>(extension));
+  }
 }
 
 const Extension* ExtensionService::GetExtensionByIdInternal(
diff --git a/chrome/browser/extensions/extension_service.h b/chrome/browser/extensions/extension_service.h
index 97fc245..eaea412 100644
--- a/chrome/browser/extensions/extension_service.h
+++ b/chrome/browser/extensions/extension_service.h
@@ -118,7 +118,8 @@ class ExtensionServiceInterface : public SyncableService {
   // themes sync to not use it directly.
   virtual void CheckForUpdatesSoon() = 0;
 
-  virtual void AddExtension(const Extension* extension) = 0;
+  // Returns true if the extension was successfully added.
+  virtual bool AddExtension(const Extension* extension) = 0;
 
   virtual void UnloadExtension(
       const std::string& extension_id,
@@ -395,7 +396,7 @@ class ExtensionService
   // Adds |extension| to this ExtensionService and notifies observers than an
   // extension has been loaded.  Called by the backend after an extension has
   // been loaded from a file and installed.
-  virtual void AddExtension(const Extension* extension) OVERRIDE;
+  virtual bool AddExtension(const Extension* extension) OVERRIDE;
 
   // Called by the backend when an extension has been installed.
   void OnExtensionInstalled(
diff --git a/chrome/browser/extensions/test_extension_service.cc b/chrome/browser/extensions/test_extension_service.cc
index e21ec88..bae3278 100644
--- a/chrome/browser/extensions/test_extension_service.cc
+++ b/chrome/browser/extensions/test_extension_service.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -95,8 +95,9 @@ bool TestExtensionService::is_ready() {
   return false;
 }
 
-void TestExtensionService::AddExtension(const Extension* extension) {
+bool TestExtensionService::AddExtension(const Extension* extension) {
   ADD_FAILURE();
+  return false;
 }
 
 void TestExtensionService::UnloadExtension(
diff --git a/chrome/browser/extensions/test_extension_service.h b/chrome/browser/extensions/test_extension_service.h
index fa3ee0d..0f6ffd0 100644
--- a/chrome/browser/extensions/test_extension_service.h
+++ b/chrome/browser/extensions/test_extension_service.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -55,7 +55,7 @@ class TestExtensionService : public ExtensionServiceInterface {
 
   virtual bool is_ready() OVERRIDE;
 
-  virtual void AddExtension(const Extension* extension) OVERRIDE;
+  virtual bool AddExtension(const Extension* extension) OVERRIDE;
 
   virtual void UnloadExtension(
       const std::string& extension_id,
diff --git a/chrome/renderer/extensions/event_bindings.cc b/chrome/renderer/extensions/event_bindings.cc
index 9f069b0..9bab21a 100644
--- a/chrome/renderer/extensions/event_bindings.cc
+++ b/chrome/renderer/extensions/event_bindings.cc
@@ -9,6 +9,7 @@
 #include "base/basictypes.h"
 #include "base/lazy_instance.h"
 #include "base/message_loop.h"
+#include "chrome/common/chrome_view_type.h"
 #include "chrome/common/extensions/extension_messages.h"
 #include "chrome/common/extensions/extension_set.h"
 #include "chrome/common/url_constants.h"
@@ -17,6 +18,7 @@
 #include "chrome/renderer/extensions/chrome_v8_extension.h"
 #include "chrome/renderer/extensions/event_bindings.h"
 #include "chrome/renderer/extensions/extension_dispatcher.h"
+#include "chrome/renderer/extensions/extension_helper.h"
 #include "chrome/renderer/extensions/schema_generated_bindings.h"
 #include "chrome/renderer/extensions/user_script_slave.h"
 #include "content/public/renderer/render_thread.h"
@@ -37,22 +39,6 @@ using content::RenderThread;
 
 namespace {
 
-// A map of event names to the number of listeners for that event. We notify
-// the browser about event listeners when we transition between 0 and 1.
-typedef std::map<std::string, int> EventListenerCounts;
-
-struct SingletonData {
-  // A map of extension IDs to listener counts for that extension.
-  std::map<std::string, EventListenerCounts> listener_counts_;
-};
-
-static base::LazyInstance<SingletonData> g_singleton_data =
-    LAZY_INSTANCE_INITIALIZER;
-
-static EventListenerCounts& GetListenerCounts(const std::string& extension_id) {
-  return g_singleton_data.Get().listener_counts_[extension_id];
-}
-
 class ExtensionImpl : public ChromeV8Extension {
  public:
   explicit ExtensionImpl(ExtensionDispatcher* dispatcher)
@@ -79,32 +65,31 @@ class ExtensionImpl : public ChromeV8Extension {
     DCHECK(args[0]->IsString() || args[0]->IsUndefined());
 
     if (args[0]->IsString()) {
-      ExtensionImpl* v8_extension = GetFromArguments<ExtensionImpl>(args);
+      ExtensionImpl* self = GetFromArguments<ExtensionImpl>(args);
       const ChromeV8ContextSet& context_set =
-          v8_extension->extension_dispatcher()->v8_context_set();
+          self->extension_dispatcher()->v8_context_set();
       ChromeV8Context* context = context_set.GetCurrent();
       CHECK(context);
-      EventListenerCounts& listener_counts =
-          GetListenerCounts(context->extension_id());
       std::string event_name(*v8::String::AsciiValue(args[0]));
 
-      if (!v8_extension->CheckCurrentContextAccessToExtensionAPI(event_name))
+      if (!self->CheckCurrentContextAccessToExtensionAPI(event_name))
         return v8::Undefined();
 
-      const ::Extension* extension = v8_extension->extension_dispatcher()->
-          extensions()->GetByID(context->extension_id());
-
+      EventListenerCounts& listener_counts =
+          self->listener_counts_[context->extension_id()];
       if (++listener_counts[event_name] == 1) {
         content::RenderThread::Get()->Send(
             new ExtensionHostMsg_AddListener(context->extension_id(),
                                              event_name));
-        // TODO(mpcomplete): restrict this to the bg page only.
-        // TODO(mpcomplete): figure out proper time to call RemoveLazyListener.
-        if (extension && !extension->background_page_persists()) {
-          content::RenderThread::Get()->Send(
-              new ExtensionHostMsg_AddLazyListener(context->extension_id(),
-                                                   event_name));
-        }
+      }
+
+      // This is called the first time the page has added a listener. Since
+      // the background page is the only lazy page, we know this is the first
+      // time this listener has been registered.
+      if (self->IsLazyBackgroundPage(context->extension_id())) {
+        content::RenderThread::Get()->Send(
+            new ExtensionHostMsg_AddLazyListener(context->extension_id(),
+                                                 event_name));
       }
     }
 
@@ -112,31 +97,63 @@ class ExtensionImpl : public ChromeV8Extension {
   }
 
   static v8::Handle<v8::Value> DetachEvent(const v8::Arguments& args) {
-    DCHECK(args.Length() == 1);
+    DCHECK(args.Length() == 2);
     // TODO(erikkay) should enforce that event name is a string in the bindings
     DCHECK(args[0]->IsString() || args[0]->IsUndefined());
 
-    if (args[0]->IsString()) {
-      ExtensionImpl* v8_extension = GetFromArguments<ExtensionImpl>(args);
+    if (args[0]->IsString() && args[1]->IsBoolean()) {
+      ExtensionImpl* self = GetFromArguments<ExtensionImpl>(args);
       const ChromeV8ContextSet& context_set =
-          v8_extension->extension_dispatcher()->v8_context_set();
+          self->extension_dispatcher()->v8_context_set();
       ChromeV8Context* context = context_set.GetCurrent();
       if (!context)
         return v8::Undefined();
 
       EventListenerCounts& listener_counts =
-          GetListenerCounts(context->extension_id());
+          self->listener_counts_[context->extension_id()];
       std::string event_name(*v8::String::AsciiValue(args[0]));
+      bool is_manual = args[1]->BooleanValue();
 
       if (--listener_counts[event_name] == 0) {
         content::RenderThread::Get()->Send(
             new ExtensionHostMsg_RemoveListener(context->extension_id(),
                                                 event_name));
       }
+
+      // DetachEvent is called when the last listener for the context is
+      // removed. If the context is the background page, and it removes the
+      // last listener manually, then we assume that it is no longer interested
+      // in being awakened for this event.
+      if (is_manual && self->IsLazyBackgroundPage(context->extension_id())) {
+        content::RenderThread::Get()->Send(
+            new ExtensionHostMsg_RemoveLazyListener(context->extension_id(),
+                                                    event_name));
+      }
     }
 
     return v8::Undefined();
   }
+
+ private:
+  // A map of event names to the number of contexts listening to that event.
+  // We notify the browser about event listeners when we transition between 0
+  // and 1.
+  typedef std::map<std::string, int> EventListenerCounts;
+
+  bool IsLazyBackgroundPage(const std::string& extension_id) {
+    content::RenderView* render_view = GetCurrentRenderView();
+    if (!render_view)
+      return false;
+
+    ExtensionHelper* helper = ExtensionHelper::Get(render_view);
+    const ::Extension* extension =
+        extension_dispatcher()->extensions()->GetByID(extension_id);
+    return (extension && !extension->background_page_persists() &&
+            helper->view_type() == chrome::VIEW_TYPE_EXTENSION_BACKGROUND_PAGE);
+  }
+
+  // A map of extension IDs to listener counts for that extension.
+  std::map<std::string, EventListenerCounts> listener_counts_;
 };
 
 }  // namespace
diff --git a/chrome/renderer/resources/extensions/event.js b/chrome/renderer/resources/extensions/event.js
index 6272067..2cf78a8 100644
--- a/chrome/renderer/resources/extensions/event.js
+++ b/chrome/renderer/resources/extensions/event.js
@@ -6,7 +6,7 @@ var chrome = chrome || {};
 (function () {
   native function GetChromeHidden();
   native function AttachEvent(eventName);
-  native function DetachEvent(eventName);
+  native function DetachEvent(eventName, manual);
   native function Print();
 
   var chromeHidden = GetChromeHidden();
@@ -157,7 +157,7 @@ var chrome = chrome || {};
 
     this.listeners_.splice(idx, 1);
     if (this.listeners_.length == 0) {
-      this.detach_();
+      this.detach_(true);
     }
   };
 
@@ -224,11 +224,11 @@ var chrome = chrome || {};
   };
 
   // Detaches this event object from its name.
-  chrome.Event.prototype.detach_ = function() {
+  chrome.Event.prototype.detach_ = function(manual) {
     var i = allAttachedEvents.indexOf(this);
     if (i >= 0)
       delete allAttachedEvents[i];
-    DetachEvent(this.eventName_);
+    DetachEvent(this.eventName_, manual);
     if (!this.eventName_)
       return;
 
@@ -243,7 +243,7 @@ var chrome = chrome || {};
   chrome.Event.prototype.destroy_ = function() {
     this.listeners_ = [];
     this.validate_ = [];
-    this.detach_();
+    this.detach_(false);
   };
 
   // Gets the declarative API object, or undefined if this extension doesn't
@@ -304,7 +304,7 @@ var chrome = chrome || {};
     for (var i = 0; i < allAttachedEvents.length; ++i) {
       var event = allAttachedEvents[i];
       if (event)
-        event.detach_();
+        event.detach_(false);
     }
   };