Sync user's custom spellcheck dictionary

chrome/browser/spellchecker/spellcheck_custom_dictionary.cc

Index: chrome/browser/spellchecker/spellcheck_custom_dictionary.cc
diff --git a/chrome/browser/spellchecker/spellcheck_custom_dictionary.cc b/chrome/browser/spellchecker/spellcheck_custom_dictionary.cc
index 588e3675c2fa3d8e22620f578cd497abef322e16..85cb9b86ec53fa0c7d1b9d243d10c91104520450 100644
--- a/chrome/browser/spellchecker/spellcheck_custom_dictionary.cc
+++ b/chrome/browser/spellchecker/spellcheck_custom_dictionary.cc
@@ -9,25 +9,42 @@
 #include "base/file_util.h"
 #include "base/files/important_file_writer.h"
 #include "base/md5.h"
+#include "base/string_number_conversions.h"
 #include "base/string_split.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/spellcheck_messages.h"
 #include "content/public/browser/browser_thread.h"
-#include "content/public/browser/render_process_host.h"
+#include "sync/api/sync_change.h"
+#include "sync/api/sync_data.h"
+#include "sync/api/sync_error_factory.h"
+#include "sync/protocol/sync.pb.h"
+#include "third_party/hunspell/src/hunspell/hunspell.hxx"

[groby-ooo-7-16, 2013/01/10 22:04:44]

We probably shouldn't include hunspell here - technically, this is
cross-platform code.

[please use gerrit instead, 2013/01/12 02:50:46]

Defined a chrome::spellcheck_common::MAX_CUSTOM_DICTIONARY_WORD_BYTES variable
in chrome/common/spellcheck_common.h that can be shared between here and
chrome/renderer/spellchecker/hunspell_engine.cc.

 
 using content::BrowserThread;
 using chrome::spellcheck_common::WordList;
 
+namespace custom_dictionary {
 namespace {
 
+// Filename extension for backup dictionary file.
 const FilePath::CharType BACKUP_EXTENSION[] = FILE_PATH_LITERAL("backup");
+
+// Prefix for the checksum in the dictionary file.
 const char CHECKSUM_PREFIX[] = "checksum_v1 = ";
 
+// The status of the checksum in a custom spellcheck dictionary.
+enum ChecksumStatus {
+  VALID_CHECKSUM,
+  INVALID_CHECKSUM,
+};
+
 // Loads the lines from the file at |file_path| into the |lines| container. If
-// the file has a valid checksum, then returns |true|. If the file has an
-// invalid checksum, then returns |false| and clears |lines|.
-bool LoadFile(FilePath file_path, std::vector<std::string>* lines) {
+// the file has a valid checksum, then returns ChecksumStatus::VALID. If the
+// file has an invalid checksum, then returns ChecksumStatus::INVALID and clears
+// |lines|. Must be called on the file thread.
+ChecksumStatus LoadFile(FilePath file_path, std::vector<std::string>* lines) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   lines->clear();
   std::string contents;
   file_util::ReadFileToString(file_path, &contents);
@@ -36,59 +53,109 @@ bool LoadFile(FilePath file_path, std::vector<std::string>* lines) {
     std::string checksum = contents.substr(pos + strlen(CHECKSUM_PREFIX));
     contents = contents.substr(0, pos);
     if (checksum != base::MD5String(contents))
-      return false;
+      return INVALID_CHECKSUM;
   }
   TrimWhitespaceASCII(contents, TRIM_ALL, &contents);
   base::SplitString(contents, '\n', lines);
-  return true;
-}
-
-bool IsValidWord(const std::string& word) {
-  return IsStringUTF8(word) && word.length() <= 128 && word.length() > 0 &&
-      std::string::npos == word.find_first_of(kWhitespaceASCII);
+  return VALID_CHECKSUM;
 }
 
+// Returns true for invalid words and false for valid words. Useful for
+// std::remove_if() calls.
 bool IsInvalidWord(const std::string& word) {
-  return !IsValidWord(word);
+  return !IsStringUTF8(word) || word.length() >= MAXWORDLEN || word.empty() ||
+      word.find_first_of(kWhitespaceASCII) != std::string::npos;
 }
 
-}  // namespace
+// Loads the custom spellcheck dictionary from |path| into |custom_words|. If
+// the dictionary checksum is not valid, but backup checksum is valid, then
+// restores the backup and loads that into |custom_words| instead. If the backup
+// is invalid too, then clears |custom_words|. Must be called on the file
+// thread.
+void LoadDictionaryFileReliably(WordList* custom_words, const FilePath& path) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
 
-SpellcheckCustomDictionary::SpellcheckCustomDictionary(Profile* profile)
-    : SpellcheckDictionary(profile),
-      custom_dictionary_path_(),
-      weak_ptr_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
-  DCHECK(profile);
-  custom_dictionary_path_ =
-      profile_->GetPath().Append(chrome::kCustomDictionaryFileName);
-}
+  // Load the contents and verify the checksum.
+  if (LoadFile(path, custom_words) == VALID_CHECKSUM)
+    return;
 
