Kill DownloadItem::IsOtr()

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 c8ec46a253234785abaca27481b172283ba5a81c..6b74c55507e78577f0b031e201feb7d5a3973663 100644
--- a/chrome/browser/extensions/api/downloads/downloads_api.cc
+++ b/chrome/browser/extensions/api/downloads/downloads_api.cc
@@ -186,7 +186,9 @@ bool ValidateFilename(const string16& filename) {
   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());
@@ -202,7 +204,7 @@ scoped_ptr<base::DictionaryValue> DownloadItemToJSON(DownloadItem* item) {
       (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) {
@@ -317,7 +319,8 @@ bool IsNotTemporaryDownloadFilter(const DownloadItem& item) {
 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()) {
@@ -392,8 +395,8 @@ void CompileDownloadQueryOrderBy(
 
 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
@@ -449,9 +452,6 @@ void RunDownloadQuery(
     }
   }
 
-  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());
@@ -570,16 +570,30 @@ 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);
@@ -790,10 +804,10 @@ 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);
 }
 
@@ -840,7 +854,8 @@ void ExtensionDownloadsEventRouter::OnDownloadUpdated(DownloadItem* item) {
   }
 
   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;
@@ -930,7 +945,7 @@ void ExtensionDownloadsEventRouter::ModelChanged(DownloadManager* manager) {
   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();