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

chrome/browser/spellchecker.h

 // Copyright (c) 2006-2009 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.
 
 #ifndef CHROME_BROWSER_SPELLCHECKER_H_
 #define CHROME_BROWSER_SPELLCHECKER_H_
 
 #include <vector>
 #include <string>
 
 #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"
 #include "chrome/common/pref_member.h"
 
 #include "base/task.h"
 #include "unicode/uscript.h"
 
 class FilePath;
 class Hunspell;
 class PrefService;
 class Profile;
 class MessageLoop;
 class URLRequestContext;
+class URLFetcher;
 
 namespace file_util {
 class MemoryMappedFile;
 }
 
 // The Browser's Spell Checker. It checks and suggests corrections.
 //
 // This object is not threadsafe. In normal usage (not unit tests) it lives on
 // the I/O thread of the browser. It is threadsafe refcounted so that I/O thread
 // and the profile on the main thread (which gives out references to it) can
 // keep it. However, all usage of this must be on the I/O thread.
 //
 // 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.
   //
   // The request context is used to download dictionaries if they do not exist.
   // This can be NULL if you don't want this (like in tests).
   // The |custom_dictionary_file_name| should be left blank so that Spellchecker
   // can figure out the custom dictionary file. It is non empty only for unit
   // testing.
   SpellChecker(const FilePath& dict_dir,
@@ skipping 48 matching lines @@
   // This function returns the corresponding language-region code for the
   // spell check language. For example, for hi, it returns hi-IN.
   static std::string GetSpellCheckLanguageRegion(std::string input_language);
 
   // This function returns ll (language code) from ll-RR where 'RR' (region
   // code) is redundant. However, if the region code matters, it's preserved.
   // That is, it returns 'hi' and 'en-GB' for 'hi-IN' and 'en-GB' respectively.
   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.
   bool CheckSpelling(const std::string& word_to_check);
 
   // When called, relays the request to fill the list with suggestions to
   // the proper backend, either hunspell or a platform-specific backend.
   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();
 
   // After |hunspell_| is initialized, this function is called to add custom
   // 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);
 
   // Returns whether or not the given word is a contraction of valid words
   // (e.g. "word:word").
   bool IsValidContraction(const string16& word);
 
   // Return the file name of the dictionary, including the path and the version
   // numbers.
   FilePath GetVersionedFileName(const std::string& language,
                                 const FilePath& dict_dir);
 
   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.
   FilePath given_dictionary_directory_;
 
   // 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_;
 
   // The hunspell dictionary in use.
   scoped_ptr<Hunspell> hunspell_;
 
   // Represents character attributes used for filtering out characters which
   // are not supported by this SpellChecker object.
   SpellcheckCharAttribute character_attributes_;
 
@@ skipping 13 matching lines @@
   // 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_;
 
   // 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_;
 
   // Remember state for auto spell correct.
   bool auto_spell_correct_turned_on_;
 
   // True if a platform-specific spellchecking engine is being used,
   // 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);
 };
 
 #endif  // CHROME_BROWSER_SPELLCHECKER_H_