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 94 matching lines @@
   sorter_types[kUrlKey] = DownloadQuery::SORT_URL;
 }
 
 bool IsNotTemporaryDownloadFilter(const DownloadItem& item) {
   return !item.IsTemporary();
 }
 
 void GetManagers(
     Profile* profile,
     bool include_incognito,
-    DownloadManager** manager, DownloadManager** incognito_manager) {
+    DownloadManager** manager,
+    DownloadManager** incognito_manager) {
   *manager = BrowserContext::GetDownloadManager(profile);
   *incognito_manager = NULL;
   if (include_incognito && profile->HasOffTheRecordProfile()) {
     *incognito_manager = BrowserContext::GetDownloadManager(
         profile->GetOffTheRecordProfile());
   }
 }
 
 DownloadItem* GetActiveItem(Profile* profile, bool include_incognito, int id) {
   DownloadManager* manager = NULL;
@@ skipping 54 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->GetDownloadItem(*query_in.id.get());
     if (!item && incognito_manager)
       item = incognito_manager->GetDownloadItem(*query_in.id.get());
     if (item)
       all_items.push_back(item);
   } else {
     manager->GetAllDownloads(FilePath(FILE_PATH_LITERAL("")), &all_items);
     if (incognito_manager)
@@ skipping 98 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 on_record = manager && manager->GetDownload(download_id);
+    bool off_record = (incognito_manager &&
+                       incognito_manager->GetDownload(download_id));
+    DCHECK(on_record ^ off_record);

[Randy Smith (Not in Mondays), 2012/07/23 18:19:26]

Just noting that this is subtly scary.  I went on a standards hunt, and it does
what you'd expect, but only because the explicit integer promotion from bool is
(false,true)->(0,1), which my memory is wasn't always the case.  It's personal
preference (not even a suggestion) but I would probably skip creating the extra
variables, pass in the def of off_record into the DownloadItemToJSON call, and
make the DCHECK more complicated (possibly split it into two separate DCHECKs)
but use logical operators rather than bitwise ones.  But that's a personal style
preference.

[benjhayden, 2012/07/25 20:59:58]

Done.

+    scoped_ptr<base::DictionaryValue> json_item(DownloadItemToJSON(
+        *it, off_record));
+    json_results->Append(json_item.release());
   }
   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(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> new_json(DownloadItemToJSON(
+        item, incognito_));
   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 69 matching lines @@
                       std::insert_iterator<DownloadIdSet>(
                         new_set, new_set.begin()));
 
   // For each download that was not in the old set of downloads but is in the
   // new set of downloads, fire an onCreated event, register as an Observer of
   // the item, store a json representation of the item so that we can easily
   // find changes in that json representation, and make an OnChangedStat.
   for (DownloadIdSet::const_iterator iter = new_set.begin();
        iter != new_set.end(); ++iter) {
     scoped_ptr<base::DictionaryValue> item(
-        DownloadItemToJSON(current_map[*iter]));
+        DownloadItemToJSON(current_map[*iter], incognito_));
     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.
@@ skipping 21 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));
 }