Move HasUserExperiments into install_static.

This is another step in bringing brand-specific constants together into one place, eventually leading to the demise of BrowserDistribution. BUG=373987 Review-Url: https://codereview.chromium.org/2777943002 Cr-Commit-Position: refs/heads/master@{#459854}
Denger-Network · Mar 27, 2017 · 00d4454 · 00d4454
1 parent c1f7bf4
commit 00d4454
Show file tree

Hide file tree

Showing 14 changed files with 54 additions and 60 deletions.
diff --git a/chrome/browser/first_run/try_chrome_dialog_view.cc b/chrome/browser/first_run/try_chrome_dialog_view.cc
@@ -15,7 +15,7 @@
 #include "chrome/grit/chromium_strings.h"
 #include "chrome/grit/generated_resources.h"
 #include "chrome/grit/theme_resources.h"
-#include "chrome/installer/util/browser_distribution.h"
+#include "chrome/install_static/install_util.h"
 #include "chrome/installer/util/user_experiment.h"
 #include "components/strings/grit/components_strings.h"
 #include "ui/aura/window.h"
@@ -172,7 +172,7 @@ TryChromeDialogView::Result TryChromeDialogView::ShowModal(
 
   // Find out what experiment we are conducting.
   installer::ExperimentDetails experiment;
-  if (!BrowserDistribution::GetDistribution()->HasUserExperiments() ||
+  if (!install_static::SupportsRetentionExperiments() ||
       !installer::CreateExperimentDetails(flavor_, &experiment) ||
       !experiment.heading) {
     NOTREACHED() << "Cannot determine which headline to show.";

diff --git a/chrome/install_static/chromium_install_modes.cc b/chrome/install_static/chromium_install_modes.cc
@@ -24,29 +24,28 @@ const wchar_t kBinariesAppGuid[] = L"";
 const wchar_t kBinariesPathName[] = L"Chromium Binaries";
 
 const InstallConstants kInstallModes[] = {
-    // clang-format off
     // The primary (and only) install mode for Chromium.
     {
         sizeof(kInstallModes[0]),
-        CHROMIUM_INDEX,
-        "",   // No install switch for the primary install mode.
-        L"",  // Empty install_suffix for the primary install mode.
-        L"",  // No logo suffix for the primary install mode.
-        L"",  // Empty app_guid since no integraion with Google Update.
+        CHROMIUM_INDEX,  // The one and only mode for Chromium.
+        "",              // No install switch for the primary install mode.
+        L"",             // Empty install_suffix for the primary install mode.
+        L"",             // No logo suffix for the primary install mode.
+        L"",          // Empty app_guid since no integraion with Google Update.
         L"Chromium",  // A distinct base_app_id.
-        L"ChromiumHTM",  // ProgID prefix.
-        L"Chromium HTML Document",  // ProgID description.
+        L"ChromiumHTM",                             // ProgID prefix.
+        L"Chromium HTML Document",                  // ProgID description.
         L"{7D2B3E1D-D096-4594-9D8F-A6667F12E0AC}",  // Active Setup GUID.
         L"{A2DF06F9-A21A-44A8-8A99-8B9C84F29160}",  // CommandExecuteImpl CLSID.
         L"",  // Empty default channel name since no update integration.
         ChannelStrategy::UNSUPPORTED,
-        true,  // Supports system-level installs.
-        true,  // Supports in-product set as default browser UX.
-        true,  // Supported multi-install.
-        icon_resources::kApplicationIndex,
-        IDR_MAINFRAME,
+        true,   // Supports system-level installs.
+        true,   // Supports in-product set as default browser UX.
+        false,  // Does not support retention experiments.
+        true,   // Supported multi-install.
+        icon_resources::kApplicationIndex,  // App icon resource index.
+        IDR_MAINFRAME,                      // App icon resource id.
     },
-    // clang-format on
 };
 
 static_assert(_countof(kInstallModes) == NUM_INSTALL_MODES,

diff --git a/chrome/install_static/google_chrome_install_modes.cc b/chrome/install_static/google_chrome_install_modes.cc
@@ -24,50 +24,50 @@ const wchar_t kBinariesAppGuid[] = L"{4DC8B4CA-1BDA-483e-B5FA-D3C12E15B62D}";
 const wchar_t kBinariesPathName[] = L"";
 
 const InstallConstants kInstallModes[] = {
-    // clang-format off
     // The primary install mode for stable Google Chrome.
     {
         sizeof(kInstallModes[0]),
-        STABLE_INDEX,
-        "",   // No install switch for the primary install mode.
-        L"",  // Empty install_suffix for the primary install mode.
-        L"",  // No logo suffix for the primary install mode.
+        STABLE_INDEX,  // The first mode is for stable/beta/dev.
+        "",            // No install switch for the primary install mode.
+        L"",           // Empty install_suffix for the primary install mode.
+        L"",           // No logo suffix for the primary install mode.
         L"{8A69D345-D564-463c-AFF1-A69D9E530F96}",
-        L"Chrome",  // A distinct base_app_id.
-        L"ChromeHTML",  // ProgID prefix.
-        L"Chrome HTML Document",  // ProgID description.
+        L"Chrome",                                  // A distinct base_app_id.
+        L"ChromeHTML",                              // ProgID prefix.
+        L"Chrome HTML Document",                    // ProgID description.
         L"{8A69D345-D564-463c-AFF1-A69D9E530F96}",  // Active Setup GUID.
         L"{5C65F4B0-3651-4514-B207-D10CB699B14B}",  // CommandExecuteImpl CLSID.
         L"",  // The empty string means "stable".
         ChannelStrategy::ADDITIONAL_PARAMETERS,
         true,  // Supports system-level installs.
         true,  // Supports in-product set as default browser UX.
+        true,  // Supports retention experiments.
         true,  // Supported multi-install.
-        icon_resources::kApplicationIndex,
-        IDR_MAINFRAME,
+        icon_resources::kApplicationIndex,  // App icon resource index.
+        IDR_MAINFRAME,                      // App icon resource id.
     },
     // A secondary install mode for Google Chrome SxS (canary).
     {
         sizeof(kInstallModes[0]),
-        CANARY_INDEX,
-        "chrome-sxs",
-        L" SxS",
-        L"Canary",
-        L"{4ea16ac7-fd5a-47c3-875b-dbf4a2008c20}",
-        L"ChromeCanary",  // A distinct base_app_id.
-        L"ChromeSSHTM",  // ProgID prefix.
-        L"Chrome Canary HTML Document",  // ProgID description.
+        CANARY_INDEX,  // The mode for the side-by-side canary channel.
+        "chrome-sxs",  // Install switch.
+        L" SxS",       // Install suffix.
+        L"Canary",     // Logo suffix.
+        L"{4ea16ac7-fd5a-47c3-875b-dbf4a2008c20}",  // A distinct app GUID.
+        L"ChromeCanary",                            // A distinct base_app_id.
+        L"ChromeSSHTM",                             // ProgID prefix.
+        L"Chrome Canary HTML Document",             // ProgID description.
         L"{4ea16ac7-fd5a-47c3-875b-dbf4a2008c20}",  // Active Setup GUID.
         L"{1BEAC3E3-B852-44F4-B468-8906C062422E}",  // CommandExecuteImpl CLSID.
-        L"canary",
+        L"canary",                                  // Forced channel name.
         ChannelStrategy::FIXED,
         false,  // Does not support system-level installs.
         false,  // Does not support in-product set as default browser UX.
+        true,   // Supports retention experiments.
         false,  // Did not support multi-install.
-        icon_resources::kSxSApplicationIndex,
-        IDR_SXS,
+        icon_resources::kSxSApplicationIndex,  // App icon resource index.
+        IDR_SXS,                               // App icon resource id.
     },
-    // clang-format on
 };
 
 static_assert(_countof(kInstallModes) == NUM_INSTALL_MODES,

diff --git a/chrome/install_static/install_constants.h b/chrome/install_static/install_constants.h
@@ -101,6 +101,10 @@ struct InstallConstants {
   // in chrome://settings are hidden when this is false.
   bool supports_set_as_default_browser;
 
+  // True if this mode supports user retention experiments run by the installer
+  // following updates.
+  bool supports_retention_experiments;
+
   // True if this mode supported the now-deprecated multi-install.
   bool supported_multi_install;
 

diff --git a/chrome/install_static/install_util.cc b/chrome/install_static/install_util.cc
@@ -355,6 +355,10 @@ bool SupportsSetAsDefaultBrowser() {
   return InstallDetails::Get().mode().supports_set_as_default_browser;
 }
 
+bool SupportsRetentionExperiments() {
+  return InstallDetails::Get().mode().supports_retention_experiments;
+}
+
 int GetIconResourceIndex() {
   return InstallDetails::Get().mode().app_icon_resource_index;
 }

diff --git a/chrome/install_static/install_util.h b/chrome/install_static/install_util.h
@@ -111,6 +111,10 @@ std::wstring GetLegacyCommandExecuteImplClsid();
 // the user's chosen default browser.
 bool SupportsSetAsDefaultBrowser();
 
+// Returns true if this mode supports user retention experiments run by the
+// installer following updates.
+bool SupportsRetentionExperiments();
+
 // Returns the index of the icon resource in the main executable for the mode.
 int GetIconResourceIndex();
 

diff --git a/chrome/installer/util/browser_distribution.cc b/chrome/installer/util/browser_distribution.cc
@@ -155,7 +155,3 @@ void BrowserDistribution::UpdateInstallStatus(bool system_install,
 bool BrowserDistribution::ShouldSetExperimentLabels() {
   return false;
 }
-
-bool BrowserDistribution::HasUserExperiments() {
-  return false;
-}
diff --git a/chrome/installer/util/browser_distribution.h b/chrome/installer/util/browser_distribution.h
@@ -84,8 +84,6 @@ class BrowserDistribution {
   // registry value.
   virtual bool ShouldSetExperimentLabels();
 
-  virtual bool HasUserExperiments();
-
  protected:
   explicit BrowserDistribution(
       std::unique_ptr<AppRegistrationData> app_reg_data);

diff --git a/chrome/installer/util/google_chrome_distribution.cc b/chrome/installer/util/google_chrome_distribution.cc
@@ -244,7 +244,3 @@ void GoogleChromeDistribution::UpdateInstallStatus(bool system_install,
 bool GoogleChromeDistribution::ShouldSetExperimentLabels() {
   return true;
 }
-
-bool GoogleChromeDistribution::HasUserExperiments() {
-  return true;
-}
diff --git a/chrome/installer/util/google_chrome_distribution.h b/chrome/installer/util/google_chrome_distribution.h
@@ -57,8 +57,6 @@ class GoogleChromeDistribution : public BrowserDistribution {
 
   bool ShouldSetExperimentLabels() override;
 
-  bool HasUserExperiments() override;
-
  protected:
   // Disallow construction from others.
   GoogleChromeDistribution();

diff --git a/chrome/installer/util/google_chrome_distribution_dummy.cc b/chrome/installer/util/google_chrome_distribution_dummy.cc
@@ -68,7 +68,3 @@ void GoogleChromeDistribution::UpdateInstallStatus(bool system_install,
 bool GoogleChromeDistribution::ShouldSetExperimentLabels() {
   return false;
 }
-
-bool GoogleChromeDistribution::HasUserExperiments() {
-  return false;
-}
diff --git a/chrome/installer/util/google_chrome_sxs_distribution.cc b/chrome/installer/util/google_chrome_sxs_distribution.cc
@@ -44,7 +44,3 @@ base::string16 GoogleChromeSxSDistribution::GetStartMenuShortcutSubfolder(
 bool GoogleChromeSxSDistribution::ShouldSetExperimentLabels() {
   return true;
 }
-
-bool GoogleChromeSxSDistribution::HasUserExperiments() {
-  return true;
-}
diff --git a/chrome/installer/util/google_chrome_sxs_distribution.h b/chrome/installer/util/google_chrome_sxs_distribution.h
@@ -24,7 +24,6 @@ class GoogleChromeSxSDistribution : public GoogleChromeDistribution {
   base::string16 GetStartMenuShortcutSubfolder(
       Subfolder subfolder_type) override;
   bool ShouldSetExperimentLabels() override;
-  bool HasUserExperiments() override;
 
  private:
   friend class BrowserDistribution;

diff --git a/chrome/installer/util/product.cc b/chrome/installer/util/product.cc
@@ -11,6 +11,7 @@
 #include "base/memory/ptr_util.h"
 #include "base/process/launch.h"
 #include "base/win/registry.h"
+#include "chrome/install_static/install_util.h"
 #include "chrome/installer/util/browser_distribution.h"
 #include "chrome/installer/util/chrome_browser_operations.h"
 #include "chrome/installer/util/google_update_constants.h"
@@ -111,7 +112,10 @@ void Product::AddDefaultShortcutProperties(
 void Product::LaunchUserExperiment(const base::FilePath& setup_path,
                                    InstallStatus status,
                                    bool system_level) const {
-  if (distribution_->HasUserExperiments()) {
+  // Assert that this is only called with the one relevant distribution.
+  // TODO(grt): Remove this when BrowserDistribution goes away.
+  DCHECK_EQ(BrowserDistribution::GetDistribution(), distribution_);
+  if (install_static::SupportsRetentionExperiments()) {
     VLOG(1) << "LaunchUserExperiment status: " << status << " product: "
             << distribution_->GetDisplayName()
             << " system_level: " << system_level;