-SpellcheckCustomDictionary::~SpellcheckCustomDictionary() {
+  // Checksum is not valid. See if there's a backup.
+  FilePath backup = path.AddExtension(BACKUP_EXTENSION);
+  if (!file_util::PathExists(backup))
+    return;
+
+  // Load the backup and verify its checksum.
+  if (LoadFile(backup, custom_words) != VALID_CHECKSUM)
+    return;
+
+  // Backup checksum is valid. Restore the backup.
+  file_util::CopyFile(backup, path);
 }
 
-void SpellcheckCustomDictionary::Load() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+// Backs up the original dictionary, saves |custom_words| and its checksum into
+// the custom spellcheck dictionary at |path|. Does not take ownership of
+// |custom_words|. Must be called on the file thread.
+void SaveDictionaryFileReliably(
+    const WordList* custom_words,
+    const FilePath& path) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+  DCHECK(custom_words);
 
-  BrowserThread::PostTaskAndReplyWithResult<WordList*>(
-      BrowserThread::FILE,
-      FROM_HERE,
-      base::Bind(&SpellcheckCustomDictionary::LoadDictionary,
-                 base::Unretained(this)),
-      base::Bind(&SpellcheckCustomDictionary::SetCustomWordListAndDelete,
-                 weak_ptr_factory_.GetWeakPtr()));
+  std::stringstream content;
+  for (WordList::const_iterator it = custom_words->begin();
+       it != custom_words->end();
+       ++it) {
+    content << *it << '\n';
+  }
+  std::string checksum = base::MD5String(content.str());
+  content << CHECKSUM_PREFIX << checksum;
+
+  file_util::CopyFile(path, path.AddExtension(BACKUP_EXTENSION));
+  base::ImportantFileWriter::WriteFileAtomically(path, content.str());
 }
 
-const WordList& SpellcheckCustomDictionary::GetWords() const {
-  return words_;
+// Applies the change in |dictionary_change| to the custom spellcheck dictionary
+// at |path|. Assumes that |dictionary_change| has already been processed to
+// clean the words that cannot be added or removed. Deletes |dictionary_change|
+// when done. Must be called on the file thread.
+void WriteAndEraseWordsInCustomDictionary(
+    scoped_ptr<SpellcheckCustomDictionary::Change> dictionary_change,
+    const FilePath& path) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+  DCHECK(dictionary_change.get());
+
+  WordList custom_words;
+  LoadDictionaryFileReliably(&custom_words, path);
+
+  // Add words.
+  custom_words.insert(custom_words.end(),
+                      dictionary_change->to_add().begin(),
+                      dictionary_change->to_add().end());
+
+  // Remove words.
+  std::sort(custom_words.begin(), custom_words.end());
+  std::sort(dictionary_change->to_remove().begin(),
+            dictionary_change->to_remove().end());
+  WordList remaining;
+  std::set_difference(custom_words.begin(),
+                      custom_words.end(),
+                      dictionary_change->to_remove().begin(),
+                      dictionary_change->to_remove().end(),
+                      std::back_inserter(remaining));
+  std::swap(custom_words, remaining);
+
+  SaveDictionaryFileReliably(&custom_words, path);
 }
 
