Only dispatch tab events when there is someone listening.
This should cut down on the number of occurrences of a crash that happens in JSON serialization.
BUG=25558,26169
Review URL: http://codereview.chromium.org/341029
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@30444 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/chrome/browser/extensions/extension_browser_event_router.cc b/chrome/browser/extensions/extension_browser_event_router.cc
index ad0d701..4e9bab30 100644
--- a/chrome/browser/extensions/extension_browser_event_router.cc
+++ b/chrome/browser/extensions/extension_browser_event_router.cc
@@ -111,6 +111,23 @@
BrowserList::AddObserver(this);
+ // Init() can happen after the browser is running, so catch up with any
+ // windows that already exist.
+ for (BrowserList::const_iterator iter = BrowserList::begin();
+ iter != BrowserList::end(); ++iter) {
+ RegisterForBrowserNotifications(*iter);
+
+ // Also catch up our internal bookkeeping of tab entries.
+ Browser* browser = *iter;
+ if (browser->tabstrip_model()) {
+ for (int i = 0; i < browser->tabstrip_model()->count(); ++i) {
+ TabContents* contents = browser->tabstrip_model()->GetTabContentsAt(i);
+ int tab_id = ExtensionTabUtil::GetTabId(contents);
+ tab_entries_[tab_id] = TabEntry(contents);
+ }
+ }
+ }
+
initialized_ = true;
}
@@ -118,12 +135,38 @@
: initialized_(false) { }
void ExtensionBrowserEventRouter::OnBrowserAdded(const Browser* browser) {
+ RegisterForBrowserNotifications(browser);
+}
+
+void ExtensionBrowserEventRouter::RegisterForBrowserNotifications(
+ const Browser* browser) {
// Start listening to TabStripModel events for this browser.
browser->tabstrip_model()->AddObserver(this);
- // The window isn't ready at this point, so we defer until it is.
+ // If this is a new window, it isn't ready at this point, so we register to be
+ // notified when it is. If this is an existing window, this is a no-op that we
+ // just do to reduce code complexity.
registrar_.Add(this, NotificationType::BROWSER_WINDOW_READY,
Source<const Browser>(browser));
+
+ if (browser->tabstrip_model()) {
+ for (int i = 0; i < browser->tabstrip_model()->count(); ++i)
+ RegisterForTabNotifications(
+ browser->tabstrip_model()->GetTabContentsAt(i));
+ }
+}
+
+void ExtensionBrowserEventRouter::RegisterForTabNotifications(
+ TabContents* contents) {
+ registrar_.Add(this, NotificationType::NAV_ENTRY_COMMITTED,
+ Source<NavigationController>(&contents->controller()));
+
+ // Observing TAB_CONTENTS_DESTROYED is necessary because it's
+ // possible for tabs to be created, detached and then destroyed without
+ // ever having been re-attached and closed. This happens in the case of
+ // a devtools TabContents that is opened in window, docked, then closed.
+ registrar_.Add(this, NotificationType::TAB_CONTENTS_DESTROYED,
+ Source<TabContents>(contents));
}
void ExtensionBrowserEventRouter::OnBrowserWindowReady(const Browser* browser) {
@@ -168,15 +211,7 @@
DispatchEvent(contents->profile(), events::kOnTabCreated, json_args);
- registrar_.Add(this, NotificationType::NAV_ENTRY_COMMITTED,
- Source<NavigationController>(&contents->controller()));
-
- // Observing TAB_CONTENTS_DESTROYED is necessary because it's
- // possible for tabs to be created, detached and then destroyed without
- // ever having been re-attached and closed. This happens in the case of
- // a devtools TabContents that is opened in window, docked, then closed.
- registrar_.Add(this, NotificationType::TAB_CONTENTS_DESTROYED,
- Source<TabContents>(contents));
+ RegisterForTabNotifications(contents);
}
void ExtensionBrowserEventRouter::TabInsertedAt(TabContents* contents,
diff --git a/chrome/browser/extensions/extension_browser_event_router.h b/chrome/browser/extensions/extension_browser_event_router.h
index 788229d5..49d3351 100644
--- a/chrome/browser/extensions/extension_browser_event_router.h
+++ b/chrome/browser/extensions/extension_browser_event_router.h
@@ -77,6 +77,14 @@
// and Observe/NAV_ENTRY_COMMITTED.
void TabUpdated(TabContents* contents, bool did_navigate);
+ // Register ourselves to receive the various notifications we are interested
+ // in for a browser.
+ void RegisterForBrowserNotifications(const Browser* browser);
+
+ // Register ourselves to receive the various notifications we are interested
+ // in for a tab.
+ void RegisterForTabNotifications(TabContents* contents);
+
ExtensionBrowserEventRouter();
friend struct DefaultSingletonTraits<ExtensionBrowserEventRouter>;
diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc
index ef6d9cd..d681e83d 100644
--- a/chrome/browser/extensions/extensions_service.cc
+++ b/chrome/browser/extensions/extensions_service.cc
@@ -127,9 +127,6 @@
// the first extension, because its members listen for loaded notifications.
g_browser_process->resource_dispatcher_host();
- // Start up the extension event routers.
- ExtensionBrowserEventRouter::GetInstance()->Init();
-
LoadAllExtensions();
// TODO(erikkay) this should probably be deferred to a future point
@@ -508,6 +505,12 @@
case Extension::ENABLED:
extensions_.push_back(scoped_extension.release());
+ // We delay starting up the browser event router until at least one
+ // extension that needs it is loaded.
+ if (extension->HasApiPermission(Extension::kTabPermission)) {
+ ExtensionBrowserEventRouter::GetInstance()->Init();
+ }
+
if (extension->location() != Extension::LOAD)
extension_prefs_->MigrateToPrefs(extension);
diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc
index ab2ff76..cb41400 100644
--- a/chrome/common/extensions/extension.cc
+++ b/chrome/common/extensions/extension.cc
@@ -99,9 +99,12 @@
const int Extension::kPageActionIconMaxSize = 19;
const int Extension::kBrowserActionIconMaxSize = 19;
+const char* Extension::kTabPermission = "tabs";
+const char* Extension::kBookmarkPermission = "bookmarks";
+
const char* Extension::kPermissionNames[] = {
- "tabs",
- "bookmarks",
+ Extension::kTabPermission,
+ Extension::kBookmarkPermission,
};
const size_t Extension::kNumPermissions =
arraysize(Extension::kPermissionNames);
diff --git a/chrome/common/extensions/extension.h b/chrome/common/extensions/extension.h
index 79ea12d7..cb5bd4b3 100644
--- a/chrome/common/extensions/extension.h
+++ b/chrome/common/extensions/extension.h
@@ -71,6 +71,9 @@
static const int kBrowserActionIconMaxSize;
// Each permission is a module that the extension is permitted to use.
+ static const char* kTabPermission;
+ static const char* kBookmarkPermission;
+
static const char* kPermissionNames[];
static const size_t kNumPermissions;
@@ -210,6 +213,12 @@
// specified URL.
bool CanAccessHost(const GURL& url) const;
+ // Returns true if the extension has the specified API permission.
+ bool HasApiPermission(const std::string& permission) const {
+ return std::find(api_permissions_.begin(), api_permissions_.end(),
+ permission) != api_permissions_.end();
+ }
+
// Returns the set of hosts that the extension effectively has access to. This
// is used in the permissions UI and is a combination of the hosts accessible
// through content scripts and the hosts accessible through XHR.
