Make sure hunspell file is not destroyed in UI thread

In this code:

  base::PostTaskAndReplyWithResult(
      task_runner_.get(), FROM_HERE,
      base::BindOnce(&InitializeDictionaryLocation, language_),
      base::BindOnce(&InitializeDictionaryLocationComplete,
                     weak_ptr_factory_.GetWeakPtr()));

When |SpellcheckHunspellDictionary| is destroyed before
|InitializeDictionaryLocation| is finished, the
|InitializeDictionaryLocationComplete| method would not be called, and
the returned |DictionaryFile| will be destroyed on UI thread, with its
|base::File| member.

It would then result in a DCHECK like this:

  [904:0518/140618.257:FATAL:thread_restrictions.cc(78)] Check failed: !g_blocking_disallowed.Get().Get(). Function marked as blocking was called from a scope that disallows blocking! If this task is running inside the ThreadPool, it needs to have MayBlock() in its TaskTraits. Otherwise, consider making this blocking work asynchronous or, as a last resort, you may use ScopedAllowBlocking (see its documentation for best practices).
  g_blocking_disallowed currently set to true by
  base::debug::CollectStackTrace [0x00007FF6805435C2+18] (o:\base\debug\stack_trace_win.cc:284)
  base::debug::StackTrace::StackTrace [0x00007FF68049BCC2+18] (o:\base\debug\stack_trace.cc:203)
  logging::LogMessage::~LogMessage [0x00007FF6804AF447+215] (o:\base\logging.cc:605)
  logging::LogMessage::~LogMessage [0x00007FF6804B0190+16] (o:\base\logging.cc:598)
  base::internal::AssertBlockingAllowed [0x00007FF680514B33+163] (o:\base\threading\thread_restrictions.cc:85)
  base::ScopedBlockingCall::ScopedBlockingCall [0x00007FF6805103EB+155] (o:\base\threading\scoped_blocking_call.cc:40)
  base::File::Close [0x00007FF680549E4B+139] (o:\base\files\file_win.cc:42)
  SpellcheckHunspellDictionary::DictionaryFile::~DictionaryFile [0x00007FF680071DDE+194] (o:\chrome\browser\spellchecker\spellcheck_hunspell_dictionary.cc:102)
  base::OnceCallback<void (SpellcheckHunspellDictionary::DictionaryFile)>::Run [0x00007FF6800749BF+75] (o:\base\callback.h:100)

To avoid have |base::File| destroyed on UI thread, we should make sure
the file is closed in the destructor of |DictionaryFile|.

