Kill DownloadItem::IsOtr()

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 168 matching lines @@
 bool ValidateFilename(const string16& filename) {
   // TODO(benjhayden): More robust validation of filename.
   if ((filename.find('/') != string16::npos) ||
       (filename.find('\\') != string16::npos))
     return false;
   if (filename.size() >= 2u && filename[0] == L'.' && filename[1] == L'.')
     return false;
   return true;
 }
 
-scoped_ptr<base::DictionaryValue> DownloadItemToJSON(DownloadItem* item) {
+scoped_ptr<base::DictionaryValue> DownloadItemToJSON(
+    DownloadItem* item,
+    bool incognito) {
   base::DictionaryValue* json = new base::DictionaryValue();
   json->SetInteger(kIdKey, item->GetId());
   json->SetString(kUrlKey, item->GetOriginalUrl().spec());
   json->SetString(kFilenameKey, item->GetFullPath().LossyDisplayName());
   json->SetString(kDangerKey, DangerString(item->GetDangerType()));
   if (item->GetDangerType() != content::DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS)
     json->SetBoolean(kDangerAcceptedKey,
         item->GetSafetyState() == DownloadItem::DANGEROUS_BUT_VALIDATED);
   json->SetString(kStateKey, StateString(item->GetState()));
   json->SetBoolean(kPausedKey, item->IsPaused());
   json->SetString(kMimeKey, item->GetMimeType());
   json->SetInteger(kStartTimeKey,
       (item->GetStartTime() - base::Time::UnixEpoch()).InMilliseconds());
   json->SetInteger(kBytesReceivedKey, item->GetReceivedBytes());
   json->SetInteger(kTotalBytesKey, item->GetTotalBytes());
-  json->SetBoolean(kIncognito, item->IsOtr());
+  json->SetBoolean(kIncognito, incognito);
   if (item->GetState() == DownloadItem::INTERRUPTED) {
     json->SetInteger(kErrorKey, static_cast<int>(item->GetLastReason()));
   } else if (item->GetState() == DownloadItem::CANCELLED) {
     json->SetInteger(kErrorKey, static_cast<int>(
         content::DOWNLOAD_INTERRUPT_REASON_USER_CANCELED));
   }
   // TODO(benjhayden): Implement endTime and fileSize.
   // json->SetInteger(kEndTimeKey, -1);
   json->SetInteger(kFileSizeKey, item->GetTotalBytes());
   return scoped_ptr<base::DictionaryValue>(json);
@@ skipping 91 matching lines @@
   sorter_types[kStartTimeKey] = DownloadQuery::SORT_START_TIME;
   sorter_types[kStateKey] = DownloadQuery::SORT_STATE;
   sorter_types[kTotalBytesKey] = DownloadQuery::SORT_TOTAL_BYTES;
   sorter_types[kUrlKey] = DownloadQuery::SORT_URL;
 }
 
 bool IsNotTemporaryDownloadFilter(const DownloadItem& item) {
   return !item.IsTemporary();
 }
 
+// Set |manager| to the on-record DownloadManager, and |incognito_manager| to
+// the off-record DownloadManager if one exists and is requested via
+// |include_incognito|. This should work regardless of whether |profile| is
+// original or incognito.
 void GetManagers(
     Profile* profile,
     bool include_incognito,
-    DownloadManager** manager, DownloadManager** incognito_manager) {
-  *manager = BrowserContext::GetDownloadManager(profile);
-  *incognito_manager = NULL;
-  if (include_incognito && profile->HasOffTheRecordProfile()) {
+    DownloadManager** manager,
+    DownloadManager** incognito_manager) {
+  *manager = BrowserContext::GetDownloadManager(profile->GetOriginalProfile());
+  if (profile->HasOffTheRecordProfile() &&
+      (include_incognito ||
+       (profile != profile->GetOriginalProfile()))) {
     *incognito_manager = BrowserContext::GetDownloadManager(
         profile->GetOffTheRecordProfile());
+  } else {
+    *incognito_manager = NULL;
   }
 }
 
 DownloadItem* GetActiveItem(Profile* profile, bool include_incognito, int id) {
   DownloadManager* manager = NULL;
   DownloadManager* incognito_manager = NULL;
   GetManagers(profile, include_incognito, &manager, &incognito_manager);
   DownloadItem* download_item = manager->GetDownload(id);
   if (!download_item && incognito_manager)
     download_item = incognito_manager->GetDownload(id);
@@ skipping 49 matching lines @@
     if (sorter_type == sorter_types.Get().end()) {
       *error = download_extension_errors::kInvalidOrderByError;
       return;
     }
     query->AddSorter(sorter_type->second, direction);
   }
 }
 
 void RunDownloadQuery(
     const extensions::api::downloads::DownloadQuery& query_in,
-    Profile* profile,
-    bool include_incognito,
+    DownloadManager* manager,
+    DownloadManager* incognito_manager,
     std::string* error,
     DownloadQuery::DownloadVector* results) {
   // TODO(benjhayden): Consider switching from LazyInstance to explicit string
   // comparisons.
   static base::LazyInstance<FilterTypeMap> filter_types =
     LAZY_INSTANCE_INITIALIZER;
   if (filter_types.Get().size() == 0)
     InitFilterTypeMap(filter_types.Get());
 
   DownloadQuery query_out;
@@ skipping 35 matching lines @@
     FilterTypeMap::const_iterator filter_type =
         filter_types.Get().find(query_json_field.key());
     if (filter_type != filter_types.Get().end()) {
       if (!query_out.AddFilter(filter_type->second, query_json_field.value())) {
         *error = download_extension_errors::kInvalidFilterError;
         return;
       }
     }
   }
 
-  DownloadManager* manager = NULL;
-  DownloadManager* incognito_manager = NULL;
-  GetManagers(profile, include_incognito, &manager, &incognito_manager);
   DownloadQuery::DownloadVector all_items;
   if (query_in.id.get()) {
     DownloadItem* item = manager->GetDownload(*query_in.id.get());
     if (!item && incognito_manager)
       item = incognito_manager->GetDownload(*query_in.id.get());
     if (item)
       all_items.push_back(item);
   } else {
     manager->GetAllDownloads(FilePath(FILE_PATH_LITERAL("")), &all_items);
     if (incognito_manager)
       incognito_manager->GetAllDownloads(
           FilePath(FILE_PATH_LITERAL("")), &all_items);
   }
   query_out.Search(all_items.begin(), all_items.end(), results);
 }
 