-void SpellcheckCustomDictionary::LoadDictionaryIntoCustomWordList(
-    WordList* custom_words) {
+}  // namespace
+
+scoped_ptr<WordList> LoadDictionary(const FilePath& path) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
 
-  LoadDictionaryFileReliably(custom_words);
+  scoped_ptr<WordList> custom_words(new WordList);
+  LoadDictionaryFileReliably(custom_words.get(), path);
   if (custom_words->empty())
-    return;
+    return custom_words.Pass();
 
   // Clean up the dictionary file contents by removing duplicates and invalid
   // words.
@@ -97,123 +164,152 @@ void SpellcheckCustomDictionary::LoadDictionaryIntoCustomWordList(
                       custom_words->end());
   custom_words->erase(std::remove_if(custom_words->begin(),
                                      custom_words->end(),
-                                     IsInvalidWord),
+                                     custom_dictionary::IsInvalidWord),
                       custom_words->end());
 
-  SaveDictionaryFileReliably(*custom_words);
-}
+  SaveDictionaryFileReliably(custom_words.get(), path);
 
-void SpellcheckCustomDictionary::SetCustomWordList(WordList* custom_words) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  return custom_words.Pass();
+}
 
-  words_.clear();
-  if (custom_words)
-    std::swap(words_, *custom_words);
+void WriteWordToCustomDictionary(
+    const std::string& word,
+    const FilePath& path) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+  scoped_ptr<SpellcheckCustomDictionary::Change> dictionary_change(
+      new SpellcheckCustomDictionary::Change);
+  dictionary_change->AddWord(word);
+  WriteAndEraseWordsInCustomDictionary(dictionary_change.Pass(), path);
+}
 
-  FOR_EACH_OBSERVER(Observer, observers_, OnCustomDictionaryLoaded());
+void EraseWordFromCustomDictionary(
+    const std::string& word,
+    const FilePath& path) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+  scoped_ptr<SpellcheckCustomDictionary::Change> dictionary_change(
+      new SpellcheckCustomDictionary::Change);
+  dictionary_change->RemoveWord(word);
+  WriteAndEraseWordsInCustomDictionary(dictionary_change.Pass(), path);
 }
 
-bool SpellcheckCustomDictionary::AddWord(const std::string& word) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  if (!IsValidWord(word))
-    return false;
+}  // namespace custom_dictionary
+
+SpellcheckCustomDictionary::Change::Result::Result()
+    : invalid_(false),
+      duplicate_(false),
+      missing_(false) {
+}
 
-  if (!CustomWordAddedLocally(word))
-    return false;
+SpellcheckCustomDictionary::Change::Result::Result(
+        const SpellcheckCustomDictionary::Change::Result& other)
+    : invalid_(other.detected_invalid_words()),
+      duplicate_(other.detected_duplicate_words()),
+      missing_(other.detected_missing_words()) {
+}
 
-  BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
-      base::Bind(&SpellcheckCustomDictionary::WriteWordToCustomDictionary,
-                 base::Unretained(this), word));
+SpellcheckCustomDictionary::Change::Result::~Result() {
+}
 
-  for (content::RenderProcessHost::iterator i(
-          content::RenderProcessHost::AllHostsIterator());
-       !i.IsAtEnd(); i.Advance()) {
-    i.GetCurrentValue()->Send(new SpellCheckMsg_WordAdded(word));
-  }
+SpellcheckCustomDictionary::Change::Change() {
+}
 
-  FOR_EACH_OBSERVER(Observer, observers_, OnCustomDictionaryWordAdded(word));
+SpellcheckCustomDictionary::Change::Change(

[groby-ooo-7-16, 2013/01/10 22:04:44]

Why pointer magic, and not just a copy ctor? (In fact, why do we need to copy
Change at all?)

[please use gerrit instead, 2013/01/12 02:50:46]

No longer used, removed.

+    const SpellcheckCustomDictionary::Change* other) {
+  DCHECK(other);
 
-  return true;
+  AddWords(&other->to_add());
+  RemoveWords(&other->to_remove());
 }
 
