[win] Create ModuleDatabase and ModuleEventSinkImpl.

chrome/browser/conflicts/module_database_impl_win.h

Index: chrome/browser/conflicts/module_database_impl_win.h
diff --git a/chrome/browser/conflicts/module_database_impl_win.h b/chrome/browser/conflicts/module_database_impl_win.h
new file mode 100644
index 0000000000000000000000000000000000000000..e29d45918b5d77ec5846f121ffeb44ec91e232e9
--- /dev/null
+++ b/chrome/browser/conflicts/module_database_impl_win.h
@@ -0,0 +1,53 @@
+// 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_IMPL_WIN_H_
+#define CHROME_BROWSER_CONFLICTS_MODULE_DATABASE_IMPL_WIN_H_
+
+#include "chrome/common/conflicts/module_database_win.mojom.h"
+#include "content/public/common/process_type.h"
+
+class ModuleDatabase;
+
+// Implementation of the mojom::ModuleDatabase interface. This is the endpoint
+// in the browser process. It simply redirects all calls to the singleton
+// ModuleDatabase object.
+class ModuleDatabaseImpl : public mojom::ModuleDatabase {

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

optional: I've been encouraging people to use chrome:: as the namespace for
mojoms. We don't appear to be consistent though, so optional.

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

Yeah, I've noticed both. A quick check on cs/ shows an order of magnitude more
usage of mojom:: without the chrome::.

+ public:
+  // Creates a service endpoint that forwards notifications from the remote
+  // process to the provided |module_database|. The |module_database| must
+  // outlive this object.
+  explicit ModuleDatabaseImpl(::ModuleDatabase* module_database);

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

is it common practice to have colliding names like this (one in mojom and one
out of it)?

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

I've seen all things. The most common is a mojom::Foo interface, and a FooImpl
implementation of that interface on the other end of the pipe. In this case, the
FooImpl is a thin facade to a singleton Foo. I was torn about naming, but in the
end preferred the colliding namespaced names.

I've also seen:

mojom::FooControllerClient (interface definition)
FooController (implementation of interface)
Foo (the central thing they all talk to)

I'm open to suggestions here!

+  ~ModuleDatabaseImpl() override;
+
+  // Factory function for use with service_manager::InterfaceRegistry. This
+  // creates a concrete implementation of mojom::ModuleDatabase interface in the
+  // current process, for the remote process represented by the provided
+  // |request|.
+  static void Create(::ModuleDatabase* module_database,

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

How do you ensure ModuleDatabase outlives this?

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

My plan was for ModuleDatabase to be a leaky singleton. It's needed for the
lifetime of the entire browser process. However, I'd be just as happy using
shared_ptr if you have a strong preference.

+                     mojom::ModuleDatabaseRequest request);
+
+  // mojom::ModuleDatabase implementation:
+  void OnProcessStarted(uint32_t process_id, uint32_t process_type) override;
+  void OnModuleEvent(mojom::ModuleEventPtr module_event) override;
+
+  // Hooked up as the connection error handler, allowing the database to track
+  // the process death. This assumes that the child process maintains a single
+  // persistent connection.
+  void OnProcessEnded();
+
+  // Accessors, for testing.
+  ::ModuleDatabase* module_database() const { return module_database_; }

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

Either have this return a const ModuleDatabase* or remove const.
optional: I would move this into the private section and friend the test, but
that isn't a standard so I leave it to you to decide.

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

I'll drop the accessors entirely and use a friend declaration.

+  uint32_t process_id() const { return process_id_; }
+
+ private:
+  // The module database this forwards events to. It must outlive this object.
+  ::ModuleDatabase* module_database_;
+  // The cached process ID of the remote process on the other end of this pipe.

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

nit: blank line before this

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

Done.

+  // This is populated on OnProcessStarted (the first function expected to be
+  // called), and forwarded along to the central database with each call.
+  uint32_t process_id_;

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

pid + creation time are needed to uniquely identify a process on windows.

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

This object is bound in lifetime to the corresponding process, and lives and
dies with it (guaranteed by Mojo). So I was worried about colliding process IDs.

There is a very minor potential for collision in the ::ModuleDatabase itself, if
the OS reuses a PID before the ProcessEnd message gets posted, and a
ProcessStart ends up getting posted before it due to a race. But the chances of
this are *very* small given that the OS tends to avoid PID reuse on short
timescales. If you feel strongly about this I could augment with the creation
time.

+};

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

DISALLOW...

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

Done.

+
+#endif  // CHROME_BROWSER_CONFLICTS_MODULE_DATABASE_IMPL_WIN_H_