Editing the custom spelling 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 a67596ebd0a974a6ac0bc09e77bcb6848e6db24a..d6832e88edb5d1f4aabe446448441c22fcceb1bb 100644
--- a/chrome/browser/spellchecker/spellcheck_custom_dictionary.cc
+++ b/chrome/browser/spellchecker/spellcheck_custom_dictionary.cc
@@ -32,6 +32,27 @@ SpellcheckCustomDictionary::SpellcheckCustomDictionary(Profile* profile)
 SpellcheckCustomDictionary::~SpellcheckCustomDictionary() {
 }
 
+void SpellcheckCustomDictionary::Load() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  BrowserThread::PostTaskAndReplyWithResult<WordList*>(
+      BrowserThread::FILE,
+      FROM_HERE,
+      base::Bind(&SpellcheckCustomDictionary::LoadDictionary,
+                 base::Unretained(this)),
+      base::Bind(&SpellcheckCustomDictionary::SetCustomWordList,
+                 weak_ptr_factory_.GetWeakPtr()));
+}
+
+const WordList& SpellcheckCustomDictionary::GetWords() const {
+  return words_;
+}
+
+WordList* SpellcheckCustomDictionary::LoadDictionary() {
+  WordList* custom_words = new WordList;
+  LoadDictionaryIntoCustomWordList(custom_words);
+  return custom_words;
+}
+
 void SpellcheckCustomDictionary::LoadDictionaryIntoCustomWordList(

[rpetterson, 2012/11/14 19:47:09]

Why doesn't this function notify observers of OnCustomDictionaryLoaded as well?

[please use gerrit instead, 2012/11/14 22:54:46]

This method does not modify the private member variable |custom_words_|. If it
was not for |custom_dictionary_path_|, this method would have been static. 

This method runs on the file thread, whereas the listeners are on the UI thread.

Here is how loading the dictionary works here:

1) [UI]   Load()
2) [FILE] LoadDictionary()
3) [FILE] LoadDictionaryIntocustomWordList()
4) [UI]   SetCustomWordList()
5) [UI]   OnCustomDictionaryLoaded()

     WordList* custom_words) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
@@ -47,12 +68,31 @@ void SpellcheckCustomDictionary::LoadDictionaryIntoCustomWordList(
     mem_fun_ref(&std::string::empty)), custom_words->end());
 }
 
-void SpellcheckCustomDictionary::Load() {
-  custom_words_.clear();
-  // We are not guaranteed to be on the FILE thread so post the task.
-  BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE, base::Bind(
-      &SpellcheckCustomDictionary::LoadDictionaryIntoCustomWordList,
-      base::Unretained(this), &custom_words_));
+void SpellcheckCustomDictionary::SetCustomWordList(WordList* custom_words) {

[rpetterson, 2012/11/14 19:47:09]

Does this need to be called on the UI thread? If so, should we have a DCHECK? It
looks like it will be when it's called from in Load(), but I'm assuming someone
can call it elsewhere.

[please use gerrit instead, 2012/11/14 22:54:46]

Adding dchecks for the correct browser thread in all methods.

+  words_.clear();
+  if (custom_words)
+    std::swap(words_, *custom_words);
+  std::vector<Observer*>::iterator it;
+  for (it = observers_.begin(); it != observers_.end(); ++it)
+    (*it)->OnCustomDictionaryLoaded();
+}
+
+void SpellcheckCustomDictionary::AddWord(const std::string& word) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  CustomWordAddedLocally(word);
+
+  BrowserThread::PostTaskAndReply(BrowserThread::FILE, FROM_HERE,
+      base::Bind(&SpellcheckCustomDictionary::WriteWordToCustomDictionary,
+                 base::Unretained(this), word),
+      base::Bind(&SpellcheckCustomDictionary::AddWordComplete,
+                 weak_ptr_factory_.GetWeakPtr(), word));
+}
+
+void SpellcheckCustomDictionary::CustomWordAddedLocally(
+    const std::string& word) {
+  words_.push_back(word);
+  // TODO(rlp): record metrics on custom word size
 }
 
 void SpellcheckCustomDictionary::WriteWordToCustomDictionary(
@@ -72,44 +112,89 @@ void SpellcheckCustomDictionary::WriteWordToCustomDictionary(
   }
 }
 
