Move InMemoryURLIndex Caching Operations to FILE Thread

chrome/browser/history/in_memory_url_index_unittest.cc

Index: chrome/browser/history/in_memory_url_index_unittest.cc
===================================================================
--- chrome/browser/history/in_memory_url_index_unittest.cc	(revision 117518)
+++ chrome/browser/history/in_memory_url_index_unittest.cc	(working copy)
@@ -6,7 +6,9 @@
 
 #include "base/file_path.h"
 #include "base/file_util.h"
+#include "base/message_loop.h"
 #include "base/path_service.h"
+#include "base/scoped_temp_dir.h"
 #include "base/string16.h"
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
@@ -14,10 +16,15 @@
 #include "chrome/browser/history/in_memory_database.h"
 #include "chrome/browser/history/in_memory_url_index.h"
 #include "chrome/browser/history/in_memory_url_index_types.h"
+#include "chrome/browser/history/url_index_private_data.h"
 #include "chrome/common/chrome_paths.h"
+#include "chrome/test/base/testing_profile.h"
+#include "content/test/test_browser_thread.h"
 #include "sql/transaction.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+using content::BrowserThread;
+
 // The test version of the history url database table ('url') is contained in
 // a database file created from a text file('url_history_provider_test.db.txt').
 // The only difference between this table and a live 'urls' table from a
