Commit 54c07418 authored by Jan Wilken Dörrie's avatar Jan Wilken Dörrie Committed by Commit Bot

[Passwords] Add UKMs for Touch To Fill

This change adds UKMs for Touch To Fill User Actions.

Launch Bug: https://crbug.com/1008937
Collection Review: go/touch-to-fill-ukm-collection-review

Bug: 1020996
Change-Id: Ie2c4c731e2f53d879b9d5df73ab4e4465b21b228
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1897932
Commit-Queue: Jan Wilken Dörrie <jdoerrie@chromium.org>
Reviewed-by: default avatarRobert Kaplow <rkaplow@chromium.org>
Reviewed-by: default avatarSteven Holte <holte@chromium.org>
Reviewed-by: default avatarFriedrich [CET] <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#714844}
parent 0221c879
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include <utility> #include <utility>
#include "base/logging.h" #include "base/logging.h"
#include "base/util/type_safety/pass_key.h"
#include "chrome/browser/password_manager/chrome_password_manager_client.h" #include "chrome/browser/password_manager/chrome_password_manager_client.h"
#include "chrome/browser/touch_to_fill/touch_to_fill_view.h" #include "chrome/browser/touch_to_fill/touch_to_fill_view.h"
#include "components/favicon/core/favicon_service.h" #include "components/favicon/core/favicon_service.h"
...@@ -14,7 +15,11 @@ ...@@ -14,7 +15,11 @@
#include "components/password_manager/core/browser/origin_credential_store.h" #include "components/password_manager/core/browser/origin_credential_store.h"
#include "components/password_manager/core/browser/password_manager_driver.h" #include "components/password_manager/core/browser/password_manager_driver.h"
#include "components/password_manager/core/browser/password_manager_metrics_util.h" #include "components/password_manager/core/browser/password_manager_metrics_util.h"
#include "components/ukm/content/source_url_recorder.h"
#include "components/url_formatter/elide_url.h" #include "components/url_formatter/elide_url.h"
#include "services/metrics/public/cpp/ukm_builders.h"
#include "services/metrics/public/cpp/ukm_recorder.h"
#include "services/metrics/public/cpp/ukm_source_id.h"
#include "services/network/public/cpp/is_potentially_trustworthy.h" #include "services/network/public/cpp/is_potentially_trustworthy.h"
using ShowVirtualKeyboard = using ShowVirtualKeyboard =
...@@ -34,10 +39,16 @@ void OnImageFetched(base::OnceCallback<void(const gfx::Image&)> callback, ...@@ -34,10 +39,16 @@ void OnImageFetched(base::OnceCallback<void(const gfx::Image&)> callback,
} // namespace } // namespace
TouchToFillController::TouchToFillController(
util::PassKey<TouchToFillControllerTest>) {}
TouchToFillController::TouchToFillController( TouchToFillController::TouchToFillController(
ChromePasswordManagerClient* password_client, ChromePasswordManagerClient* password_client,
favicon::FaviconService* favicon_service) favicon::FaviconService* favicon_service)
: password_client_(password_client), favicon_service_(favicon_service) {} : password_client_(password_client),
favicon_service_(favicon_service),
source_id_(ukm::GetSourceIdForWebContentsDocument(
password_client_->web_contents())) {}
TouchToFillController::~TouchToFillController() = default; TouchToFillController::~TouchToFillController() = default;
...@@ -66,6 +77,10 @@ void TouchToFillController::OnCredentialSelected( ...@@ -66,6 +77,10 @@ void TouchToFillController::OnCredentialSelected(
driver_->TouchToFillClosed(ShowVirtualKeyboard(false)); driver_->TouchToFillClosed(ShowVirtualKeyboard(false));
std::exchange(driver_, nullptr) std::exchange(driver_, nullptr)
->FillSuggestion(credential.username, credential.password); ->FillSuggestion(credential.username, credential.password);
ukm::builders::TouchToFill_Shown(source_id_)
.SetUserAction(static_cast<int64_t>(UserAction::kSelectedCredential))
.Record(ukm::UkmRecorder::Get());
} }
void TouchToFillController::OnManagePasswordsSelected() { void TouchToFillController::OnManagePasswordsSelected() {
...@@ -76,6 +91,10 @@ void TouchToFillController::OnManagePasswordsSelected() { ...@@ -76,6 +91,10 @@ void TouchToFillController::OnManagePasswordsSelected() {
->TouchToFillClosed(ShowVirtualKeyboard(false)); ->TouchToFillClosed(ShowVirtualKeyboard(false));
password_client_->NavigateToManagePasswordsPage( password_client_->NavigateToManagePasswordsPage(
password_manager::ManagePasswordsReferrer::kTouchToFill); password_manager::ManagePasswordsReferrer::kTouchToFill);
ukm::builders::TouchToFill_Shown(source_id_)
.SetUserAction(static_cast<int64_t>(UserAction::kSelectedManagePasswords))
.Record(ukm::UkmRecorder::Get());
} }
void TouchToFillController::OnDismiss() { void TouchToFillController::OnDismiss() {
...@@ -83,6 +102,10 @@ void TouchToFillController::OnDismiss() { ...@@ -83,6 +102,10 @@ void TouchToFillController::OnDismiss() {
return; return;
std::exchange(driver_, nullptr)->TouchToFillClosed(ShowVirtualKeyboard(true)); std::exchange(driver_, nullptr)->TouchToFillClosed(ShowVirtualKeyboard(true));
ukm::builders::TouchToFill_Shown(source_id_)
.SetUserAction(static_cast<int64_t>(UserAction::kDismissed))
.Record(ukm::UkmRecorder::Get());
} }
gfx::NativeView TouchToFillController::GetNativeView() { gfx::NativeView TouchToFillController::GetNativeView() {
......
...@@ -12,9 +12,11 @@ ...@@ -12,9 +12,11 @@
#include "base/containers/span.h" #include "base/containers/span.h"
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "base/task/cancelable_task_tracker.h" #include "base/task/cancelable_task_tracker.h"
#include "base/util/type_safety/pass_key.h"
#include "chrome/browser/touch_to_fill/touch_to_fill_view.h" #include "chrome/browser/touch_to_fill/touch_to_fill_view.h"
#include "chrome/browser/touch_to_fill/touch_to_fill_view_factory.h" #include "chrome/browser/touch_to_fill/touch_to_fill_view_factory.h"
#include "components/favicon_base/favicon_types.h" #include "components/favicon_base/favicon_types.h"
#include "services/metrics/public/cpp/ukm_source_id.h"
#include "ui/gfx/native_widget_types.h" #include "ui/gfx/native_widget_types.h"
#include "url/gurl.h" #include "url/gurl.h"
...@@ -31,6 +33,23 @@ class ChromePasswordManagerClient; ...@@ -31,6 +33,23 @@ class ChromePasswordManagerClient;
class TouchToFillController { class TouchToFillController {
public: public:
// The action a user took when interacting with the Touch To Fill sheet.
//
// These values are persisted to logs. Entries should not be renumbered and
// numeric values should never be reused. Needs to stay in sync with
// TouchToFill.UserAction in enums.xml and UserAction in
// TouchToFillComponent.java.
//
// TODO(crbug.com/1013134): De-duplicate the Java and C++ enum.
enum class UserAction {
kSelectedCredential = 0,
kDismissed = 1,
kSelectedManagePasswords = 2,
};
// No-op constructor for tests.
explicit TouchToFillController(
util::PassKey<class TouchToFillControllerTest>);
TouchToFillController(ChromePasswordManagerClient* web_contents, TouchToFillController(ChromePasswordManagerClient* web_contents,
favicon::FaviconService* favicon_service); favicon::FaviconService* favicon_service);
TouchToFillController(const TouchToFillController&) = delete; TouchToFillController(const TouchToFillController&) = delete;
...@@ -81,6 +100,8 @@ class TouchToFillController { ...@@ -81,6 +100,8 @@ class TouchToFillController {
// The favicon service used to retrieve icons for a given origin. // The favicon service used to retrieve icons for a given origin.
favicon::FaviconService* favicon_service_ = nullptr; favicon::FaviconService* favicon_service_ = nullptr;
ukm::SourceId source_id_ = ukm::kInvalidSourceId;
// Used to track requested favicons. On destruction, requests are cancelled. // Used to track requested favicons. On destruction, requests are cancelled.
base::CancelableTaskTracker favicon_tracker_; base::CancelableTaskTracker favicon_tracker_;
......
...@@ -9,8 +9,12 @@ ...@@ -9,8 +9,12 @@
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "base/test/metrics/histogram_tester.h" #include "base/test/metrics/histogram_tester.h"
#include "base/test/task_environment.h"
#include "base/util/type_safety/pass_key.h"
#include "components/password_manager/core/browser/origin_credential_store.h" #include "components/password_manager/core/browser/origin_credential_store.h"
#include "components/password_manager/core/browser/stub_password_manager_driver.h" #include "components/password_manager/core/browser/stub_password_manager_driver.h"
#include "components/ukm/test_ukm_recorder.h"
#include "services/metrics/public/cpp/ukm_builders.h"
#include "testing/gmock/include/gmock/gmock.h" #include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
...@@ -50,6 +54,8 @@ struct MockTouchToFillView : TouchToFillView { ...@@ -50,6 +54,8 @@ struct MockTouchToFillView : TouchToFillView {
class TouchToFillControllerTest : public testing::Test { class TouchToFillControllerTest : public testing::Test {
protected: protected:
using UkmBuilder = ukm::builders::TouchToFill_Shown;
TouchToFillControllerTest() { TouchToFillControllerTest() {
auto mock_view = std::make_unique<MockTouchToFillView>(); auto mock_view = std::make_unique<MockTouchToFillView>();
mock_view_ = mock_view.get(); mock_view_ = mock_view.get();
...@@ -63,14 +69,19 @@ class TouchToFillControllerTest : public testing::Test { ...@@ -63,14 +69,19 @@ class TouchToFillControllerTest : public testing::Test {
MockTouchToFillView& view() { return *mock_view_; } MockTouchToFillView& view() { return *mock_view_; }
ukm::TestAutoSetUkmRecorder& test_recorder() { return test_recorder_; }
TouchToFillController& touch_to_fill_controller() { TouchToFillController& touch_to_fill_controller() {
return touch_to_fill_controller_; return touch_to_fill_controller_;
} }
private: private:
base::test::TaskEnvironment task_environment_;
MockTouchToFillView* mock_view_ = nullptr; MockTouchToFillView* mock_view_ = nullptr;
MockPasswordManagerDriver driver_; MockPasswordManagerDriver driver_;
TouchToFillController touch_to_fill_controller_{nullptr, nullptr}; ukm::TestAutoSetUkmRecorder test_recorder_;
TouchToFillController touch_to_fill_controller_{
util::PassKey<TouchToFillControllerTest>()};
}; };
TEST_F(TouchToFillControllerTest, Show_And_Fill) { TEST_F(TouchToFillControllerTest, Show_And_Fill) {
...@@ -90,6 +101,13 @@ TEST_F(TouchToFillControllerTest, Show_And_Fill) { ...@@ -90,6 +101,13 @@ TEST_F(TouchToFillControllerTest, Show_And_Fill) {
touch_to_fill_controller().OnCredentialSelected(credentials[0]); touch_to_fill_controller().OnCredentialSelected(credentials[0]);
tester.ExpectUniqueSample("PasswordManager.FilledCredentialWasFromAndroidApp", tester.ExpectUniqueSample("PasswordManager.FilledCredentialWasFromAndroidApp",
false, 1); false, 1);
auto entries = test_recorder().GetEntriesByName(UkmBuilder::kEntryName);
ASSERT_EQ(1u, entries.size());
test_recorder().ExpectEntryMetric(
entries[0], UkmBuilder::kUserActionName,
static_cast<int64_t>(
TouchToFillController::UserAction::kSelectedCredential));
} }
TEST_F(TouchToFillControllerTest, Show_Insecure_Origin) { TEST_F(TouchToFillControllerTest, Show_Insecure_Origin) {
...@@ -126,6 +144,13 @@ TEST_F(TouchToFillControllerTest, Show_And_Fill_Android_Credential) { ...@@ -126,6 +144,13 @@ TEST_F(TouchToFillControllerTest, Show_And_Fill_Android_Credential) {
touch_to_fill_controller().OnCredentialSelected(credentials[1]); touch_to_fill_controller().OnCredentialSelected(credentials[1]);
tester.ExpectUniqueSample("PasswordManager.FilledCredentialWasFromAndroidApp", tester.ExpectUniqueSample("PasswordManager.FilledCredentialWasFromAndroidApp",
true, 1); true, 1);
auto entries = test_recorder().GetEntriesByName(UkmBuilder::kEntryName);
ASSERT_EQ(1u, entries.size());
test_recorder().ExpectEntryMetric(
entries[0], UkmBuilder::kUserActionName,
static_cast<int64_t>(
TouchToFillController::UserAction::kSelectedCredential));
} }
TEST_F(TouchToFillControllerTest, Dismiss) { TEST_F(TouchToFillControllerTest, Dismiss) {
...@@ -139,4 +164,10 @@ TEST_F(TouchToFillControllerTest, Dismiss) { ...@@ -139,4 +164,10 @@ TEST_F(TouchToFillControllerTest, Dismiss) {
EXPECT_CALL(driver(), TouchToFillClosed(ShowVirtualKeyboard(true))); EXPECT_CALL(driver(), TouchToFillClosed(ShowVirtualKeyboard(true)));
touch_to_fill_controller().OnDismiss(); touch_to_fill_controller().OnDismiss();
auto entries = test_recorder().GetEntriesByName(UkmBuilder::kEntryName);
ASSERT_EQ(1u, entries.size());
test_recorder().ExpectEntryMetric(
entries[0], UkmBuilder::kUserActionName,
static_cast<int64_t>(TouchToFillController::UserAction::kDismissed));
} }
...@@ -27,7 +27,9 @@ public interface TouchToFillComponent { ...@@ -27,7 +27,9 @@ public interface TouchToFillComponent {
* The different reasons that the sheet's state can change. * The different reasons that the sheet's state can change.
* *
* These values are persisted to logs. Entries should not be renumbered and numeric values * These values are persisted to logs. Entries should not be renumbered and numeric values
* should never be reused. Needs to stay in sync with TouchToFill.UserAction in enums.xml. * should never be reused. Needs to stay in sync with TouchToFill.UserAction in enums.xml and
* UserAction in touch_to_fill_controller.h.
* TODO(crbug.com/1013134): Deduplicate the Java and C++ enum.
*/ */
@IntDef({UserAction.SELECT_CREDENTIAL, UserAction.DISMISS, UserAction.SELECT_MANAGE_PASSWORDS, @IntDef({UserAction.SELECT_CREDENTIAL, UserAction.DISMISS, UserAction.SELECT_MANAGE_PASSWORDS,
UserAction.MAX_VALUE}) UserAction.MAX_VALUE})
......
...@@ -8425,6 +8425,19 @@ be describing additional metrics about the same event. ...@@ -8425,6 +8425,19 @@ be describing additional metrics about the same event.
</metric> </metric>
</event> </event>
<event name="TouchToFill.Shown">
<owner>jdoerrie@chromium.org</owner>
<owner>fhorschig@chromium.org</owner>
<summary>
Recorded when the Touch To Fill sheet is shown to the user.
</summary>
<metric name="UserAction" enum="TouchToFill.UserAction">
<summary>
The action a user took when interacting with the Touch To Fill sheet.
</summary>
</metric>
</event>
<event name="Translate"> <event name="Translate">
<owner>hamelphi@chromium.org</owner> <owner>hamelphi@chromium.org</owner>
<summary> <summary>
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment