The new toast experiments.

chrome/installer/util/google_chrome_distribution.cc

 // Copyright (c) 2012 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.
 //
 // This file defines specific implementation of BrowserDistribution class for
 // Google Chrome.
 
 #include "chrome/installer/util/google_chrome_distribution.h"
 
 #include <vector>
@@ skipping 42 matching lines @@
 const wchar_t kCommandExecuteImplUuid[] =
     L"{5C65F4B0-3651-4514-B207-D10CB699B14B}";
 const wchar_t kDelegateExecuteLibUuid[] =
     L"{4E805ED8-EBA0-4601-9681-12815A56EBFD}";
 const wchar_t kDelegateExecuteLibVersion[] = L"1.0";
 const wchar_t kICommandExecuteImplUuid[] =
     L"{0BA0D4E9-2259-4963-B9AE-A839F7CB7544}";
 
 // The following strings are the possible outcomes of the toast experiment
 // as recorded in the |client| field.
-const wchar_t kToastExpControlGroup[] =      L"01";
-const wchar_t kToastExpCancelGroup[] =       L"02";
-const wchar_t kToastExpUninstallGroup[] =    L"04";
-const wchar_t kToastExpTriesOkGroup[] =      L"18";
-const wchar_t kToastExpTriesErrorGroup[] =   L"28";
-const wchar_t kToastActiveGroup[] =          L"40";
-const wchar_t kToastUDDirFailure[] =         L"40";
-const wchar_t kToastExpBaseGroup[] =         L"80";
+const wchar_t kToastExpControlGroup[] =        L"01";
+const wchar_t kToastExpCancelGroup[] =         L"02";
+const wchar_t kToastExpUninstallGroup[] =      L"04";
+const wchar_t kToastExpTriesOkGroup[] =        L"18";
+const wchar_t kToastExpTriesOkDefaultGroup[] = L"28";

[Finnur, 2012/07/26 12:27:07]

This would change the result code that gets sent back to the server. Are you
sure it is OK to change the error code from 28 to 48 and reuse 28 for another
purpose?

[Mark Larson, 2012/07/26 16:32:13]

I'm also concerned about how this will show up on the backend. I think it's
probably OK, but it might make more sense to swap 28 and 48 for backward
compatibility. Is there a reason to change the meaning of 28?

BTW, you don't seem to use TriesOKDefaultGroup anywhere else.

[cpu_(ooo_6.6-7.5), 2012/07/27 03:06:23]

Done.

+const wchar_t kToastExpTriesErrorGroup[] =     L"48";
+const wchar_t kToastActiveGroup[] =            L"40";
+const wchar_t kToastUDDirFailure[] =           L"40";
+const wchar_t kToastExpBaseGroup[] =           L"80";
 
 // Substitute the locale parameter in uninstall URL with whatever
 // Google Update tells us is the locale. In case we fail to find
 // the locale, we use US English.
 string16 LocalizeUrl(const wchar_t* url) {
   string16 language;
   if (!GoogleUpdateSettings::GetLanguage(&language))
     language = L"en-US";  // Default to US English.
   return ReplaceStringPlaceholders(url, language.c_str(), NULL);
 }
@@ skipping 535 matching lines @@
   // This struct determines which experiment flavors we show for each locale and
   // brand.
   //
   // The big experiment in Dec 2009 used TGxx and THxx.
   // The big experiment in Feb 2010 used TKxx and TLxx.
   // The big experiment in Apr 2010 used TMxx and TNxx.
   // The big experiment in Oct 2010 used TVxx TWxx TXxx TYxx.
   // The big experiment in Feb 2011 used SJxx SKxx SLxx SMxx.
   // Note: the plugin infobar experiment uses PIxx codes.
   using namespace attrition_experiments;
+  struct FlavorDetails {
+    int heading_id;
+    int flags;
+  };

[Finnur, 2012/07/26 12:27:07]

This struct should probably move to line 617, as the comment there no longer
describes the struct it was intended to describe...

Also, document it?

