The current user custom spell check dictionary is shared among profiles. This...

The current user custom spell check dictionary is shared among profiles. This should be per profile.

BUG=None
TEST=SpellCheckProfileTest


Review URL: http://codereview.chromium.org/8345034

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@107062 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/profiles/profile_impl.cc

@@ -1681,6 +1681,6 @@ void ProfileImpl::ClearNetworkingHistorySince(base::Time time) {
 
 SpellCheckProfile* ProfileImpl::GetSpellCheckProfile() {
   if (!spellcheck_profile_.get())
-    spellcheck_profile_.reset(new SpellCheckProfile());
+    spellcheck_profile_.reset(new SpellCheckProfile(path_));
   return spellcheck_profile_.get();
 }

chrome/browser/spellchecker/spellcheck_host_impl.cc

@@ -239,15 +239,8 @@ void SpellCheckHostImpl::InitializeInternal() {
   request_context_getter_ = NULL;
 
   scoped_ptr<CustomWordList> custom_words(new CustomWordList());
-  if (file_ != base::kInvalidPlatformFileValue) {
-    // Load custom dictionary.
-    std::string contents;
-    file_util::ReadFileToString(custom_dictionary_file_, &contents);
-    CustomWordList list_of_words;
-    base::SplitString(contents, '\n', &list_of_words);
-    for (size_t i = 0; i < list_of_words.size(); ++i)
-      custom_words->push_back(list_of_words[i]);
-  }
+  if (file_ != base::kInvalidPlatformFileValue)
+    LoadCustomDictionary(custom_words.get());
 
   BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
       NewRunnableMethod(
@@ -310,15 +303,33 @@ void SpellCheckHostImpl::DownloadDictionary() {
   request_context_getter_ = NULL;
 }
 
+void SpellCheckHostImpl::LoadCustomDictionary(CustomWordList* custom_words) {
+  if (!custom_words)
+    return;
+
+  // Load custom dictionary for profile.
+  if (profile_)
+    profile_->LoadCustomDictionary(custom_words);
+
+  // Load custom dictionary.
+  std::string contents;
+  file_util::ReadFileToString(custom_dictionary_file_, &contents);
+  if (contents.empty())
+    return;
+
+  CustomWordList list_of_words;
+  base::SplitString(contents, '\n', &list_of_words);
+  for (size_t i = 0; i < list_of_words.size(); ++i) {
+    if (list_of_words[i] != "")
+      custom_words->push_back(list_of_words[i]);
+  }
+}
+
 void SpellCheckHostImpl::WriteWordToCustomDictionary(const std::string& word) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
 
-  // Stored in UTF-8.
-  std::string word_to_add(word + "\n");
-  FILE* f = file_util::OpenFile(custom_dictionary_file_, "a+");
-  if (f)
-    fputs(word_to_add.c_str(), f);
-  file_util::CloseFile(f);
+  if (profile_)
+    profile_->WriteWordToCustomDictionary(word);
 }
 
 void SpellCheckHostImpl::OnURLFetchComplete(const URLFetcher* source) {

chrome/browser/spellchecker/spellcheck_host_impl.h

@@ -92,6 +92,9 @@ class SpellCheckHostImpl : public SpellCheckHost,
   // If |dictionary_file_| is missing, we attempt to download it.
   void DownloadDictionary();
 
+  // Loads a custom dictionary from disk.
+  void LoadCustomDictionary(CustomWordList* custom_words);
+
   // Write a custom dictionary addition to disk.
   void WriteWordToCustomDictionary(const std::string& word);
 

chrome/browser/spellchecker/spellcheck_profile.cc

@@ -4,11 +4,15 @@
 
 #include "chrome/browser/spellchecker/spellcheck_profile.h"
 
+#include "base/file_util.h"
 #include "base/lazy_instance.h"
+#include "base/string_split.h"
+#include "base/string_util.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/spellchecker/spellcheck_host.h"
 #include "chrome/browser/spellchecker/spellcheck_host_metrics.h"
+#include "chrome/common/chrome_constants.h"
 #include "chrome/common/pref_names.h"
 
 namespace {
@@ -16,8 +20,9 @@ base::LazyInstance<SpellCheckProfile::CustomWordList> g_empty_list(
     base::LINKER_INITIALIZED);
 }  // namespace
 
-SpellCheckProfile::SpellCheckProfile()
-    : host_ready_(false) {
+SpellCheckProfile::SpellCheckProfile(const FilePath& profile_dir)
+    : host_ready_(false),
+      profile_dir_(profile_dir) {
 }
 
 SpellCheckProfile::~SpellCheckProfile() {
@@ -98,3 +103,45 @@ void SpellCheckProfile::CustomWordAddedLocally(const std::string& word) {
   if (metrics_.get())
     metrics_->RecordCustomWordCountStats(custom_words_->size());
 }
+
+void SpellCheckProfile::LoadCustomDictionary(CustomWordList* custom_words) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE) || IsTesting());
+
+  if (!custom_words)
+    return;
+
+  std::string contents;
+  file_util::ReadFileToString(GetCustomDictionaryPath(), &contents);
+  if (contents.empty())
+    return;
+
+  CustomWordList list_of_words;
+  base::SplitString(contents, '\n', &list_of_words);
+  for (size_t i = 0; i < list_of_words.size(); ++i) {
+    if (list_of_words[i] != "")
+      custom_words->push_back(list_of_words[i]);
+  }
+}
+
+void SpellCheckProfile::WriteWordToCustomDictionary(const std::string& word) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE) || IsTesting());
+
+  // Stored in UTF-8.
+  DCHECK(IsStringUTF8(word));
+
+  std::string word_to_add(word + "\n");
+  FILE* f = file_util::OpenFile(GetCustomDictionaryPath(), "a+");
+  if (f) {
+    fputs(word_to_add.c_str(), f);
+    file_util::CloseFile(f);
+  }
+}
+
+const FilePath& SpellCheckProfile::GetCustomDictionaryPath() {
+  if (!custom_dictionary_path_.get()) {
+    custom_dictionary_path_.reset(
+        new FilePath(profile_dir_.Append(chrome::kCustomDictionaryFileName)));
+  }
+
+  return *custom_dictionary_path_;
+}

