Add IsGuest to ProfileAttributesStorage and update tests.

Guest profiles are not added to profile attribute storage since they are
only stored in memory. But we need to add ephemeral Guest profiles to
the storage (similar to ephemeral profiles) so that after a crash, they
would be found and deleted.
This CL adds IsGuest to ProfileAttributeStorage to keep track of
ephemeral Guest profiles and updates related metrics tests.

Please see go/ephemeral-guest-profiles for more context.

Bug: 1125474
Change-Id: I5f169cc7c5757368303bfbdb1abf78563692e248
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2523111
Commit-Queue: Ramin Halavati <rhalavati@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Cr-Commit-Position: refs/heads/master@{#827229}

chrome/browser/profiles/profile_activity_metrics_recorder.cc

@@ -98,6 +98,8 @@ void RecordProfilesState() {
 
 void RecordAccountMetrics(const Profile* profile) {
   DCHECK(profile);
+  if (profile->IsEphemeralGuestProfile())
+    return;
 
   ProfileAttributesEntry* entry;
   if (!g_browser_process->profile_manager()

chrome/browser/profiles/profile_activity_metrics_recorder_unittest.cc

@@ -29,11 +29,19 @@ constexpr base::TimeDelta kLongTimeOfInactivity =
 
 }  // namespace
 
-class ProfileActivityMetricsRecorderTest : public testing::Test {
+class ProfileActivityMetricsRecorderTest
+    : public testing::Test,
+      public ::testing::WithParamInterface<bool> {
  public:
   ProfileActivityMetricsRecorderTest()
       : task_environment_(base::test::TaskEnvironment::TimeSource::MOCK_TIME),
         profile_manager_(TestingBrowserProcess::GetGlobal()) {
+    is_ephemeral_ = GetParam();
+
+    // Change the value if Ephemeral is not supported.
+    is_ephemeral_ &=
+        TestingProfile::SetScopedFeatureListForEphemeralGuestProfiles(
+            scoped_feature_list_, is_ephemeral_);
     base::SetRecordActionTaskRunner(
         task_environment_.GetMainThreadTaskRunner());
   }
@@ -68,6 +76,13 @@ class ProfileActivityMetricsRecorderTest : public testing::Test {
     ActivateBrowser(profile->GetPrimaryOTRProfile());
   }
 
+  void ActivateGuestBrowser(Profile* profile) {
+    if (IsEphemeral())
+      ActivateBrowser(profile);
+    else
+      ActivateBrowser(profile->GetPrimaryOTRProfile());
+  }
+
   void SimulateUserEvent() {
     metrics::DesktopSessionDurationTracker::Get()->OnUserEvent();
   }
@@ -83,6 +98,7 @@ class ProfileActivityMetricsRecorderTest : public testing::Test {
                                  /*count=*/1);
   }
 
+  bool IsEphemeral() { return is_ephemeral_; }
   TestingProfileManager* profile_manager() { return &profile_manager_; }
   base::HistogramTester* histograms() { return &histogram_tester_; }
   content::BrowserTaskEnvironment* task_environment() {
@@ -91,7 +107,9 @@ class ProfileActivityMetricsRecorderTest : public testing::Test {
 
  private:
   content::BrowserTaskEnvironment task_environment_;
+  base::test::ScopedFeatureList scoped_feature_list_;
 
+  bool is_ephemeral_;
   TestingProfileManager profile_manager_;
   base::HistogramTester histogram_tester_;
 
@@ -100,7 +118,7 @@ class ProfileActivityMetricsRecorderTest : public testing::Test {
   DISALLOW_COPY_AND_ASSIGN(ProfileActivityMetricsRecorderTest);
 };
 
-TEST_F(ProfileActivityMetricsRecorderTest, GuestProfile) {
+TEST_P(ProfileActivityMetricsRecorderTest, GuestProfile) {
   Profile* regular_profile = profile_manager()->CreateTestingProfile("p1");
   Profile* guest_profile = profile_manager()->CreateGuestProfile();
   histograms()->ExpectTotalCount("Profile.BrowserActive.PerProfile", 0);
@@ -114,9 +132,7 @@ TEST_F(ProfileActivityMetricsRecorderTest, GuestProfile) {
   histograms()->ExpectTotalCount("Profile.NumberOfProfilesAtProfileSwitch",
                                  /*count=*/0);
 
-  // Activate an incognito browser instance of the guest profile.
-  // Note: Creating a non-incognito guest browser instance is not possible.
-  ActivateIncognitoBrowser(guest_profile);
+  ActivateGuestBrowser(guest_profile);
   histograms()->ExpectBucketCount("Profile.BrowserActive.PerProfile",
                                   /*bucket=*/0, /*count=*/1);
   SimulateUserActionAndExpectRecording(/*bucket=*/0);
@@ -126,7 +142,7 @@ TEST_F(ProfileActivityMetricsRecorderTest, GuestProfile) {
   histograms()->ExpectTotalCount("Profile.BrowserActive.PerProfile", 2);
 }
 
-TEST_F(ProfileActivityMetricsRecorderTest, IncognitoProfile) {
+TEST_P(ProfileActivityMetricsRecorderTest, IncognitoProfile) {
   Profile* regular_profile = profile_manager()->CreateTestingProfile("p1");
   histograms()->ExpectTotalCount("Profile.BrowserActive.PerProfile", 0);
 
@@ -143,7 +159,7 @@ TEST_F(ProfileActivityMetricsRecorderTest, IncognitoProfile) {
                                  /*count=*/0);
 }
 
-TEST_F(ProfileActivityMetricsRecorderTest, MultipleProfiles) {
+TEST_P(ProfileActivityMetricsRecorderTest, MultipleProfiles) {
   // Profile 1: Profile is created. This does not affect the histogram.
   Profile* profile1 = profile_manager()->CreateTestingProfile("p1");
   // Profile 2: Profile is created. This does not affect the histogram.
@@ -205,7 +221,7 @@ TEST_F(ProfileActivityMetricsRecorderTest, MultipleProfiles) {
   histograms()->ExpectTotalCount("Profile.BrowserActive.PerProfile", 4);
 }
 
-TEST_F(ProfileActivityMetricsRecorderTest, SessionInactivityNotRecorded) {
+TEST_P(ProfileActivityMetricsRecorderTest, SessionInactivityNotRecorded) {
   Profile* profile = profile_manager()->CreateTestingProfile("p1");
 
   ActivateBrowser(profile);
@@ -224,7 +240,7 @@ TEST_F(ProfileActivityMetricsRecorderTest, SessionInactivityNotRecorded) {
                                   /*bucket=*/1, /*count=*/2);
 }
 
-TEST_F(ProfileActivityMetricsRecorderTest, ProfileState) {
+TEST_P(ProfileActivityMetricsRecorderTest, ProfileState) {
   Profile* regular_profile = profile_manager()->CreateTestingProfile("p1");
   Profile* guest_profile = profile_manager()->CreateGuestProfile();
   histograms()->ExpectTotalCount("Profile.State.Avatar_All", 0);
@@ -239,8 +255,8 @@ TEST_F(ProfileActivityMetricsRecorderTest, ProfileState) {
   histograms()->ExpectTotalCount("Profile.State.Avatar_All", 1);
 
   // Repeating the same thing immediately has no impact (neither for any other
-  // profile). Note that guest profile can only get created with incognito.
-  ActivateIncognitoBrowser(guest_profile);
+  // profile).
+  ActivateGuestBrowser(guest_profile);
   histograms()->ExpectTotalCount("Profile.State.Avatar_All", 1);
 
   // Stay inactive so the session ends and stay inactive long after that.
@@ -256,7 +272,7 @@ TEST_F(ProfileActivityMetricsRecorderTest, ProfileState) {
   histograms()->ExpectTotalCount("Profile.State.Avatar_All", 2);
 }
 
-TEST_F(ProfileActivityMetricsRecorderTest, AccountMetrics) {
+TEST_P(ProfileActivityMetricsRecorderTest, AccountMetrics) {
   Profile* regular_profile = profile_manager()->CreateTestingProfile("p1");
   Profile* guest_profile = profile_manager()->CreateGuestProfile();
   histograms()->ExpectTotalCount("Profile.AllAccounts.Names", 0);
@@ -269,6 +285,10 @@ TEST_F(ProfileActivityMetricsRecorderTest, AccountMetrics) {
   histograms()->ExpectTotalCount("Profile.AllAccounts.Names", 2);
 
   // We don't record for the guest profile.
-  ActivateIncognitoBrowser(guest_profile);
+  ActivateGuestBrowser(guest_profile);
   histograms()->ExpectTotalCount("Profile.AllAccounts.Names", 2);
 }
+
+INSTANTIATE_TEST_SUITE_P(AllGuestTypes,
+                         ProfileActivityMetricsRecorderTest,
+                         /*is_ephemeral=*/testing::Bool());

chrome/browser/profiles/profile_attributes_entry.cc

@@ -110,6 +110,7 @@ const char ProfileAttributesEntry::kIsOmittedFromProfileListKey[] =
 const char ProfileAttributesEntry::kAvatarIconKey[] = "avatar_icon";
 const char ProfileAttributesEntry::kBackgroundAppsKey[] = "background_apps";
 const char ProfileAttributesEntry::kProfileIsEphemeral[] = "is_ephemeral";
+const char ProfileAttributesEntry::kProfileIsGuest[] = "is_guest";
 const char ProfileAttributesEntry::kUserNameKey[] = "user_name";
 const char ProfileAttributesEntry::kGAIAIdKey[] = "gaia_id";
 const char ProfileAttributesEntry::kIsConsentedPrimaryAccountKey[] =
@@ -394,6 +395,10 @@ bool ProfileAttributesEntry::IsEphemeral() const {
   return GetBool(kProfileIsEphemeral);
 }
 
+bool ProfileAttributesEntry::IsGuest() const {
+  return GetBool(kProfileIsGuest);
+}
+
 bool ProfileAttributesEntry::IsUsingDefaultName() const {
   return GetBool(kIsUsingDefaultNameKey);
 }
@@ -572,6 +577,10 @@ void ProfileAttributesEntry::SetIsEphemeral(bool value) {
   SetBool(kProfileIsEphemeral, value);
 }
 
+void ProfileAttributesEntry::SetIsGuest(bool value) {
+  SetBool(kProfileIsGuest, value);
+}
+
 void ProfileAttributesEntry::SetIsUsingDefaultName(bool value) {
   if (SetBool(kIsUsingDefaultNameKey, value))
     profile_info_cache_->NotifyIfProfileNamesHaveChanged();

chrome/browser/profiles/profile_attributes_entry.h

@@ -121,6 +121,10 @@ class ProfileAttributesEntry {
   std::string GetSupervisedUserId() const;
   // Returns true if the profile is an ephemeral profile.
   bool IsEphemeral() const;
+  // Returns true if the profile is a Guest profile.
+  // Only ephemeral Guest profiles are stored in profile attributes and
+  // therefore a Guest profile here is always ephemeral as well.
+  bool IsGuest() const;
   // Returns true if the profile is using a default name, typically of the
   // format "Person %d".
   bool IsUsingDefaultName() const;
@@ -167,6 +171,7 @@ class ProfileAttributesEntry {
   void SetIsSigninRequired(bool value);
   void SetSignedInWithCredentialProvider(bool value);
   void SetIsEphemeral(bool value);
+  void SetIsGuest(bool value);
   void SetIsUsingDefaultName(bool value);
   void SetIsUsingDefaultAvatar(bool value);
   void SetIsAuthError(bool value);
@@ -205,6 +210,7 @@ class ProfileAttributesEntry {
   static const char kAvatarIconKey[];
   static const char kBackgroundAppsKey[];
   static const char kProfileIsEphemeral[];
+  static const char kProfileIsGuest[];
   static const char kUserNameKey[];
   static const char kGAIAIdKey[];
   static const char kIsConsentedPrimaryAccountKey[];

chrome/browser/profiles/profile_attributes_storage.cc

@@ -158,7 +158,7 @@ MultiProfileUserType GetMultiProfileUserType(
 
   int active_count = std::count_if(
       entries.begin(), entries.end(), [](ProfileAttributesEntry* entry) {
-        return ProfileMetrics::IsProfileActive(entry);
+        return ProfileMetrics::IsProfileActive(entry) && !entry->IsGuest();
       });
 
   if (active_count <= 1)
@@ -412,6 +412,8 @@ void ProfileAttributesStorage::RecordProfilesState() {
   MultiProfileUserType type = GetMultiProfileUserType(entries);
 
   for (ProfileAttributesEntry* entry : entries) {
+    if (entry->IsGuest())
+      continue;
     RecordProfileState(entry, profile_metrics::StateSuffix::kAll);
 
     switch (type) {

chrome/browser/profiles/profile_info_cache.cc

@@ -171,6 +171,7 @@ void ProfileInfoCache::AddProfileToCache(const base::FilePath& profile_path,
   info->SetBoolean(ProfileAttributesEntry::kIsOmittedFromProfileListKey,
                    !supervised_user_id.empty());
   info->SetBoolean(ProfileAttributesEntry::kProfileIsEphemeral, false);
+  info->SetBoolean(ProfileAttributesEntry::kProfileIsGuest, false);
   // Either the user has provided a name manually on purpose, and in this case
   // we should not check for legacy profile names or this a new profile but then
   // it is not a legacy name, so we dont need to check for legacy names.

chrome/browser/profiles/profile_manager.cc

@@ -522,7 +522,18 @@ Profile* ProfileManager::GetProfile(const base::FilePath& profile_dir) {
 }
 
 size_t ProfileManager::GetNumberOfProfiles() {
-  return GetProfileAttributesStorage().GetNumberOfProfiles();
+  ProfileAttributesStorage& storage = GetProfileAttributesStorage();
+  int offset = 0;
+  // Ephemeral Guest profile is registered in profile attributes storage,
+  // because if Chrome crashes we need the registry to find and delete it.
+  // But it should not be counted as a regular profile.
+  if (!guest_profile_path_.empty()) {
+    ProfileAttributesEntry* entry;
+    if (storage.GetProfileAttributesWithPath(guest_profile_path_, &entry))
+      offset = 1;
+  }
+
+  return storage.GetNumberOfProfiles() - offset;
 }
 
 bool ProfileManager::LoadProfile(const std::string& profile_name,
@@ -1857,8 +1868,10 @@ void ProfileManager::AddProfileToStorage(Profile* profile) {
       storage.GetProfileAttributesWithPath(profile->GetPath(), &entry);
   DCHECK(has_entry);
 
-  if (profile->IsEphemeralGuestProfile())
+  if (profile->IsEphemeralGuestProfile()) {
     profile->GetPrefs()->SetBoolean(prefs::kForceEphemeralProfiles, true);
+    entry->SetIsGuest(true);
+  }
 
   if (profile->GetPrefs()->GetBoolean(prefs::kForceEphemeralProfiles))
     entry->SetIsEphemeral(true);