+
   static const struct UserExperimentDetails {
     const wchar_t* locale;  // Locale to show this experiment for (* for all).
     const wchar_t* brands;  // Brand codes show this experiment for (* for all).
     int control_group;      // Size of the control group, in percentages.
-    const wchar_t prefix1;  // The first letter for the experiment code.
-    const wchar_t prefix2;  // The second letter for the experiment code. This
-                            // will be incremented by one for each additional
-                            // experiment flavor beyond the first.
-    int flavors;            // Numbers of flavors for this experiment. Should
-                            // always be positive and never exceed the number
-                            // of headings (below).
-    int headings[kMax];     // A list of IDs per experiment. 0 == no heading.
-  } kExperimentFlavors[] = {
-    // This list should be ordered most-specific rule first (catch-all, like all
-    // brands or all locales should be last).
-
-    // The experiment with the more compact bubble. This one is a bit special
-    // because it is split into two: CAxx is regular style bubble and CBxx is
-    // compact style bubble. See |compact_bubble| below.
-    {L"en-US", kBrief, 1, L'C', L'A', 2, { kEnUs3, kEnUs3, 0, 0 } },
-
-    // Catch-all rules.
-    {kAll, kAll, 1, L'B', L'A', 1, {kEnUs3, 0, 0, 0} },
+    const wchar_t* prefix;  // The two letter experiment code. The second letter
+                            // will be incremented with the flavor.
+    FlavorDetails flavors[kMax];
+  } kExperiments[] = {
+    // The first match from top to bottom is used so this list should be ordered
+    // most-specific rule first.
+    { L"*", L"CHMA",  // all locales, CHMA brand.

[Finnur, 2012/07/26 12:27:07]

nit: Capitalize first letter of comments in this line and below.

[cpu_(ooo_6.6-7.5), 2012/07/27 03:06:23]

Done.

+      25,             // 25 percent control group.
+      L"ZA",          // experiment is ZAxx, ZBxx, ZCxx, ZDxx etc.
+      // Three flavors.
+      { { IDS_TRY_TOAST_HEADING3, kDontBugMeAsButton | kUninstall | kWhyLink },

[Finnur, 2012/07/26 12:27:07]

I thought we'd decided not to have a Why link, since it doesn't actually work
(Chrome startup is halted at the toast dialog and pressing the link will launch
a new process that will also halt until you dismiss the dialog). No?

[cpu_(ooo_6.6-7.5), 2012/07/27 03:06:23]

No idea. Can you please raise the issue in the bug?

On 2012/07/26 12:27:07, Finnur wrote:

[Finnur, 2012/07/29 19:09:44]

OK, I refreshed my memory by looking up the bug...
http://code.google.com/p/chromium/issues/detail?id=110672
The consensus was that we should not show the link if Chrome is the default
browser...

On 2012/07/27 03:06:23, cpu wrote:

+        { IDS_TRY_TOAST_HEADING3, 0 },

[Finnur, 2012/07/26 12:27:07]

I'm confused about what these enums mean. What does 0 mean for example?  Is it
TryMe and Don'tBugMe?

Also, the Why link isn't being requested in the bug?

[cpu_(ooo_6.6-7.5), 2012/07/27 03:06:23]

it means don't bug me is a radio button, because the flag just controls how is
shown, and no uninstall or why link. Zero gives you the second ascii art example
in the header :)

+        { IDS_TRY_TOAST_HEADING3, kMakeDefault },
+        { 0, 0 },
+      }
+    },
+    { L"*", L"GGRV",  // all locales, GGRV is enterprise.
+      0,              // 0 percent control group.
+      L"EA",          // experiment is EAxx, EBxx, etc.

[Finnur, 2012/07/26 12:27:07]

Just curious... There's no experiment, so why have an experiment code?

[cpu_(ooo_6.6-7.5), 2012/07/27 03:06:23]

No very good reason: In case we f*ck up we can see in the omaha dashboard. We
would see EAxx, EBxx.

[Finnur, 2012/07/29 19:09:44]

Fair enough. Consider documenting on line 624?

On 2012/07/27 03:06:23, cpu wrote:

[cpu_(ooo_6.6-7.5), 2012/07/31 04:04:09]

Others missing. I'll change here after I talk to larson,or he will document.

On 2012/07/29 19:09:44, Finnur wrote:

+      // No flavors means no experiment.
+      { { 0, 0 },
+        { 0, 0 },
+        { 0, 0 },
+        { 0, 0 }
+      }
+    }
   };
 
   string16 locale;
   GoogleUpdateSettings::GetLanguage(&locale);
   if (locale.empty() || (locale == ASCIIToWide("en")))
     locale = ASCIIToWide("en-US");
 
   string16 brand;
   if (!GoogleUpdateSettings::GetBrand(&brand))
     brand = ASCIIToWide("");  // Could still be viable for catch-all rules.
-  if (brand == kEnterprise)
-    return false;
 
