Regression fix: Downloads page in Incognito mode works fine now.

Additionally, it fixes another ancient bug that prevented downloads search
from working in Incognito mode.

The design of DownloadHistory is also nicer now, with no dependencies
on DownloadManager.

TEST=see bug
BUG=51955

Review URL: http://codereview.chromium.org/3112011

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@56266 0039d316-1c4b-4281-b951-d872f2087c98

16 files changed, 86 insertions, 162 deletions

diff --git a/chrome/browser/dom_ui/downloads_dom_handler.cc b/chrome/browser/dom_ui/downloads_dom_handler.cc
index 004b6a3..54d6f36 100644
--- a/chrome/browser/dom_ui/downloads_dom_handler.cc
+++ b/chrome/browser/dom_ui/downloads_dom_handler.cc
@@ -116,18 +116,8 @@ void DownloadsDOMHandler::OnDownloadUpdated(DownloadItem* download) {
 // current set of downloads.
 void DownloadsDOMHandler::ModelChanged() {
   ClearDownloadItems();
-  download_manager_->download_history()->Search(
-      WideToUTF16(search_text_),
-      callback_factory_.NewCallback(
-          &DownloadsDOMHandler::OnSearchDownloadsComplete));
-}
-
-void DownloadsDOMHandler::OnSearchDownloadsComplete(
-    std::vector<DownloadItem*> downloads) {
-  ClearDownloadItems();
-
-  // Swap new downloads in.
-  download_items_.swap(downloads);
+  download_manager_->SearchDownloads(WideToUTF16(search_text_),
+                                     &download_items_);
   sort(download_items_.begin(), download_items_.end(), DownloadItemSorter());
 
   // Scan for any in progress downloads and add ourself to them as an observer.
@@ -153,10 +143,7 @@ void DownloadsDOMHandler::HandleGetDownloads(const Value* value) {
   std::wstring new_search = ExtractStringValue(value);
   if (search_text_.compare(new_search) != 0) {
     search_text_ = new_search;
-    ClearDownloadItems();
-    download_manager_->download_history()->Search(
-        WideToUTF16(search_text_),
-        NewCallback(this, &DownloadsDOMHandler::OnSearchDownloadsComplete));
+    ModelChanged();
   } else {
     SendCurrentDownloads();
   }
diff --git a/chrome/browser/dom_ui/downloads_dom_handler.h b/chrome/browser/dom_ui/downloads_dom_handler.h
index 3427a5a..73efa56 100644
--- a/chrome/browser/dom_ui/downloads_dom_handler.h
+++ b/chrome/browser/dom_ui/downloads_dom_handler.h
@@ -37,8 +37,6 @@ class DownloadsDOMHandler : public DOMMessageHandler,
   // DownloadManager::Observer interface
   virtual void ModelChanged();
 
-  void OnSearchDownloadsComplete(std::vector<DownloadItem*> downloads);
-
   // Callback for the "getDownloads" message.
   void HandleGetDownloads(const Value* value);
 
diff --git a/chrome/browser/download/download_history.cc b/chrome/browser/download/download_history.cc
index 1f59338..d232a87 100644
--- a/chrome/browser/download/download_history.cc
+++ b/chrome/browser/download/download_history.cc
@@ -17,12 +17,10 @@
 // static
 const int DownloadHistory::kUninitializedHandle = 0;
 
-DownloadHistory::DownloadHistory(Profile* profile, DownloadItemMapper* mapper)
+DownloadHistory::DownloadHistory(Profile* profile)
     : profile_(profile),
-      next_fake_db_handle_(kUninitializedHandle - 1),
-      download_item_mapper_(mapper) {
+      next_fake_db_handle_(kUninitializedHandle - 1) {
   DCHECK(profile);
-  DCHECK(mapper);
 }
 
 void DownloadHistory::Load(HistoryService::DownloadQueryCallback* callback) {
@@ -38,21 +36,6 @@ void DownloadHistory::Load(HistoryService::DownloadQueryCallback* callback) {
   hs->CleanUpInProgressEntries();
 }
 
-void DownloadHistory::Search(const string16& query,
-                             DownloadSearchCallback* callback) {
-  HistoryService* hs = profile_->GetHistoryService(Profile::EXPLICIT_ACCESS);
-  if (!hs) {
-    delete callback;
-    return;
-  }
-
-  HistoryService::Handle handle = hs->SearchDownloads(
-      query,
-      &history_consumer_,
-      NewCallback(this, &DownloadHistory::OnSearchDownloadsComplete));
-  history_consumer_.SetClientData(hs, handle, callback);
-}
-
 void DownloadHistory::AddEntry(
     const DownloadCreateInfo& info,
     DownloadItem* download_item,
@@ -126,27 +109,6 @@ void DownloadHistory::RemoveEntriesBetween(const base::Time remove_begin,
     hs->RemoveDownloadsBetween(remove_begin, remove_end);
 }
 
-void DownloadHistory::OnSearchDownloadsComplete(HistoryService::Handle handle,
-                                                std::vector<int64>* results) {
-  HistoryService* hs = profile_->GetHistoryService(Profile::EXPLICIT_ACCESS);
-  DownloadSearchCallback* callback =
-      history_consumer_.GetClientData(hs, handle);
-  if (!callback)
-    return;
-
-  std::vector<DownloadItem*> download_items;
-  for (std::vector<int64>::iterator i = results->begin();
-       i != results->end(); ++i) {
-    DownloadItem* download_item =
-        download_item_mapper_->GetDownloadItemFromDbHandle(*i);
-    if (download_item)
-      download_items.push_back(download_item);
-  }
-
-  callback->RunWithParams(MakeTuple(download_items));
-  delete callback;
-}
-
 int64 DownloadHistory::GetNextFakeDbHandle() {
   return next_fake_db_handle_--;
 }
diff --git a/chrome/browser/download/download_history.h b/chrome/browser/download/download_history.h
index 3fe789e..cc1407f 100644
--- a/chrome/browser/download/download_history.h
+++ b/chrome/browser/download/download_history.h
@@ -22,31 +22,15 @@ class Time;
 // Interacts with the HistoryService on behalf of the download subsystem.
 class DownloadHistory {
  public:
-  // Converts history database handles to download items.
-  class DownloadItemMapper {
-   public:
-    virtual ~DownloadItemMapper() {}
-
-    // Returns a DownloadItem* corresponding to |db_handle|, or NULL if no such
-    // DownloadItem exists.
-    virtual DownloadItem* GetDownloadItemFromDbHandle(int64 db_handle) = 0;
-  };
-
   // A fake download table ID which represents a download that has started,
   // but is not yet in the table.
   static const int kUninitializedHandle;
 
-  typedef Callback1<std::vector<DownloadItem*> >::Type DownloadSearchCallback;
-
-  DownloadHistory(Profile* profile, DownloadItemMapper* mapper);
+  explicit DownloadHistory(Profile* profile);
 
   // Retrieves DownloadCreateInfos saved in the history.
   void Load(HistoryService::DownloadQueryCallback* callback);
 
-  // Starts a history search for downloads and invokes |callback| when done.
-  void Search(const string16& query,
-              DownloadSearchCallback* callback);
-
   // Adds a new entry for a download to the history database.
   void AddEntry(const DownloadCreateInfo& info,
                 DownloadItem* download_item,
@@ -70,9 +54,6 @@ class DownloadHistory {
   int64 GetNextFakeDbHandle();
 
  private:
-  void OnSearchDownloadsComplete(HistoryService::Handle handle,
-                                 std::vector<int64>* results);
-
   Profile* profile_;
 
   // In case we don't have a valid db_handle, we use |fake_db_handle_| instead.
@@ -81,9 +62,7 @@ class DownloadHistory {
   // fake handle value on every use.
   int64 next_fake_db_handle_;
 
-  DownloadItemMapper* download_item_mapper_;
-
-  CancelableRequestConsumerTSimple<DownloadSearchCallback*> history_consumer_;
+  CancelableRequestConsumer history_consumer_;
 
   DISALLOW_COPY_AND_ASSIGN(DownloadHistory);
 };
diff --git a/chrome/browser/download/download_item.cc b/chrome/browser/download/download_item.cc
index 269f0cb..c05823a 100644
--- a/chrome/browser/download/download_item.cc
+++ b/chrome/browser/download/download_item.cc
@@ -4,15 +4,21 @@
 
 #include "chrome/browser/download/download_item.h"
 
+#include "app/l10n_util.h"
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/timer.h"
+#include "base/utf_string_conversions.h"
+#include "net/base/net_util.h"
 #include "chrome/browser/chrome_thread.h"
 #include "chrome/browser/download/download_history.h"
 #include "chrome/browser/download/download_manager.h"
 #include "chrome/browser/download/download_util.h"
 #include "chrome/browser/history/download_types.h"
+#include "chrome/browser/pref_service.h"
+#include "chrome/browser/profile.h"
 #include "chrome/common/extensions/extension.h"
+#include "chrome/common/pref_names.h"
 
 namespace {
 
@@ -298,6 +304,35 @@ void DownloadItem::OnNameFinalized() {
     NotifyObserversDownloadFileCompleted();
 }
 
+bool DownloadItem::MatchesQuery(const string16& query) const {
+  if (query.empty())
+    return true;
+
+  DCHECK_EQ(query, l10n_util::ToLower(query));
+
+  string16 url_raw(l10n_util::ToLower(UTF8ToUTF16(url_.spec())));
+  if (url_raw.find(query) != string16::npos)
+    return true;
+
+  // TODO(phajdan.jr): write a test case for the following code.
+  // A good test case would be:
+  //   "/\xe4\xbd\xa0\xe5\xa5\xbd\xe4\xbd\xa0\xe5\xa5\xbd",
+  //   L"/\x4f60\x597d\x4f60\x597d",
+  //   "/%E4%BD%A0%E5%A5%BD%E4%BD%A0%E5%A5%BD"
+  PrefService* prefs = download_manager_->profile()->GetPrefs();
+  std::wstring languages(UTF8ToWide(prefs->GetString(prefs::kAcceptLanguages)));
+  string16 url_formatted(
+      l10n_util::ToLower(WideToUTF16(net::FormatUrl(url_, languages))));
+  if (url_formatted.find(query) != string16::npos)
+    return true;
+
+  string16 path(l10n_util::ToLower(WideToUTF16(full_path_.ToWStringHack())));
+  if (path.find(query) != std::wstring::npos)
+    return true;
+
+  return false;
+}
+
 FilePath DownloadItem::GetFileName() const {
   if (safety_state_ == DownloadItem::SAFE)
     return file_name_;
diff --git a/chrome/browser/download/download_item.h b/chrome/browser/download/download_item.h
index 110f2fa..c3a5bd6 100644
--- a/chrome/browser/download/download_item.h
+++ b/chrome/browser/download/download_item.h
@@ -155,6 +155,9 @@ class DownloadItem {
   // Called when the name of the download is finalized.
   void OnNameFinalized();
 
+  // Returns true if this item matches |query|. |query| must be lower-cased.
+  bool MatchesQuery(const string16& query) const;
+
   // Accessors
   DownloadState state() const { return state_; }
   FilePath full_path() const { return full_path_; }
diff --git a/chrome/browser/download/download_manager.cc b/chrome/browser/download/download_manager.cc
index 87cc999..9b2d4b5 100644
--- a/chrome/browser/download/download_manager.cc
+++ b/chrome/browser/download/download_manager.cc
@@ -208,6 +208,35 @@ void DownloadManager::GetCurrentDownloads(
   }
 }
 
+void DownloadManager::SearchDownloads(const string16& query,
+                                      std::vector<DownloadItem*>* result) {
+  DCHECK(result);
+
+  string16 query_lower(l10n_util::ToLower(query));
+
+  for (DownloadMap::iterator it = downloads_.begin();
+       it != downloads_.end(); ++it) {
+    DownloadItem* download_item = it->second;
+
+    if (download_item->is_temporary() || download_item->is_extension_install())
+      continue;
+
+    // Display Incognito downloads only in Incognito window, and vice versa.
+    // The Incognito Downloads page will get the list of non-Incognito downloads
+    // from its parent profile.
+    if (profile_->IsOffTheRecord() != download_item->is_otr())
+      continue;
+
+    if (download_item->MatchesQuery(query_lower))
+      result->push_back(download_item);
+  }
+
+  // If we have a parent profile, let it add its downloads to the results.
+  Profile* original_profile = profile_->GetOriginalProfile();
+  if (original_profile != profile_)
+    original_profile->GetDownloadManager()->SearchDownloads(query, result);
+}
+
 // Query the history service for information about all persisted downloads.
 bool DownloadManager::Init(Profile* profile) {
   DCHECK(profile);
@@ -216,7 +245,7 @@ bool DownloadManager::Init(Profile* profile) {
 
   profile_ = profile;
   request_context_getter_ = profile_->GetRequestContext();
-  download_history_.reset(new DownloadHistory(profile, this));
+  download_history_.reset(new DownloadHistory(profile));
   download_history_->Load(
       NewCallback(this, &DownloadManager::OnQueryDownloadEntriesComplete));
 
@@ -981,16 +1010,6 @@ void DownloadManager::SaveAutoOpens() {
   }
 }
 
-DownloadItem* DownloadManager::GetDownloadItemFromDbHandle(int64 db_handle) {
-  for (DownloadMap::iterator it = downloads_.begin();
-       it != downloads_.end(); ++it) {
-    DownloadItem* item = it->second;
-    if (item->db_handle() == db_handle)
-      return item;
-  }
-  return NULL;
-}
-
 void DownloadManager::FileSelected(const FilePath& path,
                                    int index, void* params) {
   DownloadCreateInfo* info = reinterpret_cast<DownloadCreateInfo*>(params);
diff --git a/chrome/browser/download/download_manager.h b/chrome/browser/download/download_manager.h
index 7ba91d4..7a45f66 100644
--- a/chrome/browser/download/download_manager.h
+++ b/chrome/browser/download/download_manager.h
@@ -39,11 +39,11 @@
 #include "base/ref_counted.h"
 #include "base/scoped_ptr.h"
 #include "base/time.h"
-#include "chrome/browser/download/download_history.h"
 #include "chrome/browser/pref_member.h"
 #include "chrome/browser/shell_dialogs.h"
 
 class DownloadFileManager;
+class DownloadHistory;
 class DownloadItem;
 class GURL;
 class PrefService;
@@ -56,7 +56,6 @@ struct DownloadSaveInfo;
 
 // Browser's download manager: manages all downloads and destination view.
 class DownloadManager : public base::RefCountedThreadSafe<DownloadManager>,
-                        public DownloadHistory::DownloadItemMapper,
                         public SelectFileDialog::Listener {
   // For testing.
   friend class DownloadManagerTest;
@@ -97,6 +96,11 @@ class DownloadManager : public base::RefCountedThreadSafe<DownloadManager>,
   void GetCurrentDownloads(const FilePath& dir_path,
                            std::vector<DownloadItem*>* result);
 
+  // Returns all non-temporary downloads matching |query|. Empty query matches
+  // everything.
+  void SearchDownloads(const string16& query,
+                       std::vector<DownloadItem*>* result);
+
   // Returns true if initialized properly.
   bool Init(Profile* profile);
 
@@ -174,6 +178,8 @@ class DownloadManager : public base::RefCountedThreadSafe<DownloadManager>,
 
   FilePath download_path() { return *download_path_; }
 
+  Profile* profile() { return profile_; }
+
   DownloadHistory* download_history() { return download_history_.get(); }
 
   // Clears the last download path, used to initialize "save as" dialogs.
@@ -197,9 +203,6 @@ class DownloadManager : public base::RefCountedThreadSafe<DownloadManager>,
   // types.
   bool HasAutoOpenFileTypesRegistered() const;
 
-  // Overridden from DownloadHistory::DownloadItemMapper:
-  virtual DownloadItem* GetDownloadItemFromDbHandle(int64 db_handle);
-
   // Overridden from SelectFileDialog::Listener:
   virtual void FileSelected(const FilePath& path, int index, void* params);
   virtual void FileSelectionCanceled(void* params);
diff --git a/chrome/browser/history/download_database.cc b/chrome/browser/history/download_database.cc
index 62853a8..e985cca 100644
--- a/chrome/browser/history/download_database.cc
+++ b/chrome/browser/history/download_database.cc
@@ -196,22 +196,4 @@ void DownloadDatabase::RemoveDownloadsBetween(base::Time delete_begin,
   statement.Run();
 }
 
-void DownloadDatabase::SearchDownloads(std::vector<int64>* results,
-                                       const string16& search_text) {
-  sql::Statement statement(GetDB().GetCachedStatement(SQL_FROM_HERE,
-      "SELECT id FROM downloads WHERE url LIKE ? "
-      "OR full_path LIKE ? ORDER BY id"));
-  if (!statement)
-    return;
-
-  std::string text("%");
-  text.append(UTF16ToUTF8(search_text));
-  text.push_back('%');
-  statement.BindString(0, text);
-  statement.BindString(1, text);
-
-  while (statement.Step())
-    results->push_back(statement.ColumnInt64(0));
-}
-
 }  // namespace history
diff --git a/chrome/browser/history/download_database.h b/chrome/browser/history/download_database.h
index 3f2bd97..56f650a 100644
--- a/chrome/browser/history/download_database.h
+++ b/chrome/browser/history/download_database.h
@@ -51,10 +51,6 @@ class DownloadDatabase {
   // all downloads that are in progress or are waiting to be cancelled.
   void RemoveDownloadsBetween(base::Time remove_begin, base::Time remove_end);
 
-  // Search for downloads matching the search text.
-  void SearchDownloads(std::vector<int64>* results,
-                       const string16& search_text);
-
  protected:
   // Returns the database for the functions in this interface.
   virtual sql::Connection& GetDB() = 0;
diff --git a/chrome/browser/history/history.cc b/chrome/browser/history/history.cc
index c001298..156f9da 100644
--- a/chrome/browser/history/history.cc
+++ b/chrome/browser/history/history.cc
@@ -539,14 +539,6 @@ void HistoryService::RemoveDownloadsBetween(Time remove_begin,
                     remove_end);
 }
 
-HistoryService::Handle HistoryService::SearchDownloads(
-    const string16& search_text,
-    CancelableRequestConsumerBase* consumer,
-    DownloadSearchCallback* callback) {
-  return Schedule(PRIORITY_NORMAL, &HistoryBackend::SearchDownloads, consumer,
-                  new history::DownloadSearchRequest(callback), search_text);
-}
-
 HistoryService::Handle HistoryService::QueryHistory(
     const string16& text_query,
     const history::QueryOptions& options,
diff --git a/chrome/browser/history/history.h b/chrome/browser/history/history.h
index bec532a..c142c97 100644
--- a/chrome/browser/history/history.h
+++ b/chrome/browser/history/history.h
@@ -445,15 +445,6 @@ class HistoryService : public CancelableRequestProvider,
   // both directions.
   void RemoveDownloadsBetween(base::Time remove_begin, base::Time remove_end);
 
-  // Implemented by the caller of 'SearchDownloads' below, and is called when
-  // the history system has retrieved the search results.
-  typedef Callback2<Handle, std::vector<int64>*>::Type DownloadSearchCallback;
-
-  // Search for downloads that match the search text.
-  Handle SearchDownloads(const string16& search_text,
-                         CancelableRequestConsumerBase* consumer,
-                         DownloadSearchCallback* callback);
-
   // Visit Segments ------------------------------------------------------------
 
   typedef Callback2<Handle, std::vector<PageUsageData*>*>::Type
diff --git a/chrome/browser/history/history_backend.cc b/chrome/browser/history/history_backend.cc
index 1c58f74..91aba75 100644
--- a/chrome/browser/history/history_backend.cc
+++ b/chrome/browser/history/history_backend.cc
@@ -1126,17 +1126,6 @@ void HistoryBackend::RemoveDownloadsBetween(const Time remove_begin,
     db_->RemoveDownloadsBetween(remove_begin, remove_end);
 }
 
-void HistoryBackend::SearchDownloads(
-    scoped_refptr<DownloadSearchRequest> request,
-    const string16& search_text) {
-  if (request->canceled())
-    return;
-  if (db_.get())
-    db_->SearchDownloads(&request->value, search_text);
-  request->ForwardResult(DownloadSearchRequest::TupleType(request->handle(),
-                                                          &request->value));
-}
-
 void HistoryBackend::QueryHistory(scoped_refptr<QueryHistoryRequest> request,
                                   const string16& text_query,
                                   const QueryOptions& options) {
diff --git a/chrome/browser/history/history_backend.h b/chrome/browser/history/history_backend.h
index cf9e076..4d333f3 100644
--- a/chrome/browser/history/history_backend.h
+++ b/chrome/browser/history/history_backend.h
@@ -225,8 +225,6 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>,
   void RemoveDownloadsBetween(const base::Time remove_begin,
                               const base::Time remove_end);
   void RemoveDownloads(const base::Time remove_end);
-  void SearchDownloads(scoped_refptr<DownloadSearchRequest>,
-                       const string16& search_text);
 
   // Segment usage -------------------------------------------------------------
 
diff --git a/chrome/browser/history/history_marshaling.h b/chrome/browser/history/history_marshaling.h
index 30cdbef..64cde29 100644
--- a/chrome/browser/history/history_marshaling.h
+++ b/chrome/browser/history/history_marshaling.h
@@ -105,10 +105,6 @@ typedef CancelableRequest1<HistoryService::DownloadQueryCallback,
 typedef CancelableRequest<HistoryService::DownloadCreateCallback>
     DownloadCreateRequest;
 
-typedef CancelableRequest1<HistoryService::DownloadSearchCallback,
-                          std::vector<int64> >
-    DownloadSearchRequest;
-
 // Deletion --------------------------------------------------------------------
 
 typedef CancelableRequest<HistoryService::ExpireHistoryCallback>
diff --git a/chrome/test/ui_test_utils.cc b/chrome/test/ui_test_utils.cc
index cdd6087..3d36b6e 100644
--- a/chrome/test/ui_test_utils.cc
+++ b/chrome/test/ui_test_utils.cc
@@ -168,7 +168,7 @@ class DownloadsCompleteObserver : public DownloadManager::Observer,
 
     download_manager_->RemoveObserver(this);
     // waiting_ will have been set if not all downloads were complete on first
-    // pass below in OnSearchDownloadsComplete().
+    // pass below in ModelChanged().
     if (waiting_)
       MessageLoopForUI::current()->Quit();
     return true;
@@ -186,14 +186,9 @@ class DownloadsCompleteObserver : public DownloadManager::Observer,
 
   // DownloadManager::Observer
   virtual void ModelChanged() {
-    download_manager_->download_history()->Search(
-        string16(),
-        NewCallback(this,
-                    &DownloadsCompleteObserver::OnSearchDownloadsComplete));
-  }
+    downloads_.clear();
+    download_manager_->SearchDownloads(string16(), &downloads_);
 
-  void OnSearchDownloadsComplete(std::vector<DownloadItem*> downloads) {
-    downloads_ = downloads;
     if (CheckAllDownloadsComplete())
       return;
 
@@ -203,7 +198,6 @@ class DownloadsCompleteObserver : public DownloadManager::Observer,
     }
   }
 
-
  private:
   // The observed download manager.
   DownloadManager* download_manager_;