NOT FOR REVIEW: quick hacks for http://codereview.chromium.org/7919003/

chrome/browser/spellchecker/spellcheck_host_impl.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/spellchecker/spellcheck_host_impl.h"
 
 #include <set>
 
+#include "base/bind.h"
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/path_service.h"
 #include "base/string_split.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/spellchecker/spellcheck_host_metrics.h"
 #include "chrome/browser/spellchecker/spellcheck_profile_provider.h"
@@ skipping 52 matching lines @@
 #endif
 
 #if defined(OS_WIN)
 FilePath GetFallbackFilePath(const FilePath& first_choice) {
   FilePath dict_dir;
   PathService::Get(chrome::DIR_USER_DATA, &dict_dir);
   return dict_dir.Append(first_choice.BaseName());
 }
 #endif
 
+void CloseDictionary(base::PlatformFile file) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+  base::ClosePlatformFile(file);
+}
+
 }  // namespace
 
 // Constructed on UI thread.
 SpellCheckHostImpl::SpellCheckHostImpl(
     SpellCheckProfileProvider* profile,
     const std::string& language,
     net::URLRequestContextGetter* request_context_getter,
     SpellCheckHostMetrics* metrics)
     : profile_(profile),
       language_(language),
       file_(base::kInvalidPlatformFileValue),
       tried_to_download_(false),
       use_platform_spellchecker_(false),
       request_context_getter_(request_context_getter),
-      metrics_(metrics) {
+      metrics_(metrics),
+      weak_ptr_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
   DCHECK(profile_);
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   FilePath personal_file_directory;
   PathService::Get(chrome::DIR_USER_DATA, &personal_file_directory);
   custom_dictionary_file_ =
       personal_file_directory.Append(chrome::kCustomDictionaryFileName);
 
-  registrar_.Add(this, content::NOTIFICATION_RENDERER_PROCESS_CREATED,
+  registrar_.Add(weak_ptr_factory_.GetWeakPtr(),
+                 content::NOTIFICATION_RENDERER_PROCESS_CREATED,
                  content::NotificationService::AllSources());
 }
 
 SpellCheckHostImpl::~SpellCheckHostImpl() {
-  if (file_ != base::kInvalidPlatformFileValue)
-    base::ClosePlatformFile(file_);
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  if (file_ != base::kInvalidPlatformFileValue) {
+    BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
+                            base::Bind(&CloseDictionary, file_));
+    file_ = base::kInvalidPlatformFileValue;
+  }
 }
 
 void SpellCheckHostImpl::Initialize() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
   if (SpellCheckerPlatform::SpellCheckerAvailable() &&
       SpellCheckerPlatform::PlatformSupportsLanguage(language_)) {
 #if defined(OS_MACOSX)
     RecordSpellCheckStats(true, language_);
 #endif
     use_platform_spellchecker_ = true;
     SpellCheckerPlatform::SetLanguage(language_);
     MessageLoop::current()->PostTask(FROM_HERE,
-        NewRunnableMethod(this,
-            &SpellCheckHostImpl::InformProfileOfInitialization));
+        base::Bind(&SpellCheckHostImpl::InformProfileOfInitialization,
+                   weak_ptr_factory_.GetWeakPtr()));
     return;
   }
 
 #if defined(OS_MACOSX)
   RecordSpellCheckStats(false, language_);
 #endif
 
-  BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
-      NewRunnableMethod(this,
-                        &SpellCheckHostImpl::InitializeDictionaryLocation));
+  BrowserThread::PostTaskAndReply(BrowserThread::FILE, FROM_HERE,
+      base::Bind(&SpellCheckHostImpl::InitializeDictionaryLocation,
+                 base::Unretained(this)),
+      base::Bind(&SpellCheckHostImpl::InitializeDictionaryLocationComplete,
+                 weak_ptr_factory_.GetWeakPtr()));
 }
 
 void SpellCheckHostImpl::UnsetProfile() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   profile_ = NULL;
   request_context_getter_ = NULL;
   fetcher_.reset();
   registrar_.RemoveAll();
 }
@@ skipping 24 matching lines @@
       profile_ ? profile_->GetCustomWords() : CustomWordList(),
       GetLanguage(),
       prefs->GetBoolean(prefs::kEnableAutoSpellCorrect)));
 }
 
 void SpellCheckHostImpl::AddWord(const std::string& word) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   if (profile_)
     profile_->CustomWordAddedLocally(word);
