Make module enumeration a low priority background task.

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 42 matching lines @@
     return a.status > b.status;
 
   if (a.location == b.location)
     return a.name < b.name;
 
   return a.location < b.location;
 }
 
 namespace {
 
+// The default amount of time between module inspections. This prevents
+// certificate inspection and validation from using a large amount of CPU and
+// battery immediately after startup.
+const base::TimeDelta kDefaultPerModuleDelay = base::TimeDelta::FromSeconds(1);

[Patrick Monette, 2016/10/14 22:45:55]

constexpr

[chrisha, 2016/10/17 15:32:21]

Done.

+
 // A struct to help de-duping modules before adding them to the enumerated
 // modules vector.
 struct FindModule {
  public:
   explicit FindModule(const ModuleEnumerator::Module& x)
     : module(x) {}
   bool operator()(const ModuleEnumerator::Module& module_in) const {
     return (module.location == module_in.location) &&
            (module.name == module_in.name);
   }
@@ skipping 313 matching lines @@
   // to them. Remove that.
   size_t first_space = module->version.find_first_of(L" ");
   if (first_space != base::string16::npos)
     module->version = module->version.substr(0, first_space);
 
   module->normalized = true;
 }
 
 ModuleEnumerator::ModuleEnumerator(EnumerateModulesModel* observer)
     : enumerated_modules_(nullptr),
-      observer_(observer) {
+      observer_(observer),
+      per_module_delay_(kDefaultPerModuleDelay) {
 }
 
 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. It will simply be leaked if the
   // scanning has not been finished before shutdown.
   BrowserThread::GetBlockingPool()->PostWorkerTaskWithShutdownBehavior(
       FROM_HERE,
-      base::Bind(&ModuleEnumerator::ScanImpl,
+      base::Bind(&ModuleEnumerator::ScanImplStart,
                  base::Unretained(this)),
       base::SequencedWorkerPool::CONTINUE_ON_SHUTDOWN);
 }
 
-void ModuleEnumerator::ScanImpl() {
+void ModuleEnumerator::SetPerModuleDelayToZero() {
+  // Set the delay to zero so the modules enumerate as quickly as possible.
+  per_module_delay_ = base::TimeDelta::FromSeconds(0);
+}
+
+void ModuleEnumerator::ScanImplStart() {
   base::TimeTicks start_time = base::TimeTicks::Now();
 
   // The provided destination for the enumerated modules should be empty, as it
   // should only be populated once, by a single ModuleEnumerator instance. See
   // EnumerateModulesModel::ScanNow for details.
   DCHECK(enumerated_modules_->empty());
 
   // Make sure the path mapping vector is setup so we can collapse paths.
   PreparePathMappings();
 
@@ skipping 10 matching lines @@
   checkpoint2 = base::TimeTicks::Now();
   UMA_HISTOGRAM_TIMES("Conflicts.EnumerateShellExtensions",
                       checkpoint2 - checkpoint);
 
   checkpoint = checkpoint2;
   EnumerateWinsockModules();
   checkpoint2 = base::TimeTicks::Now();
   UMA_HISTOGRAM_TIMES("Conflicts.EnumerateWinsockModules",
                       checkpoint2 - checkpoint);
 
+  enumeration_total_time_ = base::TimeTicks::Now() - start_time;
+
+  // Post a delayed task to scan the first module. This forwards directly to
+  // ScanImplFinish if there are no modules to scan.
+  BrowserThread::GetBlockingPool()->PostDelayedWorkerTask(
+      FROM_HERE,
+      base::Bind(&ModuleEnumerator::ScanImplModule,
+                 base::Unretained(this),
+                 0),
+      per_module_delay_);
+}
+
+void ModuleEnumerator::ScanImplDelay(size_t index) {
+  // Bounce this over to a CONTINUE_ON_SHUTDOWN task in the same pool. This is
+  // necessary to prevent shutdown hangs while inspecting a module.
+  BrowserThread::GetBlockingPool()->PostWorkerTaskWithShutdownBehavior(
+      FROM_HERE,
+      base::Bind(&ModuleEnumerator::ScanImplModule,
+                 base::Unretained(this),
+                 index),
+      base::SequencedWorkerPool::CONTINUE_ON_SHUTDOWN);
+}
+
+void ModuleEnumerator::ScanImplModule(size_t index) {
+  while (index < enumerated_modules_->size()) {
+    base::TimeTicks start_time = base::TimeTicks::Now();
+    Module& entry = enumerated_modules_->at(index);
+    PopulateModuleInformation(&entry);
+    NormalizeModule(&entry);
+    CollapsePath(&entry);
+    base::TimeDelta elapsed = base::TimeTicks::Now() - start_time;
+    enumeration_inspection_time_ += elapsed;
+    enumeration_total_time_ += elapsed;
+
+    // If the delay has been set to zero then simply finish the rest of the
+    // enumeration in this already started task.
+    if (per_module_delay_.is_zero()) {

[Patrick Monette, 2016/10/14 22:45:55]

Reverse the conditional for simpler code? (Let you remove the else{} and
continue; statement.)

[chrisha, 2016/10/17 15:32:21]

Done.

+      ++index;
+      continue;
+    } else {
+      // Bounce back over to ScanImplDelay, which will bounce back to this
+      // function and inspect the next module.
+      BrowserThread::GetBlockingPool()->PostDelayedWorkerTask(
+          FROM_HERE,
+          base::Bind(&ModuleEnumerator::ScanImplDelay,
+                     base::Unretained(this),
+                     index + 1),
+          per_module_delay_);
+      return;
+    }
+  }
+
+  // Getting here means that all of the modules have been inspected.
+  return ScanImplFinish();
+}
+
+void ModuleEnumerator::ScanImplFinish() {
   // TODO(chrisha): Annotate any modules that are suspicious/bad.
 
   ReportThirdPartyMetrics();
 
   std::sort(enumerated_modules_->begin(),
             enumerated_modules_->end(), ModuleSort);
 
+  UMA_HISTOGRAM_TIMES("Conflicts.EnumerationInspectionTime",
+                      enumeration_inspection_time_);
   UMA_HISTOGRAM_TIMES("Conflicts.EnumerationTotalTime",
-                      base::TimeTicks::Now() - start_time);
+                      enumeration_total_time_);
 
   // Send a reply back on the UI thread. The |observer_| outlives this
   // enumerator, so posting a raw pointer is safe. This is done last as
   // DoneScanning will then reap this ModuleEnumerator.
   BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
                           base::Bind(&EnumerateModulesModel::DoneScanning,
                                      base::Unretained(observer_)));
 }
 
 void ModuleEnumerator::EnumerateLoadedModules() {
   // Get all modules in the current process.
   base::win::ScopedHandle snap(::CreateToolhelp32Snapshot(TH32CS_SNAPMODULE,
                                ::GetCurrentProcessId()));
   if (!snap.Get())
     return;
 
   // Walk the module list.
   MODULEENTRY32 module = { sizeof(module) };
   if (!::Module32First(snap.Get(), &module))
     return;
 
   do {
     // It would be weird to present chrome.exe as a loaded module.
     if (_wcsicmp(chrome::kBrowserProcessExecutableName, module.szModule) == 0)
       continue;
 
     Module entry;
     entry.type = LOADED_MODULE;
     entry.location = module.szExePath;
-    PopulateModuleInformation(&entry);
-
-    NormalizeModule(&entry);
-    CollapsePath(&entry);
     enumerated_modules_->push_back(entry);
   } while (::Module32Next(snap.Get(), &module));
 }
 
 void ModuleEnumerator::EnumerateShellExtensions() {
   ReadShellExtensions(HKEY_LOCAL_MACHINE);
   ReadShellExtensions(HKEY_CURRENT_USER);
 }
 
 void ModuleEnumerator::ReadShellExtensions(HKEY parent) {
   base::win::RegistryValueIterator registration(parent, kRegPath);
   while (registration.Valid()) {
     std::wstring key(std::wstring(L"CLSID\\") + registration.Name() +
         L"\\InProcServer32");
     base::win::RegKey clsid;
     if (clsid.Open(HKEY_CLASSES_ROOT, key.c_str(), KEY_READ) != ERROR_SUCCESS) {
       ++registration;
       continue;
     }
     base::string16 dll;
     if (clsid.ReadValue(L"", &dll) != ERROR_SUCCESS) {
       ++registration;
       continue;
     }
     clsid.Close();
 
     Module entry;
     entry.type = SHELL_EXTENSION;
     entry.location = dll;
-    PopulateModuleInformation(&entry);
-
-    NormalizeModule(&entry);
-    CollapsePath(&entry);
     AddToListWithoutDuplicating(entry);
 
     ++registration;
   }
 }
 
 void ModuleEnumerator::EnumerateWinsockModules() {
   // Add to this list the Winsock LSP DLLs.
   WinsockLayeredServiceProviderList layered_providers;
   GetWinsockLayeredServiceProviders(&layered_providers);
   for (size_t i = 0; i < layered_providers.size(); ++i) {
     Module entry;
     entry.type = WINSOCK_MODULE_REGISTRATION;
     entry.status = NOT_MATCHED;
     entry.normalized = false;
     entry.location = layered_providers[i].path;
     entry.description = layered_providers[i].name;
     entry.recommended_action = NONE;
     entry.duplicate_count = 0;
 
     wchar_t expanded[MAX_PATH];
     DWORD size = ExpandEnvironmentStrings(
         entry.location.c_str(), expanded, MAX_PATH);
-    if (size != 0 && size <= MAX_PATH) {
-      GetCertificateInfo(base::FilePath(expanded), &entry.cert_info);
-    }
+    if (size != 0 && size <= MAX_PATH)
+      entry.location = expanded;
     entry.version = base::IntToString16(layered_providers[i].version);
-
-    // Paths have already been collapsed.
-    NormalizeModule(&entry);
     AddToListWithoutDuplicating(entry);
   }
 }
 
 void ModuleEnumerator::PopulateModuleInformation(Module* module) {
   module->status = NOT_MATCHED;
   module->duplicate_count = 0;
   module->normalized = false;
   GetCertificateInfo(base::FilePath(module->location), &module->cert_info);
   module->recommended_action = NONE;
@@ skipping 187 matching lines @@
   return modules_to_notify_about_;
 }
 
 void EnumerateModulesModel::MaybePostScanningTask() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   static bool done = false;
   if (!done) {
     BrowserThread::PostAfterStartupTask(
         FROM_HERE,
         BrowserThread::GetTaskRunnerForThread(BrowserThread::UI),
-        base::Bind(&EnumerateModulesModel::ScanNow, base::Unretained(this)));
+        base::Bind(&EnumerateModulesModel::ScanNow,
+                   base::Unretained(this),
+                   true));
     done = true;
   }
 }
 
