[Profiles] Report profile switches into metrics

This CL adds a new metric that records profile switching (incl. window
switching for concurrently open profiles) into metrics.

The CL renames some variables in the activity recorder to keep the code
clear.

Bug: 1121944
Change-Id: Ia9b7fffb022f8013e4eeb4f94c8187dab16e57b9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2377712
Commit-Queue: Jan Krcal <jkrcal@chromium.org>
Commit-Queue: Alex Ilin <alexilin@chromium.org>
Reviewed-by: Alex Ilin <alexilin@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Auto-Submit: Jan Krcal <jkrcal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#802183}

chrome/browser/profiles/profile_activity_metrics_recorder.cc

@@ -71,6 +71,13 @@ void RecordBrowserActivation(const Profile* profile) {
   }
 }
 
+void RecordProfileSwitch() {
+  int profiles_count =
+      g_browser_process->profile_manager()->GetNumberOfProfiles();
+  UMA_HISTOGRAM_COUNTS_100("Profile.NumberOfProfilesAtProfileSwitch",
+                           profiles_count);
+}
+
 void RecordUserAction(const Profile* profile) {
   if (!profile)
     return;
@@ -124,32 +131,37 @@ void ProfileActivityMetricsRecorder::OnBrowserSetLastActive(Browser* browser) {
   RecordBrowserActivation(active_profile);
   RecordAccountMetrics(active_profile);
 
-  if (last_active_profile_ != active_profile) {
-    // No-op, if starting a new session (|last_active_profile_| is nullptr).
+  if (running_session_profile_ != active_profile) {
+    // No-op, if starting a new session (|running_session_profile_| is nullptr).
     RecordProfileSessionDuration(
-        last_active_profile_, base::TimeTicks::Now() - profile_session_start_);
+        running_session_profile_,
+        base::TimeTicks::Now() - running_session_start_);
 
-    last_active_profile_ = active_profile;
-    profile_session_start_ = base::TimeTicks::Now();
+    running_session_profile_ = active_profile;
+    running_session_start_ = base::TimeTicks::Now();
     profile_observer_.RemoveAll();
-    profile_observer_.Add(last_active_profile_);
-
-    // Record state at startup (when last_profile_session_end_ is 0) and
-    // whenever the user starts browsing after a longer time of inactivity. Do
-    // it asynchronously because active_time of the just activated profile is
-    // also updated from OnBrowserSetLastActive() in another BrowserListObserver
-    // and we have no guarantee if this happens before or after this function
-    // call.
-    if (last_profile_session_end_.is_null() ||
-        (profile_session_start_ - last_profile_session_end_ >
-         kLongTimeOfInactivity)) {
+    profile_observer_.Add(running_session_profile_);
+
+    // Record state at startup (when |last_session_end_| is 0) and whenever the
+    // user starts browsing after a longer time of inactivity. Do it
+    // asynchronously because active_time of the just activated profile is also
+    // updated from OnBrowserSetLastActive() in another BrowserListObserver and
+    // we have no guarantee if this happens before or after this function call.
+    if (last_session_end_.is_null() ||
+        (running_session_start_ - last_session_end_ > kLongTimeOfInactivity)) {
       base::SequencedTaskRunnerHandle::Get()->PostTask(
           FROM_HERE, base::BindOnce(&RecordProfilesState));
     }
   }
 
+  if (last_active_profile_ != active_profile) {
+    if (last_active_profile_ != nullptr)
+      RecordProfileSwitch();
+    last_active_profile_ = active_profile;
+  }
+
   // This browsing session is still lasting.
-  last_profile_session_end_ = base::TimeTicks::Now();
+  last_session_end_ = base::TimeTicks::Now();
 }
 
 void ProfileActivityMetricsRecorder::OnSessionEnded(
@@ -157,21 +169,21 @@ void ProfileActivityMetricsRecorder::OnSessionEnded(
     base::TimeTicks session_end) {
   // If this call is emitted after OnProfileWillBeDestroyed, return
   // early. We already logged the session duration there.
-  if (!last_active_profile_)
+  if (!running_session_profile_)
     return;
 
   // |session_length| can't be used here because it was measured across all
   // profiles.
-  RecordProfileSessionDuration(last_active_profile_,
-                               session_end - profile_session_start_);
-  profile_observer_.Remove(last_active_profile_);
-  last_active_profile_ = nullptr;
-  last_profile_session_end_ = base::TimeTicks::Now();
+  RecordProfileSessionDuration(running_session_profile_,
+                               session_end - running_session_start_);
+  profile_observer_.Remove(running_session_profile_);
+  running_session_profile_ = nullptr;
+  last_session_end_ = base::TimeTicks::Now();
 }
 
 void ProfileActivityMetricsRecorder::OnProfileWillBeDestroyed(
     Profile* profile) {
-  DCHECK_EQ(profile, last_active_profile_);
+  DCHECK_EQ(profile, running_session_profile_);
 
   // The profile may be deleted without an OnSessionEnded call if, for
   // example, the browser shuts down.
@@ -179,9 +191,10 @@ void ProfileActivityMetricsRecorder::OnProfileWillBeDestroyed(
   // TODO(crbug.com/1096145): explore having
   // DesktopSessionDurationTracker call OnSessionEnded() when the
   // profile is destroyed. Remove this workaround if this is done.
-  profile_observer_.Remove(last_active_profile_);
+  profile_observer_.Remove(running_session_profile_);
+  running_session_profile_ = nullptr;
   last_active_profile_ = nullptr;
-  last_profile_session_end_ = base::TimeTicks::Now();
+  last_session_end_ = base::TimeTicks::Now();
 }
 
 ProfileActivityMetricsRecorder::ProfileActivityMetricsRecorder() {
@@ -200,5 +213,5 @@ ProfileActivityMetricsRecorder::~ProfileActivityMetricsRecorder() {
 
 void ProfileActivityMetricsRecorder::OnUserAction(const std::string& action,
                                                   base::TimeTicks action_time) {
-  RecordUserAction(last_active_profile_);
+  RecordUserAction(running_session_profile_);
 }

chrome/browser/profiles/profile_activity_metrics_recorder.h

@@ -50,9 +50,14 @@ class ProfileActivityMetricsRecorder
 
   void OnUserAction(const std::string& action, base::TimeTicks action_time);
 
+  // The profile of the last active window.
   Profile* last_active_profile_ = nullptr;
-  base::TimeTicks profile_session_start_;
-  base::TimeTicks last_profile_session_end_;
+
+  // Profile of the currently running session, if there is any. Reset after
+  // inactivity.
+  Profile* running_session_profile_ = nullptr;
+  base::TimeTicks running_session_start_;
+  base::TimeTicks last_session_end_;
 
   base::ActionCallback action_callback_;
 

chrome/browser/profiles/profile_activity_metrics_recorder_unittest.cc

@@ -111,6 +111,8 @@ TEST_F(ProfileActivityMetricsRecorderTest, GuestProfile) {
   histograms()->ExpectBucketCount("Profile.BrowserActive.PerProfile",
                                   /*bucket=*/1, /*count=*/1);
   SimulateUserActionAndExpectRecording(/*bucket=*/1);
+  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.
@@ -118,6 +120,8 @@ TEST_F(ProfileActivityMetricsRecorderTest, GuestProfile) {
   histograms()->ExpectBucketCount("Profile.BrowserActive.PerProfile",
                                   /*bucket=*/0, /*count=*/1);
   SimulateUserActionAndExpectRecording(/*bucket=*/0);
+  histograms()->ExpectUniqueSample("Profile.NumberOfProfilesAtProfileSwitch",
+                                   /*bucket=*/1, /*count=*/1);
 
   histograms()->ExpectTotalCount("Profile.BrowserActive.PerProfile", 2);
 }
@@ -135,6 +139,8 @@ TEST_F(ProfileActivityMetricsRecorderTest, IncognitoProfile) {
                                   /*bucket=*/1, /*count=*/2);
 
   histograms()->ExpectTotalCount("Profile.BrowserActive.PerProfile", 2);
+  histograms()->ExpectTotalCount("Profile.NumberOfProfilesAtProfileSwitch",
+                                 /*count=*/0);
 }
 
 TEST_F(ProfileActivityMetricsRecorderTest, MultipleProfiles) {
@@ -160,6 +166,9 @@ TEST_F(ProfileActivityMetricsRecorderTest, MultipleProfiles) {
   histograms()->ExpectBucketCount("Profile.BrowserActive.PerProfile",
                                   /*bucket=*/1, /*count=*/2);
   SimulateUserActionAndExpectRecording(/*bucket=*/1);
+  // No profile switch, so far.
+  histograms()->ExpectTotalCount("Profile.NumberOfProfilesAtProfileSwitch",
+                                 /*count=*/0);
 
   // Profile 1: Session lasts 2 minutes.
   task_environment()->FastForwardBy(base::TimeDelta::FromMinutes(2));
@@ -170,6 +179,8 @@ TEST_F(ProfileActivityMetricsRecorderTest, MultipleProfiles) {
   histograms()->ExpectBucketCount("Profile.BrowserActive.PerProfile",
                                   /*bucket=*/2, /*count=*/1);
   SimulateUserActionAndExpectRecording(/*bucket=*/2);
+  histograms()->ExpectUniqueSample("Profile.NumberOfProfilesAtProfileSwitch",
+                                   /*bucket=*/3, /*count=*/1);
 
   // Profile 1: Session ended. The duration(2 minutes) is recorded.
   histograms()->ExpectBucketCount("Profile.SessionDuration.PerProfile",
@@ -184,6 +195,8 @@ TEST_F(ProfileActivityMetricsRecorderTest, MultipleProfiles) {
   histograms()->ExpectBucketCount("Profile.BrowserActive.PerProfile",
                                   /*bucket=*/3, /*count=*/1);
   SimulateUserActionAndExpectRecording(/*bucket=*/3);
+  histograms()->ExpectUniqueSample("Profile.NumberOfProfilesAtProfileSwitch",
+                                   /*bucket=*/3, /*count=*/2);
 
   // Profile 3: Session ended. The duration(2 minutes) is recorded.
   histograms()->ExpectBucketCount("Profile.SessionDuration.PerProfile",

tools/metrics/histograms/histograms.xml

@@ -141996,6 +141996,18 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
   </summary>
 </histogram>
 
+<histogram name="Profile.NumberOfProfilesAtProfileSwitch" units="profiles"
+    expires_after="2021-03-01">
+  <owner>jkrcal@chromium.org</owner>
+  <owner>droger@chromium.org</owner>
+  <summary>
+    Records the count of profiles for each profile switch (incl. switching
+    between concurrently open profiles by activating their browser windows). The
+    count excludes the Guest profile so records in bucket 1 denote switches
+    between the single profile and the Guest profile.
+  </summary>
+</histogram>
+
 <histogram name="Profile.NumberOfProfilesOnStartup" units="units"
     expires_after="2014-01-29">
   <obsolete>