Revert "arc: auto-login to MGS with blocked UI screen"

This reverts commit 725f9a2e.

Reason for revert: This CL is likly cause components_unittests failure for Linux ChromiumOS MSan Tests build starting from: https://ci.chromium.org/p/chromium/builders/ci/Linux%20ChromiumOS%20MSan%20Tests/20905
The failed tests are ArcDataSnapshotdManagerFlowTest and ArcDataSnapshotdManagerBasicTest

Original change's description:
> arc: auto-login to MGS with blocked UI screen
>
> Add an auto-login to Managed Guest Session / public account to start ARC
> snapshot creation.
> Block auto-login flow even if specified by policy.
> Wait for ArcDataSnapshotdManager successfully generates a key pair and
> auto-login to MGS once it is ready.
>
> BUG=b:161221001
> TEST=components_unittests
> TEST=browser_tests
>
> Change-Id: Ib7895f49f9295b3959226cde3e304c9c78614e93
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2440872
> Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
> Reviewed-by: Roman Sorokin [CET] <rsorokin@chromium.org>
> Reviewed-by: Yusuke Sato <yusukes@chromium.org>
> Commit-Queue: Polina Bondarenko <pbond@chromium.org>
> Auto-Submit: Polina Bondarenko <pbond@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#816186}

TBR=yusukes@chromium.org,achuith@chromium.org,rsorokin@chromium.org,pbond@chromium.org

Change-Id: Idfc1e81798a013b6f86f57d7fe3de2d2dabeb09a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: b:161221001
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2465894Reviewed-by: Maggie Cai <mxcai@chromium.org>
Commit-Queue: Maggie Cai <mxcai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#816387}

chrome/browser/chromeos/chrome_browser_main_chromeos.cc

@@ -680,8 +680,7 @@ void ChromeBrowserMainPartsChromeos::PreProfileInit() {
 
   arc_data_snapshotd_manager_ =
       std::make_unique<arc::data_snapshotd::ArcDataSnapshotdManager>(
-          g_browser_process->local_state(),
-          base::BindOnce(chrome::AttemptUserExit));
+          g_browser_process->local_state());
   if (base::FeatureList::IsEnabled(::features::kWilcoDtc))
     wilco_dtc_supportd_manager_ = std::make_unique<WilcoDtcSupportdManager>();
 

chrome/browser/chromeos/login/existing_user_controller.cc

@@ -94,7 +94,6 @@
 #include "chromeos/strings/grit/chromeos_strings.h"
 #include "components/account_id/account_id.h"
 #include "components/arc/arc_util.h"
-#include "components/arc/enterprise/arc_data_snapshotd_manager.h"
 #include "components/google/core/common/google_util.h"
 #include "components/policy/core/common/cloud/cloud_policy_client.h"
 #include "components/policy/core/common/cloud/cloud_policy_core.h"
@@ -346,42 +345,6 @@ base::TimeDelta TimeToOnlineSignIn(base::Time last_online_signin,
   return offline_signin_limit - (now - last_online_signin);
 }
 