-  for (int i = 0; i < arraysize(kExperimentFlavors); ++i) {
-    // A maximum of four flavors are supported at the moment.
-    CHECK_LE(kExperimentFlavors[i].flavors, kMax);
-    CHECK_GT(kExperimentFlavors[i].flavors, 0);
-    // Make sure each experiment has valid headings.
-    for (int f = 0; f < kMax; ++f) {
-      if (f < kExperimentFlavors[i].flavors) {
-        CHECK_GT(kExperimentFlavors[i].headings[f], 0);
-      } else {
-        CHECK_EQ(kExperimentFlavors[i].headings[f], 0);
-      }
-    }
-    // The prefix has to be a valid two letter combo.
-    CHECK(kExperimentFlavors[i].prefix1 >= 'A');
-    CHECK(kExperimentFlavors[i].prefix2 >= 'A');
-    CHECK(kExperimentFlavors[i].prefix2 +
-          kExperimentFlavors[i].flavors - 1 <= 'Z');
-
-    if (kExperimentFlavors[i].locale != locale &&
-        kExperimentFlavors[i].locale != ASCIIToWide("*"))
+  for (int i = 0; i < arraysize(kExperiments); ++i) {
+    if (kExperiments[i].locale != locale &&
+        kExperiments[i].locale != ASCIIToWide("*"))
       continue;
 
     std::vector<string16> brand_codes;
-    base::SplitString(kExperimentFlavors[i].brands, L',', &brand_codes);
+    base::SplitString(kExperiments[i].brands, L',', &brand_codes);
     if (brand_codes.empty())
       return false;
     for (std::vector<string16>::iterator it = brand_codes.begin();
          it != brand_codes.end(); ++it) {
       if (*it != brand && *it != L"*")
         continue;
+      // We have found our match.
+      const UserExperimentDetails& match = kExperiments[i];
+      // Find out how many flavors we have. Zero means no experiment.
+      int num_flavors = 0;
+      while (match.flavors[num_flavors].heading_id) { ++num_flavors; }
+      if (!num_flavors)
+        return false;
 
-      // We have found our match.
       if (flavor < 0)
-        flavor = base::RandInt(0, kExperimentFlavors[i].flavors - 1);
+        flavor = base::RandInt(0, num_flavors - 1);
       experiment->flavor = flavor;
-      experiment->heading = kExperimentFlavors[i].headings[flavor];
-      experiment->control_group = kExperimentFlavors[i].control_group;
-      experiment->prefix.resize(2);
-      experiment->prefix[0] = kExperimentFlavors[i].prefix1;
-      experiment->prefix[1] = kExperimentFlavors[i].prefix2 + flavor;
-      experiment->compact_bubble = (brand == kBrief) && (flavor == 1);
+      experiment->heading = match.flavors[flavor].heading_id;
+      experiment->control_group = match.control_group;
+      const wchar_t prefix[] = { match.prefix[0], match.prefix[1] + flavor, 0 };
+      experiment->prefix = prefix;
+      experiment->flags = match.flavors[flavor].flags;
       return true;
     }
   }
 
   return false;
 }
 
 // Currently we only have one experiment: the inactive user toast. Which only
 // applies for users doing upgrades.
 
@@ skipping 116 matching lines @@
     // The command line should now have the url added as:
     // "chrome.exe -- <url>"
     DCHECK_NE(string16::npos,
         options.GetCommandLineString().find(L" -- " + url));
   }
   // Launch chrome now. It will show the toast UI.
   int32 exit_code = 0;
   if (!installation.LaunchChromeAndWait(application_path, options, &exit_code))
     return;
 
   // The chrome process has exited, figure out what happened.

[Mark Larson, 2012/07/26 16:32:13]

Don't we need something here that sets outcome to kToastExpTriesOkDefaultGroup?

[cpu_(ooo_6.6-7.5), 2012/07/27 03:06:23]

Yes, I am trying to figure how to do this. Stay tuned for a followup change.

[cpu_(ooo_6.6-7.5), 2012/07/31 04:04:09]

Done.

   const wchar_t* outcome = NULL;
   switch (exit_code) {
     case content::RESULT_CODE_NORMAL_EXIT:
       outcome = kToastExpTriesOkGroup;
       break;
     case chrome::RESULT_CODE_NORMAL_EXIT_CANCEL:
       outcome = kToastExpCancelGroup;
       break;
     case chrome::RESULT_CODE_NORMAL_EXIT_EXP2:
       outcome = kToastExpUninstallGroup;
@@ skipping 12 matching lines @@
   // we waited for chrome to exit so the uninstall would not detect chrome
   // running.
   bool system_level_toast = CommandLine::ForCurrentProcess()->HasSwitch(
       installer::switches::kSystemLevelToast);
 
   CommandLine cmd(InstallUtil::GetChromeUninstallCmd(system_level_toast,
                                                      GetType()));
   base::LaunchProcess(cmd, base::LaunchOptions(), NULL);
 }
 #endif