[win] Create ModuleDatabase and ModuleEventSinkImpl.

chrome/browser/conflicts/module_database_win.h

+// Copyright 2016 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_MODULE_DATABASE_WIN_H_
+#define CHROME_BROWSER_CONFLICTS_MODULE_DATABASE_WIN_H_
+
+#include <set>
+#include <unordered_set>
+
+#include "base/files/file_path.h"
+#include "base/memory/weak_ptr.h"
+#include "base/sequenced_task_runner.h"
+#include "chrome/common/conflicts/module_event_win.mojom.h"
+#include "content/public/common/process_type.h"
+
+// A class that keeps track of all modules loaded across Chrome processes.
+// Drives the chrome://conflicts UI.
+class ModuleDatabase {
+ public:
+  // A ModuleDatabase is by default bound to a provided sequenced task runner.
+  // Alls calls must be made in the context of this task runner, unless
+  // otherwise noted. For calls from other contexts this task runner is used to
+  // bounce the call when appropriate.
+  explicit ModuleDatabase(scoped_refptr<base::SequencedTaskRunner> task_runner);
+  ~ModuleDatabase();
+
+  // ModuleDatabaseImpl notifications are eventually routed here. These should
+  // only be invoked on the |task_runner| provided at time of construction. They
+  // are exposed so that the browser process can call these directly. These can
+  // be called from any thread, but will bounce to the provided |task_runner|
+  // if not called there.
+  void OnProcessStarted(uint32_t process_id, content::ProcessType process_type);
+  void OnModuleEvent(uint32_t process_id, const mojom::ModuleEvent& event);
+  void OnProcessEnded(uint32_t process_id);
+
+  // TODO(chrisha): Module analysis code, and various accessors for use by
+  // chrome://conflicts.
+
+ private:
+  friend class ModuleDatabaseTest;
+  friend class ModuleDatabaseImplTest;
+
+  // Converts |process_type| to the corresponding bit. Exposed in the header for

[sky, 2016/12/14 21:58:08]

Without looking at the implementation it isn't clear what 'corresponding bit'
means here.

[chrisha, 2016/12/19 20:15:36]

Done.

+  // testing.
+  static uint32_t ProcessTypeToBit(content::ProcessType process_type);

[grt (UTC plus 2), 2016/12/15 09:07:12]

nit: move these function decls down below the types as per
https://google.github.io/styleguide/cppguide.html#Declaration_Order

[chrisha, 2016/12/19 20:15:37]

Done.

+
+  // Converts a |bit_index| to the corresponding process type. Exposed in the
+  // header for testing.
+  static content::ProcessType BitIndexToProcessType(uint32_t bit_index);

[sky, 2016/12/14 21:58:08]

Same comment about bit index here.

[chrisha, 2016/12/19 20:15:37]

Done.

+
+  // Maintains information about a module. Modules are permanent once added to
+  // the ModuleSet, so this structure grows monotonically. In practice this is
+  // not an issue as the modules themselves are vastly bigger than the minor
+  // amount of metadata tracked here.
+  struct ModuleInfo {
+    ModuleInfo(const base::FilePath& module_path, uint32_t module_id);
+
+    // Full path to the module on disk. This is the "key" of the object,
+    // and is how the ModuleInfo is indexed in std::set.
+    const base::FilePath module_path;
+
+    // The ID of this module. This is a strictly incrementing value, and is used
+    // to tie a module to the list of running processes in which it is found.
+    // It is not part of the key for the module, but it is immutable. This is
+    // simply the index of the module in the insertion order.
+    const uint32_t module_id;

[grt (UTC plus 2), 2016/12/15 09:07:12]

int is the type of choice for things that don't explicitly need to be otherwise
(https://google.github.io/styleguide/cppguide.html#Integer_Types).

also, since the sets in ProcessInfo contain these, i mildly suggest making an
explicit type for this (e.g., "using ModuleId = int;" in this struct, or
somesuch).

[chrisha, 2016/12/19 20:15:37]

Done.

+
+    // Everything below this point is implicitly mutable data. Const versions of
+    // this struct will have their constness casted away at runtime, allowing

[grt (UTC plus 2), 2016/12/15 09:07:12]

use "mutable" keyword rather than casting?

[chrisha, 2016/12/19 20:15:37]

I get the ability to have const-correct accessors by not using mutable. For
example, with "CreateProcessInfo".

(I don't feel strongly about this though.)

+    // map-like behaviour for a set without duplicating the key externally.
+
+    // 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
+    // "BitIndexToProcessType".
+    uint32_t process_types;
+  };
+
+  // Key comparator for ModuleInfo, allowing it to be used in a set.

[grt (UTC plus 2), 2016/12/15 09:07:12]

is it relevant to document that it compares by module_path?

[chrisha, 2016/12/19 20:15:36]

Done.

+  struct ModuleInfoKeyComparator {
+    // Implements the less-than operator.
+    bool operator()(const ModuleInfo& mi1, const ModuleInfo& mi2) const;
+  };
+
+  using ModuleSet = std::set<ModuleInfo, ModuleInfoKeyComparator>;
+
+  // Information about a running process. This ties modules in a ModuleSet to
+  // processes in which they are (or have been) loaded.
+  struct ProcessInfo {
+    ProcessInfo(uint32_t process_id, content::ProcessType process_type);
+
+    const uint32_t process_id;
+    const content::ProcessType process_type;
+
+    // Everything below this point is implicitly mutable data. Const versions of
+    // this struct will have their constness casted away at runtime, allowing
+    // map-like behaviour for a set without duplicating the key externally.
+
+    // The set of modules that are loaded/unloaded in this process, by ID. This
+    // is an unordered_set in order to maintain density of memory use as well as
+    // O(1) updates/accesses.
+    std::unordered_set<uint32_t> loaded_module_ids;

[grt (UTC plus 2), 2016/12/15 09:07:12]

will these be large enough that std::vector (sorted or unsorted) won't be better
in all regards (less memory overhead, better cache performance, etc)?

[chrisha, 2016/12/19 20:15:37]

These won't be enormous... the long tail sees something like ~200 modules in a
process. I dislike having to iterate in order to modify these, but I hate the
fragmented storage and high overhead of a std::set. I chose unordered_set as a
compromise (uses linear storage, O(1) access, etc).

But, looking briefly into this it looks like the overhead is actually pretty
atrocious. Given that loads/unloads are actually relatively rare events, I'll
move to using a vector.

+    std::unordered_set<uint32_t> unloaded_module_ids;
+  };
+
+  // Key comparator for ProcessInfo, allowing it to be used in a set.
+  struct ProcessInfoKeyComparator {
+    // Implements the less-than operator.
+    bool operator()(const ProcessInfo& pi1, const ProcessInfo& pi2) const;
+  };
+
+  using ProcessSet = std::set<ProcessInfo, ProcessInfoKeyComparator>;
+
+  // Finds or creates a mutable ModuleInfo entry.
+  ModuleInfo* FindOrCreateModuleInfo(const base::FilePath& module_path);
+
+  // Finds a process info entry. Returns nullptr if none is found.
+  ProcessInfo* GetProcessInfo(uint32_t process_id);
+
+  // Creates a process info entry. Returns nullptr if one already existed.
+  const ProcessInfo* CreateProcessInfo(uint32_t process_id,
+                                       content::ProcessType process_type);
+
+  // Deletes a process info entry. Returns true on success, false otherwise.
+  bool DeleteProcessInfo(uint32_t process_id);
+
+  // The task runner to which this object is bound.
+  scoped_refptr<base::SequencedTaskRunner> task_runner_;
+
+  // Weak pointer factory for this object. This is used when bouncing
+  // incoming events to |task_runner_|.
+  base::WeakPtrFactory<ModuleDatabase> weak_ptr_factory_;

[sky, 2016/12/14 21:58:08]

Make this the last members (I'm surprised you didn't hit a warning when
uploading).

[chrisha, 2016/12/19 20:15:37]

Done.

+
+  // A map of all known modules.
+  ModuleSet modules_;
+
+  // The set of all known running processes, and modules loaded/unloaded in
+  // them.
+  ProcessSet processes_;
+
+  DISALLOW_COPY_AND_ASSIGN(ModuleDatabase);
+};
+
+#endif  // CHROME_BROWSER_CONFLICTS_MODULE_DATABASE_WIN_H_