-  BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
-      NewRunnableMethod(this,
-          &SpellCheckHostImpl::WriteWordToCustomDictionary, word));
-  for (RenderProcessHost::iterator i(RenderProcessHost::AllHostsIterator());
-       !i.IsAtEnd(); i.Advance()) {
-    i.GetCurrentValue()->Send(new SpellCheckMsg_WordAdded(word));
-  }
+
+  BrowserThread::PostTaskAndReply(BrowserThread::FILE, FROM_HERE,
+      base::Bind(&SpellCheckHostImpl::WriteWordToCustomDictionary,
+                 base::Unretained(this), word),
+      base::Bind(&SpellCheckHostImpl::AddWordComplete,
+                 weak_ptr_factory_.GetWeakPtr(), word));
 }
 
 void SpellCheckHostImpl::InitializeDictionaryLocation() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
 
   // Initialize the BDICT path. This initialization should be in the FILE thread
   // because it checks if there is a "Dictionaries" directory and create it.
   if (bdict_file_path_.empty())
     bdict_file_path_ = GetFirstChoiceFilePath(language_);
 
 #if defined(OS_WIN)
   // Check if the dictionary exists in the fallback location. If so, use it
   // rather than downloading anew.
   FilePath fallback = GetFallbackFilePath(bdict_file_path_);
   if (!file_util::PathExists(bdict_file_path_) &&
       file_util::PathExists(fallback)) {
     bdict_file_path_ = fallback;
   }
 #endif
 
-  InitializeInternal();
-}
-
-void SpellCheckHostImpl::InitializeInternal() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
-
   if (!profile_)
     return;
 
   if (!VerifyBDict(bdict_file_path_)) {
     DCHECK_EQ(file_, base::kInvalidPlatformFileValue);
     file_util::Delete(bdict_file_path_, false);
 
     // Notify browser tests that this dictionary is corrupted. We also skip
     // downloading the dictionary when we run this function on browser tests.
     if (SignalStatusEvent(BDICT_CORRUPTED))
       tried_to_download_ = true;
   } else {
     file_ = base::CreatePlatformFile(
         bdict_file_path_,
         base::PLATFORM_FILE_READ | base::PLATFORM_FILE_OPEN,
         NULL, NULL);
   }
 
   // File didn't exist. Download it.
   if (file_ == base::kInvalidPlatformFileValue && !tried_to_download_ &&
       request_context_getter_) {
+    // Return this function so InitializeDictionaryLocationComplete() can start
+    // downloading the dictionary.
+    return;
+  }
+
+  request_context_getter_ = NULL;
+
+  custom_words_.reset(new CustomWordList());
+  if (file_ != base::kInvalidPlatformFileValue)
+    LoadCustomDictionary(custom_words_.get());
+}
+
+void SpellCheckHostImpl::InitializeDictionaryLocationComplete() {
+  if (file_ == base::kInvalidPlatformFileValue && !tried_to_download_ &&
+      request_context_getter_) {
     // We download from the ui thread because we need to know that
     // |request_context_getter_| is still valid before initiating the download.
     BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
-        NewRunnableMethod(this, &SpellCheckHostImpl::DownloadDictionary));
-    return;
+        base::Bind(&SpellCheckHostImpl::DownloadDictionary,
+                   weak_ptr_factory_.GetWeakPtr()));
+  } else {
+    // TODO: This could be a reply point for the calling PostTaskAndReply().
+    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+        base::Bind(
+            &SpellCheckHostImpl::InformProfileOfInitializationWithCustomWords,
+            weak_ptr_factory_.GetWeakPtr(),
+            custom_words_.release()));
   }
-
-  request_context_getter_ = NULL;
-
-  scoped_ptr<CustomWordList> custom_words(new CustomWordList());
-  if (file_ != base::kInvalidPlatformFileValue)
-    LoadCustomDictionary(custom_words.get());
-
-  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
-      NewRunnableMethod(
-          this,
-          &SpellCheckHostImpl::InformProfileOfInitializationWithCustomWords,
-          custom_words.release()));
-}
-
-void SpellCheckHostImpl::InitializeOnFileThread() {
-  DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::FILE));
-
-  BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
-      NewRunnableMethod(this, &SpellCheckHostImpl::Initialize));
 }
 
 void SpellCheckHostImpl::InformProfileOfInitialization() {
   InformProfileOfInitializationWithCustomWords(NULL);
 }
 
 void SpellCheckHostImpl::InformProfileOfInitializationWithCustomWords(
     CustomWordList* custom_words) {
   // Non-null |custom_words| should be given only if the profile is available
   // for simplifying the life-cycle management of the word list.
   DCHECK(profile_ || !custom_words);
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   if (profile_)
     profile_->SpellCheckHostInitialized(custom_words);
 
   for (RenderProcessHost::iterator i(RenderProcessHost::AllHostsIterator());
        !i.IsAtEnd(); i.Advance()) {
     RenderProcessHost* process = i.GetCurrentValue();
     if (process)
       InitForRenderer(process);
   }
 }
 
 void SpellCheckHostImpl::DownloadDictionary() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-
-  if (!request_context_getter_) {
-    InitializeOnFileThread();
-    return;
-  }
+  DCHECK(request_context_getter_);
 
   // Determine URL of file to download.
   static const char kDownloadServerUrl[] =
       "http://cache.pack.google.com/edgedl/chrome/dict/";
   std::string bdict_file = bdict_file_path_.BaseName().MaybeAsASCII();
   if (bdict_file.empty()) {
     NOTREACHED();
     return;
   }
   GURL url = GURL(std::string(kDownloadServerUrl) +
                   StringToLowerASCII(bdict_file));
