Improve reliability of custom spelling dictionary file.

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 4510332e59e3032ee7b35bf26f640ff6f3f2885c..e3f2751373df501471d32f8df4be053447fb09fb 100644
--- a/chrome/browser/spellchecker/spellcheck_custom_dictionary.cc
+++ b/chrome/browser/spellchecker/spellcheck_custom_dictionary.cc
@@ -7,6 +7,8 @@
 #include <functional>
 
 #include "base/file_util.h"
+#include "base/files/important_file_writer.h"
+#include "base/md5.h"
 #include "base/string_split.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/chrome_constants.h"
@@ -19,11 +21,16 @@ using chrome::spellcheck_common::WordList;
 
 SpellcheckCustomDictionary::SpellcheckCustomDictionary(Profile* profile)
     : SpellcheckDictionary(profile),
-      custom_dictionary_path_(),
+      custom_dictionary_path_(profile->GetPath().Append(
+          chrome::kCustomDictionaryFileName)),
+      custom_dictionary_checksum_path_(profile->GetPath().Append(
+          chrome::kCustomDictionaryFileName).AddExtension("checksum")),
+      custom_dictionary_backup_path_(profile->GetPath().Append(
+          chrome::kCustomDictionaryFileName).AddExtension("backup")),
+      custom_dictionary_checksum_backup_path_(profile->GetPath().Append(
+          chrome::kCustomDictionaryFileName).AddExtension("checksum").
+                AddExtension("backup")),
       weak_ptr_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
-  DCHECK(profile);

[groby-ooo-7-16, 2012/12/03 17:12:44]

Why'd you kill the DCHECK?

[please use gerrit instead, 2012/12/03 23:16:51]

My mistake. It's back.