-bool SpellcheckCustomDictionary::CustomWordAddedLocally(
-    const std::string& word) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DCHECK(IsValidWord(word));
+SpellcheckCustomDictionary::Change::~Change() {
+}
 
-  WordList::iterator it = std::find(words_.begin(), words_.end(), word);
-  if (it == words_.end()) {
-    words_.push_back(word);
-    return true;
-  }
-  return false;
-  // TODO(rlp): record metrics on custom word size
+void SpellcheckCustomDictionary::Change::AddWord(const std::string& word) {
+  to_add_.push_back(word);
 }
 
-void SpellcheckCustomDictionary::WriteWordToCustomDictionary(
-    const std::string& word) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
-  DCHECK(IsValidWord(word));
+void SpellcheckCustomDictionary::Change::RemoveWord(const std::string& word) {
+  to_remove_.push_back(word);
+}
 
-  WordList custom_words;
-  LoadDictionaryFileReliably(&custom_words);
-  custom_words.push_back(word);
-  SaveDictionaryFileReliably(custom_words);
+void SpellcheckCustomDictionary::Change::AddWords(const WordList* words) {
+  to_add_.insert(to_add_.end(), words->begin(), words->end());
 }
 
-bool SpellcheckCustomDictionary::RemoveWord(const std::string& word) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  if (!IsValidWord(word))
-    return false;
+void SpellcheckCustomDictionary::Change::RemoveWords(const WordList* words) {
+  to_remove_.insert(to_remove_.end(), words->begin(), words->end());
+}
 
-  if (!CustomWordRemovedLocally(word))
-    return false;
+SpellcheckCustomDictionary::SpellcheckCustomDictionary(Profile* profile)
+    : SpellcheckDictionary(profile),
+      custom_dictionary_path_(),
+      weak_ptr_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)),
+      is_loaded_(false) {
+  DCHECK(profile);
 

[groby-ooo-7-16, 2013/01/10 22:04:44]

Kill a newline, save some space :)

[please use gerrit instead, 2013/01/12 02:50:46]

Killed newlines after all dchecks.

-  BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
-      base::Bind(&SpellcheckCustomDictionary::EraseWordFromCustomDictionary,
-                 base::Unretained(this), word));
+  custom_dictionary_path_ =
+      profile_->GetPath().Append(chrome::kCustomDictionaryFileName);
+}
 
-  for (content::RenderProcessHost::iterator i(
-          content::RenderProcessHost::AllHostsIterator());
-       !i.IsAtEnd(); i.Advance()) {
-    i.GetCurrentValue()->Send(new SpellCheckMsg_WordRemoved(word));
-  }
+SpellcheckCustomDictionary::~SpellcheckCustomDictionary() {
+}
 
-  FOR_EACH_OBSERVER(Observer, observers_, OnCustomDictionaryWordRemoved(word));
+const WordList& SpellcheckCustomDictionary::GetWords() const {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  return true;
+  return words_;
 }
 
-bool SpellcheckCustomDictionary::CustomWordRemovedLocally(
+SpellcheckCustomDictionary::Change::Result SpellcheckCustomDictionary::AddWord(
     const std::string& word) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  DCHECK(IsValidWord(word));
 
-  WordList::iterator it = std::find(words_.begin(), words_.end(), word);
-  if (it != words_.end()) {
-    words_.erase(it);
-    return true;
-  }
-  return false;
+  scoped_ptr<Change> dictionary_change(new Change);
+  dictionary_change->AddWord(word);
+  Apply(dictionary_change.get());
+  Change::Result result(dictionary_change->result());
+  Notify(dictionary_change.get());
+  Sync(dictionary_change.get());
+  Save(dictionary_change.Pass());
+  return result;
 }
 
