[go: up one dir, main page]

Add ProfileMetricsRecorder

...to track and record profile related usage metrics during runtime.
Metrics are recorded per profile, i.e. each profile is assigned an ID
which corresponds to a histogram bucket.

The histogram Profile.BrowserActive.PerProfile is added.

(cherry picked from commit c67ace4a61ef46140193228317a636e7ce19d35c)

Bug: 965469
Change-Id: I2012eb8f3ec1a7ed6ddb54f703453d8d559d0f23
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1625117
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Reviewed-by: Robert Kaplow (slow) <rkaplow@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Commit-Queue: Thomas Tangl <tangltom@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#667281}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1655788
Reviewed-by: Thomas Tangl <tangltom@chromium.org>
Cr-Commit-Position: refs/branch-heads/3809@{#249}
Cr-Branched-From: d82dec1a818f378c464ba307ddd9c92133eac355-refs/heads/master@{#665002}
diff --git a/chrome/browser/BUILD.gn b/chrome/browser/BUILD.gn
index 04aff03..540fe15f 100644
--- a/chrome/browser/BUILD.gn
+++ b/chrome/browser/BUILD.gn
@@ -3888,6 +3888,8 @@
       "profiles/avatar_menu_actions_desktop.h",
       "profiles/avatar_menu_desktop.cc",
       "profiles/avatar_menu_observer.h",
+      "profiles/profile_activity_metrics_recorder.cc",
+      "profiles/profile_activity_metrics_recorder.h",
       "profiles/profile_list.h",
       "profiles/profile_list_desktop.cc",
       "profiles/profile_list_desktop.h",
diff --git a/chrome/browser/chrome_browser_main.cc b/chrome/browser/chrome_browser_main.cc
index e90273c..b84ed25 100644
--- a/chrome/browser/chrome_browser_main.cc
+++ b/chrome/browser/chrome_browser_main.cc
@@ -262,6 +262,7 @@
 #if defined(OS_WIN) || defined(OS_MACOSX) || \
     (defined(OS_LINUX) && !defined(OS_CHROMEOS))
 #include "chrome/browser/metrics/desktop_session_duration/desktop_session_duration_tracker.h"
+#include "chrome/browser/profiles/profile_activity_metrics_recorder.h"
 #endif
 
 #if defined(OS_WIN)
@@ -1106,6 +1107,7 @@
 #if defined(OS_WIN) || defined(OS_MACOSX) || \
     (defined(OS_LINUX) && !defined(OS_CHROMEOS))
   metrics::DesktopSessionDurationTracker::Initialize();
+  ProfileActivityMetricsRecorder::Initialize();
 #endif
   metrics::RendererUptimeTracker::Initialize();
 
diff --git a/chrome/browser/prefs/browser_prefs.cc b/chrome/browser/prefs/browser_prefs.cc
index e783e9a..50312cd 100644
--- a/chrome/browser/prefs/browser_prefs.cc
+++ b/chrome/browser/prefs/browser_prefs.cc
@@ -53,6 +53,7 @@
 #include "chrome/browser/previews/previews_offline_helper.h"
 #include "chrome/browser/profiles/chrome_version_service.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/profiles/profile_attributes_entry.h"
 #include "chrome/browser/profiles/profile_impl.h"
 #include "chrome/browser/profiles/profile_info_cache.h"
 #include "chrome/browser/profiles/profiles_state.h"
@@ -528,6 +529,7 @@
   OriginTrialPrefs::RegisterPrefs(registry);
   password_manager::PasswordManager::RegisterLocalPrefs(registry);
   PrefProxyConfigTrackerImpl::RegisterPrefs(registry);
+  ProfileAttributesEntry::RegisterLocalStatePrefs(registry);
   ProfileInfoCache::RegisterPrefs(registry);
   profiles::RegisterPrefs(registry);
   rappor::RapporServiceImpl::RegisterPrefs(registry);
diff --git a/chrome/browser/profiles/profile_activity_metrics_recorder.cc b/chrome/browser/profiles/profile_activity_metrics_recorder.cc
new file mode 100644
index 0000000..53d27e8
--- /dev/null
+++ b/chrome/browser/profiles/profile_activity_metrics_recorder.cc
@@ -0,0 +1,61 @@
+// Copyright 2019 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_activity_metrics_recorder.h"
+
+#include "base/logging.h"
+#include "base/metrics/histogram_macros.h"
+#include "base/no_destructor.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/profiles/profile.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/browser.h"
+#include "chrome/browser/ui/browser_list.h"
+
+namespace {
+
+// The maximum number of profiles that are recorded. This means that all
+// profiles with bucket index greater than |kMaxProfileBucket| won't be included
+// in the metrics.
+constexpr int kMaxProfileBucket = 100;
+
+int GetMetricsBucketIndex(Profile* profile) {
+  if (profile->IsGuestSession())
+    return 0;
+
+  ProfileAttributesEntry* entry;
+  if (!g_browser_process->profile_manager()
+           ->GetProfileAttributesStorage()
+           .GetProfileAttributesWithPath(profile->GetPath(), &entry)) {
+    // This can happen if the profile is deleted.
+    VLOG(1) << "Failed to read profile bucket index because attributes entry "
+               "doesn't exist.";
+    return -1;
+  }
+  return entry->GetMetricsBucketIndex();
+}
+
+}  // namespace
+
+// static
+void ProfileActivityMetricsRecorder::Initialize() {
+  static base::NoDestructor<ProfileActivityMetricsRecorder>
+      s_profile_activity_metrics_recorder;
+}
+
+ProfileActivityMetricsRecorder::ProfileActivityMetricsRecorder() {
+  BrowserList::AddObserver(this);
+}
+
+void ProfileActivityMetricsRecorder::OnBrowserSetLastActive(Browser* browser) {
+  int profile_bucket =
+      GetMetricsBucketIndex(browser->profile()->GetOriginalProfile());
+
+  if (0 <= profile_bucket && profile_bucket <= kMaxProfileBucket) {
+    UMA_HISTOGRAM_EXACT_LINEAR("Profile.BrowserActive.PerProfile",
+                               profile_bucket, kMaxProfileBucket);
+  }
+}
diff --git a/chrome/browser/profiles/profile_activity_metrics_recorder.h b/chrome/browser/profiles/profile_activity_metrics_recorder.h
new file mode 100644
index 0000000..9de532e
--- /dev/null
+++ b/chrome/browser/profiles/profile_activity_metrics_recorder.h
@@ -0,0 +1,36 @@
+// Copyright 2019 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.
+
+#ifndef CHROME_BROWSER_PROFILES_PROFILE_ACTIVITY_METRICS_RECORDER_H_
+#define CHROME_BROWSER_PROFILES_PROFILE_ACTIVITY_METRICS_RECORDER_H_
+
+#include <stddef.h>
+
+#include "base/macros.h"
+#include "chrome/browser/ui/browser_list_observer.h"
+
+class Browser;
+
+namespace base {
+template <typename T>
+class NoDestructor;
+}
+
+class ProfileActivityMetricsRecorder : public BrowserListObserver {
+ public:
+  // Initializes a |ProfileActivityMetricsRecorder| object and starts
+  // tracking/recording.
+  static void Initialize();
+
+  // BrowserListObserver overrides:
+  void OnBrowserSetLastActive(Browser* browser) override;
+
+ private:
+  friend class base::NoDestructor<ProfileActivityMetricsRecorder>;
+  ProfileActivityMetricsRecorder();
+
+  DISALLOW_COPY_AND_ASSIGN(ProfileActivityMetricsRecorder);
+};
+
+#endif  // CHROME_BROWSER_PROFILES_PROFILE_ACTIVITY_METRICS_RECORDER_H_
diff --git a/chrome/browser/profiles/profile_activity_metrics_recorder_unittest.cc b/chrome/browser/profiles/profile_activity_metrics_recorder_unittest.cc
new file mode 100644
index 0000000..ecbcd7d
--- /dev/null
+++ b/chrome/browser/profiles/profile_activity_metrics_recorder_unittest.cc
@@ -0,0 +1,124 @@
+// Copyright 2019 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 <string>
+#include <vector>
+
+#include "base/test/metrics/histogram_tester.h"
+#include "chrome/browser/profiles/profile_activity_metrics_recorder.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_list.h"
+#include "chrome/test/base/scoped_testing_local_state.h"
+#include "chrome/test/base/test_browser_window.h"
+#include "chrome/test/base/testing_browser_process.h"
+#include "chrome/test/base/testing_profile.h"
+#include "chrome/test/base/testing_profile_manager.h"
+#include "content/public/test/test_browser_thread_bundle.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+class ProfileActivityMetricsRecorderTest : public testing::Test {
+ public:
+  ProfileActivityMetricsRecorderTest()
+      : profile_manager_(TestingBrowserProcess::GetGlobal()) {}
+
+  void SetUp() override {
+    Test::SetUp();
+    ASSERT_TRUE(profile_manager_.SetUp());
+
+    ProfileActivityMetricsRecorder::Initialize();
+  }
+
+  void ActivateBrowser(Profile* profile) {
+    Browser::CreateParams browser_params(profile, false);
+    TestBrowserWindow browser_window;
+    browser_params.window = &browser_window;
+    std::unique_ptr<Browser> browser(Browser::Create(browser_params));
+
+    BrowserList::SetLastActive(browser.get());
+  }
+
+  void ActivateIncognitoBrowser(Profile* profile) {
+    ActivateBrowser(profile->GetOffTheRecordProfile());
+  }
+
+  TestingProfileManager* profile_manager() { return &profile_manager_; }
+  base::HistogramTester* histograms() { return &histogram_tester_; }
+
+ private:
+  content::TestBrowserThreadBundle thread_bundle_;
+
+  TestingProfileManager profile_manager_;
+  base::HistogramTester histogram_tester_;
+
+  DISALLOW_COPY_AND_ASSIGN(ProfileActivityMetricsRecorderTest);
+};
+
+TEST_F(ProfileActivityMetricsRecorderTest, GuestProfile) {
+  Profile* regular_profile = profile_manager()->CreateTestingProfile("p1");
+  Profile* guest_profile = profile_manager()->CreateGuestProfile();
+  histograms()->ExpectTotalCount("Profile.BrowserActive.PerProfile", 0);
+
+  // Check whether the regular profile is counted in bucket 2. (Bucket 1 is
+  // reserved for the guest profile.)
+  ActivateBrowser(regular_profile);
+  histograms()->ExpectBucketCount("Profile.BrowserActive.PerProfile",
+                                  /*bucket=*/1, /*count=*/1);
+
+  // Activate an incognito browser instance of the guest profile.
+  // Note: Creating a non-incognito guest browser instance is not possible.
+  ActivateIncognitoBrowser(guest_profile);
+  histograms()->ExpectBucketCount("Profile.BrowserActive.PerProfile",
+                                  /*bucket=*/0, /*count=*/1);
+
+  histograms()->ExpectTotalCount("Profile.BrowserActive.PerProfile", 2);
+}
+
+TEST_F(ProfileActivityMetricsRecorderTest, IncognitoProfile) {
+  Profile* regular_profile = profile_manager()->CreateTestingProfile("p1");
+  histograms()->ExpectTotalCount("Profile.BrowserActive.PerProfile", 0);
+
+  ActivateBrowser(regular_profile);
+  histograms()->ExpectBucketCount("Profile.BrowserActive.PerProfile",
+                                  /*bucket=*/1, /*count=*/1);
+
+  ActivateIncognitoBrowser(regular_profile);
+  histograms()->ExpectBucketCount("Profile.BrowserActive.PerProfile",
+                                  /*bucket=*/1, /*count=*/2);
+
+  histograms()->ExpectTotalCount("Profile.BrowserActive.PerProfile", 2);
+}
+
+TEST_F(ProfileActivityMetricsRecorderTest, MultipleProfiles) {
+  Profile* profile1 = profile_manager()->CreateTestingProfile("p1");
+  Profile* profile2 = profile_manager()->CreateTestingProfile("p2");
+  histograms()->ExpectTotalCount("Profile.BrowserActive.PerProfile", 0);
+
+  // The browser of profile 1 is activated for the first time. It is assigned
+  // bucket 1.
+  ActivateBrowser(profile1);
+  histograms()->ExpectBucketCount("Profile.BrowserActive.PerProfile",
+                                  /*bucket=*/1, /*count=*/1);
+
+  // Profile creation doesn't influence the histogram.
+  Profile* profile3 = profile_manager()->CreateTestingProfile("p3");
+  histograms()->ExpectTotalCount("Profile.BrowserActive.PerProfile", 1);
+
+  ActivateBrowser(profile1);
+  histograms()->ExpectBucketCount("Profile.BrowserActive.PerProfile",
+                                  /*bucket=*/1, /*count=*/2);
+
+  // The browser of profile 3 is activated for the first time. It is assigned
+  // bucket 2.
+  ActivateBrowser(profile3);
+  histograms()->ExpectBucketCount("Profile.BrowserActive.PerProfile",
+                                  /*bucket=*/2, /*count=*/1);
+
+  // The browser of profile 2 is activated for the first time. It is assigned
+  // bucket 3.
+  ActivateBrowser(profile2);
+  histograms()->ExpectBucketCount("Profile.BrowserActive.PerProfile",
+                                  /*bucket=*/3, /*count=*/1);
+
+  histograms()->ExpectTotalCount("Profile.BrowserActive.PerProfile", 4);
+}
diff --git a/chrome/browser/profiles/profile_attributes_entry.cc b/chrome/browser/profiles/profile_attributes_entry.cc
index 4b84cbc..7302e632 100644
--- a/chrome/browser/profiles/profile_attributes_entry.cc
+++ b/chrome/browser/profiles/profile_attributes_entry.cc
@@ -6,11 +6,13 @@
 
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
+#include "chrome/browser/browser_process.h"
 #include "chrome/browser/profiles/profile_attributes_entry.h"
 #include "chrome/browser/profiles/profile_avatar_icon_util.h"
 #include "chrome/browser/profiles/profile_info_cache.h"
 #include "chrome/browser/signin/signin_util.h"
 #include "chrome/common/pref_names.h"
+#include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
 #include "components/prefs/scoped_user_pref_update.h"
 #include "ui/base/resource/resource_bundle.h"
@@ -22,6 +24,22 @@
 const char kAuthCredentialsKey[] = "local_auth_credentials";
 const char kPasswordTokenKey[] = "gaia_password_token";
 const char kIsAuthErrorKey[] = "is_auth_error";
+const char kMetricsBucketIndex[] = "metrics_bucket_index";
+
+// Local state pref to keep track of the next available profile bucket.
+const char kNextMetricsBucketIndex[] = "profile.metrics.next_bucket_index";
+
+// Returns the next available metrics bucket index and increases the index
+// counter. I.e. two consecutive calls will return two consecutive numbers.
+int NextAvailableMetricsBucketIndex() {
+  PrefService* local_prefs = g_browser_process->local_state();
+  int next_index = local_prefs->GetInteger(kNextMetricsBucketIndex);
+  DCHECK_GT(next_index, 0);
+
+  local_prefs->SetInteger(kNextMetricsBucketIndex, next_index + 1);
+
+  return next_index;
+}
 
 }  // namespace
 
