EnumerateModules: Address UI review comments....

chrome/browser/enumerate_modules_model_win.cc

 // Copyright (c) 2010 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/enumerate_modules_model_win.h"
 
 #include <Tlhelp32.h>
 #include <wintrust.h>
 
 #include "app/l10n_util.h"
@@ skipping 19 matching lines @@
 #include "grit/generated_resources.h"
 
 // The period of time (in milliseconds) to wait until checking to see if any
 // incompatible modules exist.
 static const int kModuleCheckDelayMs = 60 * 1000;
 
 // The path to the Shell Extension key in the Windows registry.
 static const wchar_t kRegPath[] =
     L"Software\\Microsoft\\Windows\\CurrentVersion\\Shell Extensions\\Approved";
 
-// A sort method that sorts by ModuleType ordinal (loaded module at the top),
-// then by full name (including path).
+// A sort method that sorts by bad modules first, then by full name (including
+// path).
 static bool ModuleSort(const ModuleEnumerator::Module& a,
                        const ModuleEnumerator::Module& b) {
-  if (a.type != b.type)
-    return a.type < b.type;
+  if (a.status != b.status)
+    return a.status > b.status;
+
   if (a.location == b.location)
     return a.name < b.name;
 
   return a.location < b.location;
 }
 
 namespace {
 
 // Used to protect the LoadedModuleVector which is accessed
 // from both the UI thread and the FILE thread.
 Lock* lock = NULL;
 
 // 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) {}
-  // Coverity issue 13896.
   bool operator()(const ModuleEnumerator::Module& module_in) const {
-    return (module.type == module_in.type) &&
-           (module.location == module_in.location) &&
+    return (module.location == module_in.location) &&
            (module.name == module_in.name);
   }
 
   const ModuleEnumerator::Module& module;
 };
 
 }
 
 // The browser process module blacklist. This lists all modules that are known
 // to cause compatibility issues within the browser process. When adding to this
 // list, make sure that all paths are lower-case, in long pathname form, end
 // with a slash and use environments variables (or just look at one of the
 // comments below and keep it consistent with that). When adding an entry with
 // an environment variable not currently used in the list below, make sure to
 // update the list in PreparePathMappings. Filename, Description/Signer, and
 // Location must be entered as hashes (see GenerateHash). Filename is mandatory.
 // Entries without any Description, Signer info, or Location will never be
 // marked as confirmed bad (only as suspicious).
 const ModuleEnumerator::BlacklistEntry ModuleEnumerator::kModuleBlacklist[] = {
   // NOTE: Please keep this list sorted by dll name, then location.
 
   // foldersizecolumn.dll.
   {"5ec91bd7", "", "", "", "", NONE},
 
   // idmmbc.dll, "%programfiles%\\internet download manager\\", "Tonec Inc.".
   // See: http://crbug.com/26892/.
-  {"b8dce5c3", "94541bf5", "d33ad640", "", "", NONE},
+  {"b8dce5c3", "94541bf5", "d33ad640", "", "6.03", UPDATE},
 
   // imon.dll.  See: http://crbug.com/21715.
   {"8f42f22e", "", "", "", "", NONE},
 
   // is3lsp.dll.  See: http://crbug.com/26892.
   {"7ffbdce9", "", "", "", "", NONE},
 
   // nvlsp.dll.  See: http://crbug.com/22083.
   {"37f907e2", "", "", "", "", NONE},
 
@@ skipping 82 matching lines @@
         Version::GetVersionFromString(blacklisted.version_from));
     scoped_ptr<Version> version_max(
         Version::GetVersionFromString(blacklisted.version_to));
     bool version_ok = !version_min.get() && !version_max.get();
     if (!version_ok) {
       bool too_low = version_min.get() &&
           (!module_version.get() ||
           module_version->CompareTo(*version_min.get()) < 0);
       bool too_high = version_max.get() &&
           (!module_version.get() ||
-          module_version->CompareTo(*version_max.get()) > 0);
+          module_version->CompareTo(*version_max.get()) >= 0);
       version_ok = !too_low && !too_high;
     }
 
     if (version_ok) {
       // At this point, the names match and there is no version specified
       // or the versions also match.
 
       std::string desc_or_signer(blacklisted.desc_or_signer);
       std::string signer_hash, description_hash;
       GenerateHash(WideToUTF8(module.digital_signer), &signer_hash);
@@ skipping 36 matching lines @@
 
 void ModuleEnumerator::ScanOnFileThread() {
   base::TimeTicks start_time = base::TimeTicks::Now();
 
   enumerated_modules_->clear();
 
   // Make sure the path mapping vector is setup so we can collapse paths.
   PreparePathMappings();
 
   base::TimeTicks checkpoint = base::TimeTicks::Now();
+
+  // Enumerating loaded modules must happen first since the other types of
+  // modules check for duplication against the loaded modules.
   EnumerateLoadedModules();
   HISTOGRAM_TIMES("Conflicts.EnumerateLoadedModules",
                   base::TimeTicks::Now() - checkpoint);
 
   checkpoint = base::TimeTicks::Now();
   EnumerateShellExtensions();
   HISTOGRAM_TIMES("Conflicts.EnumerateShellExtensions",
                   base::TimeTicks::Now() - checkpoint);
 
   checkpoint = base::TimeTicks::Now();
@@ skipping 132 matching lines @@
 void ModuleEnumerator::AddToListWithoutDuplicating(const Module& module) {
   DCHECK(module.normalized);
   // These are registered modules, not loaded modules so the same module
   // can be registered multiple times, often dozens of times. There is no need
   // to list each registration, so we just increment the count for each module
   // that is counted multiple times.
   ModulesVector::iterator iter;
   iter = std::find_if(enumerated_modules_->begin(),
                       enumerated_modules_->end(),
                       FindModule(module));
-  if (iter != enumerated_modules_->end())
+  if (iter != enumerated_modules_->end()) {
     iter->duplicate_count++;
-  else
+    iter->type = static_cast<ModuleType>(iter->type | module.type);
+  } else {
     enumerated_modules_->push_back(module);
+  }
 }
 
 void ModuleEnumerator::PreparePathMappings() {
   path_mapping_.clear();
 
   scoped_ptr<base::Environment> environment(base::Environment::Create());
   std::vector<string16> env_vars;
   env_vars.push_back(L"LOCALAPPDATA");
   env_vars.push_back(L"ProgramFiles");
   env_vars.push_back(L"USERPROFILE");
@@ skipping 186 matching lines @@
     return NULL;
   }
 
   ListValue* list = new ListValue();
 
   for (ModuleEnumerator::ModulesVector::const_iterator module =
            enumerated_modules_.begin();
        module != enumerated_modules_.end(); ++module) {
     DictionaryValue* data = new DictionaryValue();
     data->SetInteger("type", module->type);
-    switch (module->type) {
-      case ModuleEnumerator::SHELL_EXTENSION:
-        data->SetString("type_description", ASCIIToWide("Shell Extension"));
-        break;
-      case ModuleEnumerator::WINSOCK_MODULE_REGISTRATION:
-        data->SetString("type_description", ASCIIToWide("Winsock"));
-        break;
-      default:
-        data->SetString("type_description", ASCIIToWide(""));
-        break;
+    string16 type_string;
+    if ((module->type & ModuleEnumerator::LOADED_MODULE) == 0) {
+      // Module is not loaded, denote type of module.
+      if (module->type & ModuleEnumerator::SHELL_EXTENSION)
+        type_string = ASCIIToWide("Shell Extension");
+      if (module->type & ModuleEnumerator::WINSOCK_MODULE_REGISTRATION) {
+        if (!type_string.empty())
+          type_string += ASCIIToWide(", ");
+        type_string += ASCIIToWide("Winsock");
+      }
+      // Must be one of the above type.
+      DCHECK(!type_string.empty());
+      type_string += ASCIIToWide(" -- ");
+      type_string += l10n_util::GetStringUTF16(IDS_CONFLICTS_NOT_LOADED_YET);
     }
+    data->SetString("type_description", type_string);
     data->SetInteger("status", module->status);
     data->SetString("location", module->location);
     data->SetString("name", module->name);
     data->SetString("product_name", module->product_name);
     data->SetString("description", module->description);
-    data->SetString("version", module->version.empty() ? ASCIIToWide("") :
-        l10n_util::GetStringF(IDS_CONFLICTS_CHECK_VERSION_STRING,
-                   module->version));
+    data->SetString("version", module->version);
     data->SetString("digital_signer", module->digital_signer);
 
     // Figure out the possible resolution help string.
     string16 actions;
     string16 separator = ASCIIToWide(" ") + l10n_util::GetStringUTF16(
         IDS_CONFLICTS_CHECK_POSSIBLE_ACTION_SEPERATOR) +
         ASCIIToWide(" ");
 
     if (module->recommended_action & ModuleEnumerator::NONE) {
       actions = l10n_util::GetStringUTF16(
@@ skipping 93 matching lines @@
   GenerateHash(WideToUTF8(module.name), &filename);
   GenerateHash(WideToUTF8(module.location), &location);
   GenerateHash(WideToUTF8(module.description), &description);
   GenerateHash(WideToUTF8(module.digital_signer), &signer);
 
   string16 url = l10n_util::GetStringF(IDS_HELP_CENTER_VIEW_CONFLICTS,
       ASCIIToWide(filename), ASCIIToWide(location),
       ASCIIToWide(description), ASCIIToWide(signer));
   return GURL(WideToUTF8(url));
 }