Spellchecker: Always destruct url request context getter on io thread.

To do this, we have to initiate downloads on the UI thread and don't hold onto a reference in the file thread.

BUG=27667

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

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

5 files changed, 78 insertions, 20 deletions

diff --git a/chrome/browser/cookies_tree_model_unittest.cc b/chrome/browser/cookies_tree_model_unittest.cc
index 43575f5..199dcaf 100644
--- a/chrome/browser/cookies_tree_model_unittest.cc
+++ b/chrome/browser/cookies_tree_model_unittest.cc
@@ -50,10 +50,14 @@ class CookieTestingProfile : public TestingProfile {
   scoped_refptr<URLRequestContextGetter> url_request_context_getter_;
 };
 
-
-
 class CookiesTreeModelTest : public testing::Test {
  public:
+  CookiesTreeModelTest() : io_thread_(ChromeThread::IO, &message_loop_) {
+  }
+
+  virtual ~CookiesTreeModelTest() {
+  }
+
   virtual void SetUp() {
     profile_.reset(new CookieTestingProfile());
   }
@@ -109,11 +113,12 @@ class CookiesTreeModelTest : public testing::Test {
     delete parent_node->GetModel()->Remove(parent_node, ct_node_index);
   }
  protected:
-  MessageLoopForUI message_loop_;
+  MessageLoop message_loop_;
+  ChromeThread io_thread_;
+
   scoped_ptr<CookieTestingProfile> profile_;
 };
 