@@ -30,6 +48,14 @@
 const char ProfileAttributesEntry::kProfileIsEphemeral[] = "is_ephemeral";
 const char ProfileAttributesEntry::kUserNameKey[] = "user_name";
 
+// static
+void ProfileAttributesEntry::RegisterLocalStatePrefs(
+    PrefRegistrySimple* registry) {
+  // Bucket 0 is reserved for the guest profile, so start new bucket indices
+  // at 1.
+  registry->RegisterIntegerPref(kNextMetricsBucketIndex, 1);
+}
+
 ProfileAttributesEntry::ProfileAttributesEntry()
     : profile_info_cache_(nullptr),
       prefs_(nullptr),
@@ -212,6 +238,15 @@
   return icon_index;
 }
 
+size_t ProfileAttributesEntry::GetMetricsBucketIndex() {
+  int bucket_index = GetInteger(kMetricsBucketIndex);
+  if (bucket_index == -1) {
+    bucket_index = NextAvailableMetricsBucketIndex();
+    SetInteger(kMetricsBucketIndex, bucket_index);
+  }
+  return bucket_index;
+}
+
 void ProfileAttributesEntry::SetName(const base::string16& name) {
   profile_info_cache_->SetNameOfProfileAtIndex(profile_index(), name);
 }
@@ -401,6 +436,13 @@
   return value && value->is_bool() && value->GetBool();
 }
 
