[Win] Create ModuleWatcher.

chrome/common/conflicts/module_watcher_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_COMMON_CONFLICTS_MODULE_WATCHER_WIN_H_
+#define CHROME_COMMON_CONFLICTS_MODULE_WATCHER_WIN_H_
+
+#include "base/callback.h"
+#include "base/files/file_path.h"
+#include "base/memory/ref_counted.h"
+#include "base/synchronization/lock.h"
+
+namespace base {
+template <class ObserverType>
+class ObserverListThreadSafe;
+}  // namespace base
+
+namespace conflicts {
+
+class ModuleWatcherTest;
+
+// This class observes modules as they are loaded and unloaded into a process'
+// address space. It works by installing a DllNotification callback, and
+// also enumerating all currently loaded modules at the time of its creation.
+// It doesn't really make sense to create more than one of these in a process,
+// although it's entirely possible.
+class ModuleWatcher {
+ public:
+  // The types of events that are observed by the watcher.
+  enum EventType {

[sky, 2016/11/04 16:27:24]

Generally favor enum class for new code.

[chrisha, 2016/11/04 19:15:49]

Good point. I'll actually be moving this in a Mojo definition to remove the need
to translate these things, which takes care of that for me.

+    // The module was already loaded at the time of creation of the
+    // ModuleWatcher.
+    MODULE_ALREADY_LOADED,
+    // The module was not loaded at the time of creation of the ModuleWatcher
+    // and is being loaded.
+    MODULE_LOADED,
+    // The module is being unloaded.
+    MODULE_UNLOADED,
+  };
+
+  // A notification about a module being loaded or unloaded.
+  struct ModuleEvent {
+    // The type of event.
+    EventType event_type;
+    // The full path to the module being loaded or unloaded.
+    base::FilePath module_path;
+    // The load address of the module.
+    void* load_address;
+    // The size of the module in memory, in bytes.
+    size_t size;
+  };
+
+  // Clients may register callbacks with the following type in order to receive
+  // notifications.
+  using OnModuleEventCallback = base::Callback<void(const ModuleEvent& event)>;
+  class Observer;
+
+  ModuleWatcher();
+  ~ModuleWatcher();
+
+  // Registers a callback with this ModuleWatcher. The thread on which the

[sky, 2016/11/04 16:27:24]

Can you motivate why you need the threading complexity here? I'm wondering why
you can't have all observers added on a single thread?

Better yet, can you detail what threading policy you are implementing here? By
that I mean can one thread call start and another call stop? Do you need to
worry about observers being added while processing start/stop?

[chrisha, 2016/11/04 19:15:49]

The problem is with the LdrDllNotification callback. The callback comes in on
the thread that is doing the DLL loading, which can be most any thread (almost
always the main thread, which becomes the UI thread). However, in the context of
injected third party software it could be on a thread that Chrome doesn't manage
or supervise. Without manually doing the thread bouncing I was looking for a
simple way to have the callback propagate to a thread of my choice.

And this component needs to run in child processes as well, so thanks for
opening up this can of worms :)

In the browser, I want the notifications to arrive on the FILE thread, as
they'll feed directly into the ModuleDatabaseImpl, which needs to go to disk and
do some work. In child processes the notifications will be thin wrappers that
push the data directly to the browser via the ModuleDatabase Mojo interface.
Thus, I actually don't care about the thread in which they're run, unless Mojo
has some threading requirements in child processes.
I expect the watcher to be controlled from the browser impl and renderer impl,
and realistically it can be a leaky singleton that is only ever started. For
unittests, I need to be able to "stop" it so that the callback gets uninstalled
and doesn't leak into other tests. The "Stop" could be made private and renamed
StopForTesting.

Thoughts?

+  // callback is registered is the thread on which the callback will be invoked.
+  // To unregister the callback simply delete the resulting Observer object.
+  // Note that the observer must be destroyed on the same thread on which it
+  // was created. (See ObserverListThreadSafe::RemoveObserver for details.)
+  // NOTE: This is a wrapper around ObserverListThreadSafe that effectively
+  // creates a CallbackListThreadSafe. If anybody goes looking for such a thing
+  // and stumbles upon this code, contact chrisha@ about generalizing it.
+  std::unique_ptr<Observer> RegisterCallback(

[sky, 2016/11/04 16:27:24]

Generally in chrome code and Observer is an interface. Using the name Observer
here is mildly confusing. Maybe CallbackToken?

[chrisha, 2016/11/04 19:15:49]

Yeah, or "Subscription" if I want to follow the CallbackList model. Point taken.

+      const OnModuleEventCallback& callback);
+
+  // Starts the watcher. This enumerates all loaded modules and registers a
+  // DllNotification callback for continued notifications. This is threadsafe.
+  // Note that it is possible to receive duplicate notifications for some
+  // modules as the initial loaded module enumerations is racy with the
+  // installation of the callback. In this case you will receive both a
+  // MODULE_LOADED and a MODULE_ALREADY_LOADED event for the same module. Since
+  // the callback is installed first no modules will be missed.
+  void Start();
+
+  // Stops the watcher. The watcher may subsequently be restarted. Doesn't
+  // need to be called explicitly as ~ModuleWatcher will do so as well. This
+  // is thread-safe.
+  void Stop();
+
+  // Returns true if the watcher is currently running, false otherwise.
+  bool IsRunning();
+
+ protected:
+  // Used as a bridge between the operating system notifications and this class.
+  class Bridge;
+
+  // The Bridge requires access to Notify.
+  friend class Bridge;
+  // For unittesting.
+  friend class ModuleWatcherTest;
+  // Observers require access to |observer_list_|.
+  friend class Observer;
+
+  // Registers a Dll Notification callback with the OS. Returns true on success,
+  // false otherwise. Modifies |dll_notification_cookie_|. Requires |lock_| to
+  // already be acquired.
+  bool RegisterDllNotificationCallback();
+
+  // Removes the installed Dll Notification callback. This should only
+  // be called if InstallDllNotificationCallback returned true. Returns true on
+  // success, false otherwise. Requires |lock_| to already be acquired.
+  bool UnregisterDllNotificationCallback();
+
+  // Enumerates all currently loaded modules, firing off callbacks. Returns
+  // true on success, false otherwise.
+  bool EnumerateAlreadyLoadedModules();
+
+  // Invoked by ModuleWatcherHelper. Dispatches the provided ModuleEvent to all
+  // registered observers.
+  void Notify(const ModuleEvent& event);
+
+  // The list of registered observers. This is directly modified by
+  // ModuleWatcher::Observer.
+  scoped_refptr<base::ObserverListThreadSafe<Observer>> observer_list_;
+
+ private:
+  base::Lock lock_;
+  // Indicates whether or not the ModuleWatcher is currently started. Under
+  // lock_.
+  bool started_;
+  // Used by the DllNotification mechanism. Under lock_.
+  void* dll_notification_cookie_;
+
+  DISALLOW_COPY_AND_ASSIGN(ModuleWatcher);
+};
+
+// Observer class. This is all implementation detail, plumbing callbacks to
+// an ObserverListThreadSafe and handling automatic unregistration. Nothing to
+// see here.
+class ModuleWatcher::Observer {
+ public:
+  ~Observer();
+
+ protected:
+  friend ModuleWatcher;
+
+  // Constructor. To actually create one of these use
+  // ModuleWatcher::RegisterCallback
+  Observer(const OnModuleEventCallback& callback,
+           ModuleWatcher* module_watcher);
+
+  // Used by ModuleWatcher to dispatch events to this observer.
+  void Notify(const ModuleEvent& event);
+
+  // The bound callback.
+  OnModuleEventCallback callback_;

[sky, 2016/11/04 16:27:24]

Style guide says no protected members.

+  // The parent ModuleWatcher to which this observer belongs.
+  ModuleWatcher* module_watcher_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(Observer);
+};
+
+}  // namespace conflicts
+
+#endif  // CHROME_COMMON_CONFLICTS_MODULE_WATCHER_WIN_H_