Revert "Remove all legacy supervised users on startup."

This reverts commit 0dd5cc0a.

Reason for revert: RemoveSupervisedUsersBrowserEnabledTest.SupervisedUserRemoved has a use-after-free
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29/31266

Original change's description:
> Remove all legacy supervised users on startup.
> 
> The flag is: RemoveSupervisedUsersOnStartup.
> It is disabled by default.
> 
> The tests in this CL verify that when the feature is disabled, no users
> are removed, and when the feature is enabled, *only* the Supervised
> users are removed.
> 
> Bug: 916685
> Change-Id: I32882e786f457a27368724415b65e6a428e4b70b
> Reviewed-on: https://chromium-review.googlesource.com/c/1384934
> Reviewed-by: Michael Giuffrida <michaelpg@chromium.org>
> Reviewed-by: Jacob Dufault <jdufault@chromium.org>
> Commit-Queue: Danan S <danan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#626865}

TBR=michaelpg@chromium.org,jdufault@chromium.org,danan@chromium.org

Change-Id: I50a08e80981cdb609d0800f5f3ef1974a04e883b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 916685
Reviewed-on: https://chromium-review.googlesource.com/c/1443033Reviewed-by: Kevin McNee <mcnee@chromium.org>
Commit-Queue: Kevin McNee <mcnee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#627016}

chrome/browser/chromeos/login/users/chrome_user_manager_impl.cc

@@ -9,7 +9,6 @@
 #include <cstddef>
 #include <set>
 #include <utility>
-#include <vector>
 
 #include "ash/public/cpp/ash_pref_names.h"
 #include "ash/public/interfaces/constants.mojom.h"