+void DispatchEventInternal(
+    Profile* target_profile,
+    const char* event_name,
+    const std::string& json_args) {
+  target_profile->GetExtensionEventRouter()->DispatchEventToRenderers(
+      event_name,
+      json_args,
+      target_profile,
+      GURL(),
+      extensions::EventFilteringInfo());
+
+  ExtensionDownloadsEventRouter::DownloadsNotificationSource
+    notification_source;
+  notification_source.event_name = event_name;
+  notification_source.profile = target_profile;
+  content::Source<ExtensionDownloadsEventRouter::DownloadsNotificationSource>
+    content_source(&notification_source);
+  std::string args_copy(json_args);
+  content::NotificationService::current()->Notify(
+      chrome::NOTIFICATION_EXTENSION_DOWNLOADS_EVENT,
+      content_source,
+      content::Details<std::string>(&args_copy));
+}
+
 }  // namespace
 
 DownloadsDownloadFunction::DownloadsDownloadFunction() {}
 DownloadsDownloadFunction::~DownloadsDownloadFunction() {}
 
 bool DownloadsDownloadFunction::RunImpl() {
   scoped_ptr<extensions::api::downloads::Download::Params> params(
       extensions::api::downloads::Download::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
   const extensions::api::downloads::DownloadOptions& options = params->options;
@@ skipping 82 matching lines @@
   SendResponse(error_.empty());
 }
 
 DownloadsSearchFunction::DownloadsSearchFunction() {}
 DownloadsSearchFunction::~DownloadsSearchFunction() {}
 
 bool DownloadsSearchFunction::RunImpl() {
   scoped_ptr<extensions::api::downloads::Search::Params> params(
       extensions::api::downloads::Search::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
+  DownloadManager* manager = NULL;
+  DownloadManager* incognito_manager = NULL;
+  GetManagers(profile(), include_incognito(), &manager, &incognito_manager);
   DownloadQuery::DownloadVector results;
-  RunDownloadQuery(params->query, profile(), include_incognito(),
-                   &error_, &results);
+  RunDownloadQuery(params->query,
+                   manager,
+                   incognito_manager,
+                   &error_,
+                   &results);
   if (!error_.empty())
     return false;
+
   base::ListValue* json_results = new base::ListValue();
   for (DownloadManager::DownloadVector::const_iterator it = results.begin();
        it != results.end(); ++it) {
-    scoped_ptr<base::DictionaryValue> item(DownloadItemToJSON(*it));
-    json_results->Append(item.release());
+    DownloadItem* item = *it;
+    int32 download_id = item->GetId();
+    bool off_record = ((incognito_manager != NULL) &&
+                       (incognito_manager->GetDownload(download_id) != NULL));
+    scoped_ptr<base::DictionaryValue> json_item(DownloadItemToJSON(
+        *it, off_record));
+    json_results->Append(json_item.release());

[Randy Smith (Not in Mondays), 2012/08/02 17:52:17]

I wince a little bit at RunDownloadQuery having the incognito/non-incognito
information and throwing it away so that it has to be regenerated here.  It
looks like this is the only place RunDownloadQuery() is called from.  Maybe
refactor so that RunDownloadQuery creates the json so that you don't need to
regenerate the incognito info?

[benjhayden, 2012/08/13 15:08:09]

The downloads are all mixed together, so that wouldn't actually help. Also,
RunDownloadQuery is about to be used by erase().

   }
   SetResult(json_results);
   RecordApiFunctions(DOWNLOADS_FUNCTION_SEARCH);
   return true;
 }
 
 DownloadsPauseFunction::DownloadsPauseFunction() {}
 DownloadsPauseFunction::~DownloadsPauseFunction() {}
 
 bool DownloadsPauseFunction::RunImpl() {
@@ skipping 190 matching lines @@
     RecordApiFunctions(DOWNLOADS_FUNCTION_GET_FILE_ICON);
     SetResult(base::Value::CreateStringValue(url));
   }
   SendResponse(error_.empty());
 }
 
 ExtensionDownloadsEventRouter::ExtensionDownloadsEventRouter(
     Profile* profile,
     DownloadManager* manager)
   : profile_(profile),
-    manager_(manager) {
+    manager_(manager),
+    incognito_(profile_->HasOffTheRecordProfile() &&
+               (profile_ == profile_->GetOffTheRecordProfile())) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DCHECK(profile_);
-  DCHECK(manager_);
   manager_->AddObserver(this);
 }
 
 ExtensionDownloadsEventRouter::~ExtensionDownloadsEventRouter() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   if (manager_ != NULL)
     manager_->RemoveObserver(this);
   for (ItemMap::const_iterator iter = downloads_.begin();
        iter != downloads_.end(); ++iter) {
     if (iter->second != NULL)
@@ skipping 26 matching lines @@
     DispatchEvent(extensions::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> new_json(DownloadItemToJSON(
+        item, incognito_));

[Randy Smith (Not in Mondays), 2012/08/02 17:52:17]

Won't this produce the wrong result when we're dispatching a regular download to
an incognito profile?

[benjhayden, 2012/08/13 15:08:09]

We discussed offline how EDER handles incognito. To summarize, each EDER
observers exactly one manager, which is always either incognito or regular. The
incognito-ness of the download isn't lost until DispatchEventInternal.

   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,
   // set the differences in the |delta| object and remember that something
   // significant changed.
   for (base::DictionaryValue::Iterator iter(*new_json.get());
@@ skipping 37 matching lines @@
 }
 
 void ExtensionDownloadsEventRouter::OnDownloadCreated(
     DownloadManager* manager, DownloadItem* download_item) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK(manager_ == manager);
   if (download_item->IsTemporary()) return;
 
   download_item->AddObserver(this);
   scoped_ptr<base::DictionaryValue> json_item(
-      DownloadItemToJSON(download_item));
+      DownloadItemToJSON(download_item, incognito_));
   DispatchEvent(extensions::event_names::kOnDownloadCreated,
                 json_item->DeepCopy());
   int32 download_id = download_item->GetId();
   DCHECK(item_jsons_.find(download_id) == item_jsons_.end());
   on_changed_stats_[download_id] = new OnChangedStat();
   item_jsons_[download_id] = json_item.release();
   downloads_[download_id] = download_item;
 }
 
 void ExtensionDownloadsEventRouter::ManagerGoingDown(
     DownloadManager* manager) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   manager_->RemoveObserver(this);
   manager_ = NULL;
 }
 
 void ExtensionDownloadsEventRouter::DispatchEvent(
     const char* event_name, base::Value* arg) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   base::ListValue args;
   args.Append(arg);
   std::string json_args;
   base::JSONWriter::Write(&args, &json_args);
-  profile_->GetExtensionEventRouter()->DispatchEventToRenderers(
-      event_name,
-      json_args,
-      profile_,
-      GURL(),
-      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));
+  DispatchEventInternal(profile_, event_name, json_args);
+  if (profile_->HasOffTheRecordProfile() &&
+      (profile_ == profile_->GetOriginalProfile())) {
+    DispatchEventInternal(
+        profile_->GetOffTheRecordProfile(), event_name, json_args);
+  }
 }