-void SpellcheckCustomDictionary::EraseWordFromCustomDictionary(
-    const std::string& word) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
-  DCHECK(IsValidWord(word));
+SpellcheckCustomDictionary::Change::Result
+    SpellcheckCustomDictionary::CustomWordAddedLocally(
+        const std::string& word) {
+  scoped_ptr<Change> dictionary_change(new Change);
+  dictionary_change->AddWord(word);
+  Apply(dictionary_change.get());
+  Notify(dictionary_change.get());
+  Sync(dictionary_change.get());
+  return dictionary_change->result();
+}
 
-  WordList custom_words;
-  LoadDictionaryFileReliably(&custom_words);
-  if (custom_words.empty())
-    return;
+SpellcheckCustomDictionary::Change::Result
+    SpellcheckCustomDictionary::RemoveWord(const std::string& word) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  WordList::iterator it = std::find(custom_words.begin(),
-                                    custom_words.end(),
-                                    word);
-  if (it != custom_words.end())
-    custom_words.erase(it);
+  scoped_ptr<Change> dictionary_change(new Change);
+  dictionary_change->RemoveWord(word);
+  Apply(dictionary_change.get());
+  Change::Result result(dictionary_change->result());

[groby-ooo-7-16, 2013/01/10 22:04:44]

Why doesn't this just call RemoveWordLocally?

[please use gerrit instead, 2013/01/12 02:50:46]

I deleted RemoveWordLocally and AddWordLocally completely.

+  Notify(dictionary_change.get());
+  Sync(dictionary_change.get());
+  Save(dictionary_change.Pass());
+  return result;
+}
+
+SpellcheckCustomDictionary::Change::Result

[groby-ooo-7-16, 2013/01/10 19:53:03]

Oy. I apologize for the confusion - in my previous comment, I intended for the
_public_ API to remain bool. (That means Change can be a private class, nobody
needs to see it)

[please use gerrit instead, 2013/01/12 02:50:46]

I moved the public API back to bool. SpellcheckCustomDictionary::Change is used
to notify observers of changes in the dictionary, so let's keep it public, but
make Result into a private struct.

+    SpellcheckCustomDictionary::CustomWordRemovedLocally(
+        const std::string& word) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  SaveDictionaryFileReliably(custom_words);
+  scoped_ptr<Change> dictionary_change(new Change);
+  dictionary_change->RemoveWord(word);
+  Apply(dictionary_change.get());
+  Notify(dictionary_change.get());
+  Sync(dictionary_change.get());
+  return dictionary_change->result();
 }
 
 void SpellcheckCustomDictionary::AddObserver(Observer* observer) {
@@ -228,58 +324,300 @@ void SpellcheckCustomDictionary::RemoveObserver(Observer* observer) {
   observers_.RemoveObserver(observer);
 }
 
-WordList* SpellcheckCustomDictionary::LoadDictionary() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+bool SpellcheckCustomDictionary::IsLoaded() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  WordList* custom_words = new WordList;
-  LoadDictionaryIntoCustomWordList(custom_words);
-  return custom_words;
+  return is_loaded_;
 }
 
-void SpellcheckCustomDictionary::SetCustomWordListAndDelete(
-    WordList* custom_words) {
+bool SpellcheckCustomDictionary::IsSyncing() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  SetCustomWordList(custom_words);
-  delete custom_words;
+  return !!sync_processor_.get();
 }
 
-void SpellcheckCustomDictionary::LoadDictionaryFileReliably(
-    WordList* custom_words) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+void SpellcheckCustomDictionary::OnLoaded(scoped_ptr<WordList> custom_words) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(custom_words.get());
 
-  // Load the contents and verify the checksum.
-  if (LoadFile(custom_dictionary_path_, custom_words))
-    return;
+  scoped_ptr<Change> dictionary_change(new Change);
+  dictionary_change->AddWords(custom_words.get());
+  Apply(dictionary_change.get());

[groby-ooo-7-16, 2013/01/10 22:04:44]

Hm. It seems every Apply causes a Sync. Why not call Sync from Apply and be
done?

[please use gerrit instead, 2013/01/12 02:50:46]

