Cache command-line switches keyed by type in UserSessionManager

Command-line switches that should be independent of policy/flag
settings are now cached in UserSessionManager, so they're not lost
when a different set of command-line switches is applied.
Currently, the only such persistent switch is --profile-requires-policy
set by UserCloudPolicyManagerChromeOS.

The long-term plan is to further develop this mechanism in a way to make
manual re-parsing of switches (see e.g.
|KioskAppManager::GetSwitchesForSessionRestore|) unnecessary. This will
be done in a follow-up CL.

      and manual: Enable IsolateOrigins user policy and
      ephemeral_user_enabled device policy, sign-in into a user account.

Bug: 831460
Test: browser_tests --gtest_filter=SiteIsolationFlagHandlingTest*
Change-Id: I1276b20018b0c4305309123e46dd3411aeda5ac0
Reviewed-on: https://chromium-review.googlesource.com/1010343
Commit-Queue: Pavol Marko <pmarko@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550765}

chrome/browser/chromeos/app_mode/kiosk_app_manager.cc

@@ -27,6 +27,7 @@
 #include "chrome/browser/chromeos/app_mode/kiosk_app_manager_observer.h"
 #include "chrome/browser/chromeos/app_mode/kiosk_external_updater.h"
 #include "chrome/browser/chromeos/extensions/external_cache_impl.h"
+#include "chrome/browser/chromeos/login/session/user_session_manager.h"
 #include "chrome/browser/chromeos/ownership/owner_settings_service_chromeos.h"
 #include "chrome/browser/chromeos/ownership/owner_settings_service_chromeos_factory.h"
 #include "chrome/browser/chromeos/policy/browser_policy_connector_chromeos.h"
@@ -42,7 +43,6 @@
 #include "chromeos/cryptohome/async_method_caller.h"
 #include "chromeos/cryptohome/cryptohome_parameters.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
-#include "chromeos/dbus/session_manager_client.h"
 #include "chromeos/settings/cros_settings_names.h"
 #include "components/ownership/owner_key_util.h"
 #include "components/prefs/pref_registry_simple.h"