@@ -848,23 +847,9 @@ void ChromeUserManagerImpl::PerformPreUserListLoadingActions() {
 }
 
 void ChromeUserManagerImpl::PerformPostUserListLoadingActions() {
-  std::vector<user_manager::User*> users_to_remove;
-
-  for (user_manager::User* user : users_) {
-    // TODO(http://crbug/866790): Remove supervised user accounts. After we have
-    // enough confidence that there are no more supervised users on devices in
-    // the wild, remove this.
-    if (base::FeatureList::IsEnabled(
-            features::kRemoveSupervisedUsersOnStartup) &&
-        user->IsSupervised()) {
-      users_to_remove.push_back(user);
-    } else {
-      GetUserImageManager(user->GetAccountId())->LoadUserImage();
-    }
-  }
-
-  for (user_manager::User* user : users_to_remove) {
-    RemoveUser(user->GetAccountId(), nullptr);
+  for (user_manager::UserList::iterator ui = users_.begin(), ue = users_.end();
+       ui != ue; ++ui) {
+    GetUserImageManager((*ui)->GetAccountId())->LoadUserImage();
   }
 }
 

chrome/browser/chromeos/login/users/remove_supervised_users_browsertest.cc

-// Copyright 2018 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include <memory>
-#include <vector>
-
-#include "base/macros.h"
-#include "base/test/scoped_feature_list.h"
-#include "chrome/browser/chromeos/login/login_manager_test.h"
-#include "chrome/browser/chromeos/login/users/chrome_user_manager.h"
-#include "chrome/browser/chromeos/login/users/chrome_user_manager_impl.h"
-#include "chrome/browser/chromeos/settings/cros_settings.h"
-#include "chrome/browser/chromeos/settings/scoped_testing_cros_settings.h"
-#include "chrome/common/chrome_features.h"
-#include "components/account_id/account_id.h"
-#include "components/user_manager/scoped_user_manager.h"
-#include "components/user_manager/user_manager.h"
-#include "components/user_manager/user_manager_base.h"
-
-namespace chromeos {
-
-namespace {
-
-struct {
-  const char* email;
-  const char* gaia_id;
-} const kTestUsers[] = {
-    {"test-user1@gmail.com", "1111111111"},
-    {"test-user2@gmail.com", "2222222222"},
-    // Test Supervised User.
-    // The domain is defined in user_manager::kSupervisedUserDomain.
-    // That const isn't directly referenced here to keep this code readable by
-    // avoiding std::string concatenations.
-    {"test-superviseduser@locally-managed.localhost", "3333333333"},
-};
-
-}  // namespace
-
-class RemoveSupervisedUsersBrowserTest : public LoginManagerTest {
- public:
-  RemoveSupervisedUsersBrowserTest() : LoginManagerTest(false, false) {}
-
-  ~RemoveSupervisedUsersBrowserTest() override = default;
-
-  void SetUpOnMainThread() override {
-    LoginManagerTest::SetUpOnMainThread();
-    InitializeTestUsers();
-  }
-
-  void TearDownOnMainThread() override {
-    LoginManagerTest::TearDownOnMainThread();
-    scoped_user_manager_.reset();
-  }
-
- protected:
-  std::vector<AccountId> test_users_;
-
- private:
-  void InitializeTestUsers() {
-    for (size_t i = 0; i < base::size(kTestUsers); ++i) {
-      test_users_.emplace_back(AccountId::FromUserEmailGaiaId(
-          kTestUsers[i].email, kTestUsers[i].gaia_id));
-      RegisterUser(test_users_[i]);
-    }
-
-    // Setup a scoped real ChromeUserManager, since this is an end-to-end
-    // test.  This will read the users registered to the local state just
-    // before this in RegisterUser.
-    scoped_user_manager_ = std::make_unique<user_manager::ScopedUserManager>(
-        chromeos::ChromeUserManagerImpl::CreateChromeUserManager());
-  }
-
-  std::unique_ptr<user_manager::ScopedUserManager> scoped_user_manager_;
-  ScopedTestingCrosSettings scoped_testing_cros_settings_;
-
-  DISALLOW_COPY_AND_ASSIGN(RemoveSupervisedUsersBrowserTest);
-};
-
-class RemoveSupervisedUsersBrowserEnabledTest
-    : public RemoveSupervisedUsersBrowserTest {
- protected:
-  void SetUpInProcessBrowserTestFixture() override {
-    scoped_feature_list_.InitAndEnableFeature(
-        features::kRemoveSupervisedUsersOnStartup);
-  }
-
- private:
-  base::test::ScopedFeatureList scoped_feature_list_;
-};
-
-class RemoveSupervisedUsersBrowserDisabledTest
-    : public RemoveSupervisedUsersBrowserTest {
- protected:
-  void SetUpInProcessBrowserTestFixture() override {
-    scoped_feature_list_.InitAndDisableFeature(
-        features::kRemoveSupervisedUsersOnStartup);
-  }
-
- private:
-  base::test::ScopedFeatureList scoped_feature_list_;
-};
-
-IN_PROC_BROWSER_TEST_F(RemoveSupervisedUsersBrowserEnabledTest,
-                       SupervisedUserRemoved) {
-  // When Supervised users are removed, only 2 test users should be returned.
-  EXPECT_EQ(2U, user_manager::UserManager::Get()->GetUsers().size());
-
-  // None of the non-supervised users should have been removed.
-  for (user_manager::User* user :
-       user_manager::UserManager::Get()->GetUsers()) {
-    EXPECT_EQ(false, user->IsSupervised());
-  }
-}
-
-IN_PROC_BROWSER_TEST_F(RemoveSupervisedUsersBrowserDisabledTest,
-                       SupervisedUserNotRemoved) {
-  // When Supervised users are not removed, all 3 test users should be returned.
-  EXPECT_EQ(3U, user_manager::UserManager::Get()->GetUsers().size());
-}
-
-}  // namespace chromeos

chrome/common/chrome_features.cc

@@ -495,12 +495,6 @@ const base::Feature kNupPrinting{"NupPrinting",
 const base::Feature kPushMessagingBackgroundMode{
     "PushMessagingBackgroundMode", base::FEATURE_DISABLED_BY_DEFAULT};
 
-#if defined(OS_CHROMEOS)
-// Enables permanent removal of Legacy Supervised Users on startup.
-const base::Feature kRemoveSupervisedUsersOnStartup{
-    "RemoveSupervisedUsersOnStartup", base::FEATURE_DISABLED_BY_DEFAULT};
-#endif
-
 const base::Feature kSafeSearchUrlReporting{"SafeSearchUrlReporting",
                                             base::FEATURE_DISABLED_BY_DEFAULT};
 

chrome/common/chrome_features.h

@@ -327,11 +327,6 @@ COMPONENT_EXPORT(CHROME_FEATURES) extern const base::Feature kNupPrinting;
 COMPONENT_EXPORT(CHROME_FEATURES)
 extern const base::Feature kPushMessagingBackgroundMode;
 
-#if defined(OS_CHROMEOS)
-COMPONENT_EXPORT(CHROME_FEATURES)
-extern const base::Feature kRemoveSupervisedUsersOnStartup;
-#endif
-
 COMPONENT_EXPORT(CHROME_FEATURES)
 extern const base::Feature kSafeSearchUrlReporting;
 

chrome/test/BUILD.gn

@@ -1787,7 +1787,6 @@ test("browser_tests") {
         "../browser/chromeos/login/users/avatar/user_image_manager_browsertest.cc",
         "../browser/chromeos/login/users/avatar/user_image_manager_test_util.cc",
         "../browser/chromeos/login/users/avatar/user_image_manager_test_util.h",
-        "../browser/chromeos/login/users/remove_supervised_users_browsertest.cc",
         "../browser/chromeos/login/users/user_manager_hide_supervised_users_browsertest.cc",
         "../browser/chromeos/login/users/wallpaper_policy_browsertest.cc",
         "../browser/chromeos/login/webview_login_browsertest.cc",