[chromium-reviews] Fix a spell check dictionary download bug, where killing the spell checker wh...

chrome/browser/spellchecker.h

Index: chrome/browser/spellchecker.h
===================================================================
--- chrome/browser/spellchecker.h	(revision 21682)
+++ chrome/browser/spellchecker.h	(working copy)
@@ -11,6 +11,7 @@
 #include "app/l10n_util.h"
 #include "base/string_util.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"
@@ -25,6 +26,7 @@
 class Profile;
 class MessageLoop;
 class URLRequestContext;
+class URLFetcher;
 
 namespace file_util {
 class MemoryMappedFile;
@@ -40,7 +42,8 @@
 // This object should also be deleted on the I/O thread only. It owns a
 // reference to URLRequestContext which in turn owns the cache, etc. and must be
 // deleted on the I/O thread itself.
-class SpellChecker : public base::RefCountedThreadSafe<SpellChecker> {
+class SpellChecker : public base::RefCountedThreadSafe<SpellChecker>,
+                     public URLFetcher::Delegate {
  public:
   // Creates the spellchecker by reading dictionaries from the given directory,
   // and defaulting to the given language. Both strings must be provided.
@@ -109,6 +112,13 @@
   static std::string GetLanguageFromLanguageRegion(std::string input_language);
 
  private:
+  // URL Fetcher delegate implementation.

[Peter Kasting, 2009/08/10 22:25:44]

Nit: I suggest changing this to:

// URLFetcher::Delegate implementation.  Called when we finish downloading a
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,
+                                  int response_code,
+                                  const ResponseCookies& cookies,
+                                  const std::string& data);
 
   // When called, relays the request to check the spelling to the proper
   // backend, either hunspell or a platform-specific backend.
@@ -119,9 +129,6 @@
   void FillSuggestionList(const std::string& wrong_word,
                           std::vector<std::wstring>* optional_suggestions);
 
-  // Download dictionary files when required.
-  class DictionaryDownloadController;
-
   // Initializes the Hunspell Dictionary.
   bool Initialize();
 
@@ -129,8 +136,6 @@
   // words from the custom dictionary to the |hunspell_|.
   void AddCustomWordsToHunspell();
 
-  void set_file_is_downloading(bool value);
-
   // 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);
@@ -147,11 +152,10 @@
   static std::string GetCorrespondingSpellCheckLanguage(
       const std::string& language);
 
-  // Start the dictionary download process in the file thread. On completion,
-  // this function calls on set_file_is_downloading() in the IO thread to notify
-  // that download has completed. This function has to be called in the IO
-  // thread.
-  void StartDictionaryDownloadInFileThread(const FilePath& file_name);
+  // Start the dictionary download process in the file thread. The UrlFetcher

[Peter Kasting, 2009/08/10 22:25:44]

Nit: This comment is no longer true because the download process is not started
in the file thread.  Also, to be technically correct, the URLFetcher does not
live in the IO thread; the URLFetcher::Core does.  I suggest the following
comment, assuming you follow my OnURLFetchComplete() comment suggestion above:

// Start downloading a dictionary from the server.  On completion, the
OnURLFetchComplete() function is invoked.

+  // lives in the IO thread while downloading the file to memory.
+  // TODO(sidchat): Do disk IO from memory to disc in the file thread.
+  void StartDictionaryDownload(const FilePath& file_name);
 
   // The given path to the directory whether SpellChecker first tries to
   // download the spellcheck bdic dictionary file.
@@ -160,6 +164,9 @@
   // Path to the custom dictionary file.
   FilePath custom_dictionary_file_name_;
 
+  // BDIC file name (e.g. en-US_1_1.bdic).
+  FilePath bdic_file_name_;
+
   // We memory-map the BDict file.
   scoped_ptr<file_util::MemoryMappedFile> bdict_file_;
 
@@ -193,15 +200,9 @@
   // File Thread Message Loop.
   MessageLoop* file_loop_;
 
-  // UI Thread Message Loop - this will be used as a proxy to access io loop.
-  MessageLoop* ui_loop_;
-
   // Used for requests. MAY BE NULL which means don't try to download.
   URLRequestContext* url_request_context_;
 
-  // DictionaryDownloadController object to download dictionary if required.
-  scoped_refptr<DictionaryDownloadController> ddc_dic_;
-
   // Set when the dictionary file is currently downloading.
   bool dic_is_downloading_;
 
@@ -212,10 +213,8 @@
   // and False if hunspell is being used.
   bool is_using_platform_spelling_engine_;
 
-  // Used for generating callbacks to spellchecker, since spellchecker is a
-  // non-reference counted object. The callback is generated by generating tasks
-  // using NewRunableMethod on these objects.
-  ScopedRunnableMethodFactory<SpellChecker> dic_download_state_changer_factory_;
+  // URLFetcher to download a file in memory.
+  scoped_ptr<URLFetcher> fetcher_;
 
   DISALLOW_COPY_AND_ASSIGN(SpellChecker);
 };