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 124934)
+++ chrome/browser/history/in_memory_url_index_unittest.cc	(working copy)
@@ -8,6 +8,7 @@
 #include "base/file_util.h"
 #include "base/message_loop.h"
 #include "base/path_service.h"
+#include "base/scoped_temp_dir.h"
 #include "base/string_util.h"
 #include "base/string16.h"
 #include "base/utf_string_conversions.h"
@@ -24,6 +25,8 @@
 #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
@@ -37,6 +40,54 @@
 
 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);
+  virtual void OnCacheSaveFinished(bool succeeded) OVERRIDE;
+
+  MessageLoop* loop_;
+  bool succeeded_;
+  DISALLOW_COPY_AND_ASSIGN(CacheFileSaverObserver);
+};
+
+CacheFileSaverObserver::CacheFileSaverObserver(MessageLoop* loop)
+    : loop_(loop),
+      succeeded_(false) {
+  DCHECK(loop);
+}
+
+void CacheFileSaverObserver::OnCacheSaveFinished(bool succeeded) {
+  succeeded_ = succeeded;
+  loop_->Quit();
+}
+
+// Observer class so the unit tests can wait while the cache is being restored.
+class CacheFileReaderObserver : public InMemoryURLIndex::RestoreCacheObserver {
+ public:
+  explicit CacheFileReaderObserver(MessageLoop* loop);
+  virtual void OnCacheRestoreFinished(bool succeeded) OVERRIDE;
+
+  MessageLoop* loop_;
+  bool succeeded_;
+  DISALLOW_COPY_AND_ASSIGN(CacheFileReaderObserver);
+};
+
+CacheFileReaderObserver::CacheFileReaderObserver(MessageLoop* loop)
+    : loop_(loop),
+      succeeded_(false) {
+  DCHECK(loop);
+}
+
+void CacheFileReaderObserver::OnCacheRestoreFinished(bool succeeded) {
+  succeeded_ = succeeded;
+  loop_->Quit();
+}
+
+// -----------------------------------------------------------------------------
+
 class InMemoryURLIndexTest : public testing::Test {
  public:
   InMemoryURLIndexTest();
@@ -72,8 +123,11 @@
 
   // Pass-through functions to simplify our friendship with InMemoryURLIndex.
   URLIndexPrivateData* GetPrivateData() const;
+  void ClearPrivateData();
+  void set_history_dir(const FilePath& dir_path);
   bool GetCacheFilePath(FilePath* file_path) const;
-  void ClearHistoryDir() const;
+  void PostRestoreFromCacheFileTask();
+  void PostSaveToCacheFileTask();
 
   // Pass-through functions to simplify our friendship with URLIndexPrivateData.
   bool UpdateURL(const URLRow& row);
@@ -98,25 +152,37 @@
 }
 
 URLIndexPrivateData* InMemoryURLIndexTest::GetPrivateData() const {
-  DCHECK(url_index_->private_data_.get());
-  return url_index_->private_data_.get();
+  DCHECK(url_index_->private_data());
+  return url_index_->private_data();
 }
 
+void InMemoryURLIndexTest::ClearPrivateData() {
+  return url_index_->ClearPrivateData();
+}
+
+void InMemoryURLIndexTest::set_history_dir(const FilePath& dir_path) {
+  return url_index_->set_history_dir(dir_path);
+}
+
 bool InMemoryURLIndexTest::GetCacheFilePath(FilePath* file_path) const {
   DCHECK(file_path);
   return url_index_->GetCacheFilePath(file_path);
 }
 