+int ProfileAttributesEntry::GetInteger(const char* key) const {
+  const base::Value* value = GetValue(key);
+  if (!value || !value->is_int())
+    return -1;
+  return value->GetInt();
+}
+
 // Type checking. Only IsDouble is implemented because others do not have
 // callsites.
 bool ProfileAttributesEntry::IsDouble(const char* key) const {
@@ -470,3 +512,18 @@
   SetEntryData(std::move(new_data));
   return true;
 }
+
+bool ProfileAttributesEntry::SetInteger(const char* key, int value) {
+  const base::Value* old_data = GetEntryData();
+  if (old_data) {
+    const base::Value* old_value = old_data->FindKey(key);
+    if (old_value && old_value->is_int() && old_value->GetInt() == value)
+      return false;
+  }
+
+  base::Value new_data = old_data ? GetEntryData()->Clone()
+                                  : base::Value(base::Value::Type::DICTIONARY);
+  new_data.SetKey(key, base::Value(value));
+  SetEntryData(std::move(new_data));
+  return true;
+}
diff --git a/chrome/browser/profiles/profile_attributes_entry.h b/chrome/browser/profiles/profile_attributes_entry.h
index 20ab69c..cf363985 100644
--- a/chrome/browser/profiles/profile_attributes_entry.h
+++ b/chrome/browser/profiles/profile_attributes_entry.h
@@ -21,11 +21,14 @@
 class Image;
 }
 
