Add UMA metrics for profile switching.

chrome/browser/profiles/profile_metrics.cc

 // Copyright (c) 2011 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/profiles/profile_metrics.h"
 
 #include "base/files/file_path.h"
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_info_cache.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/signin/signin_header_helper.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/installer/util/google_update_settings.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/user_metrics.h"
 
 namespace {
 
 const int kMaximumReportedProfileCount = 5;
 const int kMaximumDaysOfDisuse = 4 * 7;  // Should be integral number of weeks.
 
+size_t number_of_switches_ = 0;

[Mike Lerman, 2015/01/21 20:58:02]

call this "number_of_profile_switches_" please.

[anthonyvd, 2015/01/23 16:01:30]

Done.

+
 ProfileMetrics::ProfileType GetProfileType(
     const base::FilePath& profile_path) {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
   ProfileMetrics::ProfileType metric = ProfileMetrics::SECONDARY;
   ProfileManager* manager = g_browser_process->profile_manager();
   base::FilePath user_data_dir;
   // In unittests, we do not always have a profile_manager so check.
   if (manager) {
     user_data_dir = manager->user_data_dir();
   }
@@ skipping 121 matching lines @@
     if (limited_total > kMaximumReportedProfileCount) {
       limited_total = kMaximumReportedProfileCount + 1;
       limited_signedin =
           (int)((float)(counts.signedin * limited_total)
           / counts.total + 0.5);
     }
     UpdateReportedOSProfileStatistics(limited_total, limited_signedin);
   }
 }
 
+void ProfileMetrics::IncrementNumberOfSwitches() {
+  ++number_of_switches_;

[Mike Lerman, 2015/01/21 20:58:03]

I don't think this needs to be a separate method; please inline within
LogProfileSwitch().

[anthonyvd, 2015/01/23 16:01:31]

Done.

+}
+
+void ProfileMetrics::LogNumberOfSwitches() {
+  UMA_HISTOGRAM_COUNTS_100("Profile.NumberofSwitches", number_of_switches_);

[Mike Lerman, 2015/01/21 20:58:03]

nit: NumberOfSwitches (capital O)

[anthonyvd, 2015/01/23 16:01:31]

Done.

+}
+
 void ProfileMetrics::LogNumberOfProfiles(ProfileManager* manager) {
   ProfileCounts counts;
   bool success = CountProfileInformation(manager, &counts);
   UMA_HISTOGRAM_COUNTS_100("Profile.NumberOfProfiles", counts.total);
 
   // Ignore other metrics if we have no profiles, e.g. in Chrome Frame tests.
   if (success) {
     UMA_HISTOGRAM_COUNTS_100("Profile.NumberOfManagedProfiles",
                              counts.supervised);
     UMA_HISTOGRAM_COUNTS_100("Profile.PercentageOfManagedProfiles",
@@ skipping 123 matching lines @@
   UMA_HISTOGRAM_ENUMERATION("Profile.NetUserCount", PROFILE_DELETED,
                             NUM_PROFILE_NET_METRICS);
 }
 
 void ProfileMetrics::LogProfileOpenMethod(ProfileOpen metric) {
   DCHECK(metric < NUM_PROFILE_OPEN_METRICS);
   UMA_HISTOGRAM_ENUMERATION("Profile.OpenMethod", metric,
                             NUM_PROFILE_OPEN_METRICS);
 }
 
+void ProfileMetrics::LogProfileSwitch(
+    ProfileOpen metric,
+    bool profile_already_open) {
+  DCHECK(metric < NUM_PROFILE_OPEN_METRICS);
+  if(profile_already_open) {
+    UMA_HISTOGRAM_ENUMERATION("Profile.ProfileSwitch.ToOpenedProfile",
+                              metric,
+                              NUM_PROFILE_OPEN_METRICS);
+  } else {
+    UMA_HISTOGRAM_ENUMERATION("Profile.ProfileSwitch.ToUnopenedProfile",
+                              metric,
+                              NUM_PROFILE_OPEN_METRICS);
+  }
+  UMA_HISTOGRAM_ENUMERATION("Profile.ProfileSwitch",

[Mike Lerman, 2015/01/21 20:58:02]

We really try not to rename histograms - doing so loses all the history
associated with them. One needs a really compelling reason to do the switch,
which I don't think is present here. Please still call it Profile.OpenMethod.

[anthonyvd, 2015/01/21 21:12:18]

So I preserved the Profile.OpenMethod histogram and its behavior.  However, it
logged switch methods as well as users opening profile related UI elements
(which we're not interested in in this particular case).  This method logs the
actual switch in Profile.ProfileSwitch then calls LogProfileOpenMethod (line
339) to preserve the other histogram's behavior.  Does that make sense?

[Mike Lerman, 2015/01/21 21:24:18]

Ah, I understand.
I'm leery of duplicating the data needlessly. I feel like we can keep the data
and still get the information we need from the dashboards, and just ignoring the
fields we don't need.

[anthonyvd, 2015/01/23 16:01:31]

Done.

+                            metric,
+                            NUM_PROFILE_OPEN_METRICS);
+
+  IncrementNumberOfSwitches();
+  //The LogOpenMethod histogram aggregates data from profile switches
+  //as well as opening of profile related UI elements

[Mike Lerman, 2015/01/21 21:24:18]

nit: Add spaces after the //'s, and end it all with a period. Also, don't wrap
text to the next line until you have to. I think some of the words from the
second line fit on the first.

[anthonyvd, 2015/01/23 16:01:30]

Done.

+  LogProfileOpenMethod(metric);
+}
+
 void ProfileMetrics::LogProfileSwitchGaia(ProfileGaia metric) {
   if (metric == GAIA_OPT_IN)
     LogProfileAvatarSelection(AVATAR_GAIA);
   UMA_HISTOGRAM_ENUMERATION("Profile.SwitchGaiaPhotoSettings",
                             metric,
                             NUM_PROFILE_GAIA_METRICS);
 }
 
-void ProfileMetrics::LogProfileSwitchUser(ProfileOpen metric) {
-  DCHECK(metric < NUM_PROFILE_OPEN_METRICS);
-  UMA_HISTOGRAM_ENUMERATION("Profile.OpenMethod", metric,
-                            NUM_PROFILE_OPEN_METRICS);
-}
-
 void ProfileMetrics::LogProfileSyncInfo(ProfileSync metric) {
   DCHECK(metric < NUM_PROFILE_SYNC_METRICS);
   UMA_HISTOGRAM_ENUMERATION("Profile.SyncCustomize", metric,
                             NUM_PROFILE_SYNC_METRICS);
 }
 
 void ProfileMetrics::LogProfileAuthResult(ProfileAuth metric) {
   UMA_HISTOGRAM_ENUMERATION("Profile.AuthResult", metric,
                             NUM_PROFILE_AUTH_METRICS);
 }
@@ skipping 131 matching lines @@
   UMA_HISTOGRAM_ENUMERATION("Profile.SyncSignIn",
                             GetProfileType(profile_path),
                             NUM_PROFILE_TYPE_METRICS);
 }
 
 void ProfileMetrics::LogProfileUpdate(const base::FilePath& profile_path) {
   UMA_HISTOGRAM_ENUMERATION("Profile.Update",
                             GetProfileType(profile_path),
                             NUM_PROFILE_TYPE_METRICS);
 }