Make Chrome OS load feature flags from command line.

Chrome OS will stop passing feature flags as raw switches, but rather
a list of feature flag names encoded in a new command line switch
value. The conversion to switches and features thus happens at Chrome
startup as is already the case for other platforms.

BUG=chromium:1073940
TEST=Manual

Change-Id: I84caee876792e19dc5603808e82b866f6e01de61
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2287291
Commit-Queue: Mattias Nissler <mnissler@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#840358}

chrome/browser/chromeos/login/session/user_session_manager.cc

@@ -525,6 +525,11 @@ void UserSessionManager::CompleteGuestSessionLogin(const GURL& start_url) {
   const base::CommandLine user_flags(base::CommandLine::NO_PROGRAM);
   if (!about_flags::AreSwitchesIdenticalToCurrentCommandLine(
           user_flags, *base::CommandLine::ForCurrentProcess(), NULL)) {
+    SessionManagerClient::Get()->SetFeatureFlagsForUser(
+        cryptohome::CreateAccountIdentifierFromAccountId(
+            user_manager::GuestAccountId()),
+        {});
+
     SessionManagerClient::Get()->SetFlagsForUser(
         cryptohome::CreateAccountIdentifierFromAccountId(
             user_manager::GuestAccountId()),
@@ -2241,6 +2246,13 @@ void UserSessionManager::SetSwitchesForUser(
                         pair.second.end());
   }
 
+  // Clear session_manager's feature flag state so it doesn't pass flags on
+  // restart. This is necessary until in-session feature flags have been
+  // converted to use the new way.
+  // TODO(crbug.com/1073940): Remove after conversion is complete.
+  SessionManagerClient::Get()->SetFeatureFlagsForUser(
+      cryptohome::CreateAccountIdentifierFromAccountId(account_id), {});
+
   SessionManagerClient::Get()->SetFlagsForUser(
       cryptohome::CreateAccountIdentifierFromAccountId(account_id),
       all_switches);

chrome/browser/chromeos/settings/owner_flags_storage.cc

@@ -5,13 +5,17 @@
 #include "chrome/browser/chromeos/settings/owner_flags_storage.h"
 
 #include "base/command_line.h"
+#include "base/json/json_reader.h"
+#include "base/logging.h"
 #include "base/values.h"
 #include "chrome/browser/about_flags.h"
 #include "chrome/common/pref_names.h"
 #include "chromeos/settings/cros_settings_names.h"
+#include "components/flags_ui/flags_storage.h"
 #include "components/flags_ui/flags_ui_pref_names.h"
 #include "components/ownership/owner_settings_service.h"
 #include "components/prefs/pref_service.h"
+#include "third_party/cros_system_api/switches/chrome_switches.h"
 
 namespace chromeos {
 namespace about_flags {
@@ -43,5 +47,55 @@ bool OwnerFlagsStorage::SetFlags(const std::set<std::string>& flags) {
   return true;
 }
 
+ReadOnlyFlagsStorage::ReadOnlyFlagsStorage(const std::set<std::string>& flags)
+    : flags_(flags) {}
+
+ReadOnlyFlagsStorage::~ReadOnlyFlagsStorage() = default;
+
+std::set<std::string> ReadOnlyFlagsStorage::GetFlags() const {
+  return flags_;
+}
+
+bool ReadOnlyFlagsStorage::SetFlags(const std::set<std::string>& flags) {
+  return false;
+}
+
+void ReadOnlyFlagsStorage::CommitPendingWrites() {}
+
+std::string ReadOnlyFlagsStorage::GetOriginListFlag(
+    const std::string& internal_entry_name) const {
+  return std::string();
+}
+
+void ReadOnlyFlagsStorage::SetOriginListFlag(
+    const std::string& internal_entry_name,
+    const std::string& origin_list_value) {}
+
+std::set<std::string> ParseFlagsFromCommandLine() {
+  std::string encoded =
+      base::CommandLine::ForCurrentProcess()->GetSwitchValueNative(
+          chromeos::switches::kFeatureFlags);
+  if (encoded.empty()) {
+    return {};
+  }
+
+  auto flags_list = base::JSONReader::Read(encoded);
+  if (!flags_list) {
+    LOG(WARNING) << "Failed to parse feature flags configuration";
+    return {};
+  }
+
+  std::set<std::string> flags;
+  for (const auto& flag : flags_list.value().GetList()) {
+    if (!flag.is_string()) {
+      LOG(WARNING) << "Invalid entry in encoded feature flags";
+      continue;
+    }
+    flags.insert(flag.GetString());
+  }
+
+  return flags;
+}
+
 }  // namespace about_flags
 }  // namespace chromeos

chrome/browser/chromeos/settings/owner_flags_storage.h

@@ -18,7 +18,8 @@ namespace about_flags {
 
 // Implements the FlagsStorage interface for the owner flags. It inherits from
 // PrefServiceFlagsStorage but extends it with storing the flags in the signed
-// settings as well which effectively applies them to the login session as well.
+// device settings as well which effectively applies them to the Chrome OS login
+// screen as well.
 class OwnerFlagsStorage : public ::flags_ui::PrefServiceFlagsStorage {
  public:
   OwnerFlagsStorage(PrefService* prefs,
@@ -31,6 +32,33 @@ class OwnerFlagsStorage : public ::flags_ui::PrefServiceFlagsStorage {
   ownership::OwnerSettingsService* owner_settings_service_;
 };
 
+// FlagsStorage implementation for Chrome OS startup. It is backed by a set of
+// flags that are provided at initialization time. Functions other than
+// GetFlags() are implemented as no-ops.
+class ReadOnlyFlagsStorage : public ::flags_ui::FlagsStorage {
+ public:
+  // Initializes the object with a given set of flags.
+  explicit ReadOnlyFlagsStorage(const std::set<std::string>& flags);
+  ~ReadOnlyFlagsStorage() override;
+
+  // ::flags_ui::FlagsStorage:
+  std::set<std::string> GetFlags() const override;
+  bool SetFlags(const std::set<std::string>& flags) override;
+  void CommitPendingWrites() override;
+  std::string GetOriginListFlag(
+      const std::string& internal_entry_name) const override;
+  void SetOriginListFlag(const std::string& internal_entry_name,
+                         const std::string& origin_list_value) override;
+
+ private:
+  std::set<std::string> flags_;
+};
+
+// Parses flags specified in the --feature-flags command line switch. This is
+// used by Chrome OS to pass flags configuration from session_manager to Chrome
+// on startup.
+std::set<std::string> ParseFlagsFromCommandLine();
+
 }  // namespace about_flags
 }  // namespace chromeos
 

chrome/browser/metrics/chrome_feature_list_creator.cc

@@ -47,6 +47,7 @@
 
 #if BUILDFLAG(IS_CHROMEOS_ASH)
 #include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
+#include "chrome/browser/chromeos/settings/owner_flags_storage.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #endif  // BUILDFLAG(IS_CHROMEOS_ASH)
 
@@ -139,19 +140,28 @@ void ChromeFeatureListCreator::CreatePrefService() {
 }
 
 void ChromeFeatureListCreator::ConvertFlagsToSwitches() {
-#if !BUILDFLAG(IS_CHROMEOS_ASH)
   // Convert active flags into switches. This needs to be done before
   // ui::ResourceBundle::InitSharedInstanceWithLocale as some loaded resources
-  // are affected by experiment flags (--touch-optimized-ui in particular). On
-  // ChromeOS system level flags are applied from the device settings from the
-  // session manager.
+  // are affected by experiment flags (--touch-optimized-ui in particular).
   DCHECK(!ui::ResourceBundle::HasSharedInstance());
   TRACE_EVENT0("startup", "ChromeFeatureListCreator::ConvertFlagsToSwitches");
+
+#if BUILDFLAG(IS_CHROMEOS_ASH)
+  // On Chrome OS, flags are passed on the command line when Chrome gets
+  // launched by session_manager. There are separate sets of flags for the login
+  // screen environment and user sessions. session_manager populates the former
+  // from signed device settings, while flags for user session are stored in
+  // preferences and applied via a chrome restart upon user login, see
+  // UserSessionManager::RestartToApplyPerSessionFlagsIfNeed for the latter.
+  chromeos::about_flags::ReadOnlyFlagsStorage flags_storage(
+      chromeos::about_flags::ParseFlagsFromCommandLine());
+#else
   flags_ui::PrefServiceFlagsStorage flags_storage(local_state_.get());
+#endif  // BUILDFLAG(IS_CHROMEOS_ASH)
+
   about_flags::ConvertFlagsToSwitches(&flags_storage,
                                       base::CommandLine::ForCurrentProcess(),
                                       flags_ui::kAddSentinels);
-#endif  // !BUILDFLAG(IS_CHROMEOS_ASH)
 }
 
 void ChromeFeatureListCreator::SetupFieldTrials() {