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..d526a80513ee9796fb5bc019d19ab46a9ba71536 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" |
@@ -20,7 +22,9 @@ using chrome::spellcheck_common::WordList; |
SpellcheckCustomDictionary::SpellcheckCustomDictionary(Profile* profile) |
: SpellcheckDictionary(profile), |
custom_dictionary_path_(), |
- weak_ptr_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) { |
+ weak_ptr_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)), |
+ backup_extension_(FILE_PATH_LITERAL("backup")), |
+ checksum_prefix_("checksum = ") { |
groby-ooo-7-16
2012/12/04 00:16:30
Why a prefix? Why not just make sure it's a valid
|
DCHECK(profile); |
custom_dictionary_path_ = |
profile_->GetPath().Append(chrome::kCustomDictionaryFileName); |
@@ -50,7 +54,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(); |
groby-ooo-7-16
2012/12/04 00:16:30
It's not your CL, but I suggest just calling custo
please use gerrit instead
2012/12/04 05:46:40
Done.
|
return; |
@@ -58,26 +63,23 @@ void SpellcheckCustomDictionary::LoadDictionaryIntoCustomWordList( |
base::SplitString(contents, '\n', custom_words); |
- // Erase duplicates. |
+ // Clean up the dictionary file contents by removing duplicates and empty |
+ // words. |
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(), |
groby-ooo-7-16
2012/12/04 00:16:30
While this works, it just occurred to me that only
please use gerrit instead
2012/12/04 05:46:40
Done.
|
+ 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) { |
@@ -87,9 +89,7 @@ void SpellcheckCustomDictionary::SetCustomWordList(WordList* custom_words) { |
if (custom_words) |
std::swap(words_, *custom_words); |
- std::vector<Observer*>::iterator it; |
- for (it = observers_.begin(); it != observers_.end(); ++it) |
- (*it)->OnCustomDictionaryLoaded(); |
+ FOR_EACH_OBSERVER(Observer, observers_, OnCustomDictionaryLoaded()); |
groby-ooo-7-16
2012/12/04 00:16:30
+1 for awesome macros!
|
} |
bool SpellcheckCustomDictionary::AddWord(const std::string& word) { |
@@ -108,9 +108,7 @@ bool SpellcheckCustomDictionary::AddWord(const std::string& word) { |
i.GetCurrentValue()->Send(new SpellCheckMsg_WordAdded(word)); |
} |
- std::vector<Observer*>::iterator it; |
- for (it = observers_.begin(); it != observers_.end(); ++it) |
- (*it)->OnCustomDictionaryWordAdded(word); |
+ FOR_EACH_OBSERVER(Observer, observers_, OnCustomDictionaryWordAdded(word)); |
return true; |
} |
@@ -131,18 +129,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'; |
groby-ooo-7-16
2012/12/04 00:16:30
Doesn't "LoadReliably" guarantee proper format?
A
please use gerrit instead
2012/12/04 05:46:40
Done.
|
+ ss << word << '\n'; |
+ |
+ SaveDictionryContentsReliably(ss.str()); |
} |
bool SpellcheckCustomDictionary::RemoveWord(const std::string& word) { |
@@ -161,9 +159,7 @@ bool SpellcheckCustomDictionary::RemoveWord(const std::string& word) { |
i.GetCurrentValue()->Send(new SpellCheckMsg_WordRemoved(word)); |
} |
- std::vector<Observer*>::iterator it; |
- for (it = observers_.begin(); it != observers_.end(); ++it) |
- (*it)->OnCustomDictionaryWordRemoved(word); |
+ FOR_EACH_OBSERVER(Observer, observers_, OnCustomDictionaryWordRemoved(word)); |
return true; |
} |
@@ -185,38 +181,35 @@ void SpellcheckCustomDictionary::EraseWordFromCustomDictionary( |
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); |
DCHECK(IsStringUTF8(word)); |
+ std::string contents; |
+ LoadDictionaryContentsReliably(&contents); |
+ if (contents.empty()) |
+ return; |
+ |
WordList custom_words; |
- LoadDictionaryIntoCustomWordList(&custom_words); |
+ base::SplitString(contents, '\n', &custom_words); |
- const char empty[] = {'\0'}; |
- const char separator[] = {'\n', '\0'}; |
- file_util::WriteFile(custom_dictionary_path_, empty, 0); |
+ std::stringstream ss; |
for (WordList::iterator it = custom_words.begin(); |
groby-ooo-7-16
2012/12/04 00:16:30
This would make the code easier to read:
it = cu
please use gerrit instead
2012/12/04 05:46:40
Done.
|
- 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); |
- } |
+ it != custom_words.end(); |
groby-ooo-7-16
2012/12/04 00:16:30
The above remark (std::erase) brings up another po
please use gerrit instead
2012/12/04 05:46:40
Done.
|
+ ++it) { |
+ if (!(*it).empty() && *it != word) |
+ ss << *it << '\n'; |
} |
+ |
+ SaveDictionryContentsReliably(ss.str()); |
} |
void SpellcheckCustomDictionary::AddObserver(Observer* observer) { |
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); |
- observers_.push_back(observer); |
+ observers_.AddObserver(observer); |
} |
void SpellcheckCustomDictionary::RemoveObserver(Observer* observer) { |
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); |
- std::vector<Observer*>::iterator it = std::find(observers_.begin(), |
- observers_.end(), |
- observer); |
- if (it != observers_.end()) |
- observers_.erase(it); |
+ observers_.RemoveObserver(observer); |
} |
WordList* SpellcheckCustomDictionary::LoadDictionary() { |
@@ -234,3 +227,69 @@ 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); |
+ std::string checksum = GetChecksum(contents); |
+ if (checksum.empty()) |
groby-ooo-7-16
2012/12/04 00:16:30
Hm. This looks like it will fail for any dictionar
please use gerrit instead
2012/12/04 05:46:40
Empty checksum most likely indicates that this is
|
+ return; |
groby-ooo-7-16
2012/12/04 00:16:30
If you go with the one-file-to-rule-them all appro
please use gerrit instead
2012/12/04 05:46:40
Done.
|
+ |
+ base::MD5Digest digest; |
+ base::MD5Sum(contents->c_str(), contents->length(), &digest); |
+ if (checksum == base::MD5DigestToBase16(digest)) |
+ return; |
+ |
+ FilePath backup_path = |
+ custom_dictionary_path_.AddExtension(backup_extension_); |
+ if (!file_util::PathExists(backup_path)) |
+ return; |
+ |
+ std::string backup; |
groby-ooo-7-16
2012/12/04 00:16:30
Backup is just the last copy saved before this one
please use gerrit instead
2012/12/04 05:46:40
Done.
|
+ file_util::ReadFileToString(backup_path, &backup); |
+ std::string backup_checksum = GetChecksum(&backup); |
+ base::MD5Digest backup_digest; |
+ base::MD5Sum(backup.c_str(), backup.length(), &backup_digest); |
+ if (backup_checksum != base::MD5DigestToBase16(backup_digest)) |
+ return; |
+ |
+ *contents = backup; |
+ file_util::CopyFile(backup_path, custom_dictionary_path_); |
+} |
+ |
+void SpellcheckCustomDictionary::SaveDictionryContentsReliably( |
+ const std::string& contents) { |
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); |
+ |
+ FilePath backup_path = |
+ custom_dictionary_path_.AddExtension(backup_extension_); |
+ if (file_util::PathExists(custom_dictionary_path_)) |
+ file_util::CopyFile(custom_dictionary_path_, backup_path); |
+ |
+ base::MD5Digest digest; |
+ base::MD5Sum(contents.c_str(), contents.length(), &digest); |
+ std::stringstream ss; |
+ ss << checksum_prefix_ << base::MD5DigestToBase16(digest) << '\n' << contents; |
+ base::ImportantFileWriter::WriteFileAtomically(custom_dictionary_path_, |
groby-ooo-7-16
2012/12/04 00:16:30
I might be wrong, but doesn't this just write the
please use gerrit instead
2012/12/04 05:46:40
It writes the prefix, the checksum, a newline, and
|
+ ss.str()); |
+} |
+ |
+std::string SpellcheckCustomDictionary::GetChecksum(std::string* contents) { |
+ std::string checksum; |
groby-ooo-7-16
2012/12/04 00:16:30
You can check if something is a checksum much easi
please use gerrit instead
2012/12/04 05:46:40
Done.
|
+ if (contents->substr(0, checksum_prefix_.length()) == checksum_prefix_) { |
+ size_t after_checksum = contents->find('\n', checksum_prefix_.length()); |
+ if (after_checksum == std::string::npos) { |
+ checksum = contents->substr( |
+ checksum_prefix_.length(), |
+ contents->length() - checksum_prefix_.length()); |
+ contents->clear(); |
+ } else { |
+ checksum = contents->substr(checksum_prefix_.length(), |
+ after_checksum - checksum_prefix_.length()); |
+ *contents = contents->substr(after_checksum + 1); |
+ } |
+ } |
+ return checksum; |
+} |