We cannot call Sync from Apply because ProcessSyncChanges calls Apply without
Sync and MergeDataAndStartSyncing calls Apply and Sync with different data.

We cannot call Notify and Save from Apply because OnLoaded calls Apply without
Notify and Save.

I think the current approach is the easiest to understand.

+  Sync(dictionary_change.get());
 
-  // Checksum is not valid. See if there's a backup.
-  FilePath backup = custom_dictionary_path_.AddExtension(BACKUP_EXTENSION);
-  if (!file_util::PathExists(backup))
-    return;
+  is_loaded_ = true;
+  FOR_EACH_OBSERVER(Observer, observers_, OnCustomDictionaryLoaded());
+}
 
-  // Load the backup and verify its checksum.
-  if (!LoadFile(backup, custom_words))
-    return;
+void SpellcheckCustomDictionary::Load() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  // Backup checksum is valid. Restore the backup.
-  file_util::CopyFile(backup, custom_dictionary_path_);
+  BrowserThread::PostTaskAndReplyWithResult<scoped_ptr<WordList> >(
+      BrowserThread::FILE,
+      FROM_HERE,
+      base::Bind(&custom_dictionary::LoadDictionary,
+                 custom_dictionary_path_),
+      base::Bind(&SpellcheckCustomDictionary::OnLoaded,
+                 weak_ptr_factory_.GetWeakPtr()));
 }
 
-void SpellcheckCustomDictionary::SaveDictionaryFileReliably(
-    const WordList& custom_words) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+syncer::SyncMergeResult SpellcheckCustomDictionary::MergeDataAndStartSyncing(
+    syncer::ModelType type,
+    const syncer::SyncDataList& initial_sync_data,
+    scoped_ptr<syncer::SyncChangeProcessor> sync_processor,
+    scoped_ptr<syncer::SyncErrorFactory> sync_error_handler) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(!sync_processor_.get());
+  DCHECK(!sync_error_handler_.get());
+  DCHECK(sync_processor.get());
+  DCHECK(sync_error_handler.get());
+  DCHECK_EQ(syncer::DICTIONARY, type);
+
+  sync_processor_ = sync_processor.Pass();
+  sync_error_handler_ = sync_error_handler.Pass();
+
+  // Add remote words locally.
+  WordList sync_words;
+  std::string word;
+  for (syncer::SyncDataList::const_iterator it = initial_sync_data.begin();
+     it != initial_sync_data.end();
+     ++it) {
+    DCHECK_EQ(syncer::DICTIONARY, it->GetDataType());
+    sync_words.push_back(it->GetSpecifics().dictionary().word());
+  }
+  scoped_ptr<Change> to_change_locally(new Change);
+  to_change_locally->AddWords(&sync_words);
+  Apply(to_change_locally.get());
+  Notify(to_change_locally.get());
+  Save(to_change_locally.Pass());
+
+  // Add as many as possible local words remotely.
+  std::sort(words_.begin(), words_.end());
+  std::sort(sync_words.begin(), sync_words.end());
+  Change to_change_remotely;
+  std::set_difference(words_.begin(),
+                      words_.end(),
+                      sync_words.begin(),
+                      sync_words.end(),
+                      std::back_inserter(to_change_remotely.to_add()));
+  syncer::SyncMergeResult result(type);
+  result.set_error(Sync(&to_change_remotely));
+  return result;
+}
 
