Remove refptr usages from SpellCheckHost

chrome/browser/spellchecker/spellcheck_host_impl.cc

Index: chrome/browser/spellchecker/spellcheck_host_impl.cc
diff --git a/chrome/browser/spellchecker/spellcheck_host_impl.cc b/chrome/browser/spellchecker/spellcheck_host_impl.cc
index 8762b1abe0e3c214e4c3cda9cca5e63a8f3ccf84..fbdf16cc07e65d29e7c30405776cc261533ab161 100644
--- a/chrome/browser/spellchecker/spellcheck_host_impl.cc
+++ b/chrome/browser/spellchecker/spellcheck_host_impl.cc
@@ -6,6 +6,7 @@
 
 #include <set>
 
+#include "base/bind.h"
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/path_service.h"
@@ -80,6 +81,11 @@ FilePath GetFallbackFilePath(const FilePath& first_choice) {
 }
 #endif
 
+void CloseDictionary(base::PlatformFile file) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+  base::ClosePlatformFile(file);
+}
+
 }  // namespace
 
 // Constructed on UI thread.
@@ -94,7 +100,8 @@ SpellCheckHostImpl::SpellCheckHostImpl(
       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)) {

[Hironori Bono, 2011/11/07 07:59:59]

Can you initialize |dictionary_saved_| to false here?

   DCHECK(profile_);
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
@@ -103,16 +110,24 @@ SpellCheckHostImpl::SpellCheckHostImpl(
   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)
@@ -121,8 +136,8 @@ void SpellCheckHostImpl::Initialize() {
     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;
   }
 
@@ -130,9 +145,11 @@ void SpellCheckHostImpl::Initialize() {
   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() {
@@ -177,13 +194,12 @@ void SpellCheckHostImpl::AddWord(const std::string& word) {
 
   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() {
@@ -204,12 +220,6 @@ void SpellCheckHostImpl::InitializeDictionaryLocation() {
   }
 #endif
 
-  InitializeInternal();
-}
-
-void SpellCheckHostImpl::InitializeInternal() {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
-
   if (!profile_)
     return;
 
@@ -231,31 +241,33 @@ void SpellCheckHostImpl::InitializeInternal() {
   // File didn't exist. Download it.
   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 this function so InitializeDictionaryLocationComplete() can start
+    // downloading the dictionary.
     return;
   }
 
   request_context_getter_ = NULL;
 
-  scoped_ptr<CustomWordList> custom_words(new CustomWordList());
+  custom_words_.reset(new CustomWordList());
   if (file_ != base::kInvalidPlatformFileValue)
-    LoadCustomDictionary(custom_words.get());
-
-  BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
-      NewRunnableMethod(
-          this,
-          &SpellCheckHostImpl::InformProfileOfInitializationWithCustomWords,
-          custom_words.release()));
+    LoadCustomDictionary(custom_words_.get());
 }
 
-void SpellCheckHostImpl::InitializeOnFileThread() {
-  DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::FILE));
-
-  BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
-      NewRunnableMethod(this, &SpellCheckHostImpl::Initialize));
+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,
+        base::Bind(&SpellCheckHostImpl::DownloadDictionary,
+                   weak_ptr_factory_.GetWeakPtr()));
+  } else {
+    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
+        base::Bind(
+            &SpellCheckHostImpl::InformProfileOfInitializationWithCustomWords,
+            weak_ptr_factory_.GetWeakPtr(),
+            custom_words_.release()));
+  }
 }
 
 void SpellCheckHostImpl::InformProfileOfInitialization() {
@@ -282,11 +294,7 @@ void SpellCheckHostImpl::InformProfileOfInitializationWithCustomWords(
 
 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[] =
@@ -298,8 +306,8 @@ void SpellCheckHostImpl::DownloadDictionary() {
   }
   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();
@@ -343,7 +351,6 @@ void SpellCheckHostImpl::OnURLFetchComplete(const content::URLFetcher* source) {
   if ((source->GetResponseCode() / 100) != 2) {
     // Initialize will not try to download the file a second time.
     LOG(ERROR) << "Failure to download dictionary.";
-    InitializeOnFileThread();
     return;
   }
 
@@ -355,13 +362,15 @@ void SpellCheckHostImpl::OnURLFetchComplete(const content::URLFetcher* source) {
   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;

[Hironori Bono, 2011/11/07 07:59:59]

Is it possible to add a line 'dictionary_saved_ = false;' here?

-  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,

[Hironori Bono, 2011/11/07 07:59:59]

Can you add a new function 'SaveDictionaryDataComplete' to this class and use it
instead of InformProfileOfInitialization?

+                 weak_ptr_factory_.GetWeakPtr()));
 }
 
 void SpellCheckHostImpl::Observe(int type,
@@ -382,9 +391,7 @@ void SpellCheckHostImpl::SaveDictionaryData() {
     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;
   }
 
@@ -406,15 +413,14 @@ void SpellCheckHostImpl::SaveDictionaryData() {
       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();
+
   Initialize();

[Hironori Bono, 2011/11/07 07:59:59]

Sorry, I have noticed this Initialize() call causes a DCHECK failure. Is it
possible to add a boolean variable |dictionary_saved_| and set it true here?

 }
 
@@ -452,3 +458,12 @@ const std::string& SpellCheckHostImpl::GetLanguage() const {
 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));
+  }
+}

[Hironori Bono, 2011/11/07 07:59:59]

Can you add the implementation of the SaveDictionaryDataComplete() here as
listed in the following snippet?

  void SpellCheckHostImpl::SaveDictionaryDataComplete() {
    if (dictionary_saved_)
      Initialize();
    else
      InformProfileOfInitialization();
  }