-  fetcher_.reset(content::URLFetcher::Create(
-      url, content::URLFetcher::GET, this));
+  fetcher_.reset(content::URLFetcher::Create(url, content::URLFetcher::GET,
+                                weak_ptr_factory_.GetWeakPtr()));
   fetcher_->SetRequestContext(request_context_getter_);
   tried_to_download_ = true;
   fetcher_->Start();
   request_context_getter_ = NULL;
 }
 
 void SpellCheckHostImpl::LoadCustomDictionary(CustomWordList* custom_words) {
   if (!custom_words)
     return;
 
@@ skipping 23 matching lines @@
 }
 
 void SpellCheckHostImpl::OnURLFetchComplete(const content::URLFetcher* source) {
   DCHECK(source);
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   scoped_ptr<content::URLFetcher> fetcher_destructor(fetcher_.release());
 
   if ((source->GetResponseCode() / 100) != 2) {
     // Initialize will not try to download the file a second time.
     LOG(ERROR) << "Failure to download dictionary.";
-    InitializeOnFileThread();
     return;
   }
 
   // Basic sanity check on the dictionary.
   // There's the small chance that we might see a 200 status code for a body
   // that represents some form of failure.
   std::string data;
   source->GetResponseAsString(&data);
   if (data.size() < 4 || data[0] != 'B' || data[1] != 'D' || data[2] != 'i' ||
       data[3] != 'c') {
     LOG(ERROR) << "Failure to download dictionary.";
-    InitializeOnFileThread();
     return;
   }
 
   data_ = data;
-  BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
-      NewRunnableMethod(this, &SpellCheckHostImpl::SaveDictionaryData));
+  BrowserThread::PostTaskAndReply(BrowserThread::FILE, FROM_HERE,
+      base::Bind(&SpellCheckHostImpl::SaveDictionaryData,
+                 base::Unretained(this)),
+      base::Bind(&SpellCheckHostImpl::InformProfileOfInitialization,
+                 weak_ptr_factory_.GetWeakPtr()));
 }
 
 void SpellCheckHostImpl::Observe(int type,
                                  const content::NotificationSource& source,
                                  const content::NotificationDetails& details) {
   DCHECK(type == content::NOTIFICATION_RENDERER_PROCESS_CREATED);
   RenderProcessHost* process = content::Source<RenderProcessHost>(source).ptr();
   InitForRenderer(process);
 }
 
 void SpellCheckHostImpl::SaveDictionaryData() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
 
   // To prevent corrupted dictionary data from causing a renderer crash, scan
   // the dictionary data and verify it is sane before save it to a file.
   bool verified = hunspell::BDict::Verify(data_.data(), data_.size());
   if (metrics_)
     metrics_->RecordDictionaryCorruptionStats(!verified);
   if (!verified) {
     LOG(ERROR) << "Failure to verify the downloaded dictionary.";
-    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
-        NewRunnableMethod(this,
-                          &SpellCheckHostImpl::InformProfileOfInitialization));
+    // Let PostTaskAndReply caller send to InformProfileOfInitialization
     return;
   }
 
   size_t bytes_written =
       file_util::WriteFile(bdict_file_path_, data_.data(), data_.length());
   if (bytes_written != data_.length()) {
     bool success = false;
 #if defined(OS_WIN)
     bdict_file_path_ = GetFallbackFilePath(bdict_file_path_);
     bytes_written =
         file_util::WriteFile(GetFallbackFilePath(bdict_file_path_),
                                                  data_.data(), data_.length());
     if (bytes_written == data_.length())
       success = true;
 #endif
     data_.clear();
 
     if (!success) {
       LOG(ERROR) << "Failure to save dictionary.";
       file_util::Delete(bdict_file_path_, false);
       // To avoid trying to load a partially saved dictionary, shortcut the
-      // Initialize() call.
-      BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
-          NewRunnableMethod(this,
-              &SpellCheckHostImpl::InformProfileOfInitialization));
+      // Initialize() call. Let PostTaskAndReply caller send to
+      // InformProfileOfInitialization.
       return;
     }
   }
 
   data_.clear();
+
+  // TODO: Bubble out Initialize() so it's not butchered.
+  // Why isn't this all built up with the contructor? Does it
+  // need to occur like this? Can the FILE and UI uses of
+  // Initialized be separated better?
   Initialize();
 }
 
 bool SpellCheckHostImpl::VerifyBDict(const FilePath& path) const {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
 
   // Read the dictionary file and scan its data. We need to close this file
   // after scanning its data so we can delete it and download a new dictionary
   // from our dictionary-download server if it is corrupted.
   file_util::MemoryMappedFile map;
@@ skipping 17 matching lines @@
   return file_;
 }
 
 const std::string& SpellCheckHostImpl::GetLanguage() const {
   return language_;
 }
 
 bool SpellCheckHostImpl::IsUsingPlatformChecker() const {
   return use_platform_spellchecker_;
 }
+
+void SpellCheckHostImpl::AddWordComplete(const std::string& word) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  for (RenderProcessHost::iterator i(RenderProcessHost::AllHostsIterator());
+       !i.IsAtEnd(); i.Advance()) {
+    i.GetCurrentValue()->Send(new SpellCheckMsg_WordAdded(word));
+  }
+}