-void InMemoryURLIndexTest::ClearHistoryDir() const {
-  url_index_->history_dir_.clear();
+void InMemoryURLIndexTest::PostRestoreFromCacheFileTask() {
+  url_index_->PostRestoreFromCacheFileTask();
 }
 
+void InMemoryURLIndexTest::PostSaveToCacheFileTask() {
+  url_index_->PostSaveToCacheFileTask();
+}
+
 bool InMemoryURLIndexTest::UpdateURL(const URLRow& row) {
-  return url_index_->private_data_->UpdateURL(row);
+  return GetPrivateData()->UpdateURL(row);
 }
 
 bool InMemoryURLIndexTest::DeleteURL(const GURL& url) {
-  return url_index_->private_data_->DeleteURL(url);
+  return GetPrivateData()->DeleteURL(url);
 }
 
 void InMemoryURLIndexTest::SetUp() {
@@ -269,8 +335,7 @@
 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.
-  sql::Connection& db(GetDB());
-  sql::Statement statement(db.GetUniqueStatement("SELECT * FROM urls;"));
+  sql::Statement statement(GetDB().GetUniqueStatement("SELECT * FROM urls;"));
   ASSERT_TRUE(statement.is_valid());
   uint64 row_count = 0;
   while (statement.Step()) ++row_count;
@@ -279,7 +344,7 @@
       new InMemoryURLIndex(&profile_, FilePath(), "en,ja,hi,zh"));
   url_index_->Init();
   url_index_->RebuildFromHistory(history_database_);
-  URLIndexPrivateData& private_data(*(url_index_->private_data_));
+  URLIndexPrivateData& private_data(*GetPrivateData());
 
   // history_info_map_ should have the same number of items as were filtered.
   EXPECT_EQ(1U, private_data.history_info_map_.size());
@@ -753,20 +818,13 @@
 }
 
 TEST_F(InMemoryURLIndexTest, CacheSaveRestore) {
-  // Save the cache to a protobuf, restore it, and compare the results.
-  in_memory_url_index::InMemoryURLIndexCacheItem index_cache;
+  ScopedTempDir temp_directory;
+  ASSERT_TRUE(temp_directory.CreateUniqueTempDir());
+  set_history_dir(temp_directory.path());
+
   URLIndexPrivateData& private_data(*GetPrivateData());
-  private_data.SavePrivateData(&index_cache);
 
-  // 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_);
-
-  // 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.
@@ -777,8 +835,18 @@
   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(private_data);
+
+  // Save then restore our private data.
+  CacheFileSaverObserver save_observer(&message_loop_);
+  url_index_->set_save_cache_observer(&save_observer);
+  PostSaveToCacheFileTask();
+  message_loop_.Run();
+  EXPECT_TRUE(save_observer.succeeded_);
+
+  // Clear and then prove it's clear before restoring.
+  ClearPrivateData();
   EXPECT_TRUE(private_data.word_list_.empty());
   EXPECT_TRUE(private_data.available_words_.empty());
   EXPECT_TRUE(private_data.word_map_.empty());
@@ -787,35 +855,42 @@
   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);
+  PostRestoreFromCacheFileTask();
+  message_loop_.Run();
+  EXPECT_TRUE(read_observer.succeeded_);
 
-  // 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(*GetPrivateData());
+
+  // 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());
@@ -832,6 +907,10 @@
  protected:
   virtual void SetUp() OVERRIDE;
 
+  // Pass-through functions to simplify our friendship with InMemoryURLIndex.
+  void set_history_dir(const FilePath& dir_path);
+  bool GetCacheFilePath(FilePath* file_path) const;
+
   ScopedTempDir temp_dir_;
   scoped_ptr<InMemoryURLIndex> url_index_;
 };
@@ -843,13 +922,22 @@
       new InMemoryURLIndex(NULL, path, "en,ja,hi,zh"));
 }
 
+void InMemoryURLIndexCacheTest::set_history_dir(const FilePath& dir_path) {
+  return url_index_->set_history_dir(dir_path);
+}
+
+bool InMemoryURLIndexCacheTest::GetCacheFilePath(FilePath* file_path) const {
+  DCHECK(file_path);
+  return url_index_->GetCacheFilePath(file_path);
+}
+
 TEST_F(InMemoryURLIndexCacheTest, CacheFilePath) {
   FilePath expectedPath =
       temp_dir_.path().Append(FILE_PATH_LITERAL("History Provider Cache"));
   std::vector<FilePath::StringType> expected_parts;
   expectedPath.GetComponents(&expected_parts);
   FilePath full_file_path;
-  ASSERT_TRUE(url_index_->GetCacheFilePath(&full_file_path));
+  ASSERT_TRUE(GetCacheFilePath(&full_file_path));
   std::vector<FilePath::StringType> actual_parts;
   full_file_path.GetComponents(&actual_parts);
   ASSERT_EQ(expected_parts.size(), actual_parts.size());
@@ -857,7 +945,7 @@
   for (size_t i = 0; i < count; ++i)
     EXPECT_EQ(expected_parts[i], actual_parts[i]);
   // Must clear the history_dir_ to satisfy the dtor's DCHECK.
-  url_index_->history_dir_.clear();
+  set_history_dir(FilePath());
 }
 
 }  // namespace history