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>