-void SpellcheckCustomDictionary::CustomWordAddedLocally(
-    const std::string& word) {
-  custom_words_.push_back(word);
-  // TODO(rlp): record metrics on custom word size
-}
-
-bool SpellcheckCustomDictionary::SetCustomWordList(WordList* custom_words) {
-  if (!custom_words)
-    return false;
-  custom_words_.clear();
-  std::swap(custom_words_, *custom_words);
-  return true;
-}
+void SpellcheckCustomDictionary::AddWordComplete(const std::string& word) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-const WordList& SpellcheckCustomDictionary::GetCustomWords() const {
-  return custom_words_;
+  for (content::RenderProcessHost::iterator i(
+          content::RenderProcessHost::AllHostsIterator());
+       !i.IsAtEnd(); i.Advance()) {
+    i.GetCurrentValue()->Send(new SpellCheckMsg_WordAdded(word));
+  }
+  std::vector<Observer*>::iterator it;
+  for (it = observers_.begin(); it != observers_.end(); ++it)
+    (*it)->OnCustomDictionaryWordAdded(word);
 }
 
-void SpellcheckCustomDictionary::AddWord(const std::string& word) {
+void SpellcheckCustomDictionary::RemoveWord(const std::string& word) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  CustomWordAddedLocally(word);
+  CustomWordRemovedLocally(word);
 
   BrowserThread::PostTaskAndReply(BrowserThread::FILE, FROM_HERE,
-      base::Bind(&SpellcheckCustomDictionary::WriteWordToCustomDictionary,
+      base::Bind(&SpellcheckCustomDictionary::EraseWordFromCustomDictionary,
                  base::Unretained(this), word),
-      base::Bind(&SpellcheckCustomDictionary::AddWordComplete,
+      base::Bind(&SpellcheckCustomDictionary::RemoveWordComplete,
                  weak_ptr_factory_.GetWeakPtr(), word));
 }
 
-void SpellcheckCustomDictionary::AddWordComplete(const std::string& word) {
+void SpellcheckCustomDictionary::CustomWordRemovedLocally(
+    const std::string& word) {
+  WordList::iterator it = std::find(words_.begin(), words_.end(), word);
+  if (it != words_.end())
+    words_.erase(it);
+}
+
+void SpellcheckCustomDictionary::EraseWordFromCustomDictionary(
+    const std::string& word) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+  DCHECK(IsStringUTF8(word));
+

[rpetterson, 2012/11/14 19:47:09]

Replace the next 8 lines with LoadDictionaryIntoCustomWordList?

[please use gerrit instead, 2012/11/14 22:54:46]

Done.

+  std::string contents;
+  file_util::ReadFileToString(custom_dictionary_path_, &contents);
+  if (contents.empty())
+    return;
+
+  WordList custom_words;
+  base::SplitString(contents, '\n', &custom_words);
+  custom_words.erase(remove_if(custom_words.begin(), custom_words.end(),
+    mem_fun_ref(&std::string::empty)), custom_words.end());
+
+  char empty[] = {'\0'};
+  char separator[] = {'\n', '\0'};
+  file_util::WriteFile(custom_dictionary_path_, empty, 0);
+  for (WordList::iterator it = custom_words.begin();
+      it != custom_words.end();
+      ++it) {
+    std::string word_to_add = *it;
+    if (word.compare(word_to_add) != 0) {
+      file_util::AppendToFile(custom_dictionary_path_, word_to_add.c_str(),
+          word_to_add.length());
+      file_util::AppendToFile(custom_dictionary_path_, separator, 1);
+    }
+  }
+}
+
+void SpellcheckCustomDictionary::RemoveWordComplete(const std::string& word) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   for (content::RenderProcessHost::iterator i(
           content::RenderProcessHost::AllHostsIterator());
        !i.IsAtEnd(); i.Advance()) {
-    i.GetCurrentValue()->Send(new SpellCheckMsg_WordAdded(word));
+    i.GetCurrentValue()->Send(new SpellCheckMsg_WordRemoved(word));
   }
+  std::vector<Observer*>::iterator it;
+  for (it = observers_.begin(); it != observers_.end(); ++it)
+    (*it)->OnCustomDictionaryWordRemoved(word);
 }
 
+void SpellcheckCustomDictionary::AddObserver(Observer* observer) {
+  observers_.push_back(observer);
+}
 
+void SpellcheckCustomDictionary::RemoveObserver(Observer* observer) {
+  std::vector<Observer*>::iterator it = std::find(observers_.begin(),
+                                                  observers_.end(),
+                                                  observer);
+  if (it != observers_.end())
+    observers_.erase(it);
+}