Kill DownloadItem::IsOtr()

chrome/browser/download/chrome_download_manager_delegate.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/download/chrome_download_manager_delegate.h"
 
 #include <string>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
@@ skipping 391 matching lines @@
 #if defined(ENABLE_SAFE_BROWSING)
   return profile_->GetPrefs()->GetBoolean(prefs::kSafeBrowsingEnabled) &&
       g_browser_process->safe_browsing_service()->DownloadBinHashNeeded();
 #else
   return false;
 #endif
 }
 
 void ChromeDownloadManagerDelegate::AddItemToPersistentStore(
     DownloadItem* item) {
+  if (profile_->HasOffTheRecordProfile() &&
+      (profile_ == profile_->GetOffTheRecordProfile())) {
+    OnItemAddedToPersistentStore(
+        item->GetId(), download_history_->GetNextFakeDbHandle());
+    return;
+  }

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

Why not in DownloadHistory?  That feels like a class that's specialized for
dealing with Download<->History interactions, whereas
ChromeDownloadManagerDelegate is a grab bad; if the functionality fits in a more
specific context, that's probably the better place for it?

I could imagine either code testing the profile otr status in the download
history, or a different (simple) implementation of download history for OTR
contexts (though long term we probably need to share state between otr & non-otr
download history contexts as I think sometimes non-otr contexts allocate fake DB
handles, and we want to avoid collision?)

   download_history_->AddEntry(item,
       base::Bind(&ChromeDownloadManagerDelegate::OnItemAddedToPersistentStore,
                  base::Unretained(this)));
 }
 
 void ChromeDownloadManagerDelegate::UpdateItemInPersistentStore(
     DownloadItem* item) {
   download_history_->UpdateEntry(item);
 }
 
 void ChromeDownloadManagerDelegate::UpdatePathForItemInPersistentStore(
     DownloadItem* item,
     const FilePath& new_path) {
   download_history_->UpdateDownloadPath(item, new_path);
 }
 
 void ChromeDownloadManagerDelegate::RemoveItemFromPersistentStore(
     DownloadItem* item) {
   download_history_->RemoveEntry(item);
 }
 
 void ChromeDownloadManagerDelegate::RemoveItemsFromPersistentStoreBetween(
     base::Time remove_begin,
     base::Time remove_end) {
+  if (profile_->HasOffTheRecordProfile() &&
+      (profile_ == profile_->GetOffTheRecordProfile()))
+    return;
   download_history_->RemoveEntriesBetween(remove_begin, remove_end);
 }
 
 void ChromeDownloadManagerDelegate::GetSaveDir(WebContents* web_contents,
                                                FilePath* website_save_dir,
                                                FilePath* download_save_dir,
                                                bool* skip_dir_check) {
   Profile* profile =
       Profile::FromBrowserContext(web_contents->GetBrowserContext());
   PrefService* prefs = profile->GetPrefs();
@@ skipping 312 matching lines @@
     int32 download_id, int64 db_handle) {
   // It's not immediately obvious, but HistoryBackend::CreateDownload() can
   // call this function with an invalid |db_handle|. For instance, this can
   // happen when the history database is offline. We cannot have multiple
   // DownloadItems with the same invalid db_handle, so we need to assign a
   // unique |db_handle| here.
   if (db_handle == DownloadItem::kUninitializedHandle)
     db_handle = download_history_->GetNextFakeDbHandle();
   download_manager_->OnItemAddedToPersistentStore(download_id, db_handle);
 }