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

This reverts commit 73bb9dcf.

Reason for revert: failing test has been fixed

Original change's description:
> 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/+/2612309
> Reviewed-by: Tsuyoshi Horo <horo@chromium.org>
> Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#840869}

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

# Not skipping CQ checks because this is a reland.

Bug: 1163675
Bug: 1063856
Change-Id: Idfd5668b970ec0fcaf805ef8bb5ee52778987bf5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2613970Reviewed-by: Alex Ilin <alexilin@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Commit-Queue: Alex Ilin <alexilin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#841161}

chrome/browser/profiles/profiles_state.cc

@@ -83,6 +83,7 @@ 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,11 +4,14 @@
 
 #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"
@@ -19,6 +22,16 @@
 
 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);
@@ -50,13 +63,28 @@ bool ProfilePicker::ShouldShowAtLaunch() {
   if (availability_on_startup == AvailabilityOnStartup::kForced)
     return true;
 
-  size_t number_of_profiles = g_browser_process->profile_manager()
-                                  ->GetProfileAttributesStorage()
-                                  .GetNumberOfProfiles();
+  ProfileManager* profile_manager = g_browser_process->profile_manager();
+
+  size_t number_of_profiles = profile_manager->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/startup/startup_browser_creator_browsertest.cc

@@ -2078,12 +2078,19 @@ class StartupBrowserCreatorPickerTestBase : public InProcessBrowserTest {
     // later in the startup process and so we need to have at least 2 fake
     // profiles.
     base::ScopedAllowBlockingForTesting allow_blocking;
-    ASSERT_TRUE(
-        profile_manager->GetProfile(profile_manager->user_data_dir().Append(
-            FILE_PATH_LITERAL("New Profile 1"))));
-    ASSERT_TRUE(
-        profile_manager->GetProfile(profile_manager->user_data_dir().Append(
-            FILE_PATH_LITERAL("New Profile 2"))));
+    std::vector<base::FilePath> profile_paths = {
+        profile_manager->user_data_dir().Append(
+            FILE_PATH_LITERAL("New Profile 1")),
+        profile_manager->user_data_dir().Append(
+            FILE_PATH_LITERAL("New Profile 2"))};
+    for (const auto& profile_path : profile_paths) {
+      ASSERT_TRUE(profile_manager->GetProfile(profile_path));
+      // Mark newly created profiles as active.
+      ProfileAttributesEntry* entry = nullptr;
+      ASSERT_TRUE(profile_manager->GetProfileAttributesStorage()
+                      .GetProfileAttributesWithPath(profile_path, &entry));
+      entry->SetActiveTimeToNow();
+    }
   }
 
  private:

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

@@ -44,6 +44,7 @@
 #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"
@@ -323,6 +324,9 @@ 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,6 +2703,9 @@ 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,6 +903,7 @@ 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

@@ -4312,6 +4312,10 @@ 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",