summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--chrome/browser/about_flags.cc113
-rw-r--r--chrome/browser/about_flags.h38
-rw-r--r--chrome/browser/about_flags_unittest.cc85
3 files changed, 178 insertions, 58 deletions
diff --git a/chrome/browser/about_flags.cc b/chrome/browser/about_flags.cc
index 009ae66..c710012 100644
--- a/chrome/browser/about_flags.cc
+++ b/chrome/browser/about_flags.cc
@@ -22,31 +22,7 @@ namespace about_flags {
namespace {
-enum { kOsMac = 1 << 0, kOsWin = 1 << 1, kOsLinux = 1 << 2 , kOsCrOS = 1 << 3 };
-
-unsigned kOsAll = kOsMac | kOsWin | kOsLinux | kOsCrOS;
-
-struct Experiment {
- // The internal name of the experiment. This is never shown to the user.
- // It _is_ however stored in the prefs file, so you shouldn't change the
- // name of existing flags.
- const char* internal_name;
-
- // String id of the message containing the experiment's name.
- int visible_name_id;
-
- // String id of the message containing the experiment's description.
- int visible_description_id;
-
- // The platforms the experiment is available on
- // Needs to be more than a compile-time #ifdef because of profile sync.
- unsigned supported_platforms; // bitmask
-
- // The commandline parameter that's added when this lab is active. This is
- // different from |internal_name| so that the commandline flag can be
- // renamed without breaking the prefs file.
- const char* command_line;
-};
+const unsigned kOsAll = kOsMac | kOsWin | kOsLinux | kOsCrOS;
const Experiment kExperiments[] = {
{
@@ -176,6 +152,9 @@ const Experiment kExperiments[] = {
}
};
+const Experiment* experiments = kExperiments;
+size_t num_experiments = arraysize(kExperiments);
+
// Stores and encapsulates the little state that about:flags has.
class FlagsState {
public:
@@ -183,7 +162,7 @@ class FlagsState {
void ConvertFlagsToSwitches(PrefService* prefs, CommandLine* command_line);
bool IsRestartNeededToCommitChanges();
void SetExperimentEnabled(
- PrefService* prefs, const std::string& internal_name, bool enable);
+ PrefService* prefs, const std::string& internal_name, bool enable);
void RemoveFlagsSwitches(
std::map<std::string, CommandLine::StringType>* switch_list);
void reset();
@@ -241,8 +220,8 @@ void SetEnabledFlags(
// and removed.
void SanitizeList(PrefService* prefs) {
std::set<std::string> known_experiments;
- for (size_t i = 0; i < arraysize(kExperiments); ++i)
- known_experiments.insert(kExperiments[i].internal_name);
+ for (size_t i = 0; i < num_experiments; ++i)
+ known_experiments.insert(experiments[i].internal_name);
std::set<std::string> enabled_experiments;
GetEnabledFlags(prefs, &enabled_experiments);
@@ -262,18 +241,29 @@ void GetSanitizedEnabledFlags(
GetEnabledFlags(prefs, result);
}
-int GetCurrentPlatform() {
-#if defined(OS_MACOSX)
- return kOsMac;
-#elif defined(OS_WIN)
- return kOsWin;
-#elif defined(OS_CHROMEOS) // Needs to be before the OS_LINUX check.
- return kOsCrOS;
-#elif defined(OS_LINUX)
- return kOsLinux;
-#else
-#error Unknown platform
-#endif
+// Variant of GetSanitizedEnabledFlags that also removes any flags that aren't
+// enabled on the current platform.
+void GetSanitizedEnabledFlagsForCurrentPlatform(
+ PrefService* prefs, std::set<std::string>* result) {
+ GetSanitizedEnabledFlags(prefs, result);
+
+ // Filter out any experiments that aren't enabled on the current platform. We
+ // don't remove these from prefs else syncing to a platform with a different
+ // set of experiments would be lossy.
+ std::set<std::string> platform_experiments;
+ int current_platform = GetCurrentPlatform();
+ for (size_t i = 0; i < num_experiments; ++i) {
+ if (experiments[i].supported_platforms & current_platform)
+ platform_experiments.insert(experiments[i].internal_name);
+ }
+
+ std::set<std::string> new_enabled_experiments;
+ std::set_intersection(
+ platform_experiments.begin(), platform_experiments.end(),
+ result->begin(), result->end(),
+ std::inserter(new_enabled_experiments, new_enabled_experiments.begin()));
+
+ result->swap(new_enabled_experiments);
}
} // namespace
@@ -289,8 +279,8 @@ ListValue* GetFlagsExperimentsData(PrefService* prefs) {
int current_platform = GetCurrentPlatform();
ListValue* experiments_data = new ListValue();
- for (size_t i = 0; i < arraysize(kExperiments); ++i) {
- const Experiment& experiment = kExperiments[i];
+ for (size_t i = 0; i < num_experiments; ++i) {
+ const Experiment& experiment = experiments[i];
if (!(experiment.supported_platforms & current_platform))
continue;
@@ -323,6 +313,20 @@ void RemoveFlagsSwitches(
FlagsState::instance()->RemoveFlagsSwitches(switch_list);
}
+int GetCurrentPlatform() {
+#if defined(OS_MACOSX)
+ return kOsMac;
+#elif defined(OS_WIN)
+ return kOsWin;
+#elif defined(OS_CHROMEOS) // Needs to be before the OS_LINUX check.
+ return kOsCrOS;
+#elif defined(OS_LINUX)
+ return kOsLinux;
+#else
+#error Unknown platform
+#endif
+}
+
//////////////////////////////////////////////////////////////////////////////
// FlagsState implementation.
@@ -334,11 +338,11 @@ void FlagsState::ConvertFlagsToSwitches(
return;
std::set<std::string> enabled_experiments;
- GetSanitizedEnabledFlags(prefs, &enabled_experiments);
+ GetSanitizedEnabledFlagsForCurrentPlatform(prefs, &enabled_experiments);
- std::map<std::string, const Experiment*> experiments;
- for (size_t i = 0; i < arraysize(kExperiments); ++i)
- experiments[kExperiments[i].internal_name] = &kExperiments[i];
+ std::map<std::string, const Experiment*> experiment_map;
+ for (size_t i = 0; i < num_experiments; ++i)
+ experiment_map[experiments[i].internal_name] = &experiments[i];
command_line->AppendSwitch(switches::kFlagSwitchesBegin);
flags_switches_.insert(switches::kFlagSwitchesBegin);
@@ -347,9 +351,9 @@ void FlagsState::ConvertFlagsToSwitches(
++it) {
const std::string& experiment_name = *it;
std::map<std::string, const Experiment*>::iterator experiment =
- experiments.find(experiment_name);
- DCHECK(experiment != experiments.end());
- if (experiment == experiments.end())
+ experiment_map.find(experiment_name);
+ DCHECK(experiment != experiment_map.end());
+ if (experiment == experiment_map.end())
continue;
command_line->AppendSwitch(experiment->second->command_line);
@@ -398,6 +402,17 @@ namespace testing {
void ClearState() {
FlagsState::instance()->reset();
}
+
+void SetExperiments(const Experiment* e, size_t count) {
+ if (!e) {
+ experiments = kExperiments;
+ num_experiments = arraysize(kExperiments);
+ } else {
+ experiments = e;
+ num_experiments = count;
+ }
+}
+
} // namespace testing
} // namespace about_flags
diff --git a/chrome/browser/about_flags.h b/chrome/browser/about_flags.h
index 81b0c2a..0fcb3b9 100644
--- a/chrome/browser/about_flags.h
+++ b/chrome/browser/about_flags.h
@@ -16,6 +16,35 @@ class PrefService;
namespace about_flags {
+// Enumeration of OSs.
+// This is exposed only for testing.
+enum { kOsMac = 1 << 0, kOsWin = 1 << 1, kOsLinux = 1 << 2 , kOsCrOS = 1 << 3 };
+
+// Experiment is used internally by about_flags to describe an experiment (and
+// for testing).
+// This is exposed only for testing.
+struct Experiment {
+ // The internal name of the experiment. This is never shown to the user.
+ // It _is_ however stored in the prefs file, so you shouldn't change the
+ // name of existing flags.
+ const char* internal_name;
+
+ // String id of the message containing the experiment's name.
+ int visible_name_id;
+
+ // String id of the message containing the experiment's description.
+ int visible_description_id;
+
+ // The platforms the experiment is available on
+ // Needs to be more than a compile-time #ifdef because of profile sync.
+ unsigned supported_platforms; // bitmask
+
+ // The commandline parameter that's added when this lab is active. This is
+ // different from |internal_name| so that the commandline flag can be
+ // renamed without breaking the prefs file.
+ const char* command_line;
+};
+
// Reads the Labs |prefs| (called "Labs" for historical reasons) and adds the
// commandline flags belonging to the active experiments to |command_line|.
void ConvertFlagsToSwitches(PrefService* prefs, CommandLine* command_line);
@@ -35,9 +64,18 @@ void SetExperimentEnabled(
void RemoveFlagsSwitches(
std::map<std::string, CommandLine::StringType>* switch_list);
+// Returns the value for the current platform. This is one of the values defined
+// by the OS enum above.
+// This is exposed only for testing.
+int GetCurrentPlatform();
+
namespace testing {
// Clears internal global state, for unit tests.
void ClearState();
+
+// Sets the list of experiments. Pass in NULL to use the default set. This does
+// NOT take ownership of the supplied Experiments.
+void SetExperiments(const Experiment* e, size_t count);
} // namespace testing
} // namespace about_flags
diff --git a/chrome/browser/about_flags_unittest.cc b/chrome/browser/about_flags_unittest.cc
index 3122cc9..6d86cc9 100644
--- a/chrome/browser/about_flags_unittest.cc
+++ b/chrome/browser/about_flags_unittest.cc
@@ -4,20 +4,50 @@
#include "chrome/browser/about_flags.h"
+#include "base/values.h"
#include "chrome/common/chrome_switches.h"
#include "chrome/common/pref_names.h"
#include "chrome/test/testing_pref_service.h"
+#include "grit/chromium_strings.h"
#include "testing/gtest/include/gtest/gtest.h"
-// These two should be two arbitrary, but valid names from about_flags.cc.
-const char kFlags1[] = "remoting";
-const char kFlags2[] = "print-preview";
+const char kFlags1[] = "flag1";
+const char kFlags2[] = "flag2";
+const char kFlags3[] = "flag3";
-// The command line flag corresponding to |kFlags1|.
-const char* const kFlag1CommandLine = switches::kEnableRemoting;
+const char kSwitch1[] = "switch";
+const char kSwitch2[] = "switch2";
+const char kSwitch3[] = "switch3";
namespace about_flags {
+// The experiments that are set for these tests. The first two experiments are
+// supported on the current platform, but the last is only supported on a
+// platform other than the current.
+static Experiment kExperiments[] = {
+ {
+ kFlags1,
+ IDS_PRODUCT_NAME,
+ IDS_PRODUCT_NAME,
+ 0, // Ends up being mapped to the current platform.
+ kSwitch1
+ },
+ {
+ kFlags2,
+ IDS_PRODUCT_NAME,
+ IDS_PRODUCT_NAME,
+ 0, // Ends up being mapped to the current platform.
+ kSwitch2
+ },
+ {
+ kFlags3,
+ IDS_PRODUCT_NAME,
+ IDS_PRODUCT_NAME,
+ 0, // This ends up enabling for an OS other than the current.
+ kSwitch3
+ },
+};
+
class AboutFlagsTest : public ::testing::Test {
protected:
AboutFlagsTest() {
@@ -25,6 +55,22 @@ class AboutFlagsTest : public ::testing::Test {
testing::ClearState();
}
+ virtual void SetUp() {
+ for (size_t i = 0; i < arraysize(kExperiments); ++i)
+ kExperiments[i].supported_platforms = GetCurrentPlatform();
+
+ int os_other_than_current = 1;
+ while (os_other_than_current == GetCurrentPlatform())
+ os_other_than_current <<= 1;
+ kExperiments[2].supported_platforms = os_other_than_current;
+
+ testing::SetExperiments(kExperiments, arraysize(kExperiments));
+ }
+
+ virtual void TearDown() {
+ testing::SetExperiments(NULL, 0);
+ }
+
TestingPrefService prefs_;
};
@@ -85,17 +131,17 @@ TEST_F(AboutFlagsTest, ConvertFlagsToSwitches) {
command_line.AppendSwitch("foo");
EXPECT_TRUE(command_line.HasSwitch("foo"));
- EXPECT_FALSE(command_line.HasSwitch(kFlag1CommandLine));
+ EXPECT_FALSE(command_line.HasSwitch(kSwitch1));
ConvertFlagsToSwitches(&prefs_, &command_line);
EXPECT_TRUE(command_line.HasSwitch("foo"));
- EXPECT_TRUE(command_line.HasSwitch(kFlag1CommandLine));
+ EXPECT_TRUE(command_line.HasSwitch(kSwitch1));
}
TEST_F(AboutFlagsTest, RemoveFlagSwitches) {
std::map<std::string, CommandLine::StringType> switch_list;
- switch_list[kFlag1CommandLine] = CommandLine::StringType();
+ switch_list[kSwitch1] = CommandLine::StringType();
switch_list[switches::kFlagSwitchesBegin] = CommandLine::StringType();
switch_list[switches::kFlagSwitchesEnd] = CommandLine::StringType();
switch_list["foo"] = CommandLine::StringType();
@@ -105,7 +151,7 @@ TEST_F(AboutFlagsTest, RemoveFlagSwitches) {
// This shouldn't do anything before ConvertFlagsToSwitches() wasn't called.
RemoveFlagsSwitches(&switch_list);
ASSERT_EQ(4u, switch_list.size());
- EXPECT_TRUE(switch_list.find(kFlag1CommandLine) != switch_list.end());
+ EXPECT_TRUE(switch_list.find(kSwitch1) != switch_list.end());
EXPECT_TRUE(switch_list.find(switches::kFlagSwitchesBegin) !=
switch_list.end());
EXPECT_TRUE(switch_list.find(switches::kFlagSwitchesEnd) !=
@@ -123,4 +169,25 @@ TEST_F(AboutFlagsTest, RemoveFlagSwitches) {
EXPECT_TRUE(switch_list.find("foo") != switch_list.end());
}
+// Tests enabling experiments that aren't supported on the current platform.
+TEST_F(AboutFlagsTest, PersistAndPrune) {
+ // Enable exerpiement 1 and experiment 3.
+ SetExperimentEnabled(&prefs_, kFlags1, true);
+ SetExperimentEnabled(&prefs_, kFlags3, true);
+ CommandLine command_line(CommandLine::NO_PROGRAM);
+ EXPECT_FALSE(command_line.HasSwitch(kSwitch1));
+ EXPECT_FALSE(command_line.HasSwitch(kSwitch3));
+
+ // Convert the flags to switches. Experiment 3 shouldn't be among the switches
+ // as it is not applicable to the current platform.
+ ConvertFlagsToSwitches(&prefs_, &command_line);
+ EXPECT_TRUE(command_line.HasSwitch(kSwitch1));
+ EXPECT_FALSE(command_line.HasSwitch(kSwitch3));
+
+ // Experiment 3 should show still be persisted in preferences though.
+ scoped_ptr<ListValue> switch_prefs(GetFlagsExperimentsData(&prefs_));
+ ASSERT_TRUE(switch_prefs.get());
+ EXPECT_EQ(2u, switch_prefs->GetSize());
+}
+
} // namespace about_flags
generated by cgit v1.1 at 2025-02-12 20:56:43 +0000