Restart Chrome if per session flags have been specified on ChromeOS.

After the profile is loaded we verify if the user has specfied any flags
or if there were flags specified per policy that differ from the user
specfied flags and in either case restart Chrome with the desired flags.

This allows non-owners to use the about:flags page and prevents policy
set flags from leaking inside user sessions.

BUG=221352
TEST=unit_tests & Manually by specifying flags in non-owner session and observing them respected.

Review URL: https://chromiumcodereview.appspot.com/16770002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@206832 0039d316-1c4b-4281-b951-d872f2087c98

11 files changed, 137 insertions, 4 deletions

diff --git a/chrome/browser/about_flags.cc b/chrome/browser/about_flags.cc
index 45b14cc..7a58abb 100644
--- a/chrome/browser/about_flags.cc
+++ b/chrome/browser/about_flags.cc
@@ -38,6 +38,7 @@
 
 #if defined(OS_CHROMEOS)
 #include "chromeos/chromeos_switches.h"
+#include "third_party/cros_system_api/switches/chrome_switches.h"
 #endif
 
 using content::UserMetricsAction;
@@ -83,6 +84,39 @@ void AddOsStrings(unsigned bitmask, ListValue* list) {
       list->Append(new StringValue(kBitsToOs[i].name));
 }
 
+// Convert switch constants to proper CommandLine::StringType strings.
+CommandLine::StringType GetSwitchString(const std::string& flag) {
+  CommandLine cmd_line(CommandLine::NO_PROGRAM);
+  cmd_line.AppendSwitch(flag);
+  DCHECK(cmd_line.argv().size() == 2);
+  return cmd_line.argv()[1];
+}
+
+// Scoops flags from a command line.
+std::set<CommandLine::StringType> ExtractFlagsFromCommandLine(
+    const CommandLine& cmdline) {
+  std::set<CommandLine::StringType> flags;
+  // First do the ones between --flag-switches-begin and --flag-switches-end.
+  CommandLine::StringVector::const_iterator first =
+      std::find(cmdline.argv().begin(), cmdline.argv().end(),
+                GetSwitchString(switches::kFlagSwitchesBegin));
+  CommandLine::StringVector::const_iterator last =
+      std::find(cmdline.argv().begin(), cmdline.argv().end(),
+                GetSwitchString(switches::kFlagSwitchesEnd));
+  if (first != cmdline.argv().end() && last != cmdline.argv().end())
+    flags.insert(first + 1, last);
+#if defined(OS_CHROMEOS)
+  // Then add those between --policy-switches-begin and --policy-switches-end.
+  first = std::find(cmdline.argv().begin(), cmdline.argv().end(),
+                    GetSwitchString(chromeos::switches::kPolicySwitchesBegin));
+  last = std::find(cmdline.argv().begin(), cmdline.argv().end(),
+                   GetSwitchString(chromeos::switches::kPolicySwitchesEnd));
+  if (first != cmdline.argv().end() && last != cmdline.argv().end())
+    flags.insert(first + 1, last);
+#endif
+  return flags;
+}
+
 const Experiment::Choice
     kOmniboxHistoryQuickProviderReorderForInliningChoices[] = {
   { IDS_FLAGS_OMNIBOX_HISTORY_QUICK_PROVIDER_REORDER_FOR_INLINING_AUTOMATIC,
@@ -1726,6 +1760,20 @@ void ConvertFlagsToSwitches(FlagsStorage* flags_storage,
                                                     command_line);
 }
 
+bool AreSwitchesIdenticalToCurrentCommandLine(
+    const CommandLine& new_cmdline, const CommandLine& active_cmdline) {
+  std::set<CommandLine::StringType> new_flags =
+      ExtractFlagsFromCommandLine(new_cmdline);
+  std::set<CommandLine::StringType> active_flags =
+      ExtractFlagsFromCommandLine(active_cmdline);
+
+  // Needed because std::equal doesn't check if the 2nd set is empty.
+  if (new_flags.size() != active_flags.size())
+    return false;
+
+  return std::equal(new_flags.begin(), new_flags.end(), active_flags.begin());
+}
+
 void GetFlagsExperimentsData(FlagsStorage* flags_storage,
                              FlagAccess access,
                              base::ListValue* supported_experiments,
diff --git a/chrome/browser/about_flags.h b/chrome/browser/about_flags.h
index 2553425..5d67e97 100644
--- a/chrome/browser/about_flags.h
+++ b/chrome/browser/about_flags.h
@@ -110,6 +110,11 @@ struct Experiment {
 void ConvertFlagsToSwitches(FlagsStorage* flags_storage,
                             CommandLine* command_line);
 
+// Compares a set of switches of the two provided command line objects and
+// returns true if they are the same and false otherwise.
+bool AreSwitchesIdenticalToCurrentCommandLine(
+    const CommandLine& new_cmdline, const CommandLine& active_cmdline);
+
 // Differentiate between generic flags available on a per session base and flags
 // that influence the whole machine and can be said by the admin only. This flag
 // is relevant for ChromeOS for now only and dictates whether entries marked
diff --git a/chrome/browser/about_flags_unittest.cc b/chrome/browser/about_flags_unittest.cc
index 0c94f45..34219c3 100644
--- a/chrome/browser/about_flags_unittest.cc
+++ b/chrome/browser/about_flags_unittest.cc
@@ -223,6 +223,34 @@ TEST_F(AboutFlagsTest, ConvertFlagsToSwitches) {
   EXPECT_TRUE(command_line.HasSwitch(kSwitch1));
 }
 
+TEST_F(AboutFlagsTest, CompareSwitchesToCurrentCommandLine) {
+  SetExperimentEnabled(&flags_storage_, kFlags1, true);
+
+  CommandLine command_line(CommandLine::NO_PROGRAM);
+  command_line.AppendSwitch("foo");
+
+  CommandLine new_command_line(CommandLine::NO_PROGRAM);
+  ConvertFlagsToSwitches(&flags_storage_, &new_command_line);
+
+  EXPECT_FALSE(AreSwitchesIdenticalToCurrentCommandLine(new_command_line,
+                                                        command_line));
+
+  ConvertFlagsToSwitches(&flags_storage_, &command_line);
+
+  EXPECT_TRUE(AreSwitchesIdenticalToCurrentCommandLine(new_command_line,
+                                                       command_line));
+
+  // Now both have flags but different.
+  SetExperimentEnabled(&flags_storage_, kFlags1, false);
+  SetExperimentEnabled(&flags_storage_, kFlags2, true);
+
+  CommandLine another_command_line(CommandLine::NO_PROGRAM);
+  ConvertFlagsToSwitches(&flags_storage_, &another_command_line);
+
+  EXPECT_FALSE(AreSwitchesIdenticalToCurrentCommandLine(new_command_line,
+                                                        another_command_line));
+}
+
 TEST_F(AboutFlagsTest, RemoveFlagSwitches) {
   std::map<std::string, CommandLine::StringType> switch_list;
   switch_list[kSwitch1] = CommandLine::StringType();
diff --git a/chrome/browser/chromeos/login/login_utils.cc b/chrome/browser/chromeos/login/login_utils.cc
index 1006b08..24cc11f 100644
--- a/chrome/browser/chromeos/login/login_utils.cc
+++ b/chrome/browser/chromeos/login/login_utils.cc
@@ -48,9 +48,11 @@
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/first_run/first_run.h"
 #include "chrome/browser/google/google_util_chromeos.h"
+#include "chrome/browser/lifetime/application_lifetime.h"
 #include "chrome/browser/managed_mode/managed_mode.h"
 #include "chrome/browser/net/chrome_url_request_context.h"
 #include "chrome/browser/net/preconnect.h"
+#include "chrome/browser/pref_service_flags_storage.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/rlz/rlz.h"
@@ -280,6 +282,21 @@ void LoginUtilsImpl::DoBrowserLaunch(Profile* profile,
   if (browser_shutdown::IsTryingToQuit())
     return;
 
+  CommandLine user_flags(CommandLine::NO_PROGRAM);
+  about_flags::PrefServiceFlagsStorage flags_storage_(profile->GetPrefs());
+  about_flags::ConvertFlagsToSwitches(&flags_storage_, &user_flags);
+  if (!about_flags::AreSwitchesIdenticalToCurrentCommandLine(
+          user_flags, *CommandLine::ForCurrentProcess())) {
+    CommandLine::StringVector flags;
+    // argv[0] is the program name |CommandLine::NO_PROGRAM|.
+    flags.assign(user_flags.argv().begin() + 1, user_flags.argv().end());
+    VLOG(1) << "Restarting to apply per-session flags...";
+    DBusThreadManager::Get()->GetSessionManagerClient()->SetFlagsForUser(
+        UserManager::Get()->GetActiveUser()->email(), flags);
+    chrome::ExitCleanly();
+    return;
+  }
+
   if (!UserManager::Get()->GetCurrentUserFlow()->ShouldLaunchBrowser()) {
     UserManager::Get()->GetCurrentUserFlow()->LaunchExtraSteps(profile);
     return;
@@ -298,9 +315,6 @@ void LoginUtilsImpl::DoBrowserLaunch(Profile* profile,
   chrome::startup::IsFirstRun first_run = first_run::IsChromeFirstRun() ?
       chrome::startup::IS_FIRST_RUN : chrome::startup::IS_NOT_FIRST_RUN;
 
-  // TODO(pastarmovj): Restart the browser and apply any flags set by the user.
-  // See: http://crosbug.com/39249
-
   browser_creator.LaunchBrowser(*CommandLine::ForCurrentProcess(),
                                 profile,
                                 base::FilePath(),
diff --git a/chrome/browser/chromeos/settings/device_settings_test_helper.cc b/chrome/browser/chromeos/settings/device_settings_test_helper.cc
index 508de87..501d9e9 100644
--- a/chrome/browser/chromeos/settings/device_settings_test_helper.cc
+++ b/chrome/browser/chromeos/settings/device_settings_test_helper.cc
@@ -167,6 +167,10 @@ void DeviceSettingsTestHelper::StoreDeviceLocalAccountPolicy(
   device_local_account_policy_[account_id].store_callbacks_.push_back(callback);
 }
 
+void DeviceSettingsTestHelper::SetFlagsForUser(
+    const std::string& account_id,
+    const std::vector<std::string>& flags) {}
+
 DeviceSettingsTestHelper::PolicyState::PolicyState()
     : store_result_(true) {}
 
diff --git a/chrome/browser/chromeos/settings/device_settings_test_helper.h b/chrome/browser/chromeos/settings/device_settings_test_helper.h
index 2d193a8..4764aa5 100644
--- a/chrome/browser/chromeos/settings/device_settings_test_helper.h
+++ b/chrome/browser/chromeos/settings/device_settings_test_helper.h
@@ -112,6 +112,9 @@ class DeviceSettingsTestHelper : public SessionManagerClient {
       const std::string& account_id,
       const std::string& policy_blob,
       const StorePolicyCallback& callback) OVERRIDE;
+  virtual void SetFlagsForUser(
+      const std::string& account_id,
+      const std::vector<std::string>& flags) OVERRIDE;
 
  private:
   struct PolicyState {
diff --git a/chromeos/dbus/fake_session_manager_client.cc b/chromeos/dbus/fake_session_manager_client.cc
index 1450fbd..f20b719 100644
--- a/chromeos/dbus/fake_session_manager_client.cc
+++ b/chromeos/dbus/fake_session_manager_client.cc
@@ -126,6 +126,11 @@ void FakeSessionManagerClient::StoreDeviceLocalAccountPolicy(
   base::MessageLoop::current()->PostTask(FROM_HERE, base::Bind(callback, true));
 }
 
+void FakeSessionManagerClient::SetFlagsForUser(
+    const std::string& username,
+    const std::vector<std::string>& flags) {
+}
+
 const std::string& FakeSessionManagerClient::device_policy() const {
   return device_policy_;
 }
diff --git a/chromeos/dbus/fake_session_manager_client.h b/chromeos/dbus/fake_session_manager_client.h
index 044314f..1acd082 100644
--- a/chromeos/dbus/fake_session_manager_client.h
+++ b/chromeos/dbus/fake_session_manager_client.h
@@ -57,6 +57,8 @@ class FakeSessionManagerClient : public chromeos::SessionManagerClient {
       const std::string& account_id,
       const std::string& policy_blob,
       const StorePolicyCallback& callback) OVERRIDE;
+  virtual void SetFlagsForUser(const std::string& username,
+                               const std::vector<std::string>& flags) OVERRIDE;
 
   const std::string& device_policy() const;
   void set_device_policy(const std::string& policy_blob);
diff --git a/chromeos/dbus/mock_session_manager_client.h b/chromeos/dbus/mock_session_manager_client.h
index c57d5d1..1596951 100644
--- a/chromeos/dbus/mock_session_manager_client.h
+++ b/chromeos/dbus/mock_session_manager_client.h
@@ -51,6 +51,9 @@ class MockSessionManagerClient : public SessionManagerClient {
                void(const std::string&,
                     const std::string&,
                     const StorePolicyCallback&));
+  MOCK_METHOD2(SetFlagsForUser,
+               void(const std::string&,
+                    const std::vector<std::string>&));
 };
 
 }  // namespace chromeos
diff --git a/chromeos/dbus/session_manager_client.cc b/chromeos/dbus/session_manager_client.cc
index d7700e7..f610275 100644
--- a/chromeos/dbus/session_manager_client.cc
+++ b/chromeos/dbus/session_manager_client.cc
@@ -271,6 +271,19 @@ class SessionManagerClientImpl : public SessionManagerClient {
         callback);
   }
 
+  virtual void SetFlagsForUser(const std::string& username,
+                               const std::vector<std::string>& flags) OVERRIDE {
+    dbus::MethodCall method_call(login_manager::kSessionManagerInterface,
+                                 login_manager::kSessionManagerSetFlagsForUser);
+    dbus::MessageWriter writer(&method_call);
+    writer.AppendString(username);
+    writer.AppendArrayOfStrings(flags);
+    session_manager_proxy_->CallMethod(
+        &method_call,
+        dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+        dbus::ObjectProxy::EmptyResponseCallback());
+  }
+
  private:
   // Makes a method call to the session manager with no arguments and no
   // response.
@@ -597,6 +610,9 @@ class SessionManagerClientStubImpl : public SessionManagerClient {
     user_policies_[account_name] = policy_blob;
     callback.Run(true);
   }
+  virtual void SetFlagsForUser(const std::string& username,
+                               const std::vector<std::string>& flags) OVERRIDE {
+  }
 
   static void StoreFileInBackground(const base::FilePath& path,
                                     const std::string& data) {
diff --git a/chromeos/dbus/session_manager_client.h b/chromeos/dbus/session_manager_client.h
index 70ca3f4..ca10707 100644
--- a/chromeos/dbus/session_manager_client.h
+++ b/chromeos/dbus/session_manager_client.h
@@ -15,7 +15,7 @@
 
 namespace dbus {
 class Bus;
-}  // namespace
+}  // namespace dbus
 
 namespace chromeos {
 
@@ -158,6 +158,11 @@ class CHROMEOS_EXPORT SessionManagerClient {
       const std::string& policy_blob,
       const StorePolicyCallback& callback) = 0;
 
+  // Sets the flags to be applied next time by the session manager when Chrome
+  // is restarted inside an already started session for a particular user.
+  virtual void SetFlagsForUser(const std::string& username,
+                               const std::vector<std::string>& flags) = 0;
+
   // Creates the instance.
   static SessionManagerClient* Create(DBusClientImplementationType type,
                                       dbus::Bus* bus);