Implement chrome.experimental.downloads.onChanged

chrome/browser/extensions/extension_downloads_api.cc

Index: chrome/browser/extensions/extension_downloads_api.cc
diff --git a/chrome/browser/extensions/extension_downloads_api.cc b/chrome/browser/extensions/extension_downloads_api.cc
index 0e1470eb1c556a3fa32a6b071d38e7cda9143ced..5630b11850ae2df30ccdf98be381d416c754f774 100644
--- a/chrome/browser/extensions/extension_downloads_api.cc
+++ b/chrome/browser/extensions/extension_downloads_api.cc
@@ -33,13 +33,13 @@
 #include "chrome/browser/ui/browser_list.h"
 #include "content/browser/download/download_file_manager.h"
 #include "content/browser/download/download_item.h"
+#include "content/browser/download/download_stats.h"
 #include "content/browser/download/download_types.h"
 #include "content/browser/download/interrupt_reasons.h"
 #include "content/browser/renderer_host/render_process_host.h"
 #include "content/browser/renderer_host/render_view_host.h"
 #include "content/browser/renderer_host/resource_dispatcher_host.h"
 #include "net/http/http_util.h"
-#include "net/url_request/url_request.h"
 
 namespace constants = extension_downloads_api_constants;
 
@@ -322,7 +322,7 @@ bool DownloadsSetDestinationFunction::ParseArgs() {
   std::string path;
   EXTENSION_FUNCTION_VALIDATE(args_->GetInteger(0, &dl_id));
   EXTENSION_FUNCTION_VALIDATE(args_->GetString(1, &path));
-  VLOG(1) << __FUNCTION__ << " " << dl_id << " " << &path;
+  VLOG(1) << __FUNCTION__ << " " << dl_id << " " << path;
   error_ = constants::kNotImplemented;
   return false;
 }
@@ -412,6 +412,9 @@ base::DictionaryValue* DownloadItemToJSON(DownloadItem* item) {
   json->SetInteger(constants::kFileSizeKey, item->total_bytes());
   return json;
 }
