Sync user's custom spellcheck dictionary

chrome/browser/spellchecker/spellcheck_custom_dictionary_unittest.cc

Index: chrome/browser/spellchecker/spellcheck_custom_dictionary_unittest.cc
diff --git a/chrome/browser/spellchecker/spellcheck_custom_dictionary_unittest.cc b/chrome/browser/spellchecker/spellcheck_custom_dictionary_unittest.cc
index c890afcaa654000c8bbbc5d2976d013fce33c5bf..339624d700a484978b44ad6e71be9c2f5f35d774 100644
--- a/chrome/browser/spellchecker/spellcheck_custom_dictionary_unittest.cc
+++ b/chrome/browser/spellchecker/spellcheck_custom_dictionary_unittest.cc
@@ -5,6 +5,7 @@
 #include <vector>
 
 #include "base/file_util.h"
+#include "base/string_number_conversions.h"
 #include "chrome/browser/spellchecker/spellcheck_custom_dictionary.h"
 #include "chrome/browser/spellchecker/spellcheck_factory.h"
 #include "chrome/browser/spellchecker/spellcheck_service.h"
@@ -12,12 +13,21 @@
 #include "chrome/common/spellcheck_common.h"
 #include "chrome/test/base/testing_profile.h"
 #include "content/public/test/test_browser_thread.h"
+#include "sync/api/sync_change.h"
+#include "sync/api/sync_data.h"
+#include "sync/api/sync_error_factory.h"
+#include "sync/api/sync_error_factory_mock.h"
+#include "sync/protocol/sync.pb.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using content::BrowserThread;
 using chrome::spellcheck_common::WordList;
 
+namespace {
+const size_t LIMIT = 1300;

[groby-ooo-7-16, 2012/12/19 22:16:00]

Please document. Or use a shared constant.

[please use gerrit instead, 2012/12/22 03:20:19]

Done. Using constant in chrome::spellcheck_common.

+}
+
 static ProfileKeyedService* BuildSpellcheckService(Profile* profile) {
   return new SpellcheckService(profile);
 }
