Add the ThirdPartyModules.Uninstallable histogram.

chrome/browser/conflicts/installed_programs_win.cc

+// Copyright 2017 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/conflicts/installed_programs_win.h"
+
+#include <algorithm>
+#include <utility>
+
+#include "base/callback.h"
+#include "base/memory/ptr_util.h"
+#include "base/strings/string_util.h"
+#include "base/strings/stringprintf.h"
+#include "base/task_scheduler/post_task.h"
+#include "base/win/registry.h"
+#include "chrome/browser/conflicts/msi_util_win.h"
+
+namespace {
+
+// Fetches a string |value| out of |key|. Return false if a non-empty value
+// could not be retrived.

[chrisha, 2017/05/02 21:28:24]

retrieved*

[Patrick Monette, 2017/05/29 22:04:54]

Done.

+bool GetRegistryKeyValue(const base::win::RegKey& key,
+                         const wchar_t* value,
+                         base::string16* result) {
+  return key.ReadValue(value, result) == ERROR_SUCCESS && !result->empty();
+}
+
+// Returns true if |candidate| is registered as a system component.
+bool IsSystemComponent(const base::win::RegKey& candidate) {
+  DWORD system_component = 0;
+  return candidate.ReadValueDW(L"SystemComponent", &system_component) ==
+             ERROR_SUCCESS &&
+         system_component == 1;
+}
+
+// Fetches |value| out of |key|. Return false if a non-empty value could not
+// be retrived.

[chrisha, 2017/05/02 21:28:25]

retrieved*

[Patrick Monette, 2017/05/29 22:04:54]

Done.

+bool GetValue(const base::win::RegKey& key,
+              const wchar_t* value,
+              base::string16* result) {
+  return key.ReadValue(value, result) == ERROR_SUCCESS && !result->empty();
+}
+
+// Try to get the |install_path| from |candidate| using the InstallLocation
+// value. Return true on success.
+bool GetInstallPathUsingInstallLocation(const base::win::RegKey& candidate,
+                                        base::FilePath* install_path) {
+  base::string16 install_location;
+  if (GetValue(candidate, L"InstallLocation", &install_location)) {
+    *install_path = base::FilePath(std::move(install_location));
+    return true;
+  }
+  return false;
+}
+
+// Returns true if the |component_path| points to a registry key. Registry key
+// paths are characterized by a number instead of a drive letter.
+// See the documentation for ::MsiGetComponentPath():
+// https://msdn.microsoft.com/en-us/library/windows/desktop/aa370112(v=vs.85).aspx
+bool IsRegistryComponentPath(const base::string16& component_path) {
+  base::string16 drive_letter =
+      component_path.substr(0, component_path.find(':'));
+
+  for (const auto& registry_drive_letter :
+       {L"00", L"01", L"02", L"03", L"20", L"21", L"22", L"23"}) {
+    if (drive_letter == registry_drive_letter)
+      return true;
+  }
+
+  return false;
+}
+
+// Returns all the dlls installed by the product identified by |product_guid|.
+// Returns true on success.
+bool GetInstalledDllsUsingMsiGuid(const base::string16& product_guid,
+                                  std::vector<base::FilePath>* dlls) {
+  // An invalid product guid may have been passed to this function. In those

[chrisha, 2017/05/02 21:28:24]

In this*

[Patrick Monette, 2017/05/29 22:04:55]

Done.

+  // case, GetMsiComponentPaths() will return false so it is not necessary to
+  // specifically filter those out.
+  std::vector<base::string16> component_paths;
+  if (!GetMsiComponentPaths(product_guid, &component_paths))
+    return false;
+
+  for (const auto& component_path : component_paths) {
+    // Exclude registry component paths.
+    if (!IsRegistryComponentPath(component_path)) {

[chrisha, 2017/05/02 21:28:25]

I have a very minor preference:

if (...)
  continue;

// Rest of logic.
...

It avoids one level of indentation. YMMV.

[Patrick Monette, 2017/05/29 22:04:55]

Done.

+      base::FilePath file_path(std::move(component_path));
+      if (base::EqualsCaseInsensitiveASCII(file_path.Extension(), L".dll"))

[chrisha, 2017/05/02 21:28:24]

This might be an artificial constraint. Loadable modules aren't required to have
.dll as an extension, and can in fact have anything.... the only real constraint
is that they be PE files.

This is probably fine for me, but let's at least add a comment.

It might also be interesting to log a metric that count show many loaded modules
don't have .dll as an extension?

[Patrick Monette, 2017/05/29 22:04:54]

This was mainly done so that searching for the associated program is a bit
faster (it doesn't have to check for unrelated files). I've removed it.

I'll add a metric to track this too.

+        dlls->push_back(std::move(file_path));
+    }
+  }
+
+  return true;
+}
+
+// Checks if the registry key that the first 3 parameters refers to references
+// an installed program in the Apps & Features settings page. If so, populates
+// |internal_data| with the program name and its related file paths.
+void CheckRegistryKeyForInstalledProgram(
+    HKEY hkey,
+    const base::string16& key_path,
+    const base::string16& key_name,
+    InstalledPrograms::InternalData* internal_data) {
+  base::win::RegKey candidate(
+      hkey,
+      base::StringPrintf(L"%ls\\%ls", key_path.c_str(), key_name.c_str())
+          .c_str(),
+      KEY_READ);
+
+  if (!candidate.Valid())
+    return;
+
+  // System components are not displayed in the Add or remove programs list.
+  if (IsSystemComponent(candidate))
+    return;
+
+  // If there is no UninstallString, the Uninstall button is grayed out.
+  base::string16 uninstall_string;
+  if (!GetValue(candidate, L"UninstallString", &uninstall_string))
+    return;
+
+  // Ignore Microsoft programs.
+  base::string16 publisher;
+  if (GetValue(candidate, L"Publisher", &publisher) &&
+      publisher == L"Microsoft Corporation") {
+    return;
+  }
+
+  // Candidates with no display names are ignored.
+  // Because this class is used to display warning to the user, not having the

[chrisha, 2017/05/02 21:28:25]

s/the//

[Patrick Monette, 2017/05/29 22:04:54]

Done.

+  // a display name renders the warning somewhat useless.
+  base::string16 display_name;
+  if (!GetValue(candidate, L"DisplayName", &display_name))

[chrisha, 2017/05/02 21:28:25]

In this case could we directly check the image file for a ProductName?

This is one failure case that might be useful to explicitly count via UMA, to
know how big an issue it really is.

[Patrick Monette, 2017/05/29 22:04:54]

It would be possible. But remember that having no DisplayName in the registry
means the entry in the Apps&Features page is not recognizable.
I agree. Done.

+    return;
+
+  base::FilePath install_path;
+  if (GetInstallPathUsingInstallLocation(candidate, &install_path)) {
+    internal_data->program_names.push_back(std::move(display_name));
+    internal_data->install_locations.push_back(std::make_pair(
+        std::move(install_path), internal_data->program_names.size() - 1));
+    return;

[chrisha, 2017/05/02 21:28:25]

So if we find an InstallPath then we don't want to also query the MSI? Why?

[Patrick Monette, 2017/05/29 22:04:54]

If the InstallPath is good, it's faster to check via IsParent() and takes less
memory than to compare against a bunch of files retrieved from the MSI.

Were you suggesting that we check both to potentially get better coverage?

+  }
+
+  std::vector<base::FilePath> dlls;
+  if (GetInstalledDllsUsingMsiGuid(key_name, &dlls)) {
+    internal_data->program_names.push_back(std::move(display_name));
+    for (size_t i = 0; i < dlls.size(); i++) {
+      internal_data->dll_map.push_back(std::make_pair(
+          std::move(dlls[i]), internal_data->program_names.size() - 1));
+    }
+  }
+}
+
+// Returns the index of the matching parent directory of |file| in |container|.
+// Returns false if no matches were found.
+bool BinarySearch(

[chrisha, 2017/05/02 21:28:25]

Yay, you can write a binary search! You're hired :)

This is a very generic and non-descriptive name. Also, don't write your own
binary search. Use std::binary_search and a custom comparator.

[Patrick Monette, 2017/05/29 22:04:55]

Yay!
Mmm this is a case of premature optimization on my part. I'll switch to a
flat_map with this Compare function which often unnecessarily call IsParent():

bool MyCompare(const base::FilePath& lhs, const base::FilePath& rhs) {
  if (lhs.IsParent(rhs) || rhs.IsParent(lhs))
    return false;
  return base::FilePath::CompareLessIgnoreCase(lhs.value(), rhs.value());
}

+    const std::vector<std::pair<base::FilePath, size_t>>& container,
+    const base::FilePath& file,
+    size_t* result) {
+  int min = 0;
+  int max = container.size() - 1;
+  while (min <= max) {
+    int mid_index = (max + min) / 2;
+    const auto& mid = container[mid_index];
+    if (mid.first.IsParent(file)) {
+      *result = mid.second;
+      return true;
+    }
+
+    if (base::FilePath::CompareLessIgnoreCase(file.value(), mid.first.value()))
+      max = mid_index - 1;
+    else
+      min = mid_index + 1;
+  }
+
+  return false;
+}
+
+}  // namespace
+
+InstalledPrograms::InstalledPrograms()
+    : initialized_(false), weak_ptr_factory_(this) {}
+
+InstalledPrograms::~InstalledPrograms() = default;
+
+bool InstalledPrograms::GetInstalledProgramName(const base::FilePath& file,
+                                                base::string16* program_name) {
+  DCHECK(initialized_);
+
+  // First check if the exact file path can be found.
+  auto iter = dll_map_.find(file);
+  if (iter != dll_map_.end()) {
+    *program_name = program_names_[iter->second];
+    return true;
+  }
+
+  // Then check if one of the install locations matches the file.
+  if (install_locations_.empty())
+    return false;
+
+  size_t index;
+  if (!BinarySearch(install_locations_, file, &index))
+    return false;
+
+  *program_name = program_names_[index];
+  return true;
+}
+
+// static
+std::unique_ptr<InstalledPrograms::InternalData>
+InstalledPrograms::GetInternalData() {
+  auto internal_data = base::MakeUnique<InternalData>();
+
+  const wchar_t* kUninstallKeyPaths[] = {
+      L"SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Uninstall",
+      L"SOFTWARE\\WOW6432Node\\Microsoft\\Windows\\CurrentVersion\\Uninstall",
+  };
+
+  for (const auto& uninstall_key_path : kUninstallKeyPaths) {
+    for (const auto& hkey : {HKEY_LOCAL_MACHINE, HKEY_CURRENT_USER}) {
+      for (base::win::RegistryKeyIterator i(hkey, uninstall_key_path);
+           i.Valid(); ++i) {
+        CheckRegistryKeyForInstalledProgram(hkey, uninstall_key_path, i.Name(),
+                                            internal_data.get());
+      }
+    }
+  }
+
+  return internal_data;
+}
+
+void InstalledPrograms::OnInternalDataReceived(
+    const base::Closure& on_initialized_callback,
+    std::unique_ptr<InternalData> internal_data) {
+  program_names_ = std::move(internal_data->program_names);
+  base::flat_map<base::FilePath, size_t, FilePathLess> flat_map(
+      internal_data->dll_map, base::KEEP_FIRST_OF_DUPES);
+  dll_map_ = std::move(flat_map);
+  install_locations_ = std::move(internal_data->install_locations);

[chrisha, 2017/05/02 21:28:25]

Just std::move the entire InternalData object?

[Patrick Monette, 2017/05/29 22:04:54]

Done.

+
+  // Sort |install_locations_| so that it is possible to use binary search.
+  std::sort(install_locations_.begin(), install_locations_.end(),
+            [](const auto& lhs, const auto& rhs) {
+              return base::FilePath::CompareLessIgnoreCase(lhs.first.value(),
+                                                           rhs.first.value());
+            });
+
+  initialized_ = true;
+  if (on_initialized_callback)
+    on_initialized_callback.Run();
+}
+
+void InstalledPrograms::Initialize(
+    const base::Closure& on_initialized_callback) {
+  DCHECK(!initialized_);
+  base::PostTaskWithTraitsAndReplyWithResult(
+      FROM_HERE,
+      base::TaskTraits()
+          .MayBlock()
+          .WithPriority(base::TaskPriority::BACKGROUND)
+          .WithShutdownBehavior(
+              base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN),
+      base::Bind(&InstalledPrograms::GetInternalData),
+      base::Bind(&InstalledPrograms::OnInternalDataReceived,
+                 weak_ptr_factory_.GetWeakPtr(), on_initialized_callback));
+}
+bool InstalledPrograms::FilePathLess::operator()(
+    const base::FilePath& lhs,
+    const base::FilePath& rhs) const {
+  return base::FilePath::CompareLessIgnoreCase(lhs.value(), rhs.value());
+}