Windows install_static refactor.

chrome/install_static/install_details.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_INSTALL_STATIC_INSTALL_DETAILS_H_
+#define CHROME_INSTALL_STATIC_INSTALL_DETAILS_H_
+
+#include <stdint.h>
+
+#include <memory>
+#include <string>
+
+#include "chrome/install_static/install_constants.h"
+
+namespace install_static {
+
+class PrimaryInstallDetails;
+
+// Details relating to how Chrome is installed. This class and
+// PrimaryInstallDetails (below) are used in tandem so that one instance of the
+// latter may be initialized early during process startup and then shared with
+// other modules in the process. For example, chrome_elf creates the instance
+// for a Chrome process and exports a GetInstallDetailsPayload function used by
+// chrome.exe and chrome.dll to create their own module-specific instances
+// referring to the same underlying payload.
+class InstallDetails {
+ public:
+  InstallDetails(const InstallDetails&) = delete;
+  InstallDetails(InstallDetails&&) = delete;
+  InstallDetails& operator=(const InstallDetails&) = delete;
+  virtual ~InstallDetails() = default;
+
+  // Returns the instance for this module.
+  static const InstallDetails& Get();
+
+  // This mode's index into the brand's array of install modes. This will match

[robertshield, 2016/10/24 13:56:21]

This is the first use of the word "mode" in this file. Could you define install
mode here, or above in the class comment?

[grt (UTC plus 2), 2016/10/24 19:36:00]

Pointer to install_modes.h added to class comment.

+  // a brand-specific InstallConstantIndex enumerator.
+  int install_mode_index() const { return payload_->mode->index; }
+
+  // The mode's install suffix (e.g., " SxS" for canary Chrome), or an empty
+  // string for a brand's primary install mode.
+  const wchar_t* install_suffix() const {
+    return payload_->mode->install_suffix;
+  }
+
+  // The app GUID with which this mode is registered with Google Update, or an
+  // empty string if this brand does not integrate with Google Update.
+  const wchar_t* app_guid() const { return payload_->mode->app_guid; }
+
+  // True if the mode supports installation at system-level.
+  bool supports_system_level() const {
+    return payload_->mode->supports_system_level;
+  }
+
+  // True if the mode supports multi-install.
+  bool supports_multi_install() const {
+    return payload_->mode->supports_multi_install;
+  }
+
+  // The install's update channel, or the string "unknown" if the brand does not
+  // integrate with Google Update.
+  std::wstring channel() const {
+    return std::wstring(payload_->channel, payload_->channel_length);
+  }
+  bool system_level() const { return payload_->system_level; }
+  bool multi_install() const { return payload_->multi_install; }
+
+  // Returns the path to the ClientState key for the installation. Returns the
+  // path for the binaries if |binaries| and Chrome is multi-install. Otherwise,
+  // returns the path for Chrome itself.
+  std::wstring GetClientStateKeyPath(bool binaries) const;
+
+  // Returns the path to the ClientStateMedium key for the installation. Returns
+  // the path for the binaries if |binaries| and Chrome is multi-install.
+  // Otherwise, returns the path for Chrome itself.
+  std::wstring GetClientStateMediumKeyPath(bool binaries) const;
+
+  // Sets the instance for the process. This must be called only once per
+  // process during startup.
+  static void SetForProcess(std::unique_ptr<PrimaryInstallDetails> details);

[robertshield, 2016/10/17 05:27:23]

It seems like this must always be called, exactly once, for the
PrimatyInstallDetails instance that gets created. Why not have
PrimaryInstallDetails set the g_module_details for the current module on
creation? 

[grt (UTC plus 2), 2016/10/24 11:17:51]

So that unittests can play with PIDs without impacting global state, and so that
tests can explicitly set the global state as needed.

+
+  // Returns a pointer to the module's payload so that it may be shared with
+  // other modules in the process.
+  static intptr_t GetPayload();
+
+  // Initializes this module's instance with another module's payload.
+  static void ImportFromModule(const wchar_t* module_name,

[robertshield, 2016/10/24 13:56:21]

bikeshedding: How about |InitializeFromModule| since this method initializes?

[grt (UTC plus 2), 2016/10/24 19:36:00]

What do you think of InitializeForModule for harmony with SetForProcess?

[robertshield, 2016/10/28 06:56:52]

Harmony is cool (https://www.youtube.com/watch?v=LRP8d7hhpoQ) but if I was
bikeshedding I'd say that 

InitializeForModule("awesome.dll", "GetTheThing") reads like it is doing
something to awesome.dll

whereas

InitializeFromModule("awesome.dll", "GetTheThing") reads like it is pulling
something from awesome.dll

this is just bikeshedding though. 

One last thought since I'm here (still in the optional nit category): what do
you think about dropping |export_name| and making the export name fixed by
implementation and declared as a constant in install_details? This would drop
the need to declare a constant in chrome_elf_constants.h|cc. This too might be
six-o'-one.

Anyhow, feel free to ignore any of the above if you like InitializeForModule
better. 

[grt (UTC plus 2), 2016/10/28 10:59:45]

I like it.
InitializeFromPrimaryModule!

+                               const char* export_name);
+
+ protected:
+  // A POD-struct containing the underlying data for an InstallDetails
+  // instance. Multiple InstallDetails instances (e.g., one per module in a
+  // process) share a single underlying Payload.
+  struct Payload {
+    // The brand-specific install mode for this install; see kInstallModes.
+    const InstallConstants* mode;
+
+    // The friendly name of this Chrome's channel, or "unknown" if the brand
+    // does not integrate with Google Update.
+    const wchar_t* channel;
+
+    // The string length of |channel| (not including the string terminator).
+    size_t channel_length;
+
+    // True if installed in C:\Program Files{, {x86)}; otherwise, false.
+    bool system_level;
+
+    // True if multi-install.
+    bool multi_install;
+  };
+
+  explicit InstallDetails(const Payload* payload) : payload_(payload) {}
+  const wchar_t* default_channel_name() const {
+    return payload_->mode->default_channel_name;
+  }
+
+ private:
+  const Payload* const payload_;
+};
+
+// A kind of InstallDetails that owns its payload. A single instance of this
+// class is initialized early on in process startup (e.g., in chrome_elf for the
+// case of chrome.exe processes). Its underlying data (its "payload") is shared
+// with other interested modules in the process.
+class PrimaryInstallDetails : public InstallDetails {
+ public:
+  PrimaryInstallDetails() : InstallDetails(&payload_) {}
+  PrimaryInstallDetails(const PrimaryInstallDetails&) = delete;
+  PrimaryInstallDetails(PrimaryInstallDetails&&) = delete;
+  PrimaryInstallDetails& operator=(const PrimaryInstallDetails&) = delete;
+
+  void set_mode(const InstallConstants* mode) { payload_.mode = mode; }
+  void set_channel(const std::wstring& channel) {
+    channel_ = channel;
+    payload_.channel = channel_.c_str();
+    payload_.channel_length = channel_.size();
+  }
+  void set_system_level(bool system_level) {
+    payload_.system_level = system_level;
+  }
+  void set_multi_install(bool multi_install) {
+    payload_.multi_install = multi_install;
+  }
+
+ private:
+  std::wstring channel_;
+  Payload payload_ = Payload();
+};
+
+}  // namespace install_static
+
+#endif  // CHROME_INSTALL_STATIC_INSTALL_DETAILS_H_