Bug: 1085227
Change-Id: I1dba3b36661ec7968b22cf6d7835e41a81a313cc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2206199
Reviewed-by: Rouslan Solomakhin <[email protected]>
Reviewed-by: Sami Kyöstilä <[email protected]>
Reviewed-by: Guillaume Jenkins <[email protected]>
Commit-Queue: Rouslan Solomakhin <[email protected]>
Cr-Commit-Position: refs/heads/master@{#771037}

AUTHORS

@@ -184,6 +184,7 @@ Chanho Park <[email protected]>
 Chansik Yun <[email protected]>
 Chaobin Zhang <[email protected]>
 Charles Vaughn <[email protected]>
+Cheng Zhao <[email protected]>
 Choongwoo Han <[email protected]>
 Chris Greene <[email protected]>
 Chris Harrelson <[email protected]>

chrome/browser/spellchecker/spellcheck_hunspell_dictionary.cc

@@ -90,21 +90,28 @@ bool SaveDictionaryData(std::unique_ptr<std::string> data,
 
 }  // namespace
 
-SpellcheckHunspellDictionary::DictionaryFile::DictionaryFile() {
-}
+SpellcheckHunspellDictionary::DictionaryFile::DictionaryFile(
+    base::TaskRunner* task_runner) : task_runner_(task_runner) {}
 
 SpellcheckHunspellDictionary::DictionaryFile::~DictionaryFile() {
+  if (file.IsValid()) {
+    task_runner_->PostTask(FROM_HERE,
+                           base::BindOnce(&CloseDictionary, std::move(file)));
+  }
 }
 
 SpellcheckHunspellDictionary::DictionaryFile::DictionaryFile(
     DictionaryFile&& other)
-    : path(other.path), file(std::move(other.file)) {}
+    : path(other.path),
+      file(std::move(other.file)),
+      task_runner_(std::move(other.task_runner_)) {}
 
 SpellcheckHunspellDictionary::DictionaryFile&
 SpellcheckHunspellDictionary::DictionaryFile::operator=(
     DictionaryFile&& other) {
   path = other.path;
   file = std::move(other.file);
+  task_runner_ = std::move(other.task_runner_);
   return *this;
 }
 
@@ -120,15 +127,10 @@ SpellcheckHunspellDictionary::SpellcheckHunspellDictionary(
       use_browser_spellchecker_(false),
       browser_context_(browser_context),
       spellcheck_service_(spellcheck_service),
-      download_status_(DOWNLOAD_NONE) {}
+      download_status_(DOWNLOAD_NONE),
+      dictionary_file_(task_runner_.get()) {}
 
 SpellcheckHunspellDictionary::~SpellcheckHunspellDictionary() {
-  if (dictionary_file_.file.IsValid()) {
-    task_runner_->PostTask(
-        FROM_HERE,
-        base::BindOnce(&CloseDictionary, std::move(dictionary_file_.file)));
-  }
-
 #if BUILDFLAG(USE_BROWSER_SPELLCHECKER)
   // Disable the language from platform spellchecker.
   if (spellcheck::UseBrowserSpellChecker() && HasPlatformSupport()) {
@@ -334,7 +336,8 @@ void SpellcheckHunspellDictionary::DownloadDictionary(GURL url) {
 #if !defined(OS_ANDROID)
 // static
 SpellcheckHunspellDictionary::DictionaryFile
-SpellcheckHunspellDictionary::OpenDictionaryFile(const base::FilePath& path) {
+SpellcheckHunspellDictionary::OpenDictionaryFile(base::TaskRunner* task_runner,
+                                                 const base::FilePath& path) {
   base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
                                                 base::BlockingType::MAY_BLOCK);
 
@@ -345,7 +348,7 @@ SpellcheckHunspellDictionary::OpenDictionaryFile(const base::FilePath& path) {
   // For systemwide installations on Windows, the default directory may not
   // have permissions for download. In that case, the alternate directory for
   // download is chrome::DIR_USER_DATA.
-  DictionaryFile dictionary;
+  DictionaryFile dictionary(task_runner);
 
 #if defined(OS_WIN)
   // Check if the dictionary exists in the fallback location. If so, use it
@@ -387,7 +390,7 @@ SpellcheckHunspellDictionary::OpenDictionaryFile(const base::FilePath& path) {
 // static
 SpellcheckHunspellDictionary::DictionaryFile
 SpellcheckHunspellDictionary::InitializeDictionaryLocation(
-    const std::string& language) {
+    base::TaskRunner* task_runner, const std::string& language) {
   base::ScopedBlockingCall scoped_blocking_call(FROM_HERE,
                                                 base::BlockingType::MAY_BLOCK);
 
@@ -402,7 +405,7 @@ SpellcheckHunspellDictionary::InitializeDictionaryLocation(
   base::FilePath dict_path =
       spellcheck::GetVersionedFileName(language, dict_dir);
 
-  return OpenDictionaryFile(dict_path);
+  return OpenDictionaryFile(task_runner, dict_path);
 }
 
 void SpellcheckHunspellDictionary::InitializeDictionaryLocationComplete(
@@ -492,7 +495,8 @@ void SpellcheckHunspellDictionary::PlatformSupportsLanguageComplete(
 #if !defined(OS_ANDROID) && BUILDFLAG(USE_RENDERER_SPELLCHECKER)
     base::PostTaskAndReplyWithResult(
         task_runner_.get(), FROM_HERE,
-        base::BindOnce(&InitializeDictionaryLocation, language_),
+        base::BindOnce(&InitializeDictionaryLocation,
+                       base::RetainedRef(task_runner_.get()), language_),
         base::BindOnce(
             &SpellcheckHunspellDictionary::InitializeDictionaryLocationComplete,
             weak_ptr_factory_.GetWeakPtr()));

chrome/browser/spellchecker/spellcheck_hunspell_dictionary.h

@@ -100,7 +100,7 @@ class SpellcheckHunspellDictionary
   // blocking sequence.
   struct DictionaryFile {
    public:
-    DictionaryFile();
+    explicit DictionaryFile(base::TaskRunner* task_runner);
     ~DictionaryFile();
 
     DictionaryFile(DictionaryFile&& other);
@@ -113,6 +113,9 @@ class SpellcheckHunspellDictionary
     base::File file;
 
    private:
+    // Task runner where the file is created.
+    scoped_refptr<base::TaskRunner> task_runner_;
+
     DISALLOW_COPY_AND_ASSIGN(DictionaryFile);
   };
 
@@ -127,11 +130,12 @@ class SpellcheckHunspellDictionary
 #if !defined(OS_ANDROID)
   // Figures out the location for the dictionary, verifies its contents, and
   // opens it.
-  static DictionaryFile OpenDictionaryFile(const base::FilePath& path);
+  static DictionaryFile OpenDictionaryFile(base::TaskRunner* task_runner,
+                                           const base::FilePath& path);
 
   // Gets the default location for the dictionary file.
   static DictionaryFile InitializeDictionaryLocation(
-      const std::string& language);
+      base::TaskRunner* task_runner, const std::string& language);
 
   // The reply point for PostTaskAndReplyWithResult, called after the dictionary
   // file has been initialized.