-  std::stringstream content;
-  for (WordList::const_iterator it = custom_words.begin();
-       it != custom_words.end();
+void SpellcheckCustomDictionary::StopSyncing(syncer::ModelType type) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK_EQ(syncer::DICTIONARY, type);
+
+  sync_processor_.reset();
+  sync_error_handler_.reset();
+}
+
+syncer::SyncDataList SpellcheckCustomDictionary::GetAllSyncData(
+    syncer::ModelType type) const {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK_EQ(syncer::DICTIONARY, type);
+
+  syncer::SyncDataList data;
+  std::string word;
+  size_t i = 0;
+  for (WordList::const_iterator it = words_.begin();
+       it != words_.end() &&
+           i < chrome::spellcheck_common::MAX_SYNCABLE_DICTIONARY_SIZE_IN_WORDS;
+       ++it, ++i) {
+    word = *it;
+    sync_pb::EntitySpecifics specifics;
+    specifics.mutable_dictionary()->set_word(word);
+    data.push_back(syncer::SyncData::CreateLocalData(word, word, specifics));
+  }
+  return data;
+}
+
+syncer::SyncError SpellcheckCustomDictionary::ProcessSyncChanges(
+    const tracked_objects::Location& from_here,
+    const syncer::SyncChangeList& change_list) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  scoped_ptr<Change> dictionary_change(new Change);
+  for (syncer::SyncChangeList::const_iterator it = change_list.begin();
+       it != change_list.end();
        ++it) {
-    content << *it << '\n';
+    DCHECK(it->IsValid());
+    std::string word = it->sync_data().GetSpecifics().dictionary().word();
+    switch (it->change_type()) {
+      case syncer::SyncChange::ACTION_ADD:
+        dictionary_change->AddWord(word);
+        break;
+      case syncer::SyncChange::ACTION_DELETE:
+        dictionary_change->RemoveWord(word);
+        break;
+      default:
+        return sync_error_handler_->CreateAndUploadError(
+            FROM_HERE,
+            "Processing sync changes failed on change type " +
+                syncer::SyncChange::ChangeTypeToString(it->change_type()));
+    }
   }
-  std::string checksum = base::MD5String(content.str());
-  content << CHECKSUM_PREFIX << checksum;
 
