Fix most of the downloads api in incognito profiles

chrome/browser/download/download_extension_api.cc

Index: chrome/browser/download/download_extension_api.cc
diff --git a/chrome/browser/download/download_extension_api.cc b/chrome/browser/download/download_extension_api.cc
index cdf0193ff2ef33882e4c8323ea762f597ede8def..0621d97d7bc9933fc986663287c7107b92bfb814 100644
--- a/chrome/browser/download/download_extension_api.cc
+++ b/chrome/browser/download/download_extension_api.cc
@@ -93,6 +93,9 @@ const char kHeaderValueKey[] = "value";
 const char kHeaderBinaryValueKey[] = "binaryValue";
 const char kHeadersKey[] = "headers";
 const char kIdKey[] = "id";
+const char kIncognito[] = "incognito";
+const char kIncognitoNone[] = "none";
+const char kIncognitoOnly[] = "only";
 const char kLimitKey[] = "limit";
 const char kMethodKey[] = "method";
 const char kMimeKey[] = "mime";
@@ -196,6 +199,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());
   if (item->GetState() == DownloadItem::INTERRUPTED) {
     json->SetInteger(kErrorKey, static_cast<int>(item->GetLastReason()));
   } else if (item->GetState() == DownloadItem::CANCELLED) {
@@ -307,6 +311,27 @@ bool IsNotTemporaryDownloadFilter(const DownloadItem& item) {
   return !item.IsTemporary();
 }
 
+DownloadItem* GetItem(Browser* browser, Profile* profile, int id) {
+  if ((browser != NULL) &&
+      (browser->profile() != NULL))
+    profile = browser->profile();

[Randy Smith (Not in Mondays), 2012/05/03 19:07:21]

This seems like a weird bit of functionality at the interface--we override
profile if we can, but accept what the consumer passed in if we can't override
it.  What are the requirements driving that interface?

[benjhayden, 2012/05/03 21:08:12]

GetCurrentBrowser() may sometimes return NULL if there is no current browser.
If there is a current browser and it's incognito, then
GetCurrentBrowser()->profile() will be incognito, whereas
ExtensionFunction::profile() will not be incognito.
So we need to check for GetCurrentBrowser() && GetCurrentBrowser()->profile()
first, then fall back to profile(). All callers of GetItem() need to do this, so
it made sense to roll the functionality into GetItem().
I'll add convenience methods to {As,S}yncDownloadFunction to wrap this GetItem()
so that callers don't need to even look at GetCurrentBrowser() or profile().

+  DownloadManager* download_manager =
+      DownloadServiceFactory::GetForProfile(profile)->GetDownloadManager();
+  DownloadItem* download_item =
+      download_manager->GetActiveDownloadItem(id);
+  if (!download_item) {
+    if (profile->IsOffTheRecord()) {
+      // Try looking for the item in the original manager.
+      download_manager = DownloadServiceFactory::GetForProfile(
+          profile->GetOriginalProfile())->GetDownloadManager();
+      download_item = download_manager->GetActiveDownloadItem(id);
+    }
+    // Do not try looking for the item in the off-the-record manager if profile
+    // is on the record.
+  }
+  return download_item;
+}
+
 }  // namespace
 
 bool DownloadsFunctionInterface::RunImplImpl(
@@ -532,6 +557,8 @@ void DownloadsDownloadFunction::OnStarted(DownloadId dl_id, net::Error error) {
 DownloadsSearchFunction::DownloadsSearchFunction()
   : SyncDownloadsFunction(DOWNLOADS_FUNCTION_SEARCH),
     query_(new DownloadQuery()),
+    include_on_the_record_(true),
+    include_incognito_(true),
     get_id_(0),
     has_get_id_(false) {
 }
@@ -560,6 +587,17 @@ bool DownloadsSearchFunction::ParseArgs() {
       EXTENSION_FUNCTION_VALIDATE(query_json_field.value().GetAsInteger(
           &get_id_));
       has_get_id_ = true;
+    } else if (query_json_field.key() == kIncognito) {
+      std::string incognito;
+      EXTENSION_FUNCTION_VALIDATE(query_json_field.value().GetAsString(
+          &incognito));
+      if (incognito == kIncognitoNone) {
+        include_incognito_ = false;
+      } else if (incognito == kIncognitoOnly) {
+        include_on_the_record_ = false;
+      } else {
+        EXTENSION_FUNCTION_VALIDATE(false);
+      }
     } else if (query_json_field.key() == kOrderByKey) {
       if (!ParseOrderBy(query_json_field.value()))
         return false;
@@ -633,14 +671,34 @@ bool DownloadsSearchFunction::ParseOrderBy(const base::Value& order_by_value) {
 
 bool DownloadsSearchFunction::RunInternal() {
   DownloadQuery::DownloadVector all_items, cpp_results;
-  DownloadManager* manager = DownloadServiceFactory::GetForProfile(profile())
-    ->GetDownloadManager();
+  Profile* original_profile = NULL;
+  Profile* otr_profile = NULL;
+  if (profile()->IsOffTheRecord()) {
+    otr_profile = profile();
+    original_profile = profile()->GetOriginalProfile();
+  } else {
+    original_profile = profile();
+  }
+  DownloadManager* original_manager = DownloadServiceFactory::GetForProfile(
+      original_profile)->GetDownloadManager();
+  DownloadManager* otr_manager = NULL;
+  if (otr_profile != NULL) {
+    otr_manager = DownloadServiceFactory::GetForProfile(otr_profile)
+      ->GetDownloadManager();
+  }
   if (has_get_id_) {
-    DownloadItem* item = manager->GetDownloadItem(get_id_);
+    DownloadItem* item = original_manager->GetDownloadItem(get_id_);
+    if ((item == NULL) && (otr_manager != NULL))
+      item = otr_manager->GetDownloadItem(get_id_);
     if (item != NULL)
       all_items.push_back(item);
   } else {
-    manager->GetAllDownloads(FilePath(FILE_PATH_LITERAL("")), &all_items);
+    if (include_on_the_record_)
+      original_manager->GetAllDownloads(
+          FilePath(FILE_PATH_LITERAL("")), &all_items);
+    if (include_incognito_ && (otr_manager != NULL))
+      otr_manager->GetAllDownloads(
+          FilePath(FILE_PATH_LITERAL("")), &all_items);
   }
   query_->Search(all_items.begin(), all_items.end(), &cpp_results);
   base::ListValue* json_results = new base::ListValue();
@@ -666,13 +724,9 @@ bool DownloadsPauseFunction::ParseArgs() {
 }
 
 bool DownloadsPauseFunction::RunInternal() {
-  DownloadManager* download_manager =
-      DownloadServiceFactory::GetForProfile(profile())->GetDownloadManager();
-  DownloadItem* download_item =
-      download_manager->GetActiveDownloadItem(download_id_);
-  DCHECK(!download_item || download_item->IsInProgress());
-
-  if (!download_item) {
+  DownloadItem* download_item = GetItem(
+      GetCurrentBrowser(), profile(), download_id_);
+  if ((download_item == NULL) || !download_item->IsInProgress()) {
     // This could be due to an invalid download ID, or it could be due to the
     // download not being currently active.
     error_ = download_extension_errors::kInvalidOperationError;
@@ -696,13 +750,9 @@ bool DownloadsResumeFunction::ParseArgs() {
 }
 
 bool DownloadsResumeFunction::RunInternal() {
-  DownloadManager* download_manager =
-      DownloadServiceFactory::GetForProfile(profile())->GetDownloadManager();
-  DownloadItem* download_item =
-      download_manager->GetActiveDownloadItem(download_id_);
-  DCHECK(!download_item || download_item->IsInProgress());
-
-  if (!download_item) {
+  DownloadItem* download_item = GetItem(
+      GetCurrentBrowser(), profile(), download_id_);
+  if (download_item == NULL) {
     // This could be due to an invalid download ID, or it could be due to the
     // download not being currently active.
     error_ = download_extension_errors::kInvalidOperationError;
@@ -726,12 +776,9 @@ bool DownloadsCancelFunction::ParseArgs() {
 }
 
 bool DownloadsCancelFunction::RunInternal() {
-  DownloadManager* download_manager =
-      DownloadServiceFactory::GetForProfile(profile())->GetDownloadManager();
-  DownloadItem* download_item =
-      download_manager->GetActiveDownloadItem(download_id_);
-
-  if (download_item)
+  DownloadItem* download_item = GetItem(
+      GetCurrentBrowser(), profile(), download_id_);
+  if (download_item != NULL)
     download_item->Cancel(true);
   // |download_item| can be NULL if the download ID was invalid or if the
   // download is not currently active.  Either way, we don't consider it a