Add InspectModule() that returns a populated ModuleInspectionResult struct

chrome/browser/conflicts/module_info_win.h

Index: chrome/browser/conflicts/module_info_win.h
diff --git a/chrome/browser/conflicts/module_info_win.h b/chrome/browser/conflicts/module_info_win.h
index 368bfd79f956b730d1c700565d793825967d9e7b..8ecccd81d4ecee31b8ef4961fd88bb8ac3ec239c 100644
--- a/chrome/browser/conflicts/module_info_win.h
+++ b/chrome/browser/conflicts/module_info_win.h
@@ -51,6 +51,10 @@ struct ModuleInfoData {
   ModuleInfoData(const ModuleInfoData& others);
   ~ModuleInfoData();
 
+  // Used to copy a ModuleInfoData returned by InspectModule() into an empty,
+  // existing one.
+  void CopyInspectionData(const ModuleInfoData& other);

[chrisha, 2017/02/28 18:51:29]

How about just a plain old operator=? Or pass a ModuleInfoData object into
InspectModule for it to use for output?

[Patrick Monette, 2017/02/28 23:37:38]

I definitely don't like my current approach. Let me know if you think of
something better. One idea I had was to move all inspection data to a new
struct, and only that part of the ModuleInfoData would need to be copied.

The problem is that some of ModuleInfoData's member should not just be blindly
copied.
For example, process_types: When the module is first seen, process_types will
only have one bit set. By the time it gets inspected on the background thread,
we may receive a new notification that the module was loaded into another
process. When we get the ModuleInfoData back from the inspection, they have to
be OR'ed together.

I could change the semantic of the assign operator but I feel like it would be a
surprising behavior to most people. Hence the named function.

[chrisha, 2017/03/01 15:45:42]

Yeah, having a separate section of ModuleInspectionResults, or something like
that might be cleaner. Then simply use the assignment operator for that when the
results are ready.

You could also consider have ModuleInfoData have a std::unique_ptr to the
inspection results, which remains null until they've been populated?

Agreed that the operator= is a bad idea in this case.

[Patrick Monette, 2017/03/02 20:27:07]

Done.

+
   // Set of all process types in which this module has been seen (may not be
   // currently present in a process of that type). This is a conversion of
   // ProcessType enumeration to a bitfield. See "ProcessTypeToBit" and
@@ -60,6 +64,9 @@ struct ModuleInfoData {
   // The following pieces of information are determined via a detailed
   // inspection of the module.
 
+  // Indicates if the module has been inspected.
+  bool inspected;
+
   // The module path, not including the basename. This is cleaned and normalized
   // so that common paths are converted to their environment variable mappings
   // (ie, %systemroot%). This makes i18n localized paths easily comparable.
@@ -82,4 +89,10 @@ struct ModuleInfoData {
   CertificateInfo certificate_info;
 };
 
+// Given a module identified by |module_key|, returns a populated ModuleInfoData
+// that contains detailed information about the module on disk. This is a
+// blocking task that requires access to disk.
+ModuleInfoData InspectModule(const StringMapping& env_variable_mapping,
+                             const ModuleInfoKey& module_key);
+
 #endif  // CHROME_BROWSER_CONFLICTS_MODULE_INFO_WIN_H_