+class PrefRegistrySimple;
 class PrefService;
 class ProfileInfoCache;
 
 class ProfileAttributesEntry {
  public:
+  static void RegisterLocalStatePrefs(PrefRegistrySimple* registry);
+
   ProfileAttributesEntry();
   virtual ~ProfileAttributesEntry() {}
 
@@ -93,6 +96,10 @@
   bool IsAuthError() const;
   // Returns the index of the default icon used by the profile.
   size_t GetAvatarIconIndex() const;
+  // Returns the metrics bucket this profile should be recorded in.
+  // Note: The bucket index is assigned once and remains the same all time. 0 is
+  // reserved for the guest profile.
+  size_t GetMetricsBucketIndex();
 
   void SetName(const base::string16& name);
   void SetShortcutName(const base::string16& name);
@@ -148,13 +155,14 @@
   const base::Value* GetValue(const char* key) const;
 
   // Internal getters that return basic data types. If the key is not present,
-  // or if the data is in a wrong data type, return empty string, 0.0, or false
-  // depending on the target data type. We do not assume that the data type is
-  // correct because the local state file can be modified by a third party.
+  // or if the data is in a wrong data type, return empty string, 0.0, false or
+  // -1 depending on the target data type. We do not assume that the data type
+  // is correct because the local state file can be modified by a third party.
   std::string GetString(const char* key) const;
   base::string16 GetString16(const char* key) const;
   double GetDouble(const char* key) const;
   bool GetBool(const char* key) const;
+  int GetInteger(const char* key) const;
 
   // Type checking. Only IsDouble is implemented because others do not have
   // callsites.
@@ -166,6 +174,7 @@
   bool SetString16(const char* key, base::string16 value);
   bool SetDouble(const char* key, double value);
   bool SetBool(const char* key, bool value);
+  bool SetInteger(const char* key, int value);
 
   // These members are an implementation detail meant to smooth the migration
   // of the ProfileInfoCache to the ProfileAttributesStorage interface. They can
diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn
index 1fb3cf7..6081b0f9 100644
--- a/chrome/test/BUILD.gn
+++ b/chrome/test/BUILD.gn
@@ -4635,6 +4635,7 @@
       "../browser/media/webrtc/native_desktop_media_list_unittest.cc",
       "../browser/metrics/upgrade_metrics_provider_unittest.cc",
       "../browser/policy/machine_level_user_cloud_policy_register_watcher_unittest.cc",
+      "../browser/profiles/profile_activity_metrics_recorder_unittest.cc",
       "../browser/signin/force_signin_verifier_unittest.cc",
       "../browser/signin/signin_global_error_unittest.cc",
       "../browser/signin/signin_ui_util_unittest.cc",
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml
index caa5ece..cf5eb11 100644
--- a/tools/metrics/histograms/histograms.xml
+++ b/tools/metrics/histograms/histograms.xml
@@ -100973,6 +100973,24 @@
   <summary>Size of the bookmarks database.</summary>
 </histogram>
 
+<histogram name="Profile.BrowserActive.PerProfile" units="Profile ID"
+    expires_after="2020-06-30">
+  <owner>msarda@chromium.org</owner>
+  <owner>tangltom@chromium.org</owner>
+  <summary>
+    Recorded every time a browser window becomes active. Each profile on a
+    client is assigned a unique bucket, i.e. whenever a browser window of
+    profile x becomes active, an entry is recorded in bucket x.
+
+    Example: A user has 2 profiles and opens 1 browser window for each of them.
+    When the user switches back and forth between the windows, multiple entries
+    will be recorded in bucket 1 and 2, corresponding to the profiles.
+
+    Note: The guest profile has bucket 0. Regular profiles start at bucket 1.
+    Incognito browser windows count towards the original profile.
+  </summary>
+</histogram>
+
 <histogram name="Profile.CookiesSize" units="MB" expires_after="2018-08-30">
   <owner>Please list the metric's owners. Add more owner tags as needed.</owner>
   <summary>Size of the cookies database.</summary>