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..c0e8f5f46edc09b08dee0e6f0ac593aa724fc1cd 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,22 @@ bool IsNotTemporaryDownloadFilter(const DownloadItem& item) {
   return !item.IsTemporary();
 }
 
+DownloadItem* GetItem(Profile* profile, int id) {
+  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);
+    }
+  }
+  return download_item;
+}
+
 }  // namespace
 
 bool DownloadsFunctionInterface::RunImplImpl(
@@ -441,6 +461,16 @@ bool DownloadsDownloadFunction::ParseArgs() {
     }
   }
   iodata_->rdh = content::ResourceDispatcherHost::Get();
+  // If the extension is running an OTR window, then
+  // GetCurrentBrowser()->profile() will be OTR, if there is a current browser
+  // at all. this->profile() will always be the original profile.
+  // Unfortunately, DownloadRequestHandle derives the DownloadManager* from
+  // render_process_host_id and render_view_host_routing_id, which don't seem to
+  // understand OTR, so this change will depend on DownloadManager::DownloadUrl.

[Randy Smith (Not in Mondays), 2012/05/02 18:57:01]

I'm confused; render_process_host_id and render_view_host_routing_id don't
understand OTR, or DownloadRequestHandle doesn't understand OTR?

Also: This comments shouldn't be committed as shown; it should either have a
TODO (and probably no code) or not be in at all.  My guess is that you're aiming
to commit DownloadUrl first, and then this--the ideal way to deal with that is
to work in a git branch that's branched off of DownloadUrl, and thus arrange for
this CL to show the diffs from that CL, and thus no-one but you has to worry
about commit order.  Let me know if you have any interest in pointers on setting
that up :-}.

[benjhayden, 2012/05/03 13:54:39]

Done.

+  // Profile* profile = profile();
+  // if ((GetCurrentBrowser() != NULL) &&
+  //     (GetCurrentBrowser()->profile() != NULL))
+  //   profile = GetCurrentBrowser()->profile();
   iodata_->resource_context = profile()->GetResourceContext();
   iodata_->render_process_host_id = render_view_host()->GetProcess()->GetID();
   iodata_->render_view_host_routing_id = render_view_host()->GetRoutingID();
@@ -532,6 +562,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 +592,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 +676,36 @@ 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();
+    if (profile()->HasOffTheRecordProfile())
+      otr_profile = profile()->GetOffTheRecordProfile();

[Randy Smith (Not in Mondays), 2012/05/02 18:57:01]

I don't think it's cool to allow access to OTR downloads from an extension
running in a regular window.  That seems like it punches a hole in the privacy
protection.  Am I confused about what you're doing here?

[benjhayden, 2012/05/03 13:54:39]

Done.

+  }
+  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 +731,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 +757,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 +783,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