@@ -27,7 +37,7 @@ class SpellcheckCustomDictionaryTest : public testing::Test {
   SpellcheckCustomDictionaryTest()
       : ui_thread_(BrowserThread::UI, &message_loop_),
         file_thread_(BrowserThread::FILE, &message_loop_),
-        profile_(new TestingProfile()) {
+        profile_(new TestingProfile) {
   }
 
   void SetUp() OVERRIDE {
@@ -84,40 +94,31 @@ TEST_F(SpellcheckCustomDictionaryTest, CustomWordAddedAndRemovedLocally) {
 }
 
 TEST_F(SpellcheckCustomDictionaryTest, SaveAndLoad) {
-  SpellcheckService* spellcheck_service =
-      SpellcheckServiceFactory::GetForProfile(profile_.get());
-  SpellcheckCustomDictionary* custom_dictionary =
-      spellcheck_service->GetCustomDictionary();
-
+  FilePath path = profile_->GetPath().Append(chrome::kCustomDictionaryFileName);
   WordList loaded_custom_words;
-  custom_dictionary->LoadDictionaryIntoCustomWordList(&loaded_custom_words);
+  SpellcheckCustomDictionary::LoadDictionaryIntoCustomWordList(
+      &loaded_custom_words, path);
 
   // The custom word list should be empty now.
   WordList expected;
   EXPECT_EQ(loaded_custom_words, expected);
 
-  custom_dictionary->WriteWordToCustomDictionary("foo");
+  SpellcheckCustomDictionary::WriteWordToCustomDictionary("foo", path);
   expected.push_back("foo");
 
-  custom_dictionary->WriteWordToCustomDictionary("bar");
+  SpellcheckCustomDictionary::WriteWordToCustomDictionary("bar", path);
   expected.push_back("bar");
 
   // The custom word list should include written words.
-  custom_dictionary->LoadDictionaryIntoCustomWordList(&loaded_custom_words);
+  SpellcheckCustomDictionary::LoadDictionaryIntoCustomWordList(
+      &loaded_custom_words, path);
   std::sort(expected.begin(), expected.end());
   EXPECT_EQ(loaded_custom_words, expected);
 
-  // Load in another instance of SpellCheckService.
-  // The result should be the same.
-  SpellcheckService spellcheck_service2(profile_.get());
-  WordList loaded_custom_words2;
-  spellcheck_service2.GetCustomDictionary()->
-      LoadDictionaryIntoCustomWordList(&loaded_custom_words2);
-  EXPECT_EQ(loaded_custom_words2, expected);
-
-  custom_dictionary->EraseWordFromCustomDictionary("foo");
-  custom_dictionary->EraseWordFromCustomDictionary("bar");
-  custom_dictionary->LoadDictionaryIntoCustomWordList(&loaded_custom_words);
+  SpellcheckCustomDictionary::EraseWordFromCustomDictionary("foo", path);
+  SpellcheckCustomDictionary::EraseWordFromCustomDictionary("bar", path);
+  SpellcheckCustomDictionary::LoadDictionaryIntoCustomWordList(
+      &loaded_custom_words, path);
   expected.clear();
   EXPECT_EQ(loaded_custom_words, expected);
 
@@ -142,23 +143,23 @@ TEST_F(SpellcheckCustomDictionaryTest, MultiProfile) {
   WordList expected1;
   WordList expected2;
 
-  custom_dictionary->WriteWordToCustomDictionary("foo");
-  custom_dictionary->WriteWordToCustomDictionary("bar");
+  custom_dictionary->CustomWordAddedLocally("foo");
+  custom_dictionary->CustomWordAddedLocally("bar");
   expected1.push_back("foo");
   expected1.push_back("bar");
 
-  custom_dictionary2->WriteWordToCustomDictionary("hoge");
-  custom_dictionary2->WriteWordToCustomDictionary("fuga");
+  custom_dictionary2->CustomWordAddedLocally("hoge");
+  custom_dictionary2->CustomWordAddedLocally("fuga");
   expected2.push_back("hoge");
   expected2.push_back("fuga");
 
-  WordList actual1;
-  custom_dictionary->LoadDictionaryIntoCustomWordList(&actual1);
+  WordList actual1 = custom_dictionary->GetWords();
+  std::sort(actual1.begin(), actual1.end());
   std::sort(expected1.begin(), expected1.end());
   EXPECT_EQ(actual1, expected1);
 
-  WordList actual2;
-  custom_dictionary2->LoadDictionaryIntoCustomWordList(&actual2);
+  WordList actual2 = custom_dictionary2->GetWords();
+  std::sort(actual2.begin(), actual2.end());
   std::sort(expected2.begin(), expected2.end());
   EXPECT_EQ(actual2, expected2);
 
@@ -167,19 +168,15 @@ TEST_F(SpellcheckCustomDictionaryTest, MultiProfile) {
   MessageLoop::current()->RunUntilIdle();
 }
 
-// Legacy empty dictionary should be converted to new format empty dicitonary.
+// Legacy empty dictionary should be converted to new format empty dictionary.
 TEST_F(SpellcheckCustomDictionaryTest, LegacyEmptyDictionaryShouldBeConverted) {
-  FilePath dictionary_path(
-      profile_->GetPath().Append(chrome::kCustomDictionaryFileName));
-  SpellcheckService* spellcheck_service =
-      SpellcheckServiceFactory::GetForProfile(profile_.get());
-  SpellcheckCustomDictionary* custom_dictionary =
-      spellcheck_service->GetCustomDictionary();
+  FilePath path = profile_->GetPath().Append(chrome::kCustomDictionaryFileName);
   WordList loaded_custom_words;
 
   std::string content;
-  file_util::WriteFile(dictionary_path, content.c_str(), content.length());
-  custom_dictionary->LoadDictionaryIntoCustomWordList(&loaded_custom_words);
+  file_util::WriteFile(path, content.c_str(), content.length());
+  SpellcheckCustomDictionary::LoadDictionaryIntoCustomWordList(
+      &loaded_custom_words, path);
   EXPECT_TRUE(loaded_custom_words.empty());
 
   // Flush the loop now to prevent service init tasks from being run during
@@ -191,18 +188,14 @@ TEST_F(SpellcheckCustomDictionaryTest, LegacyEmptyDictionaryShouldBeConverted) {
 // with two words.
 TEST_F(SpellcheckCustomDictionaryTest,
        LegacyDictionaryWithTwoWordsShouldBeConverted) {
-  FilePath dictionary_path(
-      profile_->GetPath().Append(chrome::kCustomDictionaryFileName));
-  SpellcheckService* spellcheck_service =
-      SpellcheckServiceFactory::GetForProfile(profile_.get());
-  SpellcheckCustomDictionary* custom_dictionary =
-      spellcheck_service->GetCustomDictionary();
+  FilePath path = profile_->GetPath().Append(chrome::kCustomDictionaryFileName);
   WordList loaded_custom_words;
   WordList expected;
 
   std::string content = "foo\nbar";
-  file_util::WriteFile(dictionary_path, content.c_str(), content.length());
-  custom_dictionary->LoadDictionaryIntoCustomWordList(&loaded_custom_words);
+  file_util::WriteFile(path, content.c_str(), content.length());
+  SpellcheckCustomDictionary::LoadDictionaryIntoCustomWordList(
+      &loaded_custom_words, path);
   expected.push_back("bar");
   expected.push_back("foo");
   EXPECT_EQ(expected, loaded_custom_words);
@@ -215,18 +208,14 @@ TEST_F(SpellcheckCustomDictionaryTest,
 // Words with spaces are illegal and should be removed.
 TEST_F(SpellcheckCustomDictionaryTest,
        IllegalWordsShouldBeRemovedFromDictionary) {
-  FilePath dictionary_path(
-      profile_->GetPath().Append(chrome::kCustomDictionaryFileName));
-  SpellcheckService* spellcheck_service =
-      SpellcheckServiceFactory::GetForProfile(profile_.get());
-  SpellcheckCustomDictionary* custom_dictionary =
-      spellcheck_service->GetCustomDictionary();
+  FilePath path = profile_->GetPath().Append(chrome::kCustomDictionaryFileName);
   WordList loaded_custom_words;
   WordList expected;
 
   std::string content = "foo\nfoo bar\nbar\nfoo bar";
-  file_util::WriteFile(dictionary_path, content.c_str(), content.length());
-  custom_dictionary->LoadDictionaryIntoCustomWordList(&loaded_custom_words);
+  file_util::WriteFile(path, content.c_str(), content.length());
+  SpellcheckCustomDictionary::LoadDictionaryIntoCustomWordList(
+      &loaded_custom_words, path);
   expected.push_back("bar");
   expected.push_back("foo");
   EXPECT_EQ(expected, loaded_custom_words);
@@ -236,35 +225,207 @@ TEST_F(SpellcheckCustomDictionaryTest,
   MessageLoop::current()->RunUntilIdle();
 }
 
-// Write to dicitonary should backup previous version and write the word to the
+// Write to dictionary should backup previous version and write the word to the
 // end of the dictionary. If the dictionary file is corrupted on disk, the
 // previous version should be reloaded.
 TEST_F(SpellcheckCustomDictionaryTest, CorruptedWriteShouldBeRecovered) {
-  FilePath dictionary_path(
-      profile_->GetPath().Append(chrome::kCustomDictionaryFileName));
-  SpellcheckService* spellcheck_service =
-      SpellcheckServiceFactory::GetForProfile(profile_.get());
-  SpellcheckCustomDictionary* custom_dictionary =
-      spellcheck_service->GetCustomDictionary();
+  FilePath path = profile_->GetPath().Append(chrome::kCustomDictionaryFileName);
   WordList loaded_custom_words;
   WordList expected;
 
   std::string content = "foo\nbar";
-  file_util::WriteFile(dictionary_path, content.c_str(), content.length());
-  custom_dictionary->LoadDictionaryIntoCustomWordList(&loaded_custom_words);
+  file_util::WriteFile(path, content.c_str(), content.length());
+  SpellcheckCustomDictionary::LoadDictionaryIntoCustomWordList(
+      &loaded_custom_words, path);
   expected.push_back("bar");
   expected.push_back("foo");
   EXPECT_EQ(expected, loaded_custom_words);
 
-  custom_dictionary->WriteWordToCustomDictionary("baz");
+  SpellcheckCustomDictionary::WriteWordToCustomDictionary("baz", path);
   content.clear();
-  file_util::ReadFileToString(dictionary_path, &content);
+  file_util::ReadFileToString(path, &content);
   content.append("corruption");
-  file_util::WriteFile(dictionary_path, content.c_str(), content.length());
-  custom_dictionary->LoadDictionaryIntoCustomWordList(&loaded_custom_words);
+  file_util::WriteFile(path, content.c_str(), content.length());
+  SpellcheckCustomDictionary::LoadDictionaryIntoCustomWordList(
+      &loaded_custom_words, path);
   EXPECT_EQ(expected, loaded_custom_words);
 
   // Flush the loop now to prevent service init tasks from being run during
   // TearDown();
   MessageLoop::current()->RunUntilIdle();
 }
+
+TEST_F(SpellcheckCustomDictionaryTest,
+       GetAllSyncDataAccuratelyReflectsDictionaryState) {
+  SpellcheckCustomDictionary* dictionary =
+      SpellcheckServiceFactory::GetForProfile(
+          profile_.get())->GetCustomDictionary();
+
+  syncer::SyncDataList data = dictionary->GetAllSyncData(syncer::DICTIONARY);
+  EXPECT_TRUE(data.empty());
+
+  EXPECT_TRUE(dictionary->CustomWordAddedLocally("foo"));
+  EXPECT_TRUE(dictionary->CustomWordAddedLocally("bar"));
+  data = dictionary->GetAllSyncData(syncer::DICTIONARY);
+  EXPECT_EQ(2UL, data.size());
+  std::vector<std::string> words;
+  words.push_back("foo");
+  words.push_back("bar");
+  for (size_t i = 0; i < data.size(); i++) {
+    EXPECT_TRUE(data[i].GetSpecifics().has_dictionary());
+    EXPECT_EQ(syncer::DICTIONARY, data[i].GetDataType());
+    EXPECT_EQ(words[i], data[i].GetTag());
+    EXPECT_EQ(words[i], data[i].GetSpecifics().dictionary().word());
+  }
+
+  EXPECT_TRUE(dictionary->CustomWordRemovedLocally("foo"));
+  EXPECT_TRUE(dictionary->CustomWordRemovedLocally("bar"));
+  data = dictionary->GetAllSyncData(syncer::DICTIONARY);
+  EXPECT_TRUE(data.empty());
+
+  // Flush the loop now to prevent service init tasks from being run during
+  // TearDown();
+  MessageLoop::current()->RunUntilIdle();

[groby-ooo-7-16, 2012/12/19 22:16:00]

Why not run them during TearDown()? It seems _every_ test needs to do this at
the end, so TearDown seems the logical place.

[please use gerrit instead, 2012/12/22 03:20:19]

We need to run RunUntilIdle here before the spellcheck goes out of scope and
gets destroyed. SpellcheckHunspellDictionary posts tasks to the file thread that
use member variables. If the object is destroyed before the file-thread task
finishes, unit tests crash.

+}
+
+TEST_F(SpellcheckCustomDictionaryTest, GetAllSyncDataHasLimit) {
+  SpellcheckCustomDictionary* dictionary =
+      SpellcheckServiceFactory::GetForProfile(
+          profile_.get())->GetCustomDictionary();
+
+  for (size_t i = 0; i < LIMIT - 1; i++) {
+    EXPECT_TRUE(dictionary->CustomWordAddedLocally(
+        "foo" + base::Uint64ToString(i)));
+  }
+  EXPECT_EQ(LIMIT - 1, dictionary->GetWords().size());
+  EXPECT_EQ(LIMIT - 1, dictionary->GetAllSyncData(syncer::DICTIONARY).size());
+
+  EXPECT_TRUE(dictionary->CustomWordAddedLocally("baz"));
+  EXPECT_EQ(LIMIT, dictionary->GetWords().size());
+  EXPECT_EQ(LIMIT, dictionary->GetAllSyncData(syncer::DICTIONARY).size());
+
+  EXPECT_TRUE(dictionary->CustomWordAddedLocally("bar"));
+  EXPECT_EQ(LIMIT + 1, dictionary->GetWords().size());
+  EXPECT_EQ(LIMIT, dictionary->GetAllSyncData(syncer::DICTIONARY).size());
+
+  EXPECT_TRUE(dictionary->CustomWordAddedLocally("snafoo"));
+  EXPECT_EQ(LIMIT + 2, dictionary->GetWords().size());
+  EXPECT_EQ(LIMIT, dictionary->GetAllSyncData(syncer::DICTIONARY).size());
+
+  // Flush the loop now to prevent service init tasks from being run during
+  // TearDown();
+  MessageLoop::current()->RunUntilIdle();
+}
+
+TEST_F(SpellcheckCustomDictionaryTest, ProcessSyncChanges) {
+  SpellcheckService* spellcheck_service =
+      SpellcheckServiceFactory::GetForProfile(profile_.get());
+  SpellcheckCustomDictionary* dictionary =
+      spellcheck_service->GetCustomDictionary();
+
+  dictionary->CustomWordAddedLocally("foo");
+  dictionary->CustomWordAddedLocally("bar");
+
+  syncer::SyncChangeList changes;
+  {
+    // Add existing word.
+    std::string word = "foo";
+    sync_pb::EntitySpecifics specifics;
+    specifics.mutable_dictionary()->set_word(word);
+    changes.push_back(syncer::SyncChange(
+        FROM_HERE,
+        syncer::SyncChange::ACTION_ADD,
+        syncer::SyncData::CreateLocalData(word, word, specifics)));
+  }
+  {
+    // Add invalid word.
+    std::string word = "foo bar";
+    sync_pb::EntitySpecifics specifics;
+    specifics.mutable_dictionary()->set_word(word);
+    changes.push_back(syncer::SyncChange(
+        FROM_HERE,
+        syncer::SyncChange::ACTION_ADD,
+        syncer::SyncData::CreateLocalData(word, word, specifics)));
+  }
+  {
+    // Add valid word.
+    std::string word = "baz";
+    sync_pb::EntitySpecifics specifics;
+    specifics.mutable_dictionary()->set_word(word);
+    changes.push_back(syncer::SyncChange(
+        FROM_HERE,
+        syncer::SyncChange::ACTION_ADD,
+        syncer::SyncData::CreateLocalData(word, word, specifics)));
+  }
+  {
+    // Remove missing word.
+    std::string word = "snafoo";
+    sync_pb::EntitySpecifics specifics;
+    specifics.mutable_dictionary()->set_word(word);
+    changes.push_back(syncer::SyncChange(
+        FROM_HERE,
+        syncer::SyncChange::ACTION_DELETE,
+        syncer::SyncData::CreateLocalData(word, word, specifics)));
+  }
+  {
+    // Remove existing word.
+    std::string word = "bar";
+    sync_pb::EntitySpecifics specifics;
+    specifics.mutable_dictionary()->set_word(word);
+    changes.push_back(syncer::SyncChange(
+        FROM_HERE,
+        syncer::SyncChange::ACTION_DELETE,
+        syncer::SyncData::CreateLocalData(word, word, specifics)));
+  }
+
+  EXPECT_FALSE(dictionary->ProcessSyncChanges(FROM_HERE, changes).IsSet());
+
+  const chrome::spellcheck_common::WordList& words = dictionary->GetWords();
+  EXPECT_EQ(2UL, words.size());
+  EXPECT_EQ(words.end(), std::find(words.begin(), words.end(), "bar"));
+  EXPECT_NE(words.end(), std::find(words.begin(), words.end(), "foo"));
+  EXPECT_NE(words.end(), std::find(words.begin(), words.end(), "baz"));
+
+  // Flush the loop now to prevent service init tasks from being run during
+  // TearDown();
+  MessageLoop::current()->RunUntilIdle();
+}
+
+TEST_F(SpellcheckCustomDictionaryTest, MergeDataAndStartSyncing) {
+  SpellcheckService* spellcheck_service =
+      SpellcheckServiceFactory::GetForProfile(profile_.get());
+  SpellcheckCustomDictionary* custom_dictionary =
+      spellcheck_service->GetCustomDictionary();
+  TestingProfile profile2;
+  SpellcheckService* spellcheck_service2 =
+      static_cast<SpellcheckService*>(
+          SpellcheckServiceFactory::GetInstance()->SetTestingFactoryAndUse(
+              &profile2, &BuildSpellcheckService));
+  SpellcheckCustomDictionary* custom_dictionary2 =
+      spellcheck_service2->GetCustomDictionary();
+
+  custom_dictionary->AddWord("foo");
+  custom_dictionary2->AddWord("bar");
+  custom_dictionary2->AddWord("baz");
+
+  EXPECT_FALSE(custom_dictionary->MergeDataAndStartSyncing(
+      syncer::DICTIONARY,
+      custom_dictionary2->GetAllSyncData(syncer::DICTIONARY),
+      scoped_ptr<syncer::SyncChangeProcessor>(custom_dictionary2),
+      scoped_ptr<syncer::SyncErrorFactory>(new syncer::SyncErrorFactoryMock)).
+          error().IsSet());
+  custom_dictionary->StopSyncingForTesting();
+
+  WordList words = custom_dictionary->GetWords();
+  WordList words2 = custom_dictionary2->GetWords();
+  EXPECT_EQ(words.size(), words2.size());
+
+  std::sort(words.begin(), words.end());
+  std::sort(words2.begin(), words2.end());
+  for (size_t i = 0; i < words.size(); ++i)
+    EXPECT_EQ(words[i], words2[i]);
+
+  // Flush the loop now to prevent service init tasks from being run during
+  // TearDown();
+  MessageLoop::current()->RunUntilIdle();
+}