@@ -307,9 +307,10 @@ void KioskAppManager::InitSession(Profile* profile,
     // set here is to be able to properly restore session if the session is
     // restarted - e.g. due to crash. For example, this will ensure restarted
     // app session restores auto-launched state.
-    DBusThreadManager::Get()->GetSessionManagerClient()->SetFlagsForUser(
-        cryptohome::Identification(
-            user_manager::UserManager::Get()->GetActiveUser()->GetAccountId()),
+    chromeos::UserSessionManager::GetInstance()->SetSwitchesForUser(
+        user_manager::UserManager::Get()->GetActiveUser()->GetAccountId(),
+        chromeos::UserSessionManager::CommandLineSwitchesType::
+            kPolicyAndFlagsAndKioskControl,
         flags);
   }
 

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

@@ -872,10 +872,9 @@ bool UserSessionManager::RestartToApplyPerSessionFlagsIfNeed(
   // argv[0] is the program name |base::CommandLine::NO_PROGRAM|.
   flags.assign(user_flags.argv().begin() + 1, user_flags.argv().end());
   LOG(WARNING) << "Restarting to apply per-session flags...";
-  session_manager_client->SetFlagsForUser(
-      cryptohome::Identification(
-          user_manager::UserManager::Get()->GetActiveUser()->GetAccountId()),
-      flags);
+  SetSwitchesForUser(
+      user_manager::UserManager::Get()->GetActiveUser()->GetAccountId(),
+      CommandLineSwitchesType::kPolicyAndFlagsAndKioskControl, flags);
   attempt_restart_closure_.Run();
   return true;
 }
@@ -2076,6 +2075,27 @@ void UserSessionManager::Shutdown() {
   first_run::GoodiesDisplayer::Delete();
 }
 
+void UserSessionManager::SetSwitchesForUser(
+    const AccountId& account_id,
+    CommandLineSwitchesType switches_type,
+    const std::vector<std::string>& switches) {
+  // TODO(pmarko): Introduce a CHECK that |account_id| is the primary user
+  // (https://crbug.com/832857).
+  command_line_switches_[switches_type] = switches;
+
+  // Apply all command-line switch types in session manager as a flat list.
+  std::vector<std::string> all_switches;
+  for (const auto& pair : command_line_switches_) {
+    all_switches.insert(all_switches.end(), pair.second.begin(),
+                        pair.second.end());
+  }
+
+  SessionManagerClient* session_manager_client =
+      DBusThreadManager::Get()->GetSessionManagerClient();
+  session_manager_client->SetFlagsForUser(
+      cryptohome::Identification(account_id), all_switches);
+}
+
 void UserSessionManager::CreateTokenUtilIfMissing() {
   if (!token_handle_util_.get())
     token_handle_util_.reset(new TokenHandleUtil());

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

@@ -9,6 +9,7 @@
 #include <string>
 
 #include "base/callback.h"
+#include "base/containers/flat_map.h"
 #include "base/macros.h"
 #include "base/memory/singleton.h"
 #include "base/memory/weak_ptr.h"
@@ -110,6 +111,21 @@ class UserSessionManager
     SECONDARY_USER_SESSION_AFTER_CRASH,
   } StartSessionType;
 
+  // Types of command-line switches for a user session. The command-line
+  // switches of all types are combined.
+  enum class CommandLineSwitchesType {
+    // Switches for controlling session initialization, such as if the profile
+    // requires enterprise policy.
+    kSessionControl,
+    // Switches derived from user policy, from user-set flags and kiosk app
+    // control switches.
+    // TODO(pmarko): Split this into multiple categories, such as kPolicy,
+    // kFlags, kKioskControl. Consider also adding sentinels automatically and
+    // pre-filling these switches from the command-line if the chrome has been
+    // started with the --login-user flag (https://crbug.com/832857).
+    kPolicyAndFlagsAndKioskControl
+  };
+
   // Parameters to use when initializing the RLZ library.  These fields need
   // to be retrieved from a blocking task and this structure is used to pass
   // the data.
@@ -276,6 +292,19 @@ class UserSessionManager
 
   void Shutdown();
 
+  // Sets the command-line switches to be set by session manager for a user
+  // session associated with |account_id| when chrome restarts. Overwrites
+  // switches for |switches_type| with |switches|. The resulting command-line
+  // switches will be the command-line switches for all types combined. Note:
+  // |account_id| is currently ignored, because session manager ignores the
+  // passed account id. For each type, only the last-set switches will be
+  // honored.
+  // TODO(pmarko): Introduce a CHECK making sure that |account_id| is the
+  // primary user (https://crbug.com/832857).
+  void SetSwitchesForUser(const AccountId& account_id,
+                          CommandLineSwitchesType switches_type,
+                          const std::vector<std::string>& switches);
+
   // Called when the user network policy has been parsed. If |send_password| is
   // true, the user's password will be sent over dbus to the session manager to
   // save in a keyring. Before the function exits, it will clear the user
@@ -529,6 +558,13 @@ class UserSessionManager
            ProfileCompare>
       fingerprint_unlock_notification_handler_;
 
+  // Maps command-line switch types to the currently set command-line switches
+  // for that type. Note: This is not per Profile/AccountId, because session
+  // manager currently doesn't support setting command-line switches per
+  // AccountId.
+  base::flat_map<CommandLineSwitchesType, std::vector<std::string>>
+      command_line_switches_;
+
   // Manages Easy unlock cryptohome keys.
   std::unique_ptr<EasyUnlockKeyManager> easy_unlock_key_manager_;
   bool running_easy_unlock_key_ops_;

chrome/browser/chromeos/policy/user_cloud_policy_manager_chromeos.cc

@@ -30,8 +30,6 @@
 #include "chrome/browser/lifetime/application_lifetime.h"
 #include "chrome/common/chrome_content_client.h"
 #include "chromeos/chromeos_switches.h"
-#include "chromeos/cryptohome/cryptohome_parameters.h"
-#include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/system/statistics_provider.h"
 #include "components/crash/core/common/crash_key.h"
 #include "components/policy/core/common/cloud/cloud_external_data_manager.h"
@@ -456,9 +454,10 @@ void UserCloudPolicyManagerChromeOS::SetPolicyRequired(bool policy_required) {
     base::CommandLine::StringVector flags;
     flags.assign(command_line.argv().begin() + 1, command_line.argv().end());
     DCHECK_EQ(1u, flags.size());
-    chromeos::DBusThreadManager::Get()
-        ->GetSessionManagerClient()
-        ->SetFlagsForUser(cryptohome::Identification(account_id_), flags);
+    chromeos::UserSessionManager::GetInstance()->SetSwitchesForUser(
+        account_id_,
+        chromeos::UserSessionManager::CommandLineSwitchesType::kSessionControl,
+        flags);
   }
 }
 

chrome/browser/ui/webui/flags_ui.cc

@@ -46,10 +46,8 @@
 #include "chrome/browser/chromeos/ownership/owner_settings_service_chromeos_factory.h"
 #include "chrome/browser/chromeos/settings/cros_settings.h"
 #include "chrome/browser/chromeos/settings/owner_flags_storage.h"
-#include "chromeos/cryptohome/cryptohome_parameters.h"
-#include "chromeos/dbus/dbus_thread_manager.h"
-#include "chromeos/dbus/session_manager_client.h"
 #include "components/pref_registry/pref_registry_syncable.h"
+#include "components/signin/core/account_id/account_id.h"
 #include "components/user_manager/user_manager.h"
 #endif
 
@@ -231,13 +229,13 @@ void FlagsDOMHandler::HandleRestartBrowser(const base::ListValue* args) {
   // argv[0] is the program name |base::CommandLine::NO_PROGRAM|.
   flags.assign(user_flags.argv().begin() + 1, user_flags.argv().end());
   VLOG(1) << "Restarting to apply per-session flags...";
-  chromeos::DBusThreadManager::Get()
-      ->GetSessionManagerClient()
-      ->SetFlagsForUser(
-          cryptohome::Identification(user_manager::UserManager::Get()
-                                         ->GetActiveUser()
-                                         ->GetAccountId()),
-          flags);
+  AccountId account_id =
+      user_manager::UserManager::Get()->GetActiveUser()->GetAccountId();
+  chromeos::UserSessionManager::GetInstance()->SetSwitchesForUser(
+      account_id,
+      chromeos::UserSessionManager::CommandLineSwitchesType::
+          kPolicyAndFlagsAndKioskControl,
+      flags);
 #endif
   chrome::AttemptRestart();
 }