@@ -31,6 +38,51 @@
 
 namespace history {
 
+// -----------------------------------------------------------------------------
+
+// Observer class so the unit tests can wait while the cache is being saved.
+class CacheFileSaverObserver : public InMemoryURLIndex::SaveCacheObserver {
+ public:
+  explicit CacheFileSaverObserver(MessageLoop* loop)

[Peter Kasting, 2012/01/14 00:12:49]

Nit: I know it's just a test file, but I'd prefer not to inline constructors or
virtual functions all the same.

[mrossetti, 2012/03/03 05:05:56]

Will do in the next CL.

+      : loop_(loop),
+        succeeded_(false) {
+    DCHECK(loop);
+  }
+
+  virtual void OnCacheSaveFinished() OVERRIDE {
+    succeeded_ = true;
+    loop_->Quit();
+  }
+  virtual void OnCacheSaveFailed() OVERRIDE { loop_->Quit(); }
+
+ private:
+  MessageLoop* loop_;
+  bool succeeded_;
+  DISALLOW_COPY_AND_ASSIGN(CacheFileSaverObserver);
+};
+
+// Observer class so the unit tests can wait while the cache is being restored.
+class CacheFileReaderObserver : public InMemoryURLIndex::RestoreCacheObserver {
+ public:
+  explicit CacheFileReaderObserver(MessageLoop* loop)
+      : loop_(loop),
+        succeeded_(false) {
+    DCHECK(loop);
+  }
+
+  virtual void OnCacheRestoreFinished(bool succeeded) OVERRIDE {
+    succeeded_ = succeeded;
+    loop_->Quit();
+  }
+
+ private:
+  MessageLoop* loop_;
+  bool succeeded_;
+  DISALLOW_COPY_AND_ASSIGN(CacheFileReaderObserver);
+};
+
+// -----------------------------------------------------------------------------
+
 class InMemoryURLIndexTest : public testing::Test,
                              public InMemoryDatabase {
  public:
@@ -62,10 +114,13 @@
   void CheckTerm(const URLIndexPrivateData::SearchTermCacheMap& cache,
                  string16 term) const;
 
-  scoped_ptr<InMemoryURLIndex> url_index_;
+  scoped_ptr<TestingProfile> profile_;
+  scoped_refptr<InMemoryURLIndex> url_index_;
 };
 
 void InMemoryURLIndexTest::SetUp() {
+  profile_.reset(new TestingProfile());
+
   // Create and populate a working copy of the URL history database.
   FilePath history_proto_path;
   PathService::Get(chrome::DIR_TEST_DATA, &history_proto_path);
@@ -195,11 +250,6 @@
   return FILE_PATH_LITERAL("url_history_provider_test_limited.db.txt");
 }
 
-TEST_F(InMemoryURLIndexTest, Construction) {
-  url_index_.reset(new InMemoryURLIndex(FilePath()));
-  EXPECT_TRUE(url_index_.get());
-}
-
 TEST_F(LimitedInMemoryURLIndexTest, Initialization) {
   // Verify that the database contains the expected number of items, which
   // is the pre-filtered count, i.e. all of the items.
@@ -208,8 +258,9 @@
   uint64 row_count = 0;
   while (statement.Step()) ++row_count;
   EXPECT_EQ(1U, row_count);
-  url_index_.reset(new InMemoryURLIndex(FilePath()));
-  url_index_->Init(this, "en,ja,hi,zh");
+  url_index_ = new InMemoryURLIndex(profile_.get(), FilePath());
+  url_index_->Init("en,ja,hi,zh");
+  url_index_->RebuildFromHistory(this);
   URLIndexPrivateData& private_data(*(url_index_->private_data_));
 
   // history_info_map_ should have the same number of items as were filtered.
@@ -218,9 +269,17 @@
   EXPECT_EQ(17U, private_data.word_map_.size());
 }
 
+//------------------------------------------------------------------------------
+
+TEST_F(InMemoryURLIndexTest, Construction) {
+  url_index_ = new InMemoryURLIndex(profile_.get(), FilePath());
+  EXPECT_TRUE(url_index_.get());
+}
+
 TEST_F(InMemoryURLIndexTest, Retrieval) {
-  url_index_.reset(new InMemoryURLIndex(FilePath()));
-  url_index_->Init(this, "en,ja,hi,zh");
+  url_index_ = new InMemoryURLIndex(profile_.get(), FilePath());
+  url_index_->Init("en,ja,hi,zh");
+  url_index_->RebuildFromHistory(this);
   // The term will be lowercased by the search.
 
   // See if a very specific term gives a single result.
@@ -268,8 +327,9 @@
 }
 
 TEST_F(InMemoryURLIndexTest, ProperStringMatching) {
-  url_index_.reset(new InMemoryURLIndex(FilePath()));
-  url_index_->Init(this, "en,ja,hi,zh");
+  url_index_ = new InMemoryURLIndex(profile_.get(), FilePath());
+  url_index_->Init("en,ja,hi,zh");
+  url_index_->RebuildFromHistory(this);
 
   // Search for the following with the expected results:
   // "atdmt view" - found
@@ -285,14 +345,15 @@
 }
 
 TEST_F(InMemoryURLIndexTest, HugeResultSet) {
-  url_index_.reset(new InMemoryURLIndex(FilePath()));
-  url_index_->Init(this, "en,ja,hi,zh");
+  url_index_ = new InMemoryURLIndex(profile_.get(), FilePath());
+  url_index_->Init("en,ja,hi,zh");
+  url_index_->RebuildFromHistory(this);
 
   // Create a huge set of qualifying history items.
   for (URLID row_id = 5000; row_id < 6000; ++row_id) {
     URLRow new_row(GURL("http://www.brokeandaloneinmanitoba.com/"), row_id);
     new_row.set_last_visit(base::Time::Now());
-    url_index_->UpdateURL(row_id, new_row);
+    url_index_->UpdateURL(new_row);
   }
 
   ScoredHistoryMatches matches =
@@ -307,8 +368,9 @@
 }
 
 TEST_F(InMemoryURLIndexTest, TitleSearch) {
-  url_index_.reset(new InMemoryURLIndex(FilePath()));
-  url_index_->Init(this, "en,ja,hi,zh");
+  url_index_ = new InMemoryURLIndex(profile_.get(), FilePath());
+  url_index_->Init("en,ja,hi,zh");
+  url_index_->RebuildFromHistory(this);
   // Signal if someone has changed the test DB.
   EXPECT_EQ(27U, url_index_->private_data_->history_info_map_.size());
 
@@ -327,8 +389,9 @@
 }
 
 TEST_F(InMemoryURLIndexTest, TitleChange) {
-  url_index_.reset(new InMemoryURLIndex(FilePath()));
-  url_index_->Init(this, "en,ja,hi,zh");
+  url_index_ = new InMemoryURLIndex(profile_.get(), FilePath());
+  url_index_->Init("en,ja,hi,zh");
+  url_index_->RebuildFromHistory(this);
 
   // Verify current title terms retrieves desired item.
   string16 original_terms =
@@ -354,7 +417,7 @@
 
   // Update the row.
   old_row.set_title(ASCIIToUTF16("Does eat oats and little lambs eat ivy"));
-  url_index_->UpdateURL(expected_id, old_row);
+  url_index_->UpdateURL(old_row);
 
   // Verify we get the row using the new terms but not the original terms.
   matches = url_index_->HistoryItemsForTerms(new_terms);
@@ -365,8 +428,9 @@
 }
 
 TEST_F(InMemoryURLIndexTest, NonUniqueTermCharacterSets) {
-  url_index_.reset(new InMemoryURLIndex(FilePath()));
-  url_index_->Init(this, "en,ja,hi,zh");
+  url_index_ = new InMemoryURLIndex(profile_.get(), FilePath());
+  url_index_->Init("en,ja,hi,zh");
+  url_index_->RebuildFromHistory(this);
 
   // The presence of duplicate characters should succeed. Exercise by cycling
   // through a string with several duplicate characters.
@@ -401,8 +465,9 @@
   typedef URLIndexPrivateData::SearchTermCacheMap::iterator CacheIter;
   typedef URLIndexPrivateData::SearchTermCacheItem CacheItem;
 
-  url_index_.reset(new InMemoryURLIndex(FilePath()));
-  url_index_->Init(this, "en,ja,hi,zh");
+  url_index_ = new InMemoryURLIndex(profile_.get(), FilePath());
+  url_index_->Init("en,ja,hi,zh");
+  url_index_->RebuildFromHistory(this);
 
   URLIndexPrivateData::SearchTermCacheMap& cache(
       url_index_->private_data_->search_term_cache_);
@@ -489,8 +554,8 @@
 }
 
 TEST_F(InMemoryURLIndexTest, AddNewRows) {
-  url_index_.reset(new InMemoryURLIndex(FilePath()));
-  url_index_->Init(this, "en,ja,hi,zh");
+  url_index_ = new InMemoryURLIndex(profile_.get(), FilePath());
+  url_index_->Init("en,ja,hi,zh");
 
   // Verify that the row we're going to add does not already exist.
   URLID new_row_id = 87654321;
@@ -501,29 +566,35 @@
 
   // Add a new row.
   URLRow new_row(GURL("http://www.brokeandaloneinmanitoba.com/"), new_row_id);
+  new_row.set_title(ASCIIToUTF16("Timothy Little and His Amazing Dog Boo"));
   new_row.set_last_visit(base::Time::Now());
-  url_index_->UpdateURL(new_row_id, new_row);
+  url_index_->UpdateURL(new_row);
 
   // Verify that we can retrieve it.
   EXPECT_EQ(1U, url_index_->HistoryItemsForTerms(
       ASCIIToUTF16("brokeandalone")).size());
 
+  // Verify that we can retrieve it by title.
+  EXPECT_EQ(1U, url_index_->HistoryItemsForTerms(
+      ASCIIToUTF16("Timothy Little")).size());
+
   // Add it again just to be sure that is harmless.
-  url_index_->UpdateURL(new_row_id, new_row);
+  url_index_->UpdateURL(new_row);
   EXPECT_EQ(1U, url_index_->HistoryItemsForTerms(
       ASCIIToUTF16("brokeandalone")).size());
 }
 
 TEST_F(InMemoryURLIndexTest, DeleteRows) {
-  url_index_.reset(new InMemoryURLIndex(FilePath()));
-  url_index_->Init(this, "en,ja,hi,zh");
+  url_index_ = new InMemoryURLIndex(profile_.get(), FilePath());
+  url_index_->Init("en,ja,hi,zh");
+  url_index_->RebuildFromHistory(this);
 
   ScoredHistoryMatches matches =
       url_index_->HistoryItemsForTerms(ASCIIToUTF16("DrudgeReport"));
   ASSERT_EQ(1U, matches.size());
 
   // Determine the row id for that result, delete that id, then search again.
-  url_index_->DeleteURL(matches[0].url_info.id());
+  url_index_->DeleteURL(matches[0].url_info.url());
   EXPECT_TRUE(url_index_->HistoryItemsForTerms(
       ASCIIToUTF16("DrudgeReport")).empty());
 }
@@ -601,7 +672,7 @@
     { "xmpp:node@example.com", false },
     { "xmpp://guest@example.com", false },
   };