-  custom_dictionary_path_ =
-      profile_->GetPath().Append(chrome::kCustomDictionaryFileName);
 }
 
 SpellcheckCustomDictionary::~SpellcheckCustomDictionary() {
@@ -50,7 +57,8 @@ void SpellcheckCustomDictionary::LoadDictionaryIntoCustomWordList(
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
 
   std::string contents;
-  file_util::ReadFileToString(custom_dictionary_path_, &contents);
+  LoadDictionaryContentsReliably(&contents);
+
   if (contents.empty()) {
     custom_words->clear();
     return;
@@ -58,26 +66,22 @@ void SpellcheckCustomDictionary::LoadDictionaryIntoCustomWordList(
 
   base::SplitString(contents, '\n', custom_words);
 
-  // Erase duplicates.
+  // Clean up the dictionary file contents.

[groby-ooo-7-16, 2012/12/03 17:12:44]

... by removing duplicates and empty words. It's nice to hace that in the
comment, since stl/algorithms don't necessarily qualify as super-readable :)

[please use gerrit instead, 2012/12/03 23:16:51]

Done.

   std::sort(custom_words->begin(), custom_words->end());
   custom_words->erase(std::unique(custom_words->begin(), custom_words->end()),
                       custom_words->end());
-
-  // Clear out empty words.
-  custom_words->erase(remove_if(custom_words->begin(), custom_words->end(),
-    mem_fun_ref(&std::string::empty)), custom_words->end());
-
-  // Write out the clean file.
+  custom_words->erase(std::remove_if(custom_words->begin(),
+                                     custom_words->end(),
+                                     std::mem_fun_ref(&std::string::empty)),
+                      custom_words->end());
   std::stringstream ss;
   for (WordList::iterator it = custom_words->begin();
        it != custom_words->end();
        ++it) {
     ss << *it << '\n';
   }
-  contents = ss.str();
-  file_util::WriteFile(custom_dictionary_path_,
-                       contents.c_str(),
-                       contents.length());
+
+  SaveDictionryContentsReliably(ss.str());
 }
 
 void SpellcheckCustomDictionary::SetCustomWordList(WordList* custom_words) {
@@ -131,18 +135,18 @@ bool SpellcheckCustomDictionary::CustomWordAddedLocally(
 void SpellcheckCustomDictionary::WriteWordToCustomDictionary(
     const std::string& word) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
-
-  // Stored in UTF-8.
   DCHECK(IsStringUTF8(word));
 
-  std::string word_to_add(word + "\n");
-  if (!file_util::PathExists(custom_dictionary_path_)) {
-    file_util::WriteFile(custom_dictionary_path_, word_to_add.c_str(),
-        word_to_add.length());
-  } else {
-    file_util::AppendToFile(custom_dictionary_path_, word_to_add.c_str(),
-        word_to_add.length());
-  }
+  std::string contents;
+  LoadDictionaryContentsReliably(&contents);
+
+  std::stringstream ss;
+  ss << contents;
+  if (contents.length() > 0 && contents[contents.length() - 1] != '\n')
+    ss << '\n';
+  ss << word << '\n';
+
+  SaveDictionryContentsReliably(ss.str());
 }
 
 bool SpellcheckCustomDictionary::RemoveWord(const std::string& word) {
@@ -185,22 +189,30 @@ void SpellcheckCustomDictionary::EraseWordFromCustomDictionary(
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
   DCHECK(IsStringUTF8(word));
 
-  WordList custom_words;
-  LoadDictionaryIntoCustomWordList(&custom_words);
-
-  const char empty[] = {'\0'};
-  const 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);
+  std::string contents;
+  LoadDictionaryContentsReliably(&contents);
+
+  size_t pos = 0;

[groby-ooo-7-16, 2012/12/03 17:12:44]

Can we just write out the whole dictionary here? For dictionaries less than a
disk block size (16K?) that's not going to be any more expensive, and it makes
much cleaner code.

[please use gerrit instead, 2012/12/03 23:16:51]

Done.

+  size_t found = contents.find(word, pos);
+  std::stringstream ss;
+  while (found != std::string::npos) {
+    if (found > pos)
+      ss.write(&contents[pos], found - pos );
+    if ((found > 0 && contents[found - 1] != '\n') ||
+        (found + word.length() < contents.length() &&
+         contents[found + word.length()] != '\n')) {
+      ss.write(&contents[found], word.length());
+      pos = found + word.length();
+    } else {
+      // skip the word and the newline character that follows it.
+      pos = found + word.length() + 1;
     }
+    found = contents.find(word, pos);
   }
+  if (contents.length() > pos)
+    ss.write(&contents[pos], contents.length() - pos);
+
+  SaveDictionryContentsReliably(ss.str());
 }
 
 void SpellcheckCustomDictionary::AddObserver(Observer* observer) {
@@ -234,3 +246,60 @@ void SpellcheckCustomDictionary::SetCustomWordListAndDelete(
   SetCustomWordList(custom_words);
   delete custom_words;
 }
+
+void SpellcheckCustomDictionary::LoadDictionaryContentsReliably(
+    std::string* contents) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+
+  file_util::ReadFileToString(custom_dictionary_path_, contents);
+
+  if (!file_util::PathExists(custom_dictionary_backup_path_) ||
+      !file_util::PathExists(custom_dictionary_checksum_backup_path_)) {
+    return;
+  }
+
+  if (file_util::PathExists(custom_dictionary_checksum_path_)) {
+    std::string checksum;
+    file_util::ReadFileToString(custom_dictionary_checksum_path_, &checksum);
+    base::MD5Digest digest;
+    base::MD5Sum(contents->c_str(), contents->length(), &digest);
+    if (checksum.compare(base::MD5DigestToBase16(digest)) == 0)
+      return;
+  }
+
+  std::string backup;
+  file_util::ReadFileToString(custom_dictionary_backup_path_, &backup);
+  std::string checksum_backup;
+  file_util::ReadFileToString(custom_dictionary_checksum_backup_path_,
+                              &checksum_backup);
+  base::MD5Digest backup_digest;
+  base::MD5Sum(backup.c_str(), backup.length(), &backup_digest);
+  if (checksum_backup.compare(base::MD5DigestToBase16(backup_digest)) != 0)
+    return;
+
+  *contents = backup;
+  file_util::CopyFile(custom_dictionary_checksum_backup_path_,
+                      custom_dictionary_checksum_path_);
+  file_util::CopyFile(custom_dictionary_backup_path_,
+                      custom_dictionary_path_);
+}
+
+void SpellcheckCustomDictionary::SaveDictionryContentsReliably(
+    const std::string& contents) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+
+  if (file_util::PathExists(custom_dictionary_checksum_path_)) {
+    file_util::CopyFile(custom_dictionary_checksum_path_,
+                        custom_dictionary_checksum_backup_path_);
+  }
+  if (file_util::PathExists(custom_dictionary_path_)) {
+    file_util::CopyFile(custom_dictionary_path_,
+                        custom_dictionary_backup_path_);
+  }
+  base::MD5Digest digest;
+  base::MD5Sum(contents.c_str(), contents.length(), &digest);
+  base::ImportantFileWriter::WriteFileAtomically(
+      custom_dictionary_checksum_path_, base::MD5DigestToBase16(digest));
+  base::ImportantFileWriter::WriteFileAtomically(
+      custom_dictionary_path_, contents);
+}