Move EnumerateModulesModel work to the blocking pool.

chrome/browser/win/enumerate_modules_model.cc

 // Copyright (c) 2012 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.
 
 #include "chrome/browser/win/enumerate_modules_model.h"
 
 #include <softpub.h>
 #include <stddef.h>
 #include <stdint.h>
 #include <tlhelp32.h>
@@ skipping 358 matching lines @@
 }
 
 ModuleEnumerator::~ModuleEnumerator() {
 }
 
 void ModuleEnumerator::ScanNow(ModulesVector* list) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   enumerated_modules_ = list;
 
   // This object can't be reaped until it has finished scanning, so its safe
-  // to post a raw pointer to another thread.
-  BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE,
-                          base::Bind(&ModuleEnumerator::ScanImpl,
-                                     base::Unretained(this)));
+  // to post a raw pointer to another thread. It will simply be leaked if the
+  // scanning has not been finished before shutdown.
+  auto* pool = BrowserThread::GetBlockingPool();
+  auto token = pool->GetSequenceToken();
+  auto runner = pool->GetSequencedTaskRunnerWithShutdownBehavior(
+      token, base::SequencedWorkerPool::CONTINUE_ON_SHUTDOWN);
+  runner->PostTask(FROM_HERE,

[gab, 2016/09/26 19:37:21]

Since you don't need a SequencedTaskRunner for multiple tasks you can use
SequencedWorkerPool::PostWorkerTaskWithShutdownBehavior() directly without
getting a temporary SequencedTaskRunner 

[chrisha, 2016/09/26 19:58:17]

Ah, even better, thanks.

+                   base::Bind(&ModuleEnumerator::ScanImpl,
+                              base::Unretained(this)));
 }
 
 void ModuleEnumerator::ScanImpl() {
   base::TimeTicks start_time = base::TimeTicks::Now();
 
   enumerated_modules_->clear();
 
   // Make sure the path mapping vector is setup so we can collapse paths.
   PreparePathMappings();
 
@@ skipping 321 matching lines @@
         FROM_HERE,
         BrowserThread::GetTaskRunnerForThread(BrowserThread::UI),
         base::Bind(&EnumerateModulesModel::ScanNow, base::Unretained(this)));
     done = true;
   }
 }
 
 void EnumerateModulesModel::ScanNow() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  // If a module enumerator exists then a scan is already underway.
+  // If a module enumerator exists then a scan is already underway. The client
+  // will receive a notification from that thread.
   if (module_enumerator_)
     return;
 
+  // Only allow a single scan per process lifetime. Immediately notify any
+  // observers that the scan is complete.
+  if (!enumerated_modules_.empty()) {
+    FOR_EACH_OBSERVER(Observer, observers_, OnScanCompleted());
+    return;
+  }

[gab, 2016/09/26 19:37:21]

I'm unclear on why this change is required/related?

[chrisha, 2016/09/26 19:58:17]

Oops, that was meant to be another CL. Will move it there.

+
   // ScanNow does not block, rather it simply schedules a task.
   module_enumerator_.reset(new ModuleEnumerator(this));
   module_enumerator_->ScanNow(&enumerated_modules_);
 }
 
 base::ListValue* EnumerateModulesModel::GetModuleList() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   // If a |module_enumerator_| is still around then scanning has not yet
   // completed, and it is unsafe to read from |enumerated_modules_|.
@@ skipping 120 matching lines @@
   module_enumerator_.reset();
 
   UMA_HISTOGRAM_COUNTS_100("Conflicts.SuspectedBadModules",
                            suspected_bad_modules_detected_);
   UMA_HISTOGRAM_COUNTS_100("Conflicts.ConfirmedBadModules",
                            confirmed_bad_modules_detected_);
 
   // Forward the callback to any registered observers.
   FOR_EACH_OBSERVER(Observer, observers_, OnScanCompleted());
 }