Save memory in the downloads extension API.

chrome/browser/extensions/api/downloads/downloads_api.cc

Index: chrome/browser/extensions/api/downloads/downloads_api.cc
diff --git a/chrome/browser/extensions/api/downloads/downloads_api.cc b/chrome/browser/extensions/api/downloads/downloads_api.cc
index 9b0f0a6099c201a761380b2826fcedc31b773744..0fedd4626dfee1c3b3c25663ca4c20d3c17d2fb9 100644
--- a/chrome/browser/extensions/api/downloads/downloads_api.cc
+++ b/chrome/browser/extensions/api/downloads/downloads_api.cc
@@ -523,6 +523,10 @@ class ExtensionDownloadsEventRouterData : public base::SupportsUserData::Data {
         static_cast<ExtensionDownloadsEventRouterData*>(data);
   }
 
+  static void Remove(DownloadItem* download_item) {
+    download_item->RemoveUserData(kKey);
+  }
+
   explicit ExtensionDownloadsEventRouterData(
       DownloadItem* download_item,
       scoped_ptr<base::DictionaryValue> json_item)
@@ -1246,6 +1250,10 @@ void ExtensionDownloadsEventRouter::OnDeterminingFilename(
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   ExtensionDownloadsEventRouterData* data =
       ExtensionDownloadsEventRouterData::Get(item);
+  if (!data) {
+    no_change.Run();
+    return;
+  }
   data->ClearPendingDeterminers();
   data->set_filename_change_callbacks(no_change, change);
   bool any_determiners = false;
@@ -1313,26 +1321,40 @@ bool ExtensionDownloadsEventRouter::DetermineFilename(
 void ExtensionDownloadsEventRouter::OnListenerRemoved(
     const extensions::EventListenerInfo& details) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  if (details.event_name !=
-      extensions::event_names::kOnDownloadDeterminingFilename)
-    return;
   DownloadManager* manager = notifier_.GetManager();
   if (!manager)
     return;
+  bool determiner_removed = (
+      details.event_name ==
+      extensions::event_names::kOnDownloadDeterminingFilename);

[Matt Perry, 2013/04/17 02:01:28]

nit: might help to put
  namespace events = extensions::event_names
at the top to shorten all these refs.

[benjhayden, 2013/04/18 18:17:55]

Done.

+  extensions::EventRouter* router = extensions::ExtensionSystem::Get(profile_)->
+    event_router();

[Matt Perry, 2013/04/17 02:01:28]

Use 4 space indent when wrapping a line like this.

[benjhayden, 2013/04/18 18:17:55]

Done.

+  bool any_listeners =
+    router->HasEventListener(extensions::event_names::kOnDownloadChanged) ||
+    router->HasEventListener(
+        extensions::event_names::kOnDownloadDeterminingFilename);
+  if (!determiner_removed && any_listeners)
+    return;
   DownloadManager::DownloadVector items;
   manager->GetAllDownloads(&items);
-  // Notify any items that may be waiting for callbacks from this
-  // extension/determiner.
   for (DownloadManager::DownloadVector::const_iterator iter =
        items.begin();
        iter != items.end(); ++iter) {
     ExtensionDownloadsEventRouterData* data =
         ExtensionDownloadsEventRouterData::Get(*iter);
-    // This will almost always be a no-op, however, it is possible for an
-    // extension renderer to be unloaded while a download item is waiting
-    // for a determiner. In that case, the download item should proceed.
-    if (data)
+    if (!data)
+      continue;
+    if (determiner_removed) {
+      // Notify any items that may be waiting for callbacks from this
+      // extension/determiner.  This will almost always be a no-op, however, it
+      // is possible for an extension renderer to be unloaded while a download
+      // item is waiting for a determiner. In that case, the download item
+      // should proceed.
       data->DeterminerRemoved(details.extension_id);
+    }
+    if (!any_listeners) {
+      ExtensionDownloadsEventRouterData::Remove(*iter);
+    }
   }
 }
 
@@ -1345,29 +1367,49 @@ void ExtensionDownloadsEventRouter::OnDownloadCreated(
   if (download_item->IsTemporary())
     return;
 
+  extensions::EventRouter* router = extensions::ExtensionSystem::Get(profile_)->
+    event_router();
+  // Avoid allocating a bunch of memory in DownloadItemToJSON if it isn't going
+  // to be used.
+  if (!router ||
+      (!router->HasEventListener(extensions::event_names::kOnDownloadCreated) &&
+       !router->HasEventListener(extensions::event_names::kOnDownloadChanged) &&
+       !router->HasEventListener(
+         extensions::event_names::kOnDownloadDeterminingFilename))) {
+    return;
+  }
   scoped_ptr<base::DictionaryValue> json_item(
       DownloadItemToJSON(download_item, profile_->IsOffTheRecord()));
   DispatchEvent(extensions::event_names::kOnDownloadCreated,
                 true,
                 extensions::Event::WillDispatchCallback(),
                 json_item->DeepCopy());
-  if (!ExtensionDownloadsEventRouterData::Get(download_item))
+  if (!ExtensionDownloadsEventRouterData::Get(download_item) &&
+      (router->HasEventListener(extensions::event_names::kOnDownloadChanged) ||
+       router->HasEventListener(
+         extensions::event_names::kOnDownloadDeterminingFilename))) {
     new ExtensionDownloadsEventRouterData(download_item, json_item.Pass());
+  }
 }
 
 void ExtensionDownloadsEventRouter::OnDownloadUpdated(
     DownloadManager* manager, DownloadItem* download_item) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  if (download_item->IsTemporary())
-    return;
-
+  extensions::EventRouter* router = extensions::ExtensionSystem::Get(profile_)->
+    event_router();
   ExtensionDownloadsEventRouterData* data =
     ExtensionDownloadsEventRouterData::Get(download_item);
-  if (!data) {
-    // The download_item probably transitioned from temporary to not temporary.
-    OnDownloadCreated(manager, download_item);
+  if (download_item->IsTemporary() ||
+      !router->HasEventListener(extensions::event_names::kOnDownloadChanged)) {
     return;
   }
+  if (!data) {
+    // The download_item probably transitioned from temporary to not temporary,
+    // or else an event listener was added.
+    data = new ExtensionDownloadsEventRouterData(
+        download_item,
+        scoped_ptr<base::DictionaryValue>(new base::DictionaryValue()));
+  }
   scoped_ptr<base::DictionaryValue> new_json(DownloadItemToJSON(
       download_item, profile_->IsOffTheRecord()));
   scoped_ptr<base::DictionaryValue> delta(new base::DictionaryValue());