Revert "[ProfilePicker] Enable picker on startup only for active users"

This reverts commit 82afac04.

Reason for revert: This CL broke StartupBrowserCreatorPickerTest.TestSetup.
 
Bug: 1163675

Original change's description:
> [ProfilePicker] Enable picker on startup only for active users
>
> Keep the picker disabled on startup until:
> - multiple profiles were used in the last 28 days, or
> - the picker has been shown once
>
> A new prefs::kBrowserProfilePickerShown pref is added to track whether
> the picker has been shown before.
>
> Bug: 1063856
> Change-Id: Ia35a3b2212b3811ed592d81fa516bc1ffc07accc
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2598848
> Commit-Queue: Alex Ilin <alexilin@chromium.org>
> Reviewed-by: David Roger <droger@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#840598}

TBR=droger@chromium.org,alexilin@chromium.org,chromium-scoped@luci-project-accounts.iam.gserviceaccount.com

Change-Id: I306f0b7fa225b9d4043e948ec4c7a15e82ed9859
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1063856
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2612309Reviewed-by: Tsuyoshi Horo <horo@chromium.org>
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#840869}

chrome/browser/profiles/profiles_state.cc

@@ -83,7 +83,6 @@ void RegisterPrefs(PrefRegistrySimple* registry) {
   registry->RegisterIntegerPref(
       prefs::kBrowserProfilePickerAvailabilityOnStartup,
       static_cast<int>(ProfilePicker::AvailabilityOnStartup::kEnabled));
-  registry->RegisterBooleanPref(prefs::kBrowserProfilePickerShown, false);
 }
 
 void SetLastUsedProfile(const std::string& profile_dir) {

chrome/browser/ui/profile_picker.cc

@@ -4,14 +4,11 @@
 
 #include "chrome/browser/ui/profile_picker.h"
 
-#include <algorithm>
 #include <string>
 
 #include "base/feature_list.h"
 #include "base/metrics/histogram_functions.h"
-#include "base/time/time.h"
 #include "chrome/browser/browser_process.h"
-#include "chrome/browser/profiles/profile_attributes_entry.h"
 #include "chrome/browser/profiles/profile_attributes_storage.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/profiles/profiles_state.h"
@@ -22,16 +19,6 @@
 
 namespace {
 
-constexpr base::TimeDelta kActiveTimeThreshold = base::TimeDelta::FromDays(28);
-
-// Returns a pref value indicating whether the profile picker has been shown to
-// the user before.
-bool ProfilePickerShown() {
-  PrefService* prefs = g_browser_process->local_state();
-  DCHECK(prefs);
-  return prefs->GetBoolean(prefs::kBrowserProfilePickerShown);
-}
-
 ProfilePicker::AvailabilityOnStartup GetAvailabilityOnStartup() {
   int availability_on_startup = g_browser_process->local_state()->GetInteger(
       prefs::kBrowserProfilePickerAvailabilityOnStartup);
@@ -63,28 +50,13 @@ bool ProfilePicker::ShouldShowAtLaunch() {
   if (availability_on_startup == AvailabilityOnStartup::kForced)
     return true;
 
-  ProfileManager* profile_manager = g_browser_process->profile_manager();
-
-  size_t number_of_profiles = profile_manager->GetNumberOfProfiles();
+  size_t number_of_profiles = g_browser_process->profile_manager()
+                                  ->GetProfileAttributesStorage()
+                                  .GetNumberOfProfiles();
   // Need to consider 0 profiles as this is what happens in some browser-tests.
   if (number_of_profiles <= 1)
     return false;
 
-  std::vector<ProfileAttributesEntry*> profile_attributes =
-      profile_manager->GetProfileAttributesStorage().GetAllProfilesAttributes();
-  int number_of_active_profiles =
-      std::count_if(profile_attributes.begin(), profile_attributes.end(),
-                    [](ProfileAttributesEntry* entry) {
-                      return (base::Time::Now() - entry->GetActiveTime() <
-                              kActiveTimeThreshold) &&
-                             !entry->IsGuest();
-                    });
-  // Don't show the profile picker at launch if the user has less than two
-  // active profiles. However, if the user has already seen the profile picker
-  // before, respect user's preference.
-  if (number_of_active_profiles < 2 && !ProfilePickerShown())
-    return false;
-
   bool pref_enabled = g_browser_process->local_state()->GetBoolean(
       prefs::kBrowserShowProfilePickerOnStartup);
   base::UmaHistogramBoolean("ProfilePicker.AskOnStartup", pref_enabled);

chrome/browser/ui/profile_picker_unittest.cc

-// Copyright 2020 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 "chrome/browser/ui/profile_picker.h"
-#include "base/test/scoped_feature_list.h"
-#include "base/test/task_environment.h"
-#include "base/time/time.h"
-#include "chrome/browser/profiles/profile_attributes_entry.h"
-#include "chrome/browser/profiles/profile_attributes_storage.h"
-#include "chrome/browser/profiles/profile_manager.h"
-#include "chrome/browser/ui/ui_features.h"
-#include "chrome/common/chrome_features.h"
-#include "chrome/common/pref_names.h"
-#include "chrome/test/base/testing_browser_process.h"
-#include "chrome/test/base/testing_profile_manager.h"
-#include "components/prefs/pref_service.h"
-#include "content/public/test/browser_task_environment.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-class ProfilePickerTest : public testing::Test {
- public:
-  ProfilePickerTest()
-      : testing_profile_manager_(TestingBrowserProcess::GetGlobal()) {
-    feature_list_.InitAndEnableFeature(features::kNewProfilePicker);
-  }
-
-  void SetUp() override { ASSERT_TRUE(testing_profile_manager_.SetUp()); }
-
-  ProfileAttributesEntry* GetProfileAttributes(Profile* profile) {
-    ProfileAttributesEntry* entry = nullptr;
-    testing_profile_manager()
-        ->profile_attributes_storage()
-        ->GetProfileAttributesWithPath(profile->GetPath(), &entry);
-    return entry;
-  }
-
-  base::test::TaskEnvironment* task_environment() { return &task_environment_; }
-
-  TestingProfileManager* testing_profile_manager() {
-    return &testing_profile_manager_;
-  }
-
-  PrefService* local_state() {
-    return testing_profile_manager()->local_state()->Get();
-  }
-
- private:
-  content::BrowserTaskEnvironment task_environment_{
-      base::test::TaskEnvironment::TimeSource::MOCK_TIME};
-  TestingProfileManager testing_profile_manager_;
-  base::test::ScopedFeatureList feature_list_;
-};
-
-TEST_F(ProfilePickerTest, ShouldShowAtLaunch_MultipleProfiles_TwoActive) {
-  TestingProfile* profile1 =
-      testing_profile_manager()->CreateTestingProfile("profile1");
-  GetProfileAttributes(profile1)->SetActiveTimeToNow();
-  TestingProfile* profile2 =
-      testing_profile_manager()->CreateTestingProfile("profile2");
-  GetProfileAttributes(profile2)->SetActiveTimeToNow();
-
-  EXPECT_TRUE(ProfilePicker::ShouldShowAtLaunch());
-
-  // Should be within the activity time threshold.
-  task_environment()->FastForwardBy(base::TimeDelta::FromDays(27));
-  EXPECT_TRUE(ProfilePicker::ShouldShowAtLaunch());
-}
-
-TEST_F(ProfilePickerTest,
-       ShouldShowAtLaunch_MultipleProfiles_Inactive_SeenPicker) {
-  testing_profile_manager()->CreateTestingProfile("profile1");
-  testing_profile_manager()->CreateTestingProfile("profile2");
-  local_state()->SetBoolean(prefs::kBrowserProfilePickerShown, true);
-
-  EXPECT_TRUE(ProfilePicker::ShouldShowAtLaunch());
-}
-
-TEST_F(ProfilePickerTest, ShouldShowAtLaunch_MultipleProfiles_OneGuest) {
-  TestingProfile* profile1 =
-      testing_profile_manager()->CreateTestingProfile("profile1");
-  GetProfileAttributes(profile1)->SetActiveTimeToNow();
-  testing_profile_manager()->CreateTestingProfile("profile2");
-  testing_profile_manager()->CreateGuestProfile();
-
-  EXPECT_FALSE(ProfilePicker::ShouldShowAtLaunch());
-}
-
-TEST_F(ProfilePickerTest,
-       ShouldShowAtLaunch_MultipleProfiles_TwoActive_Disabled) {
-  TestingProfile* profile1 =
-      testing_profile_manager()->CreateTestingProfile("profile1");
-  GetProfileAttributes(profile1)->SetActiveTimeToNow();
-  TestingProfile* profile2 =
-      testing_profile_manager()->CreateTestingProfile("profile2");
-  GetProfileAttributes(profile2)->SetActiveTimeToNow();
-  local_state()->SetBoolean(prefs::kBrowserShowProfilePickerOnStartup, false);
-
-  EXPECT_FALSE(ProfilePicker::ShouldShowAtLaunch());
-}
-
-TEST_F(ProfilePickerTest, ShouldShowAtLaunch_MultipleProfiles_Inactive) {
-  testing_profile_manager()->CreateTestingProfile("profile1");
-  testing_profile_manager()->CreateTestingProfile("profile2");
-
-  EXPECT_FALSE(ProfilePicker::ShouldShowAtLaunch());
-}
-
-TEST_F(ProfilePickerTest, ShouldShowAtLaunch_MultipleProfiles_Expired) {
-  TestingProfile* profile1 =
-      testing_profile_manager()->CreateTestingProfile("profile1");
-  GetProfileAttributes(profile1)->SetActiveTimeToNow();
-  TestingProfile* profile2 =
-      testing_profile_manager()->CreateTestingProfile("profile2");
-  GetProfileAttributes(profile2)->SetActiveTimeToNow();
-  // Should be outside of the activity time threshold.
-  task_environment()->FastForwardBy(base::TimeDelta::FromDays(29));
-
-  EXPECT_FALSE(ProfilePicker::ShouldShowAtLaunch());
-}
-
-TEST_F(ProfilePickerTest, ShouldShowAtLaunch_MultipleProfiles_OneActive) {
-  TestingProfile* profile1 =
-      testing_profile_manager()->CreateTestingProfile("profile1");
-  GetProfileAttributes(profile1)->SetActiveTimeToNow();
-  testing_profile_manager()->CreateTestingProfile("profile2");
-
-  EXPECT_FALSE(ProfilePicker::ShouldShowAtLaunch());
-}
-
-TEST_F(ProfilePickerTest, ShouldShowAtLaunch_SingleProfile) {
-  testing_profile_manager()->CreateTestingProfile("profile1");
-  local_state()->SetBoolean(prefs::kBrowserProfilePickerShown, true);
-
-  EXPECT_FALSE(ProfilePicker::ShouldShowAtLaunch());
-}
-
-class ProfilePickerTestEphemeralGuest : public ProfilePickerTest {
- public:
-  ProfilePickerTestEphemeralGuest() {
-    feature_list_.InitAndEnableFeature(
-        features::kEnableEphemeralGuestProfilesOnDesktop);
-  }
-
- private:
-  base::test::ScopedFeatureList feature_list_;
-};
-
-TEST_F(ProfilePickerTestEphemeralGuest,
-       ShouldShowAtLaunch_MultipleProfiles_OneGuest) {
-  TestingProfile* profile1 =
-      testing_profile_manager()->CreateTestingProfile("profile1");
-  GetProfileAttributes(profile1)->SetActiveTimeToNow();
-  testing_profile_manager()->CreateTestingProfile("profile2");
-  TestingProfile* guest_profile =
-      testing_profile_manager()->CreateGuestProfile();
-  GetProfileAttributes(guest_profile)->SetActiveTimeToNow();
-
-  EXPECT_FALSE(ProfilePicker::ShouldShowAtLaunch());
-}
-
-TEST_F(ProfilePickerTestEphemeralGuest,
-       ShouldShowAtLaunch_MultipleProfiles_OneGuest_SeenPicker) {
-  testing_profile_manager()->CreateTestingProfile("profile1");
-  testing_profile_manager()->CreateGuestProfile();
-  local_state()->SetBoolean(prefs::kBrowserProfilePickerShown, true);
-
-  EXPECT_FALSE(ProfilePicker::ShouldShowAtLaunch());
-}

chrome/browser/ui/views/profiles/profile_picker_view.cc

@@ -44,7 +44,6 @@
 #include "chrome/grit/chromium_strings.h"
 #include "chrome/grit/google_chrome_strings.h"
 #include "components/keep_alive_registry/keep_alive_types.h"
-#include "components/prefs/pref_service.h"
 #include "components/signin/public/identity_manager/account_info.h"
 #include "components/startup_metric_utils/browser/startup_metric_utils.h"
 #include "content/public/browser/browser_context.h"
@@ -324,9 +323,6 @@ void ProfilePickerView::Init(ProfilePicker::EntryPoint entry_point,
   GetWidget()->Show();
   state_ = kReady;
 
-  PrefService* prefs = g_browser_process->local_state();
-  prefs->SetBoolean(prefs::kBrowserProfilePickerShown, true);
-
   if (entry_point == ProfilePicker::EntryPoint::kOnStartup) {
     DCHECK(!creation_time_on_startup_.is_null());
     base::UmaHistogramTimes("ProfilePicker.StartupTime.WebViewCreated",

chrome/common/pref_names.cc

@@ -2703,9 +2703,6 @@ const char kForceBrowserSignin[] = "profile.force_browser_signin";
 const char kBrowserProfilePickerAvailabilityOnStartup[] =
     "profile.picker_availability_on_startup";
 
-// Whether the profile picker has been shown at least once.
-const char kBrowserProfilePickerShown[] = "profile.picker_shown";
-
 // Whether to show the profile picker on startup or not.
 const char kBrowserShowProfilePickerOnStartup[] =
     "profile.show_picker_on_startup";

chrome/common/pref_names.h

@@ -903,7 +903,6 @@ extern const char kBrowserGuestModeEnforced[];
 extern const char kBrowserAddPersonEnabled[];
 extern const char kForceBrowserSignin[];
 extern const char kBrowserProfilePickerAvailabilityOnStartup[];
-extern const char kBrowserProfilePickerShown[];
 extern const char kBrowserShowProfilePickerOnStartup[];
 extern const char kSigninAllowedOnNextStartup[];
 extern const char kSigninInterceptionEnabled[];

chrome/test/BUILD.gn

@@ -4314,10 +4314,6 @@ test("unit_tests") {
     ]
   }
 
-  if (is_win || is_mac || (is_linux && !is_chromeos_lacros)) {
-    sources += [ "../browser/ui/profile_picker_unittest.cc" ]
-  }
-
   if (enable_offline_pages) {
     sources += [
       "../browser/offline_pages/background_loader_offliner_unittest.cc",