-void EnumerateModulesModel::ScanNow() {
+void EnumerateModulesModel::ScanNow(bool background_mode) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   // |module_enumerator_| is used as a lock to know whether or not there are
   // active/pending blocking pool tasks. If a module enumerator exists then a
   // scan is already underway. Otherwise, either no scan has been completed or
   // a scan has terminated.
-  if (module_enumerator_)
+  if (module_enumerator_) {
+    // If a scan is in progress and this request is for immediate results, then
+    // inform the background scan. This is done without any locks because the
+    // other thread only reads from the value that is being modified, and on
+    // Windows its an atomic write.

[Patrick Monette, 2016/10/14 22:45:55]

Is this really safe?

[chrisha, 2016/10/17 15:32:21]

Yeah, this is a standard trick. The variable is being used as a one-way switch.
Writing can only happen on one thread (UI), and reading can only happen on one
thread (a thread in the worker-pool). A 4-byte write/read is atomic, so the
reader will only ever see the original value, or the zero value, and nothing
else is possible.

(We'd require a lock if we had multiple threads doing read-update-write logic,
as they'd be racing with each other. It's even safe to have multiple writer
threads as long as the value they're writing doesn't depend on the current value
of the variable.)

[Patrick Monette, 2016/10/17 19:52:40]

Acknowledged.

+    if (!background_mode)
+      module_enumerator_->SetPerModuleDelayToZero();
     return;
+  }
 
   // Only allow a single scan per process lifetime. Immediately notify any
   // observers that the scan is complete. At this point |enumerated_modules_| is
   // safe to access as no potentially racing blocking pool task can exist.
   if (!enumerated_modules_.empty()) {
     FOR_EACH_OBSERVER(Observer, observers_, OnScanCompleted());
     return;
   }
 
   // ScanNow does not block, rather it simply schedules a task.
   module_enumerator_.reset(new ModuleEnumerator(this));
+  if (!background_mode)
+    module_enumerator_->SetPerModuleDelayToZero();
   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_|.
   if (module_enumerator_.get())
     return nullptr;
@@ skipping 118 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());
 }