[OsSettingsMetrics] Create wrapper for User Action Tracker

Part of the set up for the metrics changes to OS Settings. Creates a
wrapper class for the existing User Action Tracker so that it can
stay alive outside of user sessions.

Bug: 1133553
Change-Id: I9f3db611f893dcfc78638c007b27b1099f63f3da
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2439021
Commit-Queue: Daniel Classon <dclasson@google.com>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#812367}

chrome/browser/chromeos/BUILD.gn

@@ -3693,9 +3693,9 @@ source_set("unit_tests") {
     "../ui/webui/settings/chromeos/multidevice_handler_unittest.cc",
     "../ui/webui/settings/chromeos/os_settings_manager_unittest.cc",
     "../ui/webui/settings/chromeos/os_settings_section_unittest.cc",
+    "../ui/webui/settings/chromeos/search/per_session_settings_user_action_tracker_unittest.cc",
     "../ui/webui/settings/chromeos/search/search_handler_unittest.cc",
     "../ui/webui/settings/chromeos/search/search_tag_registry_unittest.cc",
-    "../ui/webui/settings/chromeos/search/settings_user_action_tracker_unittest.cc",
   ]
   if (use_cups) {
     sources += [

chrome/browser/ui/BUILD.gn

@@ -2507,17 +2507,19 @@ static_library("ui") {
       "webui/settings/chromeos/quick_unlock_handler.h",
       "webui/settings/chromeos/reset_section.cc",
       "webui/settings/chromeos/reset_section.h",
+      "webui/settings/chromeos/search/per_session_settings_user_action_tracker.cc",
+      "webui/settings/chromeos/search/per_session_settings_user_action_tracker.h",
       "webui/settings/chromeos/search/search_concept.h",
       "webui/settings/chromeos/search/search_handler.cc",
       "webui/settings/chromeos/search/search_handler.h",
       "webui/settings/chromeos/search/search_tag_registry.cc",
       "webui/settings/chromeos/search/search_tag_registry.h",
-      "webui/settings/chromeos/search/settings_user_action_tracker.cc",
-      "webui/settings/chromeos/search/settings_user_action_tracker.h",
       "webui/settings/chromeos/search_section.cc",
       "webui/settings/chromeos/search_section.h",
       "webui/settings/chromeos/server_printer_url_util.cc",
       "webui/settings/chromeos/server_printer_url_util.h",
+      "webui/settings/chromeos/settings_user_action_tracker.cc",
+      "webui/settings/chromeos/settings_user_action_tracker.h",
       "webui/settings/chromeos/wallpaper_handler.cc",
       "webui/settings/chromeos/wallpaper_handler.h",
       "webui/settings/tts_handler.cc",

chrome/browser/ui/webui/settings/chromeos/os_settings_manager.cc

@@ -10,6 +10,7 @@
 #include "chrome/browser/ui/webui/settings/chromeos/os_settings_sections.h"
 #include "chrome/browser/ui/webui/settings/chromeos/search/search_handler.h"
 #include "chrome/browser/ui/webui/settings/chromeos/search/search_tag_registry.h"
+#include "chrome/browser/ui/webui/settings/chromeos/settings_user_action_tracker.h"
 #include "chromeos/components/local_search_service/local_search_service.h"
 #include "chromeos/constants/chromeos_features.h"
 #include "content/public/browser/web_ui_data_source.h"
@@ -44,6 +45,8 @@ OsSettingsManager::OsSettingsManager(
                                                identity_manager,
                                                android_sms_service,
                                                printers_manager)),
+      settings_user_action_tracker_(
+          std::make_unique<SettingsUserActionTracker>()),
       hierarchy_(std::make_unique<Hierarchy>(sections_.get())),
       search_handler_(
           std::make_unique<SearchHandler>(search_tag_registry_.get(),

chrome/browser/ui/webui/settings/chromeos/os_settings_manager.h

@@ -54,6 +54,7 @@ class Hierarchy;
 class OsSettingsSections;
 class SearchHandler;
 class SearchTagRegistry;
+class SettingsUserActionTracker;
 
 // Manager for the Chrome OS settings page. This class is implemented as a
 // KeyedService, so one instance of the class is intended to be active for the
@@ -109,6 +110,10 @@ class OsSettingsManager : public KeyedService {
 
   SearchHandler* search_handler() { return search_handler_.get(); }
 
+  SettingsUserActionTracker* settings_user_action_tracker() {
+    return settings_user_action_tracker_.get();
+  }
+
   const Hierarchy* hierarchy() const { return hierarchy_.get(); }
 
  private:
@@ -119,6 +124,7 @@ class OsSettingsManager : public KeyedService {
 
   std::unique_ptr<SearchTagRegistry> search_tag_registry_;
   std::unique_ptr<OsSettingsSections> sections_;
+  std::unique_ptr<SettingsUserActionTracker> settings_user_action_tracker_;
   std::unique_ptr<Hierarchy> hierarchy_;
   std::unique_ptr<SearchHandler> search_handler_;
 };

chrome/browser/ui/webui/settings/chromeos/os_settings_ui.cc

@@ -20,7 +20,7 @@
 #include "chrome/browser/ui/webui/settings/chromeos/os_settings_manager_factory.h"
 #include "chrome/browser/ui/webui/settings/chromeos/pref_names.h"
 #include "chrome/browser/ui/webui/settings/chromeos/search/search_handler.h"
-#include "chrome/browser/ui/webui/settings/chromeos/search/settings_user_action_tracker.h"
+#include "chrome/browser/ui/webui/settings/chromeos/settings_user_action_tracker.h"
 #include "chrome/browser/ui/webui/webui_util.h"
 #include "chrome/common/webui_url_constants.h"
 #include "chrome/grit/os_settings_resources.h"
@@ -139,8 +139,9 @@ void OSSettingsUI::BindInterface(
 
 void OSSettingsUI::BindInterface(
     mojo::PendingReceiver<mojom::UserActionRecorder> receiver) {
-  user_action_recorder_ =
-      std::make_unique<SettingsUserActionTracker>(std::move(receiver));
+  OsSettingsManagerFactory::GetForProfile(Profile::FromWebUI(web_ui()))
+      ->settings_user_action_tracker()
+      ->BindInterface(std::move(receiver));
 }
 
 void OSSettingsUI::BindInterface(

chrome/browser/ui/webui/settings/chromeos/search/settings_user_action_tracker.cc → chrome/browser/ui/webui/settings/chromeos/search/per_session_settings_user_action_tracker.cc

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/ui/webui/settings/chromeos/search/settings_user_action_tracker.h"
+#include "chrome/browser/ui/webui/settings/chromeos/search/per_session_settings_user_action_tracker.h"
 
 #include "base/metrics/histogram_functions.h"
 
@@ -35,18 +35,13 @@ void LogDurationMetric(const char* metric_name, base::TimeDelta duration) {
 
 }  // namespace
 
-SettingsUserActionTracker::SettingsUserActionTracker(
-    mojo::PendingReceiver<mojom::UserActionRecorder> pending_receiver)
-    : SettingsUserActionTracker() {
-  receiver_.Bind(std::move(pending_receiver));
-}
-
-SettingsUserActionTracker::SettingsUserActionTracker()
+PerSessionSettingsUserActionTracker::PerSessionSettingsUserActionTracker()
     : metric_start_time_(base::TimeTicks::Now()) {}
 
-SettingsUserActionTracker::~SettingsUserActionTracker() = default;
+PerSessionSettingsUserActionTracker::~PerSessionSettingsUserActionTracker() =
+    default;
 
-void SettingsUserActionTracker::RecordPageFocus() {
+void PerSessionSettingsUserActionTracker::RecordPageFocus() {
   if (last_blur_timestamp_.is_null())
     return;
 
@@ -65,23 +60,23 @@ void SettingsUserActionTracker::RecordPageFocus() {
   }
 }
 
-void SettingsUserActionTracker::RecordPageBlur() {
+void PerSessionSettingsUserActionTracker::RecordPageBlur() {
   last_blur_timestamp_ = base::TimeTicks::Now();
 }
 
-void SettingsUserActionTracker::RecordClick() {
+void PerSessionSettingsUserActionTracker::RecordClick() {
   ++num_clicks_since_start_time_;
 }
 
-void SettingsUserActionTracker::RecordNavigation() {
+void PerSessionSettingsUserActionTracker::RecordNavigation() {
   ++num_navigations_since_start_time_;
 }
 
-void SettingsUserActionTracker::RecordSearch() {
+void PerSessionSettingsUserActionTracker::RecordSearch() {
   ++num_searches_since_start_time_;
 }
 
-void SettingsUserActionTracker::RecordSettingChange() {
+void PerSessionSettingsUserActionTracker::RecordSettingChange() {
   base::TimeTicks now = base::TimeTicks::Now();
 
   if (!last_record_setting_changed_timestamp_.is_null()) {
@@ -120,7 +115,7 @@ void SettingsUserActionTracker::RecordSettingChange() {
   last_record_setting_changed_timestamp_ = now;
 }
 
-void SettingsUserActionTracker::ResetMetricsCountersAndTimestamp() {
+void PerSessionSettingsUserActionTracker::ResetMetricsCountersAndTimestamp() {
   metric_start_time_ = base::TimeTicks::Now();
   num_clicks_since_start_time_ = 0u;
   num_navigations_since_start_time_ = 0u;

chrome/browser/ui/webui/settings/chromeos/search/settings_user_action_tracker.h → chrome/browser/ui/webui/settings/chromeos/search/per_session_settings_user_action_tracker.h

@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_BROWSER_UI_WEBUI_SETTINGS_CHROMEOS_SEARCH_SETTINGS_USER_ACTION_TRACKER_H_
-#define CHROME_BROWSER_UI_WEBUI_SETTINGS_CHROMEOS_SEARCH_SETTINGS_USER_ACTION_TRACKER_H_
+#ifndef CHROME_BROWSER_UI_WEBUI_SETTINGS_CHROMEOS_SEARCH_PER_SESSION_SETTINGS_USER_ACTION_TRACKER_H_
+#define CHROME_BROWSER_UI_WEBUI_SETTINGS_CHROMEOS_SEARCH_PER_SESSION_SETTINGS_USER_ACTION_TRACKER_H_
 
 #include "base/time/time.h"
 #include "chrome/browser/ui/webui/settings/chromeos/search/user_action_recorder.mojom.h"
@@ -17,14 +17,14 @@ namespace settings {
 // This class is only meant to track actions from an individual settings
 // session; if the settings window is closed and reopened again, a new instance
 // should be created for that new session.
-class SettingsUserActionTracker : public mojom::UserActionRecorder {
+class PerSessionSettingsUserActionTracker : public mojom::UserActionRecorder {
  public:
-  explicit SettingsUserActionTracker(
-      mojo::PendingReceiver<mojom::UserActionRecorder> pending_receiver);
-  SettingsUserActionTracker(const SettingsUserActionTracker& other) = delete;
-  SettingsUserActionTracker& operator=(const SettingsUserActionTracker& other) =
-      delete;
-  ~SettingsUserActionTracker() override;
+  PerSessionSettingsUserActionTracker();
+  PerSessionSettingsUserActionTracker(
+      const PerSessionSettingsUserActionTracker& other) = delete;
+  PerSessionSettingsUserActionTracker& operator=(
+      const PerSessionSettingsUserActionTracker& other) = delete;
+  ~PerSessionSettingsUserActionTracker() override;
 
   // mojom::UserActionRecorder:
   void RecordPageFocus() override;
@@ -35,10 +35,7 @@ class SettingsUserActionTracker : public mojom::UserActionRecorder {
   void RecordSettingChange() override;
 
  private:
-  friend class SettingsUserActionTrackerTest;
-
-  // For unit tests.
-  SettingsUserActionTracker();
+  friend class PerSessionSettingsUserActionTrackerTest;
 
   void ResetMetricsCountersAndTimestamp();
 
@@ -71,4 +68,4 @@ class SettingsUserActionTracker : public mojom::UserActionRecorder {
 }  // namespace settings
 }  // namespace chromeos
 
-#endif  // CHROME_BROWSER_UI_WEBUI_SETTINGS_CHROMEOS_SEARCH_SETTINGS_USER_ACTION_TRACKER_H_
+#endif  // CHROME_BROWSER_UI_WEBUI_SETTINGS_CHROMEOS_SEARCH_PER_SESSION_SETTINGS_USER_ACTION_TRACKER_H_

chrome/browser/ui/webui/settings/chromeos/search/settings_user_action_tracker_unittest.cc → chrome/browser/ui/webui/settings/chromeos/search/per_session_settings_user_action_tracker_unittest.cc

@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/ui/webui/settings/chromeos/search/settings_user_action_tracker.h"
+#include "chrome/browser/ui/webui/settings/chromeos/search/per_session_settings_user_action_tracker.h"
 
 #include "base/test/metrics/histogram_tester.h"
 #include "base/test/task_environment.h"
@@ -12,18 +12,18 @@
 namespace chromeos {
 namespace settings {
 
-class SettingsUserActionTrackerTest : public testing::Test {
+class PerSessionSettingsUserActionTrackerTest : public testing::Test {
  protected:
-  SettingsUserActionTrackerTest() = default;
-  ~SettingsUserActionTrackerTest() override = default;
+  PerSessionSettingsUserActionTrackerTest() = default;
+  ~PerSessionSettingsUserActionTrackerTest() override = default;
 
   base::test::TaskEnvironment task_environment_{
       base::test::TaskEnvironment::TimeSource::MOCK_TIME};
   base::HistogramTester histogram_tester_;
-  SettingsUserActionTracker tracker_;
+  PerSessionSettingsUserActionTracker tracker_;
 };
 
-TEST_F(SettingsUserActionTrackerTest, TestRecordMetrics) {
+TEST_F(PerSessionSettingsUserActionTrackerTest, TestRecordMetrics) {
   // Focus the page, perform some tasks, and change a setting.
   tracker_.RecordPageFocus();
   tracker_.RecordClick();
@@ -117,7 +117,7 @@ TEST_F(SettingsUserActionTrackerTest, TestRecordMetrics) {
       /*count=*/2);
 }
 
-TEST_F(SettingsUserActionTrackerTest, TestBlurAndFocus) {
+TEST_F(PerSessionSettingsUserActionTrackerTest, TestBlurAndFocus) {
   // Focus the page, click, and change a setting.
   tracker_.RecordPageFocus();
   tracker_.RecordClick();

chrome/browser/ui/webui/settings/chromeos/settings_user_action_tracker.cc

+// Copyright 2020 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/ui/webui/settings/chromeos/settings_user_action_tracker.h"
+
+#include <memory>
+#include <utility>
+
+#include "base/bind.h"
+
+namespace chromeos {
+namespace settings {
+
+SettingsUserActionTracker::SettingsUserActionTracker() = default;
+
+SettingsUserActionTracker::~SettingsUserActionTracker() = default;
+
+void SettingsUserActionTracker::BindInterface(
+    mojo::PendingReceiver<mojom::UserActionRecorder> pending_receiver) {
+  // Only one user session should be active at a time.
+  EndCurrentSession();
+  receiver_.Bind(std::move(pending_receiver));
+  receiver_.set_disconnect_handler(
+      base::BindOnce(&SettingsUserActionTracker::OnBindingDisconnected,
+                     base::Unretained(this)));
+
+  // New session started, so create a new per session tracker.
+  per_session_tracker_ =
+      std::make_unique<PerSessionSettingsUserActionTracker>();
+}
+
+void SettingsUserActionTracker::EndCurrentSession() {
+  // Session ended, so delete the per session tracker.
+  per_session_tracker_.reset();
+  receiver_.reset();
+}
+
+void SettingsUserActionTracker::OnBindingDisconnected() {
+  EndCurrentSession();
+}
+
+void SettingsUserActionTracker::RecordPageFocus() {
+  per_session_tracker_->RecordPageFocus();
+}
+
+void SettingsUserActionTracker::RecordPageBlur() {
+  per_session_tracker_->RecordPageBlur();
+}
+
+void SettingsUserActionTracker::RecordClick() {
+  per_session_tracker_->RecordClick();
+}
+
+void SettingsUserActionTracker::RecordNavigation() {
+  per_session_tracker_->RecordNavigation();
+}
+
+void SettingsUserActionTracker::RecordSearch() {
+  per_session_tracker_->RecordSearch();
+}
+
+void SettingsUserActionTracker::RecordSettingChange() {
+  per_session_tracker_->RecordSettingChange();
+}
+
+}  // namespace settings
+}  // namespace chromeos

chrome/browser/ui/webui/settings/chromeos/settings_user_action_tracker.h

+// Copyright 2020 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_UI_WEBUI_SETTINGS_CHROMEOS_SETTINGS_USER_ACTION_TRACKER_H_
+#define CHROME_BROWSER_UI_WEBUI_SETTINGS_CHROMEOS_SETTINGS_USER_ACTION_TRACKER_H_
+
+#include <memory>
+
+#include "chrome/browser/ui/webui/settings/chromeos/search/per_session_settings_user_action_tracker.h"
+#include "chrome/browser/ui/webui/settings/chromeos/search/user_action_recorder.mojom.h"
+#include "mojo/public/cpp/bindings/pending_receiver.h"
+#include "mojo/public/cpp/bindings/receiver.h"
+
+namespace chromeos {
+namespace settings {
+
+// Records user actions within Settings. Utilizes a per session tracker that
+// measures the user's effort required to change a setting. Eventually uses
+// a per section tracker to record metrics in each section.
+class SettingsUserActionTracker : public mojom::UserActionRecorder {
+ public:
+  SettingsUserActionTracker();
+  SettingsUserActionTracker(const SettingsUserActionTracker& other) = delete;
+  SettingsUserActionTracker& operator=(const SettingsUserActionTracker& other) =
+      delete;
+  ~SettingsUserActionTracker() override;
+
+  void BindInterface(
+      mojo::PendingReceiver<mojom::UserActionRecorder> pending_receiver);
+
+ private:
+  // mojom::UserActionRecorder:
+  void RecordPageFocus() override;
+  void RecordPageBlur() override;
+  void RecordClick() override;
+  void RecordNavigation() override;
+  void RecordSearch() override;
+  void RecordSettingChange() override;
+
+  void EndCurrentSession();
+  void OnBindingDisconnected();
+
+  std::unique_ptr<PerSessionSettingsUserActionTracker> per_session_tracker_;
+
+  mojo::Receiver<mojom::UserActionRecorder> receiver_{this};
+};
+
+}  // namespace settings
+}  // namespace chromeos
+
+#endif  // CHROME_BROWSER_UI_WEBUI_SETTINGS_CHROMEOS_SETTINGS_USER_ACTION_TRACKER_H_