Fix two bugs in the toast

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 631 matching lines @@
     // 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} },
   };
 
   string16 locale;
+  GoogleUpdateSettings::GetLanguage(&locale);
+  if (locale.empty() || (locale == ASCIIToWide("en")))
+    locale = ASCIIToWide("en-US");
+
   string16 brand;
-
-  if (!GoogleUpdateSettings::GetLanguage(&locale))
-    locale = ASCIIToWide("en-US");
   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.
-    DCHECK_LE(kExperimentFlavors[i].flavors, kMax);
-    DCHECK_GT(kExperimentFlavors[i].flavors, 0);
+    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) {
-        DCHECK_GT(kExperimentFlavors[i].headings[f], 0);
+        CHECK_GT(kExperimentFlavors[i].headings[f], 0);
       } else {
-        DCHECK_EQ(kExperimentFlavors[i].headings[f], 0);
+        CHECK_EQ(kExperimentFlavors[i].headings[f], 0);
       }
     }
-    // Make sure we don't overflow on the second letter of the experiment code.
-    DCHECK(kExperimentFlavors[i].prefix2 +
-           kExperimentFlavors[i].flavors - 1 <= 'Z');
+    // 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("*"))
       continue;
 
     std::vector<string16> brand_codes;
     base::SplitString(kExperimentFlavors[i].brands, L',', &brand_codes);
     if (brand_codes.empty())
       return false;
     for (std::vector<string16>::iterator it = brand_codes.begin();
@@ skipping 58 matching lines @@
     return;
   }
   int flavor = experiment.flavor;
   string16 base_group = experiment.prefix;
 
   string16 brand;
   if (GoogleUpdateSettings::GetBrand(&brand) && (brand == L"CHXX")) {
     // Testing only: the user automatically qualifies for the experiment.
     VLOG(1) << "Experiment qualification bypass";
   } else {
+    // Check that the user was not already drafted in this experiment.
+    string16 client;
+    GoogleUpdateSettings::GetClient(&client);
+    if (client.size() > 2) {
+      if (base_group == client.substr(0, 2)) {

[Finnur, 2012/07/25 10:51:10]

nit: Probably more readable to combine these into one if:

if (client.size() > 2 && base_group == client.substr(0, 2)) {
...
}

+        VLOG(1) << "User already participated in this experiment";
+        return;

[Finnur, 2012/07/25 10:51:10]

Are you intentionally setting no results for Omaha to pick up and report back?
In other words, shouldn't you be using SetClient with a specific results code,
like we do on lines 782, 787, 793 (to name a few)?

+      }
+    }
     // Check browser usage inactivity by the age of the last-write time of the
     // chrome user data directory.
     FilePath user_data_dir(product.GetUserDataPath());
 
     const bool experiment_enabled = false;
     const int kThirtyDays = 30 * 24;
 
     int dir_age_hours = GetDirectoryWriteAgeInHours(
         user_data_dir.value().c_str());
     if (!experiment_enabled) {
@@ skipping 86 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