-  file_util::CopyFile(custom_dictionary_path_,
-                      custom_dictionary_path_.AddExtension(BACKUP_EXTENSION));
-  base::ImportantFileWriter::WriteFileAtomically(custom_dictionary_path_,
-                                                 content.str());
+  Apply(dictionary_change.get());
+  Notify(dictionary_change.get());
+  Save(dictionary_change.Pass());
+
+  return syncer::SyncError();
+}
+
+// TODO(rlp): record metrics on custom word size
+void SpellcheckCustomDictionary::Apply(
+    SpellcheckCustomDictionary::Change* dictionary_change) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(dictionary_change);
+
+  if (!dictionary_change->to_add().empty()) {
+    // Do not add duplicate words.
+    std::sort(dictionary_change->to_add().begin(),
+              dictionary_change->to_add().end());
+    std::sort(words_.begin(), words_.end());
+    WordList to_add;
+    std::set_difference(dictionary_change->to_add().begin(),

[groby-ooo-7-16, 2013/01/10 22:04:44]

That's similar to the sanitizing code at load time. Should we just unify the
two? (Maybe a "SanitizeData()" on Change?

[please use gerrit instead, 2013/01/12 02:50:46]

Good idea! Done.

+                        dictionary_change->to_add().end(),
+                        words_.begin(),
+                        words_.end(),
+                        std::back_inserter(to_add));
+    to_add.erase(std::unique(to_add.begin(), to_add.end()),
+                 to_add.end());
+    if (dictionary_change->to_add().size() != to_add.size())
+      dictionary_change->result().set_detected_duplicate_words();
+
+    // Do not add invalid words.
+    size_t size = to_add.size();
+    to_add.erase(std::remove_if(to_add.begin(),
+                                to_add.end(),
+                                custom_dictionary::IsInvalidWord),
+                 to_add.end());
+    if (size != to_add.size())
+      dictionary_change->result().set_detected_invalid_words();
+
+    // Add the words to the dictionary.
+    words_.insert(words_.end(), to_add.begin(), to_add.end());
+
+    // Clean up the dictionary change by removing duplicates and invalid words.
+    std::swap(dictionary_change->to_add(), to_add);
+  }
+
+  if (!dictionary_change->to_remove().empty()) {
+    // Do not remove words that are missing from the dictionary.
+    std::sort(dictionary_change->to_remove().begin(),
+              dictionary_change->to_remove().end());
+    std::sort(words_.begin(), words_.end());
+    WordList to_remove;
+    std::set_intersection(words_.begin(),
+                          words_.end(),
+                          dictionary_change->to_remove().begin(),
+                          dictionary_change->to_remove().end(),
+                          std::back_inserter(to_remove));
+    if (dictionary_change->to_remove().size() > to_remove.size())
+      dictionary_change->result().set_detected_missing_words();
+
+    // Remove the words from the dictionary.
+    WordList updated_words;
+    std::set_difference(words_.begin(),
+                        words_.end(),
+                        to_remove.begin(),
+                        to_remove.end(),
+                        std::back_inserter(updated_words));
+    std::swap(words_, updated_words);
+
+    // Clean up the dictionary change by removing missing words.
+    std::swap(dictionary_change->to_remove(), to_remove);
+  }
+}
+
+void SpellcheckCustomDictionary::Save(
+    scoped_ptr<SpellcheckCustomDictionary::Change> dictionary_change) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(dictionary_change.get());
+
+  BrowserThread::PostTask(
+      BrowserThread::FILE,
+      FROM_HERE,
+      base::Bind(
+          &custom_dictionary::WriteAndEraseWordsInCustomDictionary,
+          base::Passed(dictionary_change.Pass()),
+          custom_dictionary_path_));
+}
+
+syncer::SyncError SpellcheckCustomDictionary::Sync(
+    const SpellcheckCustomDictionary::Change* dictionary_change) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(dictionary_change);
+
+  syncer::SyncError error;
+  if (!IsSyncing() || dictionary_change->empty())
+    return error;
+
+  // The number of words on the sync server should not exceed the limits.
+  int server_size = static_cast<int>(words_.size()) -
+      static_cast<int>(dictionary_change->to_add().size());
+  int max_upload_size = std::max(
+      0,
+      static_cast<int>(
+          chrome::spellcheck_common::MAX_SYNCABLE_DICTIONARY_SIZE_IN_WORDS) -
+          server_size);
+  int upload_size = std::min(
+      static_cast<int>(dictionary_change->to_add().size()),
+      max_upload_size);
+
+  syncer::SyncChangeList sync_change_list;
+  std::string word;
+  int i = 0;
+
+  for (WordList::const_iterator it = dictionary_change->to_add().begin();
+       it != dictionary_change->to_add().end() && i < upload_size;
+       ++it, ++i) {
+    word = *it;
+    sync_pb::EntitySpecifics specifics;
+    specifics.mutable_dictionary()->set_word(word);
+    sync_change_list.push_back(syncer::SyncChange(
+        FROM_HERE,
+        syncer::SyncChange::ACTION_ADD,
+        syncer::SyncData::CreateLocalData(word, word, specifics)));
+  }
+
+  for (WordList::const_iterator it = dictionary_change->to_remove().begin();
+       it != dictionary_change->to_remove().end();
+       ++it) {
+    word = *it;
+    sync_pb::EntitySpecifics specifics;
+    specifics.mutable_dictionary()->set_word(word);
+    sync_change_list.push_back(syncer::SyncChange(
+        FROM_HERE,
+        syncer::SyncChange::ACTION_DELETE,
+        syncer::SyncData::CreateLocalData(word, word, specifics)));
+  }
+
+  // Send the changes to the sync processor.
+  error = sync_processor_->ProcessSyncChanges(FROM_HERE, sync_change_list);
+  if (error.IsSet())
+    return error;
+
+  // Turn off syncing of this dictionary if the server already has the maximum
+  // number of words.
+  if (words_.size() >
+          chrome::spellcheck_common::MAX_SYNCABLE_DICTIONARY_SIZE_IN_WORDS)
+    StopSyncing(syncer::DICTIONARY);
+
+  return error;
+}
+
+void  SpellcheckCustomDictionary::Notify(
+    const SpellcheckCustomDictionary::Change* dictionary_change) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(dictionary_change);
+
+  if (!IsLoaded() || dictionary_change->empty())
+    return;
+
+  FOR_EACH_OBSERVER(Observer,
+                    observers_,
+                    OnCustomDictionaryChanged(dictionary_change));
 }