Reland DownloadItem::Observer::OnDownloadDestroyed() replaces DownloadItem::REMOVING

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

 // 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.
 
 #include "chrome/browser/extensions/api/downloads/downloads_api.h"
 
 #include <algorithm>
 #include <cctype>
 #include <iterator>
 #include <set>
@@ skipping 119 matching lines @@
 };
 COMPILE_ASSERT(arraysize(kDangerStrings) == content::DOWNLOAD_DANGER_TYPE_MAX,
                download_danger_type_enum_changed);
 
 // Note: Any change to the state strings, should be accompanied by a
 // corresponding change to downloads.json.
 const char* kStateStrings[] = {
   kStateInProgress,
   kStateComplete,
   kStateInterrupted,
-  NULL,
   kStateInterrupted,
 };
 COMPILE_ASSERT(arraysize(kStateStrings) == DownloadItem::MAX_DOWNLOAD_STATE,
                download_item_state_enum_changed);
 
 const char* DangerString(content::DownloadDangerType danger) {
   DCHECK(danger >= 0);
   DCHECK(danger < static_cast<content::DownloadDangerType>(
       arraysize(kDangerStrings)));
   return kDangerStrings[danger];
 }
 
 content::DownloadDangerType DangerEnumFromString(const std::string& danger) {
   for (size_t i = 0; i < arraysize(kDangerStrings); ++i) {
     if (danger == kDangerStrings[i])
       return static_cast<content::DownloadDangerType>(i);
   }
   return content::DOWNLOAD_DANGER_TYPE_MAX;
 }
 
 const char* StateString(DownloadItem::DownloadState state) {
   DCHECK(state >= 0);
   DCHECK(state < static_cast<DownloadItem::DownloadState>(
       arraysize(kStateStrings)));
-  DCHECK(state != DownloadItem::REMOVING);
   return kStateStrings[state];
 }
 
 DownloadItem::DownloadState StateEnumFromString(const std::string& state) {
   for (size_t i = 0; i < arraysize(kStateStrings); ++i) {
     if ((kStateStrings[i] != NULL) && (state == kStateStrings[i]))
       return static_cast<DownloadItem::DownloadState>(i);
   }
   return DownloadItem::MAX_DOWNLOAD_STATE;
 }
@@ skipping 766 matching lines @@
 ExtensionDownloadsEventRouter::OnChangedStat::OnChangedStat()
   : fires(0),
     total(0) {
 }
 
 ExtensionDownloadsEventRouter::OnChangedStat::~OnChangedStat() {
   if (total > 0)
     UMA_HISTOGRAM_PERCENTAGE("Download.OnChanged", (fires * 100 / total));
 }
 
+void ExtensionDownloadsEventRouter::OnDownloadDestroyed(DownloadItem* item) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  if (!profile_)
+    return;

[Randy Smith (Not in Mondays), 2012/07/11 17:55:37]

If we're getting notification and the download is being destroyed, we have to
get rid of our pointer to it, whether there's a profile around or not.

[benjhayden, 2012/07/13 20:03:17]

Done.

+  int download_id = item->GetId();
+  downloads_.erase(download_id);
+  item->RemoveObserver(this);
+  delete item_jsons_[download_id];
+  item_jsons_.erase(download_id);
+  delete on_changed_stats_[download_id];
+  on_changed_stats_.erase(download_id);
+}
+
+void ExtensionDownloadsEventRouter::OnDownloadRemoved(DownloadItem* item) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  if (!profile_)
+    return;
+  int download_id = item->GetId();
+  DispatchEvent(extension_event_names::kOnDownloadErased,
+                base::Value::CreateIntegerValue(download_id));
+}
+
 void ExtensionDownloadsEventRouter::OnDownloadUpdated(DownloadItem* item) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  if (!profile_)
+    return;
   int download_id = item->GetId();
-  if (item->GetState() == DownloadItem::REMOVING) {
-    // The REMOVING state indicates that this item is being erased.
-    // Let's unregister as an observer so that we don't see any more updates
-    // from it, dispatch the onErased event, and remove its json and is
-    // OnChangedStat from our maps.
-    downloads_.erase(download_id);
-    item->RemoveObserver(this);
-    DispatchEvent(extension_event_names::kOnDownloadErased,
-                  base::Value::CreateIntegerValue(download_id));
-    delete item_jsons_[download_id];
-    item_jsons_.erase(download_id);
-    delete on_changed_stats_[download_id];
-    on_changed_stats_.erase(download_id);
-    return;
-  }
 
   base::DictionaryValue* old_json = item_jsons_[download_id];
   scoped_ptr<base::DictionaryValue> new_json(DownloadItemToJSON(item));
   scoped_ptr<base::DictionaryValue> delta(new base::DictionaryValue());
   delta->SetInteger(kIdKey, download_id);
   std::set<std::string> new_fields;
   bool changed = false;
 
   // For each field in the new json representation of the item except the
   // bytesReceived field, if the field has changed from the previous old json,
@@ skipping 28 matching lines @@
   // Update the OnChangedStat and dispatch the event if something significant
   // changed. Replace the stored json with the new json.
   ++(on_changed_stats_[download_id]->total);
   if (changed) {
     DispatchEvent(extension_event_names::kOnDownloadChanged, delta.release());
     ++(on_changed_stats_[download_id]->fires);
   }
   item_jsons_[download_id]->Swap(new_json.get());
 }
 
-void ExtensionDownloadsEventRouter::OnDownloadOpened(DownloadItem* item) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-}
-
 void ExtensionDownloadsEventRouter::ModelChanged(DownloadManager* manager) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(manager_ == manager);
   typedef std::set<int> DownloadIdSet;
 
   // Get all the download items.
   DownloadManager::DownloadVector current_vec;
   manager_->SearchDownloads(string16(), &current_vec);
 
   // Populate set<>s of download item identifiers so that we can find
@@ skipping 28 matching lines @@
        iter != new_set.end(); ++iter) {
     scoped_ptr<base::DictionaryValue> item(
         DownloadItemToJSON(current_map[*iter]));
     DispatchEvent(extension_event_names::kOnDownloadCreated, item->DeepCopy());
     DCHECK(item_jsons_.find(*iter) == item_jsons_.end());
     on_changed_stats_[*iter] = new OnChangedStat();
     current_map[*iter]->AddObserver(this);
     item_jsons_[*iter] = item.release();
   }
   downloads_.swap(current_map);
-
-  // Dispatching onErased is handled in OnDownloadUpdated when an item
-  // transitions to the REMOVING state.
 }
 
 void ExtensionDownloadsEventRouter::ManagerGoingDown(
     DownloadManager* manager) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   manager_->RemoveObserver(this);
   manager_ = NULL;
 }
 
 void ExtensionDownloadsEventRouter::DispatchEvent(
@@ skipping 11 matching lines @@
       extensions::EventFilteringInfo());
 
   DownloadsNotificationSource notification_source;
   notification_source.event_name = event_name;
   notification_source.profile = profile_;
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_EXTENSION_DOWNLOADS_EVENT,
       content::Source<DownloadsNotificationSource>(&notification_source),
       content::Details<std::string>(&json_args));
 }