-
 TEST_F(CookiesTreeModelTest, RemoveAll) {
   net::CookieMonster* monster = profile_->GetCookieMonster();
   monster->SetCookie(GURL("http://foo"), "A=1");
diff --git a/chrome/browser/gtk/options/cookies_view_unittest.cc b/chrome/browser/gtk/options/cookies_view_unittest.cc
index 10f757e..2160bfa 100644
--- a/chrome/browser/gtk/options/cookies_view_unittest.cc
+++ b/chrome/browser/gtk/options/cookies_view_unittest.cc
@@ -61,6 +61,12 @@ class CookieTestingProfile : public TestingProfile {
 
 class CookiesViewTest : public testing::Test {
  public:
+  CookiesViewTest() : io_thread_(ChromeThread::IO, &message_loop_) {
+  }
+
+  virtual ~CookiesViewTest() {
+  }
+
   virtual void SetUp() {
     profile_.reset(new CookieTestingProfile());
   }
@@ -153,7 +159,9 @@ class CookiesViewTest : public testing::Test {
   }
 
  protected:
-  MessageLoopForUI message_loop_;
+  MessageLoop message_loop_;
+  ChromeThread io_thread_;
+
   scoped_ptr<CookieTestingProfile> profile_;
 };
 
diff --git a/chrome/browser/net/url_request_context_getter.h b/chrome/browser/net/url_request_context_getter.h
index 78edbd1..51c2ca9 100644
--- a/chrome/browser/net/url_request_context_getter.h
+++ b/chrome/browser/net/url_request_context_getter.h
@@ -6,6 +6,7 @@
 #define CHROME_BROWSER_NET_URL_REQUEST_CONTEXT_GETTER_H_
 
 #include "base/ref_counted.h"
+#include "chrome/browser/chrome_thread.h"
 
 namespace net {
 class CookieStore;
@@ -15,7 +16,8 @@ class URLRequestContext;
 
 // Interface for retrieving an URLRequestContext.
 class URLRequestContextGetter
-    : public base::RefCountedThreadSafe<URLRequestContextGetter> {
+    : public base::RefCountedThreadSafe<URLRequestContextGetter,
+                                        ChromeThread::DeleteOnIOThread> {
  public:
   virtual URLRequestContext* GetURLRequestContext() = 0;
 
@@ -24,7 +26,8 @@ class URLRequestContextGetter
   virtual net::CookieStore* GetCookieStore();
 
  protected:
-  friend class base::RefCountedThreadSafe<URLRequestContextGetter>;
+  friend class ChromeThread;
+  friend class DeleteTask<URLRequestContextGetter>;
 
   virtual ~URLRequestContextGetter() {}
 };
diff --git a/chrome/browser/spellcheck_host.cc b/chrome/browser/spellcheck_host.cc
index e276473..446d19d 100644
--- a/chrome/browser/spellcheck_host.cc
+++ b/chrome/browser/spellcheck_host.cc
@@ -169,6 +169,8 @@ void SpellCheckHost::UnsetObserver() {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
 
   observer_ = NULL;
+  request_context_getter_ = NULL;
+  fetcher_.reset();
 }
 
 void SpellCheckHost::AddWord(const std::string& word) {
@@ -210,11 +212,17 @@ void SpellCheckHost::InitializeInternal() {
       NULL);
 
   // File didn't exist. Download it.
-  if (file_ == base::kInvalidPlatformFileValue && !tried_to_download_) {
-    DownloadDictionary();
+  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.
+    ChromeThread::PostTask(ChromeThread::UI, FROM_HERE,
+        NewRunnableMethod(this, &SpellCheckHost::DownloadDictionary));
     return;
   }
 
+  request_context_getter_ = NULL;
+
   if (file_ != base::kInvalidPlatformFileValue) {
     // Load custom dictionary.
     std::string contents;
@@ -230,6 +238,13 @@ void SpellCheckHost::InitializeInternal() {
           &SpellCheckHost::InformObserverOfInitialization));
 }
 
+void SpellCheckHost::InitializeOnFileThread() {
+  DCHECK(!ChromeThread::CurrentlyOn(ChromeThread::FILE));
+
+  ChromeThread::PostTask(ChromeThread::FILE, FROM_HERE,
+      NewRunnableMethod(this, &SpellCheckHost::Initialize));
+}
+
 void SpellCheckHost::InformObserverOfInitialization() {
   DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
 
@@ -238,7 +253,12 @@ void SpellCheckHost::InformObserverOfInitialization() {
 }
 
 void SpellCheckHost::DownloadDictionary() {
-  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
+
+  if (!request_context_getter_) {
+    InitializeOnFileThread();
+    return;
+  }
 
   // Determine URL of file to download.
   static const char kDownloadServerUrl[] =
@@ -246,9 +266,10 @@ void SpellCheckHost::DownloadDictionary() {
   GURL url = GURL(std::string(kDownloadServerUrl) + WideToUTF8(
       l10n_util::ToLower(bdict_file_path_.BaseName().ToWStringHack())));
   fetcher_.reset(new URLFetcher(url, URLFetcher::GET, this));
-  fetcher_->set_request_context(request_context_getter_.get());
+  fetcher_->set_request_context(request_context_getter_);
   tried_to_download_ = true;
   fetcher_->Start();
+  request_context_getter_ = NULL;
 }
 
 void SpellCheckHost::WriteWordToCustomDictionary(const std::string& word) {
@@ -269,12 +290,13 @@ void SpellCheckHost::OnURLFetchComplete(const URLFetcher* source,
                                         const ResponseCookies& cookies,
                                         const std::string& data) {
   DCHECK(source);
-  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::UI));
+  fetcher_.reset();
 
   if ((response_code / 100) != 2) {
     // Initialize will not try to download the file a second time.
     LOG(ERROR) << "Failure to download dictionary.";
-    Initialize();
+    InitializeOnFileThread();
     return;
   }
 
@@ -284,25 +306,35 @@ void SpellCheckHost::OnURLFetchComplete(const URLFetcher* source,
   if (data.size() < 4 || data[0] != 'B' || data[1] != 'D' || data[2] != 'i' ||
       data[3] != 'c') {
     LOG(ERROR) << "Failure to download dictionary.";
-    Initialize();
+    InitializeOnFileThread();
     return;
   }
 
+  data_ = data;
+  ChromeThread::PostTask(ChromeThread::FILE, FROM_HERE,
+      NewRunnableMethod(this, &SpellCheckHost::SaveDictionaryData));
+}
+
+void SpellCheckHost::SaveDictionaryData() {
+  DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
+
   size_t bytes_written =
-      file_util::WriteFile(bdict_file_path_, data.data(), data.length());
-  if (bytes_written != data.length()) {
+      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())
+                                                 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.
       ChromeThread::PostTask(ChromeThread::UI, FROM_HERE,
@@ -312,5 +344,6 @@ void SpellCheckHost::OnURLFetchComplete(const URLFetcher* source,
     }
   }
 
+  data_.clear();
   Initialize();
 }
diff --git a/chrome/browser/spellcheck_host.h b/chrome/browser/spellcheck_host.h
index 57bacdd..6b25cd0 100644
--- a/chrome/browser/spellcheck_host.h
+++ b/chrome/browser/spellcheck_host.h
@@ -64,6 +64,8 @@ class SpellCheckHost : public base::RefCountedThreadSafe<SpellCheckHost,
   // Executed on the file thread.
   void InitializeInternal();
 
+  void InitializeOnFileThread();
+
   // Inform |observer_| that initialization has finished.
   void InformObserverOfInitialization();
 
@@ -82,6 +84,9 @@ class SpellCheckHost : public base::RefCountedThreadSafe<SpellCheckHost,
                                   const ResponseCookies& cookies,
                                   const std::string& data);
 
+  // Saves |data_| to disk. Run on the file thread.
+  void SaveDictionaryData();
+
   // May be NULL.
   Observer* observer_;
 
@@ -104,8 +109,12 @@ class SpellCheckHost : public base::RefCountedThreadSafe<SpellCheckHost,
   // once.
   bool tried_to_download_;
 
-  // Used for downloading the dictionary file.
-  scoped_refptr<URLRequestContextGetter> request_context_getter_;
+  // Data received from the dictionary download.
+  std::string data_;
+
+  // Used for downloading the dictionary file. We don't hold a reference, and
+  // it is only valid to use it on the UI thread.
+  URLRequestContextGetter* request_context_getter_;
 
   // Used for downloading the dictionary file.
   scoped_ptr<URLFetcher> fetcher_;