-  url_index_.reset(new InMemoryURLIndex(FilePath()));
+  url_index_ = new InMemoryURLIndex(profile_.get(), FilePath());
   URLIndexPrivateData& private_data(*(url_index_->private_data_.get()));
   for (size_t i = 0; i < ARRAYSIZE_UNSAFE(data); ++i) {
     GURL url(data[i].url_spec);
@@ -611,8 +682,8 @@
 }
 
 TEST_F(InMemoryURLIndexTest, CacheFilePath) {
-  url_index_.reset(new InMemoryURLIndex(FilePath(FILE_PATH_LITERAL(
-      "/flammmy/frammy/"))));
+  url_index_ = new InMemoryURLIndex(profile_.get(), FilePath(FILE_PATH_LITERAL(
+      "/flammmy/frammy/")));
   FilePath full_file_path;
   url_index_->GetCacheFilePath(&full_file_path);
   std::vector<FilePath::StringType> expected_parts;
@@ -629,23 +700,20 @@
 }
 
 TEST_F(InMemoryURLIndexTest, CacheSaveRestore) {
+  MessageLoop message_loop;
+  content::TestBrowserThread fake_ui_thread(BrowserThread::UI, &message_loop);
+  content::TestBrowserThread fake_file_thread(BrowserThread::FILE,
+                                              &message_loop);
+
   // Save the cache to a protobuf, restore it, and compare the results.
-  url_index_.reset(new InMemoryURLIndex(FilePath()));
+  url_index_ = new InMemoryURLIndex(profile_.get(), FilePath());
   InMemoryURLIndex& url_index(*(url_index_.get()));
-  url_index.Init(this, "en,ja,hi,zh");
-  in_memory_url_index::InMemoryURLIndexCacheItem index_cache;
-  URLIndexPrivateData& private_data(*(url_index_->private_data_.get()));
-  private_data.SavePrivateData(&index_cache);
+  url_index.Init("en,ja,hi,zh");
+  url_index.RebuildFromHistory(this);
 
-  // Capture our private data so we can later compare for equality.
-  String16Vector word_list(private_data.word_list_);
-  WordMap word_map(private_data.word_map_);
-  CharWordIDMap char_word_map(private_data.char_word_map_);
-  WordIDHistoryMap word_id_history_map(private_data.word_id_history_map_);
-  HistoryIDWordMap history_id_word_map(private_data.history_id_word_map_);
-  HistoryInfoMap history_info_map(private_data.history_info_map_);
+  URLIndexPrivateData& private_data(*(url_index_->private_data_));
 
-  // Prove that there is really something there.
+  // Ensure that there is really something there to be saved.
   EXPECT_FALSE(private_data.word_list_.empty());
   // available_words_ will already be empty since we have freshly built the
   // data set for this test.
@@ -656,8 +724,22 @@
   EXPECT_FALSE(private_data.history_id_word_map_.empty());
   EXPECT_FALSE(private_data.history_info_map_.empty());
 
-  // Clear and then prove it's clear.
-  private_data.Clear();
+  // Capture the current private data for later comparison to restored data.
+  URLIndexPrivateData old_data(*(url_index_->private_data_));
+
+  // Save then restore our private data.
+  ScopedTempDir temp_directory;
+  ASSERT_TRUE(temp_directory.CreateUniqueTempDir());
+  FilePath temp_file;
+  ASSERT_TRUE(file_util::CreateTemporaryFileInDir(temp_directory.path(),
+                                                  &temp_file));
+  CacheFileSaverObserver save_observer(&message_loop);
+  url_index_->set_save_cache_observer(&save_observer);
+  url_index.DoSaveToCacheFile(temp_file);
+  message_loop.Run();
+
+  // Clear and then prove it's clear before restoring.
+  url_index.ClearPrivateData();
   EXPECT_TRUE(private_data.word_list_.empty());
   EXPECT_TRUE(private_data.available_words_.empty());
   EXPECT_TRUE(private_data.word_map_.empty());
@@ -666,35 +748,41 @@
   EXPECT_TRUE(private_data.history_id_word_map_.empty());
   EXPECT_TRUE(private_data.history_info_map_.empty());
 
-  // Restore the cache.
-  EXPECT_TRUE(private_data.RestorePrivateData(index_cache));
+  CacheFileReaderObserver read_observer(&message_loop);
+  url_index_->set_restore_cache_observer(&read_observer);
+  url_index.DoRestoreFromCacheFile(temp_file);
+  message_loop.Run();
 
-  // Compare the restored and captured for equality.
-  EXPECT_EQ(word_list.size(), private_data.word_list_.size());
-  EXPECT_EQ(word_map.size(), private_data.word_map_.size());
-  EXPECT_EQ(char_word_map.size(), private_data.char_word_map_.size());
-  EXPECT_EQ(word_id_history_map.size(),
-            private_data.word_id_history_map_.size());
-  EXPECT_EQ(history_id_word_map.size(),
-            private_data.history_id_word_map_.size());
-  EXPECT_EQ(history_info_map.size(), private_data.history_info_map_.size());
+  URLIndexPrivateData& new_data(*(url_index_->private_data_));
+
+  // Compare the captured and restored for equality.
+  EXPECT_EQ(old_data.word_list_.size(), new_data.word_list_.size());
+  EXPECT_EQ(old_data.word_map_.size(), new_data.word_map_.size());
+  EXPECT_EQ(old_data.char_word_map_.size(), new_data.char_word_map_.size());
+  EXPECT_EQ(old_data.word_id_history_map_.size(),
+            new_data.word_id_history_map_.size());
+  EXPECT_EQ(old_data.history_id_word_map_.size(),
+            new_data.history_id_word_map_.size());
+  EXPECT_EQ(old_data.history_info_map_.size(),
+            new_data.history_info_map_.size());
   // WordList must be index-by-index equal.
-  size_t count = word_list.size();
+  size_t count = old_data.word_list_.size();
   for (size_t i = 0; i < count; ++i)
-    EXPECT_EQ(word_list[i], private_data.word_list_[i]);
+    EXPECT_EQ(old_data.word_list_[i], new_data.word_list_[i]);
 
-  ExpectMapOfContainersIdentical(char_word_map,
-                                 private_data.char_word_map_);
-  ExpectMapOfContainersIdentical(word_id_history_map,
-                                 private_data.word_id_history_map_);
-  ExpectMapOfContainersIdentical(history_id_word_map,
-                                 private_data.history_id_word_map_);
+  ExpectMapOfContainersIdentical(old_data.char_word_map_,
+                                 new_data.char_word_map_);
+  ExpectMapOfContainersIdentical(old_data.word_id_history_map_,
+                                 new_data.word_id_history_map_);
+  ExpectMapOfContainersIdentical(old_data.history_id_word_map_,
+                                 new_data.history_id_word_map_);
 
-  for (HistoryInfoMap::const_iterator expected = history_info_map.begin();
-       expected != history_info_map.end(); ++expected) {
+  for (HistoryInfoMap::const_iterator expected =
+       old_data.history_info_map_.begin();
+       expected != old_data.history_info_map_.end(); ++expected) {
     HistoryInfoMap::const_iterator actual =
-        private_data.history_info_map_.find(expected->first);
-    ASSERT_FALSE(private_data.history_info_map_.end() == actual);
+        new_data.history_info_map_.find(expected->first);
+    ASSERT_FALSE(new_data.history_info_map_.end() == actual);
     const URLRow& expected_row(expected->second);
     const URLRow& actual_row(actual->second);
     EXPECT_EQ(expected_row.visit_count(), actual_row.visit_count());