-// Returns account ID of a public session account if it is unique, otherwise
-// returns invalid account ID.
-AccountId GetArcDataSnapshotAutoLoginAccountId(
-    const std::vector<policy::DeviceLocalAccount>& device_local_accounts) {
-  AccountId auto_login_account_id = EmptyAccountId();
-  for (std::vector<policy::DeviceLocalAccount>::const_iterator it =
-           device_local_accounts.begin();
-       it != device_local_accounts.end(); ++it) {
-    if (it->type == policy::DeviceLocalAccount::TYPE_PUBLIC_SESSION) {
-      // Do not perform ARC data snapshot auto-login if more than one public
-      // session account is configured.
-      if (auto_login_account_id.is_valid())
-        return EmptyAccountId();
-      auto_login_account_id = AccountId::FromUserEmail(it->user_id);
-      VLOG(2) << "PublicSession autologin found: " << it->user_id;
-    }
-  }
-  return auto_login_account_id;
-}
-
-// Returns account ID if a corresponding to |auto_login_account_id| device local
-// account exists, otherwise returns invalid account ID.
-AccountId GetPublicSessionAutoLoginAccountId(
-    const std::vector<policy::DeviceLocalAccount>& device_local_accounts,
-    const std::string& auto_login_account_id) {
-  for (std::vector<policy::DeviceLocalAccount>::const_iterator it =
-           device_local_accounts.begin();
-       it != device_local_accounts.end(); ++it) {
-    if (it->account_id == auto_login_account_id) {
-      VLOG(2) << "PublicSession autologin found: " << it->user_id;
-      return AccountId::FromUserEmail(it->user_id);
-    }
-  }
-  return EmptyAccountId();
-}
-
 class AutoLaunchNotificationDelegate
     : public message_center::HandleNotificationClickDelegate {
  public:
@@ -1592,17 +1555,16 @@ void ExistingUserController::ConfigureAutoLogin() {
       policy::GetDeviceLocalAccounts(cros_settings_);
   const bool show_update_required_screen = IsUpdateRequiredDeadlineReached();
 
-  auto* data_snapshotd_manager =
-      arc::data_snapshotd::ArcDataSnapshotdManager::Get();
-  bool is_arc_data_snapshot_autologin =
-      (data_snapshotd_manager &&
-       data_snapshotd_manager->IsAutoLoginConfigured());
-  if (is_arc_data_snapshot_autologin) {
-    public_session_auto_login_account_id_ =
-        GetArcDataSnapshotAutoLoginAccountId(device_local_accounts);
-  } else {
-    public_session_auto_login_account_id_ = GetPublicSessionAutoLoginAccountId(
-        device_local_accounts, auto_login_account_id);
+  public_session_auto_login_account_id_ = EmptyAccountId();
+  for (std::vector<policy::DeviceLocalAccount>::const_iterator it =
+           device_local_accounts.begin();
+       it != device_local_accounts.end(); ++it) {
+    if (it->account_id == auto_login_account_id) {
+      public_session_auto_login_account_id_ =
+          AccountId::FromUserEmail(it->user_id);
+      VLOG(2) << "PublicSession autologin found: " << it->user_id;
+      break;
+    }
   }
 
   const user_manager::User* public_session_user =
@@ -1614,8 +1576,7 @@ void ExistingUserController::ConfigureAutoLogin() {
     public_session_auto_login_account_id_ = EmptyAccountId();
   }
 
-  if (is_arc_data_snapshot_autologin ||
-      !cros_settings_->GetInteger(kAccountsPrefDeviceLocalAccountAutoLoginDelay,
+  if (!cros_settings_->GetInteger(kAccountsPrefDeviceLocalAccountAutoLoginDelay,
                                   &auto_login_delay_)) {
     auto_login_delay_ = 0;
   }
@@ -1694,20 +1655,6 @@ void ExistingUserController::StartAutoLoginTimer() {
     StopAutoLoginTimer();
   }
 
-  // Block auto-login flow until ArcDataSnapshotdManager is ready to enter an
-  // auto-login session.
-  // ArcDataSnapshotdManager stores a reset auto-login callback to fire it once
-  // it is ready.
-  auto* data_snapshotd_manager =
-      arc::data_snapshotd::ArcDataSnapshotdManager::Get();
-  if (data_snapshotd_manager && !data_snapshotd_manager->IsAutoLoginAllowed() &&
-      data_snapshotd_manager->IsAutoLoginConfigured()) {
-    data_snapshotd_manager->set_reset_autologin_callback(
-        base::BindOnce(&ExistingUserController::ResetAutoLoginTimer,
-                       weak_factory_.GetWeakPtr()));
-    return;
-  }
-
   // Start the auto-login timer.
   if (!auto_login_timer_)
     auto_login_timer_.reset(new base::OneShotTimer);

chrome/browser/chromeos/login/existing_user_controller_browsertest.cc

@@ -59,7 +59,6 @@
 #include "chromeos/network/network_state_test_helper.h"
 #include "chromeos/settings/cros_settings_names.h"
 #include "chromeos/settings/cros_settings_provider.h"
-#include "components/arc/enterprise/arc_data_snapshotd_manager.h"
 #include "components/password_manager/core/common/password_manager_pref_names.h"
 #include "components/policy/core/common/cloud/cloud_policy_constants.h"
 #include "components/policy/core/common/cloud/cloud_policy_core.h"
@@ -109,8 +108,6 @@ const char kPassword[] = "test_password";
 const char kHash[] = "test_hash";
 
 const char kPublicSessionUserEmail[] = "public_session_user@localhost";
-const char kPublicSessionSecondUserEmail[] =
-    "public_session_second_user@localhost";
 const int kAutoLoginNoDelay = 0;
 const int kAutoLoginShortDelay = 1;
 const int kAutoLoginLongDelay = 10000;
@@ -146,10 +143,6 @@ base::FilePath GetKerberosCredentialsCachePath() {
   return path.Append("kerberos").Append("krb5cc");
 }
 
-arc::data_snapshotd::ArcDataSnapshotdManager* arc_data_snapshotd_manager() {
-  return arc::data_snapshotd::ArcDataSnapshotdManager::Get();
-}
-
 }  // namespace
 
 class ExistingUserControllerTest : public policy::DevicePolicyCrosBrowserTest {
@@ -335,12 +328,6 @@ class ExistingUserControllerPublicSessionTest
   void SetUpOnMainThread() override {
     ExistingUserControllerTest::SetUpOnMainThread();
 
-    // By default ArcDataSnapshotdManager does not influence an auto login
-    // flow.
-    EXPECT_TRUE(arc_data_snapshotd_manager());
-    EXPECT_TRUE(arc_data_snapshotd_manager()->IsAutoLoginAllowed());
-    EXPECT_FALSE(arc_data_snapshotd_manager()->IsAutoLoginConfigured());
-
     // Wait for the public session user to be created.
     if (!user_manager::UserManager::Get()->IsKnownUser(
             public_session_account_id_)) {
@@ -719,98 +706,6 @@ IN_PROC_BROWSER_TEST_F(ExistingUserControllerPublicSessionTest,
   FireAutoLogin();
 }
 
-IN_PROC_BROWSER_TEST_F(ExistingUserControllerPublicSessionTest,
-                       ArcDataSnapshotdAutoLogin) {
-  arc_data_snapshotd_manager()->set_state_for_testing(
-      arc::data_snapshotd::ArcDataSnapshotdManager::State::kBlockedUi);
-  EXPECT_FALSE(arc_data_snapshotd_manager()->IsAutoLoginAllowed());
-  EXPECT_TRUE(arc_data_snapshotd_manager()->IsAutoLoginConfigured());
-
-  ConfigureAutoLogin();
-  existing_user_controller()->OnSigninScreenReady();
-
-  // Do not start an auto-login public account session when in blocked UI mode.
-  EXPECT_TRUE(auto_login_account_id().is_valid());
-  EXPECT_EQ(public_session_account_id_, auto_login_account_id());
-  EXPECT_EQ(0, auto_login_delay());
-  EXPECT_FALSE(auto_login_timer());
-
-  // Allow to launch public account session (MGS).
-  arc_data_snapshotd_manager()->set_state_for_testing(
-      arc::data_snapshotd::ArcDataSnapshotdManager::State::kMgsToLaunch);
-  EXPECT_TRUE(arc_data_snapshotd_manager()->IsAutoLoginAllowed());
-  EXPECT_TRUE(arc_data_snapshotd_manager()->IsAutoLoginConfigured());
-
-  // Set up mocks to check login success.
-  UserContext user_context(user_manager::USER_TYPE_PUBLIC_ACCOUNT,
-                           public_session_account_id_);
-  user_context.SetUserIDHash(user_context.GetAccountId().GetUserEmail());
-  ExpectSuccessfulLogin(user_context);
-  existing_user_controller()->OnSigninScreenReady();
-
-  // Start auto-login and wait for login tasks to complete.
-  content::RunAllPendingInMessageLoop();
-
-  arc_data_snapshotd_manager()->OnSessionStateChanged();
-
-  EXPECT_TRUE(auto_login_account_id().is_valid());
-  EXPECT_EQ(0, auto_login_delay());
-  EXPECT_TRUE(auto_login_timer());
-  EXPECT_EQ(arc::data_snapshotd::ArcDataSnapshotdManager::State::kMgsLaunched,
-            arc_data_snapshotd_manager()->state());
-}
-
-class ExistingUserControllerSecondPublicSessionTest
-    : public ExistingUserControllerPublicSessionTest {
- public:
-  void SetUpInProcessBrowserTestFixture() override {
-    ExistingUserControllerPublicSessionTest::SetUpInProcessBrowserTestFixture();
-    AddSecondPublicSessionAccount();
-  }
-
- private:
-  void AddSecondPublicSessionAccount() {
-    // Setup the device policy.
-    em::ChromeDeviceSettingsProto& proto(device_policy()->payload());
-    em::DeviceLocalAccountInfoProto* account =
-        proto.mutable_device_local_accounts()->add_account();
-    account->set_account_id(kPublicSessionSecondUserEmail);
-    account->set_type(
-        em::DeviceLocalAccountInfoProto::ACCOUNT_TYPE_PUBLIC_SESSION);
-    RefreshDevicePolicy();
-
-    // Setup the device local account policy.
-    policy::UserPolicyBuilder device_local_account_policy;
-    device_local_account_policy.policy_data().set_username(
-        kPublicSessionSecondUserEmail);
-    device_local_account_policy.policy_data().set_policy_type(
-        policy::dm_protocol::kChromePublicAccountPolicyType);
-    device_local_account_policy.policy_data().set_settings_entity_id(
-        kPublicSessionSecondUserEmail);
-    device_local_account_policy.Build();
-    session_manager_client()->set_device_local_account_policy(
-        kPublicSessionSecondUserEmail, device_local_account_policy.GetBlob());
-  }
-};
-// Test that if two public session accounts are configured for the device, auto
-// login is not happening.
-IN_PROC_BROWSER_TEST_F(ExistingUserControllerSecondPublicSessionTest,
-                       ArcDataSnapshotdTwoAccounts) {
-  // Allow to launch public account session (MGS).
-  arc_data_snapshotd_manager()->set_state_for_testing(
-      arc::data_snapshotd::ArcDataSnapshotdManager::State::kMgsToLaunch);
-  EXPECT_TRUE(arc_data_snapshotd_manager()->IsAutoLoginAllowed());
-  EXPECT_TRUE(arc_data_snapshotd_manager()->IsAutoLoginConfigured());
-
-  ConfigureAutoLogin();
-  existing_user_controller()->OnSigninScreenReady();
-
-  // Do not configure auto login if more than one public session is configured.
-  EXPECT_FALSE(auto_login_account_id().is_valid());
-  EXPECT_EQ(0, auto_login_delay());
-  EXPECT_FALSE(auto_login_timer());
-}
-
 class ExistingUserControllerActiveDirectoryTest
     : public ExistingUserControllerTest {
  public:

components/arc/enterprise/arc_data_snapshotd_manager.cc

@@ -11,15 +11,12 @@
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
-#include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "chromeos/constants/chromeos_switches.h"
 #include "chromeos/dbus/upstart/upstart_client.h"
 #include "components/arc/arc_prefs.h"
 #include "components/arc/enterprise/arc_data_snapshotd_bridge.h"
 #include "components/prefs/pref_service.h"
-#include "components/session_manager/core/session_manager.h"
-#include "components/user_manager/user_manager.h"
 #include "ui/ozone/public/ozone_switches.h"
 
 namespace arc {
@@ -37,7 +34,7 @@ constexpr char kUpdated[] = "updated";
 constexpr char kPrevious[] = "previous";
 constexpr char kLast[] = "last";
 constexpr char kBlockedUiReboot[] = "blocked_ui_reboot";
-constexpr char kStarted[] = "started";
+constexpr char kStartedDate[] = "started_date";
 
 bool IsSnapshotEnabled() {
   // TODO(pbond): implement policy processing.
@@ -62,9 +59,6 @@ void EnableHeadlessMode() {
 
 bool ArcDataSnapshotdManager::is_snapshot_enabled_for_testing_ = false;
 
-// This class is owned by ChromeBrowserMainPartsChromeos.
-static ArcDataSnapshotdManager* g_arc_data_snapshotd_manager = nullptr;
-
 ArcDataSnapshotdManager::SnapshotInfo::SnapshotInfo(const base::Value* value,
                                                     bool last)
     : is_last_(last) {
@@ -159,11 +153,11 @@ std::unique_ptr<ArcDataSnapshotdManager::Snapshot>
 ArcDataSnapshotdManager::Snapshot::CreateForTesting(
     PrefService* local_state,
     bool blocked_ui_mode,
-    bool started,
+    const std::string& started_date,
     std::unique_ptr<SnapshotInfo> last,
     std::unique_ptr<SnapshotInfo> previous) {
   return base::WrapUnique(new ArcDataSnapshotdManager::Snapshot(
-      local_state, blocked_ui_mode, started, std::move(last),
+      local_state, blocked_ui_mode, started_date, std::move(last),
       std::move(previous)));
 }
 
@@ -188,9 +182,9 @@ void ArcDataSnapshotdManager::Snapshot::Parse() {
       blocked_ui_mode_ = found.value();
   }
   {
-    auto found = dict->FindBoolPath(kStarted);
-    if (found.has_value())
-      started_ = found.value();
+    auto* found = dict->FindStringPath(kStartedDate);
+    if (found)
+      started_date_ = *found;
   }
 }
 
@@ -201,7 +195,7 @@ void ArcDataSnapshotdManager::Snapshot::Sync() {
   if (last_)
     last_->Sync(&dict);
   dict.SetBoolKey(kBlockedUiReboot, blocked_ui_mode_);
-  dict.SetBoolKey(kStarted, started_);
+  dict.SetStringKey(kStartedDate, started_date_);
   local_state_->Set(arc::prefs::kArcSnapshotInfo, std::move(dict));
 }
 
@@ -211,42 +205,23 @@ void ArcDataSnapshotdManager::Snapshot::ClearSnapshot(bool last) {
   Sync();
 }
 
-void ArcDataSnapshotdManager::Snapshot::StartNewSnapshot() {
-  previous_ = std::move(last_);
-  last_ = nullptr;
-
-  started_ = true;
-  Sync();
-}
-
 ArcDataSnapshotdManager::Snapshot::Snapshot(
     PrefService* local_state,
     bool blocked_ui_mode,
-    bool started,
+    const std::string& started_date,
     std::unique_ptr<SnapshotInfo> last,
     std::unique_ptr<SnapshotInfo> previous)
     : local_state_(local_state),
       blocked_ui_mode_(blocked_ui_mode),
-      started_(started),
+      started_date_(started_date),
       last_(std::move(last)),
       previous_(std::move(previous)) {
   DCHECK(local_state_);
 }
 
-// static
-ArcDataSnapshotdManager* ArcDataSnapshotdManager::Get() {
-  return g_arc_data_snapshotd_manager;
-}
-
-ArcDataSnapshotdManager::ArcDataSnapshotdManager(
-    PrefService* local_state,
-    base::OnceClosure attempt_user_exit_callback)
-    : snapshot_{local_state},
-      attempt_user_exit_callback_(std::move(attempt_user_exit_callback)) {
-  DCHECK(!g_arc_data_snapshotd_manager);
+ArcDataSnapshotdManager::ArcDataSnapshotdManager(PrefService* local_state)
+    : snapshot_{local_state} {
   DCHECK(local_state);
-  g_arc_data_snapshotd_manager = this;
-
   snapshot_.Parse();
 
   if (IsRestoredSession()) {
@@ -262,11 +237,6 @@ ArcDataSnapshotdManager::ArcDataSnapshotdManager(
 }
 
 ArcDataSnapshotdManager::~ArcDataSnapshotdManager() {
-  DCHECK(g_arc_data_snapshotd_manager);
-  g_arc_data_snapshotd_manager = nullptr;
-
-  session_manager::SessionManager::Get()->RemoveObserver(this);
-
   snapshot_.Sync();
   EnsureDaemonStopped(base::DoNothing());
 }
@@ -291,37 +261,6 @@ void ArcDataSnapshotdManager::EnsureDaemonStopped(base::OnceClosure callback) {
   StopDaemon(std::move(callback));
 }
 
-bool ArcDataSnapshotdManager::IsAutoLoginConfigured() {
-  switch (state_) {
-    case ArcDataSnapshotdManager::State::kBlockedUi:
-    case ArcDataSnapshotdManager::State::kMgsToLaunch:
-    case ArcDataSnapshotdManager::State::kMgsLaunched:
-      return true;
-    case ArcDataSnapshotdManager::State::kNone:
-    case ArcDataSnapshotdManager::State::kRestored:
-      return false;
-  }
-}
-
-bool ArcDataSnapshotdManager::IsAutoLoginAllowed() {
-  switch (state_) {
-    case ArcDataSnapshotdManager::State::kBlockedUi:
-      return false;
-    case ArcDataSnapshotdManager::State::kNone:
-    case ArcDataSnapshotdManager::State::kRestored:
-    case ArcDataSnapshotdManager::State::kMgsLaunched:
-    case ArcDataSnapshotdManager::State::kMgsToLaunch:
-      return true;
-  }
-}
-
-void ArcDataSnapshotdManager::OnSessionStateChanged() {
-  if (state_ != State::kMgsToLaunch)
-    return;
-  if (user_manager::UserManager::Get()->IsLoggedInAsPublicAccount())
-    state_ = State::kMgsLaunched;
-}
-
 void ArcDataSnapshotdManager::StopDaemon(base::OnceClosure callback) {
   VLOG(1) << "Stopping arc-data-snapshotd";
   daemon_weak_ptr_factory_.InvalidateWeakPtrs();
@@ -392,22 +331,10 @@ void ArcDataSnapshotdManager::OnSnapshotsCleared(bool success) {
 
 void ArcDataSnapshotdManager::OnKeyPairGenerated(bool success) {
   if (success) {
-    VLOG(1) << "Managed Guest Session is ready to be started with blocked UI.";
     state_ = State::kMgsToLaunch;
-    session_manager::SessionManager::Get()->AddObserver(this);
-    // Move last to previous snapshot:
-    snapshot_.StartNewSnapshot();
-
-    if (!reset_autologin_callback_.is_null())
-      std::move(reset_autologin_callback_).Run();
   } else {
+    // TODO(pbond): restart browser to normal.
     LOG(ERROR) << "Key pair generation failed. Abort snapshot creation.";
-
-    snapshot_.set_blocked_ui_mode(false);
-    snapshot_.Sync();
-
-    DCHECK(!attempt_user_exit_callback_.is_null());
-    EnsureDaemonStopped(std::move(attempt_user_exit_callback_));
   }
 }
 

components/arc/enterprise/arc_data_snapshotd_manager.h

@@ -10,7 +10,6 @@
 
 #include "base/callback.h"
 #include "base/memory/weak_ptr.h"
-#include "components/session_manager/core/session_manager_observer.h"
 
 class PrefService;
 
@@ -27,8 +26,7 @@ class ArcDataSnapshotdBridge;
 
 // This class manages ARC data/ directory snapshots and controls the lifetime of
 // the arc-data-snapshotd daemon.
-class ArcDataSnapshotdManager final
-    : public session_manager::SessionManagerObserver {
+class ArcDataSnapshotdManager final {
  public:
   // State of the flow.
   enum class State {
@@ -111,7 +109,7 @@ class ArcDataSnapshotdManager final
     static std::unique_ptr<Snapshot> CreateForTesting(
         PrefService* local_state,
         bool blocked_ui_mode,
-        bool started,
+        const std::string& started_date,
         std::unique_ptr<SnapshotInfo> last,
         std::unique_ptr<SnapshotInfo> previous);
 
@@ -123,22 +121,14 @@ class ArcDataSnapshotdManager final
     // if |last| is true or previous otherwise.
     void ClearSnapshot(bool last);
 
-    // Moves last snapshot to previous and updates a |start_date| to the current
-    // date.
-    void StartNewSnapshot();
-
-    void set_blocked_ui_mode(bool blocked_ui_mode) {
-      blocked_ui_mode_ = blocked_ui_mode;
-    }
     bool is_blocked_ui_mode() const { return blocked_ui_mode_; }
-    bool started() const { return started_; }
     SnapshotInfo* last() { return last_.get(); }
     SnapshotInfo* previous() { return previous_.get(); }
 
    private:
     Snapshot(PrefService* local_state,
              bool blocked_ui_mode,
-             bool started,
+             const std::string& started_date,
              std::unique_ptr<SnapshotInfo> last,
              std::unique_ptr<SnapshotInfo> previous);
 
@@ -148,49 +138,32 @@ class ArcDataSnapshotdManager final
     // Values should be kept in sync with values stored in arc.snapshot
     // preference.
     bool blocked_ui_mode_ = false;
-    bool started_;
+    std::string started_date_;
     std::unique_ptr<SnapshotInfo> last_;
     std::unique_ptr<SnapshotInfo> previous_;
   };
 
-  ArcDataSnapshotdManager(PrefService* local_state,
-                          base::OnceClosure attempt_user_exit_callback);
+  explicit ArcDataSnapshotdManager(PrefService* local_state);
   ArcDataSnapshotdManager(const ArcDataSnapshotdManager&) = delete;
   ArcDataSnapshotdManager& operator=(const ArcDataSnapshotdManager&) = delete;
-  ~ArcDataSnapshotdManager() override;
-
-  static ArcDataSnapshotdManager* Get();
+  ~ArcDataSnapshotdManager();
 
   // Starts arc-data-snapshotd.
   void EnsureDaemonStarted(base::OnceClosure callback);
   // Stops arc-data-snapshotd.
   void EnsureDaemonStopped(base::OnceClosure callback);
 
-  // Returns true if autologin to public account should be performed.
-  bool IsAutoLoginConfigured();
-  // Returns true if autologin is allowed to be performed and manager is not
-  // waiting for the response from arc-data-snapshotd daemon.
-  bool IsAutoLoginAllowed();
-
-  // session_manager::SessionManagerObserver:
-  void OnSessionStateChanged() override;
-
   // Get |bridge_| for testing.
   ArcDataSnapshotdBridge* bridge() { return bridge_.get(); }
 
   State state() const { return state_; }
 
-  void set_reset_autologin_callback(base::OnceClosure callback) {
-    reset_autologin_callback_ = std::move(callback);
-  }
-
   static void set_snapshot_enabled_for_testing(bool enabled) {
     is_snapshot_enabled_for_testing_ = enabled;
   }
   static bool is_snapshot_enabled_for_testing() {
     return is_snapshot_enabled_for_testing_;
   }
-  void set_state_for_testing(State state) { state_ = state; }
 
  private:
   // Attempts to arc-data-snapshotd daemon regardless of state of the class.
@@ -228,11 +201,6 @@ class ArcDataSnapshotdManager final
 
   std::unique_ptr<ArcDataSnapshotdBridge> bridge_;
 
-  base::OnceClosure attempt_user_exit_callback_;
-
-  // Callback to reset an autologin timer once userless MGS is ready to start.
-  base::OnceClosure reset_autologin_callback_;
-
   // Used for cancelling previously posted tasks to daemon.
   base::WeakPtrFactory<ArcDataSnapshotdManager> daemon_weak_ptr_factory_{this};
   // WeakPtrFactory to use for callbacks.

components/arc/enterprise/arc_data_snapshotd_manager_unittest.cc

@@ -9,7 +9,6 @@
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/memory/ptr_util.h"
-#include "base/test/bind_test_util.h"
 #include "base/test/task_environment.h"
 #include "chromeos/constants/chromeos_switches.h"
 #include "chromeos/dbus/arc/fake_arc_data_snapshotd_client.h"
@@ -18,7 +17,6 @@
 #include "components/arc/arc_prefs.h"
 #include "components/arc/enterprise/arc_data_snapshotd_bridge.h"
 #include "components/prefs/testing_pref_service.h"
-#include "components/session_manager/core/session_manager.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/ozone/public/ozone_switches.h"
@@ -105,7 +103,6 @@ class ArcDataSnapshotdManagerBasicTest : public testing::Test {
  private:
   TestingPrefServiceSimple local_state_;
   std::unique_ptr<TestUpstartClient> upstart_client_;
-  session_manager::SessionManager session_manager_;
 };
 
 // Tests flows in ArcDataSnapshotdManager:
@@ -128,9 +125,7 @@ class ArcDataSnapshotdManagerFlowTest
   }
 
   // Check number of snapshots in local_state.
-  void CheckSnapshots(int expected_snapshots_number,
-                      bool expected_blocked_ui = true,
-                      bool expected_snapshot_started = false) {
+  void CheckSnapshotsNumber(int expected_number) {
     ArcDataSnapshotdManager::Snapshot snapshot(local_state());
     snapshot.Parse();
     int actual_number = 0;
@@ -140,14 +135,12 @@ class ArcDataSnapshotdManagerFlowTest
     if (snapshot.last()) {
       actual_number++;
     }
-    EXPECT_EQ(expected_snapshots_number, actual_number);
-    EXPECT_EQ(expected_blocked_ui, snapshot.is_blocked_ui_mode());
-    EXPECT_EQ(expected_snapshot_started, snapshot.started());
+    EXPECT_EQ(expected_number, actual_number);
   }
 
   // Set up local_state with info for previous and last snapshots and blocked ui
   // mode.
-  void SetupLocalState(bool blocked_ui_mode) {
+  void SetupLocalState() {
     auto last = ArcDataSnapshotdManager::SnapshotInfo::CreateForTesting(
         "" /* os_version */, "" /* creation_date */, false /* verified */,
         false /* updated */, true /* last */);
@@ -155,16 +148,11 @@ class ArcDataSnapshotdManagerFlowTest
         "" /* os_version */, "" /* creation_date */, false /* verified */,
         false /* updated */, false /* last */);
     auto snapshot = ArcDataSnapshotdManager::Snapshot::CreateForTesting(
-        local_state(), blocked_ui_mode, false /* started */, std::move(last),
-        std::move(previous));
+        local_state(), true /* blocked_ui_mode */, "" /* started_date */,
+        std::move(last), std::move(previous));
     snapshot->Sync();
   }
 
-  void TearDownLocalState() {
-    ArcDataSnapshotdManager::Snapshot snapshot(local_state());
-    snapshot.Sync();
-  }
-
   void RunUntilIdle() {
     if (is_dbus_client_available()) {
       task_environment_.RunUntilIdle();
@@ -182,9 +170,7 @@ class ArcDataSnapshotdManagerFlowTest
 
 // Test basic scenario: start / stop arc-data-snapshotd.
 TEST_F(ArcDataSnapshotdManagerBasicTest, Basic) {
-  // Daemon stopped in ctor, since no need to be running.
-  ExpectStopDaemon(false /* success */);
-  ArcDataSnapshotdManager manager(local_state(), base::DoNothing());
+  ArcDataSnapshotdManager manager(local_state());
   EXPECT_EQ(manager.state(), ArcDataSnapshotdManager::State::kNone);
   EXPECT_FALSE(manager.bridge());
 
@@ -200,9 +186,7 @@ TEST_F(ArcDataSnapshotdManagerBasicTest, Basic) {
 // Test a double start scenario: start arc-data-snapshotd twice.
 // Upstart job returns "false" if the job is already running.
 TEST_F(ArcDataSnapshotdManagerBasicTest, DoubleStart) {
-  // Daemon stopped in ctor, since no need to be running.
-  ExpectStopDaemon(false /* success */);
-  ArcDataSnapshotdManager manager(local_state(), base::DoNothing());
+  ArcDataSnapshotdManager manager(local_state());
   EXPECT_EQ(manager.state(), ArcDataSnapshotdManager::State::kNone);
   EXPECT_FALSE(manager.bridge());
 
@@ -224,11 +208,7 @@ TEST_F(ArcDataSnapshotdManagerBasicTest, DoubleStart) {
 // Test that arc-data-snapshotd daemon is already stopped when |manager| tries
 // to stop it.
 TEST_F(ArcDataSnapshotdManagerBasicTest, UpstartFailures) {
-  // Daemon stopped in ctor, since no need to be running.
-  ExpectStopDaemon(false /* success */);
-
-  ArcDataSnapshotdManager manager(local_state(), base::DoNothing());
-
+  ArcDataSnapshotdManager manager(local_state());
   EXPECT_EQ(manager.state(), ArcDataSnapshotdManager::State::kNone);
   EXPECT_FALSE(manager.bridge());
 
@@ -245,11 +225,8 @@ TEST_F(ArcDataSnapshotdManagerBasicTest, RestoredAfterCrash) {
   SetUpRestoredSessionCommandLine();
   // The attempt to stop the daemon, started before crash.
   ExpectStopDaemon(true /*success */);
-  ArcDataSnapshotdManager manager(local_state(), base::DoNothing());
+  ArcDataSnapshotdManager manager(local_state());
   EXPECT_EQ(manager.state(), ArcDataSnapshotdManager::State::kRestored);
-  EXPECT_FALSE(manager.IsAutoLoginConfigured());
-  EXPECT_TRUE(manager.IsAutoLoginAllowed());
-
   EXPECT_FALSE(manager.bridge());
 
   ExpectStartDaemon(true /*success */);
@@ -262,9 +239,8 @@ TEST_F(ArcDataSnapshotdManagerBasicTest, RestoredAfterCrash) {
 // Test clear snapshots flow.
 TEST_P(ArcDataSnapshotdManagerFlowTest, ClearSnapshotsBasic) {
   // Set up two snapshots (previous and last) in local_state.
-  SetupLocalState(false /* blocked_ui_mode */);
-  CheckSnapshots(2 /* expected_snapshots_number */,
-                 false /* expected_blocked_ui_mode */);
+  SetupLocalState();
+  CheckSnapshotsNumber(2 /* expected_number */);
 
   // Once |manager| is created, it tries to clear both snapshots, because the
   // mechanism is disabled by default, and stop the daemon.
@@ -272,25 +248,21 @@ TEST_P(ArcDataSnapshotdManagerFlowTest, ClearSnapshotsBasic) {
   ExpectStartDaemon(true /*success */);
   // Stop once finished clearing.
   ExpectStopDaemon(true /*success */);
-  ArcDataSnapshotdManager manager(local_state(), base::DoNothing());
+  ArcDataSnapshotdManager manager(local_state());
   RunUntilIdle();
 
   // No snapshots in local_state either.
   EXPECT_EQ(manager.state(), ArcDataSnapshotdManager::State::kNone);
-  EXPECT_FALSE(manager.IsAutoLoginConfigured());
-  EXPECT_TRUE(manager.IsAutoLoginAllowed());
-  CheckSnapshots(0 /* expected_snapshots_number */,
-                 false /* expected_blocked_ui_mode */);
+  CheckSnapshotsNumber(0 /* expected_number */);
 
   EXPECT_FALSE(manager.bridge());
-  TearDownLocalState();
 }
 
 // Test blocked UI mode flow.
 TEST_P(ArcDataSnapshotdManagerFlowTest, BlockedUiBasic) {
   // Set up two snapshots (previous and last) in local_state.
-  SetupLocalState(true /* blocked_ui_mode */);
-  CheckSnapshots(2 /* expected_snapshots_number */);
+  SetupLocalState();
+  CheckSnapshotsNumber(2 /* expected_number */);
   // Enable snapshotting mechanism for testing.
   ArcDataSnapshotdManager::set_snapshot_enabled_for_testing(true /* enabled */);
 
@@ -300,39 +272,21 @@ TEST_P(ArcDataSnapshotdManagerFlowTest, BlockedUiBasic) {
   ExpectStartDaemon(true /*success */);
   // Stop once finished clearing.
   ExpectStopDaemon(true /*success */);
-  bool is_attempt_user_exit_called = false;
-  ArcDataSnapshotdManager manager(
-      local_state(),
-      base::BindLambdaForTesting([&is_attempt_user_exit_called]() {
-        is_attempt_user_exit_called = true;
-      }));
+  ArcDataSnapshotdManager manager(local_state());
   CheckHeadlessMode();
   EXPECT_EQ(manager.state(), ArcDataSnapshotdManager::State::kBlockedUi);
-  EXPECT_TRUE(manager.IsAutoLoginConfigured());
-  EXPECT_FALSE(manager.IsAutoLoginAllowed());
-
   RunUntilIdle();
 
-  if (is_dbus_client_available()) {
-    EXPECT_FALSE(is_attempt_user_exit_called);
-    EXPECT_EQ(manager.state(), ArcDataSnapshotdManager::State::kMgsToLaunch);
-    EXPECT_TRUE(manager.IsAutoLoginConfigured());
-    EXPECT_TRUE(manager.IsAutoLoginAllowed());
-
-    EXPECT_TRUE(manager.bridge());
-    // Starts a last snapshot creation. last became previous.
-    CheckSnapshots(1 /* expected_snapshots_number */,
-                   true /*expected_blocked_ui */,
-                   true /* expected_snapshot_started */);
-  } else {
-    EXPECT_TRUE(is_attempt_user_exit_called);
-    EXPECT_EQ(manager.state(), ArcDataSnapshotdManager::State::kBlockedUi);
-    EXPECT_FALSE(manager.bridge());
-    // Snapshots are valid. No need to clear.
-    CheckSnapshots(2 /* expected_snapshots_number */,
-                   false /* expected_blocked_ui */);
-  }
-  TearDownLocalState();
+  // Snapshots are valid, no need to clear.
+  CheckSnapshotsNumber(2 /* expected_number */);
+
+  auto expected_state = is_dbus_client_available()
+                            ? ArcDataSnapshotdManager::State::kMgsToLaunch
+                            : ArcDataSnapshotdManager::State::kBlockedUi;
+
+  // The communication is established and MGS can be launched.
+  EXPECT_EQ(manager.state(), expected_state);
+  EXPECT_TRUE(manager.bridge());
 }
 
 INSTANTIATE_TEST_SUITE_P(ArcDataSnapshotdManagerFlowTest,