Add the ThirdPartyModules.Uninstallable histogram.

chrome/browser/conflicts/installed_programs_win.h

Index: chrome/browser/conflicts/installed_programs_win.h
diff --git a/chrome/browser/conflicts/installed_programs_win.h b/chrome/browser/conflicts/installed_programs_win.h
new file mode 100644
index 0000000000000000000000000000000000000000..dda1c0fc1f503cee8da510f99036fa5303d0637d
--- /dev/null
+++ b/chrome/browser/conflicts/installed_programs_win.h
@@ -0,0 +1,72 @@
+// 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.
+
+#ifndef CHROME_BROWSER_CONFLICTS_INSTALLED_PROGRAMS_WIN_H_
+#define CHROME_BROWSER_CONFLICTS_INSTALLED_PROGRAMS_WIN_H_
+
+#include <memory>
+#include <utility>
+#include <vector>
+
+#include "base/callback_forward.h"
+#include "base/containers/flat_map.h"
+#include "base/files/file_path.h"
+#include "base/macros.h"
+#include "base/memory/weak_ptr.h"
+#include "base/strings/string16.h"
+
+class InstalledPrograms {

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

Some class documentation might be useful. How is this thing intended to be used?
Who will own it? What is it's lifetime? What is the threading model?

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

I added documentation. I feel like only a small description and a note on
thread-safety is needed. Nothing special is going on with the lifetime and it's
only meant to be owned via composition.

+ public:
+  // This structure has the same representation than the internal state of

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

as* the internal

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

I got rid of InternalData.

+  // InstalledPrograms. It is created on a background sequenced and passed back

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

sequence*

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

Ditto.

+  // on the sequence that this instance was initialized.
+  struct InternalData {

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

Is there a reason why we don't simply document these variables here, and have
InstalledPrograms own an instance of InternalData? Which is then moved on the
OnInternalDataReceived callback?

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

No longer applicable.

+    std::vector<base::string16> program_names;
+    std::vector<std::pair<base::FilePath, size_t>> dll_map;
+    std::vector<std::pair<base::FilePath, size_t>> install_locations;
+  };
+
+  InstalledPrograms();
+  ~InstalledPrograms();
+
+  // Initializes this class on a background sequence.
+  void Initialize(const base::Closure& on_initialized_callback);

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

Invokes the callback on the background sequence, or on the sequence calling
Initialize? Please clarify.

+
+  // Given a |dll|, checks if it matches with an installed program on the user's
+  // machine and return the matching program name. Do not call this before the
+  // initialization is done.
+  bool GetInstalledProgramName(const base::FilePath& dll,
+                               base::string16* program_name);
+
+ private:
+  // Generates an InternalData instance. Should be called on a background
+  // sequence.

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

Document the fact that these will be used by Initialize, and *will* be executed
in the task scheduler with background priority (rather than "should")

(It wasn't clear to me how these were going to be used until I read the full
impl.)

+  static std::unique_ptr<InternalData> GetInternalData();
+  void OnInternalDataReceived(const base::Closure& on_initialized_callback,
+                              std::unique_ptr<InternalData> internal_data);
+
+  // Used to assert that initialization was completed before calling
+  // GetInstalledProgramName().
+  bool initialized_;
+
+  // Program names are stored in this vector because multiple entries in
+  // dll_map_ could point to the same one. This is to avoid duplicating them.
+  std::vector<base::string16> program_names_;
+  struct FilePathLess {
+    bool operator()(const base::FilePath& lhs, const base::FilePath& rhs) const;
+  };
+
+  // Contains all the dll files from programs installed via Microsoft Installer.
+  base::flat_map<base::FilePath, size_t, FilePathLess> dll_map_;

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

Is there a reason why we use a flat_map here and a std::vector above?

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

Switched both to flat_map.

+
+  // For some programs, the best information available is the directory of the
+  // installation.
+  std::vector<std::pair<base::FilePath, size_t>> install_locations_;
+
+  base::WeakPtrFactory<InstalledPrograms> weak_ptr_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(InstalledPrograms);
+};
+
+#endif  // CHROME_BROWSER_CONFLICTS_INSTALLED_PROGRAMS_WIN_H_