Spellchecker:

Move file accesses during initialization to file thread. Also fix a rare (and harmless) memory leak. Also introduce a rare (and harmless) memory leak.

TEST=added dchecks, ran spellcheck unit tests, clicked around, changed dictionaries, added words
BUG=22984
BUG=21924
BUG=1123096

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

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

4 files changed, 231 insertions, 137 deletions

diff --git a/chrome/browser/spellcheck_unittest.cc b/chrome/browser/spellcheck_unittest.cc
index 8c048b2..f1f8387 100644
--- a/chrome/browser/spellcheck_unittest.cc
+++ b/chrome/browser/spellcheck_unittest.cc
@@ -292,7 +292,6 @@ TEST_F(SpellCheckTest, SpellCheckStrings_EN_US) {
   }
 }
 
-
 TEST_F(SpellCheckTest, SpellCheckSuggestions_EN_US) {
   static const struct {
     // A string to be tested.
@@ -608,7 +607,7 @@ TEST_F(SpellCheckTest, SpellCheckSuggestions_EN_US) {
     {L"jum", false, 0, 0, L"hum"},
     {L"jum", false, 0, 0, L"sum"},
     {L"jum", false, 0, 0, L"um"},
-#endif //!OS_MACOSX
+#endif  // !OS_MACOSX
     // TODO (Sidchat): add many more examples.
   };
 
diff --git a/chrome/browser/spellchecker.cc b/chrome/browser/spellchecker.cc
index 0a9f03e..128c7f3 100644
--- a/chrome/browser/spellchecker.cc
+++ b/chrome/browser/spellchecker.cc
@@ -2,10 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "app/l10n_util.h"
 #include "chrome/browser/spellchecker.h"
-#include "chrome/browser/spellchecker_common.h"
-#include "chrome/browser/spellchecker_platform_engine.h"
+
+#include "app/l10n_util.h"
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
 #include "base/file_util.h"
@@ -16,22 +15,23 @@
 #include "base/string_util.h"
 #include "base/thread.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/chrome_thread.h"
 #include "chrome/browser/net/url_fetcher.h"
 #include "chrome/browser/profile.h"
+#include "chrome/browser/spellchecker_common.h"
+#include "chrome/browser/spellchecker_platform_engine.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_counters.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/pref_service.h"
-#include "third_party/hunspell/src/hunspell/hunspell.hxx"
 #include "grit/generated_resources.h"
 #include "grit/locale_settings.h"
 #include "net/url_request/url_request.h"
+#include "third_party/hunspell/src/hunspell/hunspell.hxx"
 
 using base::TimeTicks;
 
-
-
 namespace {
 
 static const struct {
@@ -93,7 +93,7 @@ bool SaveBufferToFile(const std::string& data,
       num_bytes;
 }
 
-}
+}  // namespace
 
 // This is a helper class which acts as a proxy for invoking a task from the
 // file loop back to the IO loop. Invoking a task from file loop to the IO
@@ -104,15 +104,20 @@ bool SaveBufferToFile(const std::string& data,
 // NULL. This object also takes ownership of the given task.
 class UIProxyForIOTask : public Task {
  public:
-  explicit UIProxyForIOTask(Task* on_dictionary_save_complete_callback_task)
-      : on_dictionary_save_complete_callback_task_(
-            on_dictionary_save_complete_callback_task) {
+  explicit UIProxyForIOTask(Task* callback_task, SpellChecker* spellchecker)
+      : callback_task_(callback_task),
+        spellchecker_(spellchecker) {
   }
 
  private:
   void Run();
 
-  Task* on_dictionary_save_complete_callback_task_;
+  Task* callback_task_;
+  // The SpellChecker that invoked the file loop task. May be NULL. If not
+  // NULL, then we will Release() on it if we don't run |callback_task_|. This
+  // balances any refs the spellchecker might have had outstanding which it
+  // would have Released() when |callback_task_| was run.
+  SpellChecker* spellchecker_;
   DISALLOW_COPY_AND_ASSIGN(UIProxyForIOTask);
 };
 
@@ -121,16 +126,20 @@ void UIProxyForIOTask::Run() {
   base::Thread* io_thread = g_browser_process->io_thread();
   if (io_thread) {  // io_thread has not been torn down yet.
     MessageLoop* io_loop = io_thread->message_loop();
-    io_loop->PostTask(FROM_HERE,
-                      on_dictionary_save_complete_callback_task_);
-    on_dictionary_save_complete_callback_task_ = NULL;
+    io_loop->PostTask(FROM_HERE, callback_task_);
+  } else {
+    if (spellchecker_)
+      spellchecker_->Release();
+    delete callback_task_;
   }
+
+  callback_task_ = NULL;
 }
 
 // Design: The spellchecker initializes hunspell_ in the Initialize() method.
 // This is done using the dictionary file on disk, e.g. "en-US_1_1.bdic".
 // Initialization of hunspell_ is held off during this process. If the
-// dictionaryis not available, we first attempt to download and save it. After
+// dictionary is not available, we first attempt to download and save it. After
 // the dictionary is downloaded and saved to disk (or the attempt to do so
 // fails)), corresponding flags are set
 // in spellchecker - in the IO thread. Since IO thread goes first during closing
@@ -144,14 +153,12 @@ class SaveDictionaryTask : public Task {
   SaveDictionaryTask(Task* on_dictionary_save_complete_callback_task,
                      const FilePath& first_attempt_file_name,
                      const FilePath& fallback_file_name,
-                     const std::string& data,
-                     MessageLoop* ui_loop)
+                     const std::string& data)
       : on_dictionary_save_complete_callback_task_(
             on_dictionary_save_complete_callback_task),
         first_attempt_file_name_(first_attempt_file_name),
         fallback_file_name_(fallback_file_name),
-        data_(data),
-        ui_loop_(ui_loop) {
+        data_(data) {
   }
 
  private:
@@ -178,7 +185,6 @@ class SaveDictionaryTask : public Task {
   std::string data_;
 
   // This invokes back to io loop when downloading is over.
-  MessageLoop* ui_loop_;
   DISALLOW_COPY_AND_ASSIGN(SaveDictionaryTask);
 };
 
@@ -191,17 +197,110 @@ void SaveDictionaryTask::Run() {
     if (!file_util::PathExists(fallback_dir))
       file_util::CreateDirectory(fallback_dir);
     SaveBufferToFile(data_, fallback_file_name_);
-  } // Unsuccessful save is taken care of in SpellChecker::Initialize().
+  }  // Unsuccessful save is taken care of in SpellChecker::Initialize().
 
   // Set Flag that dictionary is not downloading anymore.
-  ui_loop_->PostTask(FROM_HERE,
-      new UIProxyForIOTask(on_dictionary_save_complete_callback_task_));
+  MessageLoop* ui_loop = ChromeThread::GetMessageLoop(ChromeThread::UI);
+  ui_loop->PostTask(FROM_HERE,
+      new UIProxyForIOTask(on_dictionary_save_complete_callback_task_, NULL));
 }
 
+// Design: this task tries to read the dictionary from disk and load it into
+// memory. It is executed on the file thread, and posts the results back to
+// the IO thread (via the UI thread---see UIProxyForIOTask).
+// The task first checks for the existence of the dictionary in one of the two
+// given locations. If it does not exist, the task informs the SpellChecker,
+// which will try to download the directory and run a new ReadDictionaryTask.
+class ReadDictionaryTask : public Task {
+ public:
+  ReadDictionaryTask(SpellChecker* spellchecker,
+                     const FilePath& dict_file_name_app,
+                     const FilePath& dict_file_name_usr)
+        : spellchecker_(spellchecker),
+          hunspell_(NULL),
+          bdict_file_(NULL),
+          custom_dictionary_file_name_(
+              spellchecker->custom_dictionary_file_name_),
+          dict_file_name_app_(dict_file_name_app),
+          dict_file_name_usr_(dict_file_name_usr) {
+  }
+
+  virtual void Run() {
+    FilePath bdict_file_path;
+    if (file_util::PathExists(dict_file_name_app_)) {
+      bdict_file_path = dict_file_name_app_;
+    } else if (file_util::PathExists(dict_file_name_usr_)) {
+      bdict_file_path = dict_file_name_usr_;
+    } else {
+      Finish(false);
+      return;
+    }
+
+    bdict_file_ = new file_util::MemoryMappedFile;
+    if (bdict_file_->Initialize(bdict_file_path)) {
+      TimeTicks start_time = TimeTicks::Now();
+
+      hunspell_ =
+          new Hunspell(bdict_file_->data(), bdict_file_->length());
+
+      // Add custom words to Hunspell.
+      std::string contents;
+      file_util::ReadFileToString(custom_dictionary_file_name_, &contents);
+      std::vector<std::string> list_of_words;
+      SplitString(contents, '\n', &list_of_words);
+      for (std::vector<std::string>::iterator it = list_of_words.begin();
+           it != list_of_words.end(); ++it) {
+        hunspell_->add(it->c_str());
+      }
+
+      DHISTOGRAM_TIMES("Spellcheck.InitTime",
+                       TimeTicks::Now() - start_time);
+    } else {
+      delete bdict_file_;
+      bdict_file_ = NULL;
+    }
+
+    Finish(true);
+  }
+
+ private:
+  void Finish(bool file_existed) {
+    Task* task = NewRunnableMethod(spellchecker_, &SpellChecker::HunspellInited,
+                                   hunspell_, bdict_file_, file_existed);
+    if (spellchecker_->file_loop_) {
+      MessageLoop* ui_loop = ChromeThread::GetMessageLoop(ChromeThread::UI);
+      // We were called on the file loop. Post back to the IO loop.
+      // If this never gets posted to the IO loop, then we will leak |hunspell_|
+      // and |bdict_file_|. But that can only happen during shutdown, so it's
+      // not worth caring about.
+      ui_loop->PostTask(FROM_HERE, new UIProxyForIOTask(task, spellchecker_));
+    } else {
+      // We were called directly (e.g., during testing). Run the task directly.
+      task->Run();
+      delete task;
+    }
+  }
+
+  // The SpellChecker we are working for. We are guaranteed to be outlived
+  // by this object because it AddRefs() itself before calling us.
+  // Accessing it is not necessarily thread safe, but are careful to only access
+  // it in ways that are.
+  SpellChecker* spellchecker_;
+  Hunspell* hunspell_;
+  file_util::MemoryMappedFile* bdict_file_;
+
+  FilePath custom_dictionary_file_name_;
+  FilePath dict_file_name_app_;
+  FilePath dict_file_name_usr_;
+
+  DISALLOW_COPY_AND_ASSIGN(ReadDictionaryTask);
+};
+
 void SpellChecker::SpellCheckLanguages(std::vector<std::string>* languages) {
   for (size_t i = 0; i < ARRAYSIZE_UNSAFE(g_supported_spellchecker_languages);
-       ++i)
+       ++i) {
     languages->push_back(g_supported_spellchecker_languages[i].language);
+  }
 }
 
 // This function returns the language-region version of language name.
@@ -370,19 +469,15 @@ SpellChecker::SpellChecker(const FilePath& dict_dir,
       custom_dictionary_file_name_(custom_dictionary_file_name),
       tried_to_init_(false),
       language_(language),
-#ifndef NDEBUG
       worker_loop_(NULL),
-#endif
       tried_to_download_dictionary_file_(false),
       file_loop_(NULL),
-      ui_loop_(MessageLoop::current()),
       url_request_context_(request_context),
       obtaining_dictionary_(false),
       auto_spell_correct_turned_on_(false),
       is_using_platform_spelling_engine_(false),
       fetcher_(NULL),
-      ALLOW_THIS_IN_INITIALIZER_LIST(
-          on_dictionary_save_complete_callback_factory_(this)) {
+      ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)) {
   if (SpellCheckerPlatform::SpellCheckerAvailable()) {
     SpellCheckerPlatform::Init();
     if (SpellCheckerPlatform::PlatformSupportsLanguage(language)) {
@@ -415,13 +510,11 @@ SpellChecker::SpellChecker(const FilePath& dict_dir,
 }
 
 SpellChecker::~SpellChecker() {
-#ifndef NDEBUG
   // This must be deleted on the I/O thread (see the header). This is the same
-  // thread thatSpellCheckWord is called on, so we verify that they were all the
-  // same thread.
+  // thread that SpellCheckWord is called on, so we verify that they were all
+  // the same thread.
   if (worker_loop_)
     DCHECK(MessageLoop::current() == worker_loop_);
-#endif
 }
 
 void SpellChecker::StartDictionaryDownload(const FilePath& file_name) {
@@ -455,18 +548,26 @@ void SpellChecker::OnURLFetchComplete(const URLFetcher* source,
       bdic_file_name_);
   FilePath user_data_dir = GetFallbackDictionaryDownloadDirectory();
   FilePath fallback_file_name = user_data_dir.Append(bdic_file_name_);
-  Task* dic_task = on_dictionary_save_complete_callback_factory_.
+  Task* dic_task = method_factory_.
       NewRunnableMethod(&SpellChecker::OnDictionarySaveComplete);
   file_loop_->PostTask(FROM_HERE, new SaveDictionaryTask(dic_task,
-      first_attempt_file_name, fallback_file_name, data, ui_loop_));
+      first_attempt_file_name, fallback_file_name, data));
+}
+
+void SpellChecker::OnDictionarySaveComplete() {
+  obtaining_dictionary_ = false;
+  // Now that the dictionary is downloaded, continue trying to download.
+  Initialize();
 }
 
 // Initialize SpellChecker. In this method, if the dictionary is not present
 // in the local disk, it is fetched asynchronously.
-// TODO(sidchat): After dictionary is downloaded, initialize hunspell in
-// file loop - this is currently being done in the io loop.
-// Bug: http://b/issue?id=1123096
 bool SpellChecker::Initialize() {
+  if (!worker_loop_)
+    worker_loop_ = MessageLoop::current();
+  else
+    DCHECK(worker_loop_ == MessageLoop::current());
+
   // Return false if the dictionary files are downloading.
   if (obtaining_dictionary_)
     return false;
@@ -497,40 +598,68 @@ bool SpellChecker::Initialize() {
   FilePath dictionary_file_name_usr = GetVersionedFileName(language_,
       dict_dir_userdata);
 
-  // Check in both the directories to see whether the spellcheck dictionary
-  // already resides in one of these.
-  FilePath bdic_file_name;
-  if (file_util::PathExists(dictionary_file_name_app)) {
-    bdic_file_name = dictionary_file_name_app;
-  } else if (file_util::PathExists(dictionary_file_name_usr)) {
-    bdic_file_name = dictionary_file_name_usr;
+  // Balances Release() in HunspellInited(), or in UIProxyForIOTask if the IO
+  // thread is torn down before the ReadDictionaryTask calls us back.
+  AddRef();
+  Task* task = new ReadDictionaryTask(this,
+      dictionary_file_name_app, dictionary_file_name_usr);
+  if (file_loop_) {
+    file_loop_->PostTask(FROM_HERE, task);
   } else {
-    // Download the dictionary file.
-    if (file_loop_ && url_request_context_) {
-      if (!tried_to_download_dictionary_file_) {
-        StartDictionaryDownload(dictionary_file_name_app);
-        tried_to_download_dictionary_file_ = true;
-        return false;
-      } else {  // There is no dictionary even after trying to download it.
-        // Stop trying to download the dictionary in this session.
-        tried_to_init_ = true;
-        return false;
-      }
+    task->Run();
+    delete task;
+  }
+
+  return hunspell_.get() != NULL;
+}
+
+void SpellChecker::HunspellInited(Hunspell* hunspell,
+                                  file_util::MemoryMappedFile* bdict_file,
+                                  bool file_existed) {
+  DCHECK(worker_loop_ == MessageLoop::current());
+
+  if (file_existed)
+    tried_to_init_ = true;
+
+  if (!hunspell) {
+    if (!file_existed) {
+      // File didn't exist. We need to download a dictionary.
+      DoDictionaryDownload();
     }
+
+    // Balances AddRef() in Initialize().
+    Release();
+    return;
   }
 
-  // Control has come so far - the BDIC dictionary file probably exists. Now try
-  // to initialize hunspell using the available bdic dictionary file.
-  TimeTicks begin_time = TimeTicks::Now();
-  bdict_file_.reset(new file_util::MemoryMappedFile());
-  if (bdict_file_->Initialize(bdic_file_name)) {
-    hunspell_.reset(new Hunspell(bdict_file_->data(), bdict_file_->length()));
-    AddCustomWordsToHunspell();
+
+  bdict_file_.reset(bdict_file);
+  hunspell_.reset(hunspell);
+  // Add all the custom words we've gotten while Hunspell was loading.
+  while (!custom_words_.empty()) {
+    hunspell_->add(custom_words_.front().c_str());
+    custom_words_.pop();
   }
-  DHISTOGRAM_TIMES("Spellcheck.InitTime", TimeTicks::Now() - begin_time);
 
-  tried_to_init_ = true;
-  return false;
+  // Balances AddRef() in Initialize().
+  Release();
+}
+
+void SpellChecker::DoDictionaryDownload() {
+  // Download the dictionary file.
+  if (file_loop_ && url_request_context_) {
+    if (!tried_to_download_dictionary_file_) {
+      FilePath dictionary_file_name_app = GetVersionedFileName(language_,
+          given_dictionary_directory_);
+      StartDictionaryDownload(dictionary_file_name_app);
+      tried_to_download_dictionary_file_ = true;
+    } else {
+      // Don't try to download a dictionary more than once.
+      tried_to_init_ = true;
+    }
+  } else {
+    NOTREACHED();
+  }
 }
 
 void SpellChecker::GetAutoCorrectionWord(const std::wstring& word, int tag,
@@ -583,23 +712,6 @@ void SpellChecker::EnableAutoSpellCorrect(bool turn_on) {
   auto_spell_correct_turned_on_ = turn_on;
 }
 
-void SpellChecker::AddCustomWordsToHunspell() {
-  // Add custom words to Hunspell.
-  // This should be done in File Loop, but since Hunspell is in this IO Loop,
-  // this too has to be initialized here.
-  // TODO(sidchat): Work out a way to initialize Hunspell in the File Loop.
-  std::string contents;
-  file_util::ReadFileToString(custom_dictionary_file_name_, &contents);
-  std::vector<std::string> list_of_words;
-  SplitString(contents, '\n', &list_of_words);
-  if (hunspell_.get()) {
-    for (std::vector<std::string>::iterator it = list_of_words.begin();
-         it != list_of_words.end(); ++it) {
-      hunspell_->add(it->c_str());
-    }
-  }
-}
-
 // Returns whether or not the given string is a valid contraction.
 // This function is a fall-back when the SpellcheckWordIterator class
 // returns a concatenated word which is not in the selected dictionary
@@ -629,20 +741,16 @@ bool SpellChecker::SpellCheckWord(
   DCHECK(in_word_len >= 0);
   DCHECK(misspelling_start && misspelling_len) << "Out vars must be given.";
 
-#ifndef NDEBUG
   // This must always be called on the same thread (normally the I/O thread).
   if (worker_loop_)
     DCHECK(MessageLoop::current() == worker_loop_);
-  else
-    worker_loop_ = MessageLoop::current();
-#endif
 
   // Check if the platform spellchecker is being used.
   if (!is_using_platform_spelling_engine_) {
     // If it isn't, try and init hunspell.
     Initialize();
 
-    // Check to see if hunspell was successful.
+    // Check to see if hunspell was successfuly initialized.
     if (!hunspell_.get())
       return true;  // Unable to spellcheck, return word is OK.
   }
@@ -728,16 +836,24 @@ void SpellChecker::AddWord(const std::wstring& word) {
 
   // Add the word to hunspell.
   std::string word_to_add = WideToUTF8(word);
-  if (!word_to_add.empty())
-    hunspell_->add(word_to_add.c_str());
+  if (!word_to_add.empty()) {
+    // Either add the word to |hunspell_|, or, if |hunspell_| is still loading,
+    // defer it till after the load completes.
+    if (hunspell_.get())
+      hunspell_->add(word_to_add.c_str());
+    else
+      custom_words_.push(word_to_add);
+  }
 
   // Now add the word to the custom dictionary file.
   Task* write_word_task =
       new AddWordToCustomDictionaryTask(custom_dictionary_file_name_, word);
-  if (file_loop_)
+  if (file_loop_) {
     file_loop_->PostTask(FROM_HERE, write_word_task);
-  else
+  } else {
     write_word_task->Run();
+    delete write_word_task;
+  }
 }
 
 bool SpellChecker::CheckSpelling(const std::string& word_to_check, int tag) {
@@ -756,9 +872,8 @@ bool SpellChecker::CheckSpelling(const std::string& word_to_check, int tag) {
   return word_correct;
 }
 
-
 void SpellChecker::FillSuggestionList(const std::string& wrong_word,
-                            std::vector<std::wstring>* optional_suggestions) {
+    std::vector<std::wstring>* optional_suggestions) {
   if (is_using_platform_spelling_engine_) {
     SpellCheckerPlatform::FillSuggestionList(wrong_word, optional_suggestions);
     return;
diff --git a/chrome/browser/spellchecker.h b/chrome/browser/spellchecker.h
index 48e4153..a8acafd 100644
--- a/chrome/browser/spellchecker.h
+++ b/chrome/browser/spellchecker.h
@@ -5,19 +5,20 @@
 #ifndef CHROME_BROWSER_SPELLCHECKER_H_
 #define CHROME_BROWSER_SPELLCHECKER_H_
 
-#include <vector>
+#include <queue>
 #include <string>
+#include <vector>
 
 #include "app/l10n_util.h"
 #include "base/string_util.h"
+#include "base/task.h"
+#include "base/time.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/net/url_fetcher.h"
 #include "chrome/browser/profile.h"
 #include "chrome/browser/spellcheck_worditerator.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/pref_member.h"
-
-#include "base/task.h"
 #include "unicode/uscript.h"
 
 class FilePath;
@@ -115,9 +116,10 @@ class SpellChecker : public base::RefCountedThreadSafe<SpellChecker>,
   static std::string GetLanguageFromLanguageRegion(std::string input_language);
 
  private:
+  friend class ReadDictionaryTask;
+
   // URLFetcher::Delegate implementation.  Called when we finish downloading the
   // spellcheck dictionary; saves the dictionary to disk.
-  // TODO(sidchat): Save to disk in the file thread instead of the IO thread.
   virtual void OnURLFetchComplete(const URLFetcher* source,
                                   const GURL& url,
                                   const URLRequestStatus& status,
@@ -137,13 +139,15 @@ class SpellChecker : public base::RefCountedThreadSafe<SpellChecker>,
   // Initializes the Hunspell Dictionary.
   bool Initialize();
 
-  // After |hunspell_| is initialized, this function is called to add custom
-  // words from the custom dictionary to the |hunspell_|.
-  void AddCustomWordsToHunspell();
+  // Called when |hunspell| is done loading, succesfully or not. If |hunspell|
+  // and |bdict_file| are non-NULL, assume ownership.
+  void HunspellInited(Hunspell* hunspell,
+                      file_util::MemoryMappedFile* bdict_file,
+                      bool file_existed);
 
-  // Memory maps the given .bdic file. On success, it will return true and will
-  // place the data and length into the given out parameters.
-  bool MapBdictFile(const unsigned char** data, size_t* length);
+  // Either start downloading a dictionary if we have not already, or do nothing
+  // if we have already tried to download one.
+  void DoDictionaryDownload();
 
   // Returns whether or not the given word is a contraction of valid words
   // (e.g. "word:word").
@@ -163,7 +167,7 @@ class SpellChecker : public base::RefCountedThreadSafe<SpellChecker>,
 
   // This method is called in the IO thread after dictionary download has
   // completed in FILE thread.
-  void OnDictionarySaveComplete(){ obtaining_dictionary_ = false; }
+  void OnDictionarySaveComplete();
 
   // The given path to the directory whether SpellChecker first tries to
   // download the spellcheck bdic dictionary file.
@@ -192,15 +196,13 @@ class SpellChecker : public base::RefCountedThreadSafe<SpellChecker>,
   // The language that this spellchecker works in.
   std::string language_;
 
-#ifndef NDEBUG
   // This object must only be used on the same thread. However, it is normally
   // created on the UI thread. This checks calls to SpellCheckWord and the
   // destructor to make sure we're only ever running on the same thread.
   //
   // This will be NULL if it is not initialized yet (not initialized in the
-  // constructor since that's on a different thread.
+  // constructor since that's on a different thread).
   MessageLoop* worker_loop_;
-#endif
 
   // Flag indicating whether we tried to download the dictionary file.
   bool tried_to_download_dictionary_file_;
@@ -208,9 +210,6 @@ class SpellChecker : public base::RefCountedThreadSafe<SpellChecker>,
   // File Thread Message Loop.
   MessageLoop* file_loop_;
 
-  // UI Thread Message Loop.
-  MessageLoop* ui_loop_;
-
   // Used for requests. MAY BE NULL which means don't try to download.
   URLRequestContext* url_request_context_;
 
@@ -227,10 +226,13 @@ class SpellChecker : public base::RefCountedThreadSafe<SpellChecker>,
   // URLFetcher to download a file in memory.
   scoped_ptr<URLFetcher> fetcher_;
 
+  // While Hunspell is loading, we add any new custom words to this queue.
+  // We will add them to |hunspell_| when it is done loading.
+  std::queue<std::string> custom_words_;
+
   // Used for generating callbacks to spellchecker, since spellchecker is a
   // non-reference counted object.
-  ScopedRunnableMethodFactory<SpellChecker>
-      on_dictionary_save_complete_callback_factory_;
+  ScopedRunnableMethodFactory<SpellChecker> method_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(SpellChecker);
 };
diff --git a/tools/valgrind/memcheck/suppressions.txt b/tools/valgrind/memcheck/suppressions.txt
index b673759..5cb55ad 100644
--- a/tools/valgrind/memcheck/suppressions.txt
+++ b/tools/valgrind/memcheck/suppressions.txt
@@ -959,28 +959,6 @@
    fun:_ZN7WebCore8V8Custom32v8MessagePortPostMessageCallbackERKN2v89ArgumentsE
 }
 {
-   bug_22984
-   Memcheck:Leak
-   fun:_Znw*
-   fun:_ZN27ScopedRunnableMethodFactoryI12SpellCheckerE17NewRunnableMethodIMS0_FvvEEEP4TaskT_
-   fun:_ZN12SpellChecker18OnURLFetchCompleteEPK10URLFetcherRK4GURLRK16URLRequestStatusiRKSt6vectorISsSaISsEERKSs
-   fun:_ZN10URLFetcher4Core21OnCompletedURLRequestERK16URLRequestStatus
-   fun:_Z16DispatchToMethodIN10URLFetcher4CoreEMS1_FvRK16URLRequestStatusES2_EvPT_T0_RK6Tuple1IT1_E
-   fun:_ZN14RunnableMethodIN10URLFetcher4CoreEMS1_FvRK16URLRequestStatusE6Tuple1IS2_EE3RunEv
-   fun:_ZN11MessageLoop7RunTaskEP4Task
-   fun:_ZN11MessageLoop21DeferOrRunPendingTaskERKNS_11PendingTaskE
-   fun:_ZN11MessageLoop6DoWorkEv
-   fun:_ZN4base19MessagePumpLibevent3RunEPNS_11MessagePump8DelegateE
-   fun:_ZN11MessageLoop11RunInternalEv
-   fun:_ZN11MessageLoop10RunHandlerEv
-   fun:_ZN11MessageLoop3RunEv
-   fun:_ZN4base6Thread3RunEP11MessageLoop
-   fun:_ZN4base6Thread10ThreadMainEv
-   fun:_Z10ThreadFuncPv
-   fun:start_thread
-   fun:clone
-}
-{
    bug_23104
    Memcheck:Leak
    fun:_Znw*