chrome/browser/spellchecker/spellcheck_profile.h

@@ -9,6 +9,8 @@
 #include <string>
 #include <vector>
 
+#include "base/compiler_specific.h"
+#include "base/file_path.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/ref_counted.h"
 #include "chrome/browser/spellchecker/spellcheck_profile_provider.h"
@@ -56,7 +58,7 @@ class SpellCheckProfile : public SpellCheckProfileProvider {
     REINITIALIZE_DID_NOTHING
   };
 
-  SpellCheckProfile();
+  explicit SpellCheckProfile(const FilePath& profile_dir);
   virtual ~SpellCheckProfile();
 
   // Retrieves SpellCheckHost object.
@@ -82,9 +84,11 @@ class SpellCheckProfile : public SpellCheckProfileProvider {
   void StartRecordingMetrics(bool spellcheck_enabled);
 
   // SpellCheckProfileProvider implementation.
-  virtual void SpellCheckHostInitialized(CustomWordList* custom_words);
-  virtual const CustomWordList& GetCustomWords() const;
-  virtual void CustomWordAddedLocally(const std::string& word);
+  virtual void SpellCheckHostInitialized(CustomWordList* custom_words) OVERRIDE;
+  virtual const CustomWordList& GetCustomWords() const OVERRIDE;
+  virtual void CustomWordAddedLocally(const std::string& word) OVERRIDE;
+  virtual void LoadCustomDictionary(CustomWordList* custom_words) OVERRIDE;
+  virtual void WriteWordToCustomDictionary(const std::string& word) OVERRIDE;
 
  protected:
   // Only tests should override this.
@@ -97,6 +101,8 @@ class SpellCheckProfile : public SpellCheckProfileProvider {
   virtual bool IsTesting() const;
 
  private:
+  const FilePath& GetCustomDictionaryPath();
+
   scoped_refptr<SpellCheckHost> host_;
   scoped_ptr<SpellCheckHostMetrics> metrics_;
 
@@ -107,6 +113,12 @@ class SpellCheckProfile : public SpellCheckProfileProvider {
   // In-memory cache of the custom words file.
   scoped_ptr<CustomWordList> custom_words_;
 
+  // A directory path of profile.
+  FilePath profile_dir_;
+
+  // A path for custom dictionary per profile.
+  scoped_ptr<FilePath> custom_dictionary_path_;
+
   DISALLOW_COPY_AND_ASSIGN(SpellCheckProfile);
 };
 

chrome/browser/spellchecker/spellcheck_profile_provider.h

@@ -25,6 +25,12 @@ class SpellCheckProfileProvider {
   // Invoked on the Ui thread when new custom word is registered.
   virtual void CustomWordAddedLocally(const std::string& word) = 0;
 
+  // Loads the custom dictionary associated with this profile
+  virtual void LoadCustomDictionary(CustomWordList* custom_words) = 0;
+
+  // Writes a word to the custom dictionary associated with this profile.
+  virtual void WriteWordToCustomDictionary(const std::string& word) = 0;
+
  protected:
   virtual ~SpellCheckProfileProvider() {}
 };

chrome/browser/spellchecker/spellcheck_profile_unittest.cc

@@ -4,6 +4,7 @@
 
 #include <vector>
 
+#include "base/scoped_temp_dir.h"
 #include "chrome/browser/spellchecker/spellcheck_host.h"
 #include "chrome/browser/spellchecker/spellcheck_profile.h"
 #include "content/browser/browser_thread.h"
@@ -29,8 +30,9 @@ class MockSpellCheckHost : public SpellCheckHost {
 
 class TestingSpellCheckProfile : public SpellCheckProfile {
  public:
-  TestingSpellCheckProfile()
-      : create_host_calls_(0) {
+  explicit TestingSpellCheckProfile(const FilePath& profile_dir)
+      : SpellCheckProfile(profile_dir),
+        create_host_calls_(0) {
   }
 
   virtual SpellCheckHost* CreateHost(
@@ -75,7 +77,9 @@ class SpellCheckProfileTest : public testing::Test {
 
 TEST_F(SpellCheckProfileTest, ReinitializeEnabled) {
   scoped_refptr<MockSpellCheckHost> host(new MockSpellCheckHost());
-  TestingSpellCheckProfile target;
+  ScopedTempDir dir;
+  ASSERT_TRUE(dir.CreateUniqueTempDir());
+  TestingSpellCheckProfile target(dir.path());
   target.SetHostToBeCreated(host.get());
 
   // The first call should create host.
@@ -96,7 +100,10 @@ TEST_F(SpellCheckProfileTest, ReinitializeEnabled) {
 
 TEST_F(SpellCheckProfileTest, ReinitializeDisabled) {
   scoped_refptr<MockSpellCheckHost> host(new MockSpellCheckHost());
-  TestingSpellCheckProfile target;
+  ScopedTempDir dir;
+  ASSERT_TRUE(dir.CreateUniqueTempDir());
+  TestingSpellCheckProfile target(dir.path());
+
   target.returning_from_create_ = host.get();
 
   // If enabled is false, nothing should happen
@@ -112,9 +119,11 @@ TEST_F(SpellCheckProfileTest, ReinitializeDisabled) {
 
 TEST_F(SpellCheckProfileTest, ReinitializeRemove) {
   scoped_refptr<MockSpellCheckHost> host(new MockSpellCheckHost());
-  TestingSpellCheckProfile target;
-  target.SetHostToBeCreated(host.get());
+  ScopedTempDir dir;
+  ASSERT_TRUE(dir.CreateUniqueTempDir());
+  TestingSpellCheckProfile target(dir.path());
 
+  target.SetHostToBeCreated(host.get());
 
   // At first, create the host.
   ResultType result1 = target.ReinitializeHost(false, true, "", NULL);
@@ -131,7 +140,10 @@ TEST_F(SpellCheckProfileTest, ReinitializeRemove) {
 
 TEST_F(SpellCheckProfileTest, ReinitializeRecreate) {
   scoped_refptr<MockSpellCheckHost> host1(new MockSpellCheckHost());
-  TestingSpellCheckProfile target;
+  ScopedTempDir dir;
+  ASSERT_TRUE(dir.CreateUniqueTempDir());
+  TestingSpellCheckProfile target(dir.path());
+
   target.SetHostToBeCreated(host1.get());
 
   // At first, create the host.
@@ -154,7 +166,10 @@ TEST_F(SpellCheckProfileTest, ReinitializeRecreate) {
 
 TEST_F(SpellCheckProfileTest, SpellCheckHostInitializedWithCustomWords) {
   scoped_refptr<MockSpellCheckHost> host(new MockSpellCheckHost());
-  TestingSpellCheckProfile target;
+  ScopedTempDir dir;
+  ASSERT_TRUE(dir.CreateUniqueTempDir());
+  TestingSpellCheckProfile target(dir.path());
+
   target.SetHostToBeCreated(host.get());
   target.ReinitializeHost(false, true, "", NULL);
 
@@ -169,7 +184,10 @@ TEST_F(SpellCheckProfileTest, SpellCheckHostInitializedWithCustomWords) {
 
 TEST_F(SpellCheckProfileTest, CustomWordAddedLocally) {
   scoped_refptr<MockSpellCheckHost> host(new MockSpellCheckHost());
-  TestingSpellCheckProfile target;
+  ScopedTempDir dir;
+  ASSERT_TRUE(dir.CreateUniqueTempDir());
+  TestingSpellCheckProfile target(dir.path());
+
   target.SetHostToBeCreated(host.get());
   target.ReinitializeHost(false, true, "", NULL);
 
@@ -185,3 +203,80 @@ TEST_F(SpellCheckProfileTest, CustomWordAddedLocally) {
   expected.push_back("bar");
   EXPECT_EQ(target.GetCustomWords(), expected);
 }
+
+TEST_F(SpellCheckProfileTest, SaveAndLoad) {
+  scoped_refptr<MockSpellCheckHost> host(new MockSpellCheckHost());
+  ScopedTempDir dir;
+  ASSERT_TRUE(dir.CreateUniqueTempDir());
+  TestingSpellCheckProfile target(dir.path());
+
+  target.SetHostToBeCreated(host.get());
+  target.ReinitializeHost(false, true, "", NULL);
+
+  scoped_ptr<SpellCheckProfile::CustomWordList> loaded_custom_words(
+    new SpellCheckProfile::CustomWordList());
+  target.LoadCustomDictionary(loaded_custom_words.get());
+
+  // The custom word list should be empty now.
+  SpellCheckProfile::CustomWordList expected;
+  EXPECT_EQ(*loaded_custom_words, expected);
+
+  target.WriteWordToCustomDictionary("foo");
+  expected.push_back("foo");
+
+  target.WriteWordToCustomDictionary("bar");
+  expected.push_back("bar");
+
+  // The custom word list should include written words.
+  target.LoadCustomDictionary(loaded_custom_words.get());
+  EXPECT_EQ(*loaded_custom_words, expected);
+
+  // Load in another instance of SpellCheckProfile.
+  // The result should be the same.
+  scoped_refptr<MockSpellCheckHost> host2(new MockSpellCheckHost());
+  TestingSpellCheckProfile target2(dir.path());
+  target2.SetHostToBeCreated(host2.get());
+  target2.ReinitializeHost(false, true, "", NULL);
+  scoped_ptr<SpellCheckProfile::CustomWordList> loaded_custom_words2(
+      new SpellCheckProfile::CustomWordList());
+  target2.LoadCustomDictionary(loaded_custom_words2.get());
+  EXPECT_EQ(*loaded_custom_words2, expected);
+}
+
+TEST_F(SpellCheckProfileTest, MultiProfile) {
+  scoped_refptr<MockSpellCheckHost> host1(new MockSpellCheckHost());
+  scoped_refptr<MockSpellCheckHost> host2(new MockSpellCheckHost());
+
+  ScopedTempDir dir1;
+  ScopedTempDir dir2;
+  ASSERT_TRUE(dir1.CreateUniqueTempDir());
+  ASSERT_TRUE(dir2.CreateUniqueTempDir());
+  TestingSpellCheckProfile target1(dir1.path());
+  TestingSpellCheckProfile target2(dir2.path());
+
+  target1.SetHostToBeCreated(host1.get());
+  target1.ReinitializeHost(false, true, "", NULL);
+  target2.SetHostToBeCreated(host2.get());
+  target2.ReinitializeHost(false, true, "", NULL);
+
+  SpellCheckProfile::CustomWordList expected1;
+  SpellCheckProfile::CustomWordList expected2;
+
+  target1.WriteWordToCustomDictionary("foo");
+  target1.WriteWordToCustomDictionary("bar");
+  expected1.push_back("foo");
+  expected1.push_back("bar");
+
+  target2.WriteWordToCustomDictionary("hoge");
+  target2.WriteWordToCustomDictionary("fuga");
+  expected2.push_back("hoge");
+  expected2.push_back("fuga");
+
+  SpellCheckProfile::CustomWordList actual1;
+  target1.LoadCustomDictionary(&actual1);
+  EXPECT_EQ(actual1, expected1);
+
+  SpellCheckProfile::CustomWordList actual2;
+  target2.LoadCustomDictionary(&actual2);
+  EXPECT_EQ(actual2, expected2);
+}