+
+typedef std::set<int> DownloadIdSet;
+
 }  // anonymous namespace
 
 ExtensionDownloadsEventRouter::ExtensionDownloadsEventRouter(
@@ -420,16 +423,72 @@ ExtensionDownloadsEventRouter::ExtensionDownloadsEventRouter(
     manager_(
         profile ?
         DownloadServiceFactory::GetForProfile(profile)->GetDownloadManager() :
-        NULL) {
+        NULL),
+    delete_item_jsons_(&item_jsons_),
+    delete_on_changed_stats_(&on_changed_stats_) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DCHECK(profile_);
-  DCHECK(manager_);
   manager_->AddObserver(this);
 }
 
 ExtensionDownloadsEventRouter::~ExtensionDownloadsEventRouter() {
   if (manager_ != NULL)
     manager_->RemoveObserver(this);
+  for (ItemMap::const_iterator iter = downloads_.begin();
+       iter != downloads_.end(); ++iter) {
+    if (iter->second != NULL)
+      iter->second->RemoveObserver(this);
+  }
+}
+
+ExtensionDownloadsEventRouter::OnChangedStat::OnChangedStat()

[Randy Smith (Not in Mondays), 2011/10/18 21:15:32]

I don't have a strong feeling on this, but I think it'd be a better code
complexity/information tradeoff to just record the number of times you get an
OnDownloadUpdated and the number of times you fire an OnChanged event.  It
doesn't show you the distribution across downloads, but as mentioned, that's not
vital information; just the fact that we're getting the gratuitous notifications
is the primary information we need.

+  : fires(0), total(0) {
+}
+
+ExtensionDownloadsEventRouter::OnChangedStat::~OnChangedStat() {
+  download_stats::RecordOnChanged(fires * 100 / total);
+}
+
+void ExtensionDownloadsEventRouter::OnDownloadUpdated(DownloadItem* item) {
+  if (item->state() == DownloadItem::REMOVING) {
+    // Remove item from downloads_, but keep the id in downloads_ so that we
+    // notice its absence in ModelChanged() and fire an onErased.
+    ItemMap::const_iterator it = downloads_.find(item->id());
+    DCHECK(it != downloads_.end());
+    DCHECK(it->second == item);
+    downloads_[item->id()] = NULL;
+    item->RemoveObserver(this);
+    return;
+  }
+  base::DictionaryValue* current_json = item_jsons_[item->id()];
+  scoped_ptr<base::DictionaryValue> new_json(DownloadItemToJSON(item));
+  scoped_ptr<base::DictionaryValue> delta(new base::DictionaryValue());
+  delta->SetInteger(constants::kIdKey, item->id());
+  bool changed = false;
+  for (base::DictionaryValue::key_iterator key = new_json->begin_keys();
+       key != new_json->end_keys(); ++key) {
+    if (*key != constants::kBytesReceivedKey) {
+      base::Value* new_value = NULL;
+      base::Value* old_value = NULL;
+      if (new_json->Get(*key, &new_value) &&
+          (!current_json->HasKey(*key) ||
+          (current_json->Get(*key, &old_value) &&
+            !new_value->Equals(old_value)))) {
+        delta->Set(*key + ".old", (old_value ? old_value->DeepCopy()
+                                            : base::Value::CreateNullValue()));
+        delta->Set(*key + ".new", new_value->DeepCopy());
+        changed = true;
+      }
+    }
+  }
+  ++(on_changed_stats_[item->id()]->total);
+  if (changed) {
+    DispatchEvent(extension_event_names::kOnDownloadChanged, delta.release());
+    ++(on_changed_stats_[item->id()]->fires);
+  }
+  current_json->Swap(new_json.get());
+}
+
+void ExtensionDownloadsEventRouter::OnDownloadOpened(DownloadItem* item) {
 }
 
 void ExtensionDownloadsEventRouter::ModelChanged() {
@@ -438,7 +497,11 @@ void ExtensionDownloadsEventRouter::ModelChanged() {
     return;
   DownloadManager::DownloadVector current_vec;
   manager_->SearchDownloads(string16(), &current_vec);
-  DownloadIdSet current_set;
+  DownloadIdSet current_set, prev_set;
+  for (ItemMap::const_iterator iter = downloads_.begin();
+       iter != downloads_.end(); ++iter) {
+    prev_set.insert(iter->first);
+  }
   ItemMap current_map;
   for (DownloadManager::DownloadVector::const_iterator iter =
        current_vec.begin();
@@ -452,28 +515,36 @@ void ExtensionDownloadsEventRouter::ModelChanged() {
     current_set.insert(item_id);
     current_map[item_id] = item;
   }
-  DownloadIdSet new_set;  // current_set - downloads_;
-  DownloadIdSet erased_set;  // downloads_ - current_set;
+  DownloadIdSet new_set;  // current_set - prev_set;
+  DownloadIdSet erased_set;  // prev_set - current_set;
   std::insert_iterator<DownloadIdSet> new_insertor(new_set, new_set.begin());
   std::insert_iterator<DownloadIdSet> erased_insertor(
       erased_set, erased_set.begin());
   std::set_difference(current_set.begin(), current_set.end(),
-                      downloads_.begin(), downloads_.end(),
+                      prev_set.begin(), prev_set.end(),
                       new_insertor);
-  std::set_difference(downloads_.begin(), downloads_.end(),
+  std::set_difference(prev_set.begin(), prev_set.end(),
                       current_set.begin(), current_set.end(),
                       erased_insertor);
   for (DownloadIdSet::const_iterator iter = new_set.begin();
        iter != new_set.end(); ++iter) {
     DispatchEvent(extension_event_names::kOnDownloadCreated,
                   DownloadItemToJSON(current_map[*iter]));
+    DCHECK(item_jsons_.find(*iter) == item_jsons_.end());
+    on_changed_stats_[*iter] = new OnChangedStat();
+    item_jsons_[*iter] = DownloadItemToJSON(current_map[*iter]);
+    current_map[*iter]->AddObserver(this);
   }
   for (DownloadIdSet::const_iterator iter = erased_set.begin();
        iter != erased_set.end(); ++iter) {
     DispatchEvent(extension_event_names::kOnDownloadErased,
                   base::Value::CreateIntegerValue(*iter));
+    delete item_jsons_[*iter];
+    item_jsons_.erase(*iter);
+    delete on_changed_stats_[*iter];
+    on_changed_stats_.erase(*iter);
   }
-  downloads_.swap(current_set);
+  downloads_.swap(current_map);
 }
 
 void ExtensionDownloadsEventRouter::ManagerGoingDown() {