Skip to content

Commit

Permalink
Add metrics for parent access code
Browse files Browse the repository at this point in the history 
Bug: 935859
Test: ParentAccessViewTest
Change-Id: Ie1ad5b0165e1f9c9387f0452b1b0f7cece1c5182
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1775555
Commit-Queue: Aga Wronska <agawronska@chromium.org>
Reviewed-by: Toni Baržić <tbarzic@chromium.org>
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#692281}
  • Loading branch information
Aga Wronska authored and Commit Bot committed Aug 30, 2019
1 parent 1e41879 commit 8dc25af
Show file tree
Hide file tree
Showing 5 changed files with 209 additions and 0 deletions.
44 changes: 44 additions & 0 deletions ash/login/ui/parent_access_view.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,15 @@
#include "ash/wallpaper/wallpaper_controller_impl.h"
#include "base/bind.h"
#include "base/logging.h"
#include "base/metrics/histogram_macros.h"
#include "base/optional.h"
#include "base/strings/strcat.h"
#include "base/strings/string16.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/utf_string_conversions.h"
#include "base/task/post_task.h"
#include "base/threading/thread_task_runner_handle.h"
#include "components/session_manager/session_manager_types.h"
#include "third_party/skia/include/core/SkColor.h"
#include "ui/accessibility/ax_node_data.h"
#include "ui/base/l10n/l10n_util.h"
Expand Down Expand Up @@ -179,6 +181,36 @@ class AccessibleInputField : public views::Textfield {
DISALLOW_COPY_AND_ASSIGN(AccessibleInputField);
};

void RecordAction(ParentAccessView::UMAAction action) {
UMA_HISTOGRAM_ENUMERATION(ParentAccessView::kUMAParentAccessCodeAction,
action);
}

void RecordUsage(ParentAccessRequestReason reason) {
switch (reason) {
case ParentAccessRequestReason::kUnlockTimeLimits: {
UMA_HISTOGRAM_ENUMERATION(ParentAccessView::kUMAParentAccessCodeUsage,
ParentAccessView::UMAUsage::kTimeLimits);
return;
}
case ParentAccessRequestReason::kChangeTime: {
bool is_login = Shell::Get()->session_controller()->GetSessionState() ==
session_manager::SessionState::LOGIN_PRIMARY;
UMA_HISTOGRAM_ENUMERATION(
ParentAccessView::kUMAParentAccessCodeUsage,
is_login ? ParentAccessView::UMAUsage::kTimeChangeLoginScreen
: ParentAccessView::UMAUsage::kTimeChangeInSession);
return;
}
case ParentAccessRequestReason::kChangeTimezone: {
UMA_HISTOGRAM_ENUMERATION(ParentAccessView::kUMAParentAccessCodeUsage,
ParentAccessView::UMAUsage::kTimezoneChange);
return;
}
}
NOTREACHED() << "Unknown ParentAccessRequestReason";
}

} // namespace

// Digital access code input view for variable length of input codes.
Expand Down Expand Up @@ -456,6 +488,12 @@ ParentAccessView::Callbacks::Callbacks(const Callbacks& other) = default;

ParentAccessView::Callbacks::~Callbacks() = default;

// static
constexpr char ParentAccessView::kUMAParentAccessCodeAction[];

// static
constexpr char ParentAccessView::kUMAParentAccessCodeUsage[];

ParentAccessView::ParentAccessView(const AccountId& account_id,
const Callbacks& callbacks,
ParentAccessRequestReason reason,
Expand Down Expand Up @@ -627,6 +665,8 @@ ParentAccessView::ParentAccessView(const AccountId& account_id,
pin_keyboard_view_->SetVisible(IsTabletMode());

tablet_mode_observer_.Add(Shell::Get()->tablet_mode_controller());

RecordUsage(request_reason_);
}

ParentAccessView::~ParentAccessView() = default;
Expand Down Expand Up @@ -681,8 +721,10 @@ base::string16 ParentAccessView::GetAccessibleWindowTitle() const {
void ParentAccessView::ButtonPressed(views::Button* sender,
const ui::Event& event) {
if (sender == back_button_) {
RecordAction(ParentAccessView::UMAAction::kCanceledByUser);
callbacks_.on_finished.Run(false);
} else if (sender == help_button_) {
RecordAction(ParentAccessView::UMAAction::kGetHelp);
Shell::Get()->login_screen_controller()->ShowParentAccessHelpApp();
} else if (sender == submit_button_) {
SubmitCode();
Expand Down Expand Up @@ -721,11 +763,13 @@ void ParentAccessView::SubmitCode() {

if (result) {
VLOG(1) << "Parent access code successfully validated";
RecordAction(ParentAccessView::UMAAction::kValidationSuccess);
callbacks_.on_finished.Run(true);
return;
}

VLOG(1) << "Invalid parent access code entered";
RecordAction(ParentAccessView::UMAAction::kValidationError);
UpdateState(State::kError);
}

Expand Down
30 changes: 30 additions & 0 deletions ash/login/ui/parent_access_view.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,36 @@ class ASH_EXPORT ParentAccessView : public views::DialogDelegateView,
OnFinished on_finished;
};

// Actions that originated in parent access dialog. These values are persisted
// to metrics. Entries should not be renumbered and numeric values should
// never be reused.
enum class UMAAction {
kValidationSuccess = 0,
kValidationError = 1,
kCanceledByUser = 2,
kGetHelp = 3,
kMaxValue = kGetHelp,
};

// Context in which parent access code was used. These values are persisted to
// metrics. Entries should not be reordered and numeric values should never be
// reused.
enum class UMAUsage {
kTimeLimits = 0,
kTimeChangeLoginScreen = 1,
kTimeChangeInSession = 2,
kTimezoneChange = 3,
kMaxValue = kTimezoneChange,
};

// Histogram to log actions that originated in parent access dialog.
static constexpr char kUMAParentAccessCodeAction[] =
"Supervision.ParentAccessCode.Action";

// Histogram to log context in which parent access code was used.
static constexpr char kUMAParentAccessCodeUsage[] =
"Supervision.ParentAccessCode.Usage";

// Creates parent access view that will validate the parent access code for a
// specific child, when |account_id| is set, or to any child signed in the
// device, when it is empty. |callbacks| will be called when user performs
Expand Down
99 changes: 99 additions & 0 deletions ash/login/ui/parent_access_view_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,19 @@
#include "ash/login/ui/login_pin_view.h"
#include "ash/login/ui/login_test_base.h"
#include "ash/login/ui/login_test_utils.h"
#include "ash/login/ui/parent_access_widget.h"
#include "ash/session/test_session_controller_client.h"
#include "ash/shell.h"
#include "ash/strings/grit/ash_strings.h"
#include "ash/wm/tablet_mode/tablet_mode_controller.h"
#include "base/bind.h"
#include "base/macros.h"
#include "base/optional.h"
#include "base/test/bind_test_util.h"
#include "base/test/metrics/histogram_tester.h"
#include "base/time/time.h"
#include "components/account_id/account_id.h"
#include "components/session_manager/session_manager_types.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/events/base_event_utils.h"
Expand Down Expand Up @@ -101,6 +105,40 @@ class ParentAccessViewTest : public LoginTestBase {
SetWidget(CreateWidgetWithContent(view_));
}

// Shows parent access widget with the specified |reason|.
void ShowWidget(ParentAccessRequestReason reason) {
ParentAccessWidget::Show(
account_id_,
base::BindRepeating(&ParentAccessViewTest::OnFinished,
base::Unretained(this)),
reason);
ParentAccessWidget* widget = ParentAccessWidget::Get();
ASSERT_TRUE(widget);
}

// Dismisses existing parent access widget with back button click. Should be
// only called when the widget is shown.
void DismissWidget() {
ParentAccessWidget* widget = ParentAccessWidget::Get();
ASSERT_TRUE(widget);

ParentAccessView* view =
ParentAccessWidget::TestApi(widget).parent_access_view();
ParentAccessView::TestApi test_api(view);
ui::MouseEvent event(ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(),
ui::EventTimeForNow(), 0, 0);
view->ButtonPressed(test_api.back_button(), event);
}

// Verifies expectation that UMA |action| was logged.
// Cannot be used when more than one action is reported.
void ExpectUMAActionReported(ParentAccessView::UMAAction action) {
histogram_tester_.ExpectBucketCount(
ParentAccessView::kUMAParentAccessCodeAction, action, 1);
histogram_tester_.ExpectTotalCount(
ParentAccessView::kUMAParentAccessCodeAction, 1);
}

const AccountId account_id_;
std::unique_ptr<MockLoginScreenClient> login_client_;

Expand All @@ -113,6 +151,8 @@ class ParentAccessViewTest : public LoginTestBase {
// Time that will be used on the code validation.
base::Time validation_time_;

base::HistogramTester histogram_tester_;

ParentAccessView* view_ = nullptr; // Owned by test widget view hierarchy.

private:
Expand Down Expand Up @@ -151,6 +191,7 @@ TEST_F(ParentAccessViewTest, BackButton) {

EXPECT_EQ(1, back_action_);
EXPECT_EQ(0, successful_validation_);
ExpectUMAActionReported(ParentAccessView::UMAAction::kCanceledByUser);
}

// Tests that submit button submits code from code input.
Expand All @@ -176,6 +217,7 @@ TEST_F(ParentAccessViewTest, SubmitButton) {
SimulateButtonPress(test_api.submit_button());
base::RunLoop().RunUntilIdle();
EXPECT_EQ(1, successful_validation_);
ExpectUMAActionReported(ParentAccessView::UMAAction::kValidationSuccess);
}

// Tests that help button opens help app.
Expand All @@ -188,6 +230,8 @@ TEST_F(ParentAccessViewTest, HelpButton) {

EXPECT_CALL(*client, ShowParentAccessHelpApp()).Times(1);
SimulateButtonPress(test_api.help_button());

ExpectUMAActionReported(ParentAccessView::UMAAction::kGetHelp);
}

// Tests that access code can be entered with numpad.
Expand All @@ -210,6 +254,7 @@ TEST_F(ParentAccessViewTest, Numpad) {
SimulateButtonPress(test_api.submit_button());
base::RunLoop().RunUntilIdle();
EXPECT_EQ(1, successful_validation_);
ExpectUMAActionReported(ParentAccessView::UMAAction::kValidationSuccess);
}

// Tests that access code can be submitted with press of 'enter' key.
Expand All @@ -234,6 +279,7 @@ TEST_F(ParentAccessViewTest, SubmitWithEnter) {
generator->PressKey(ui::KeyboardCode::VKEY_RETURN, ui::EF_NONE);
base::RunLoop().RunUntilIdle();
EXPECT_EQ(1, successful_validation_);
ExpectUMAActionReported(ParentAccessView::UMAAction::kValidationSuccess);
}

// Tests that 'enter' key does not submit incomplete code.
Expand Down Expand Up @@ -273,6 +319,7 @@ TEST_F(ParentAccessViewTest, PressEnterOnIncompleteCode) {
generator->PressKey(ui::KeyboardCode::VKEY_RETURN, ui::EF_NONE);
base::RunLoop().RunUntilIdle();
EXPECT_EQ(1, successful_validation_);
ExpectUMAActionReported(ParentAccessView::UMAAction::kValidationSuccess);
}

// Tests that backspace button works.
Expand Down Expand Up @@ -322,6 +369,7 @@ TEST_F(ParentAccessViewTest, Backspace) {
SimulateButtonPress(test_api.submit_button());
base::RunLoop().RunUntilIdle();
EXPECT_EQ(1, successful_validation_);
ExpectUMAActionReported(ParentAccessView::UMAAction::kValidationSuccess);
}

// Tests input with virtual pin keyboard.
Expand All @@ -347,6 +395,7 @@ TEST_F(ParentAccessViewTest, PinKeyboard) {
SimulateButtonPress(test_api.submit_button());
base::RunLoop().RunUntilIdle();
EXPECT_EQ(1, successful_validation_);
ExpectUMAActionReported(ParentAccessView::UMAAction::kValidationSuccess);
}

// Tests that pin keyboard visibility changes upon tablet mode changes.
Expand Down Expand Up @@ -387,6 +436,7 @@ TEST_F(ParentAccessViewTest, ErrorState) {
base::RunLoop().RunUntilIdle();
EXPECT_EQ(ParentAccessView::State::kError, test_api.state());
EXPECT_EQ(0, successful_validation_);
ExpectUMAActionReported(ParentAccessView::UMAAction::kValidationError);

// After access code is completed, focus moves to submit button.
// Move focus back to access code input.
Expand All @@ -406,6 +456,12 @@ TEST_F(ParentAccessViewTest, ErrorState) {
SimulateButtonPress(test_api.submit_button());
base::RunLoop().RunUntilIdle();
EXPECT_EQ(1, successful_validation_);

histogram_tester_.ExpectBucketCount(
ParentAccessView::kUMAParentAccessCodeAction,
ParentAccessView::UMAAction::kValidationError, 1);
histogram_tester_.ExpectTotalCount(
ParentAccessView::kUMAParentAccessCodeAction, 2);
}

// Tests children views traversal with tab key.
Expand Down Expand Up @@ -477,4 +533,47 @@ TEST_F(ParentAccessViewTest, BackwardTabKeyTraversal) {
EXPECT_TRUE(HasFocusInAnyChildView(test_api.access_code_view()));
}

// Tests that correct usage metric is reported.
TEST_F(ParentAccessViewTest, UMAUsageMetric) {
ShowWidget(ParentAccessRequestReason::kUnlockTimeLimits);
DismissWidget();
histogram_tester_.ExpectBucketCount(
ParentAccessView::kUMAParentAccessCodeUsage,
ParentAccessView::UMAUsage::kTimeLimits, 1);

ShowWidget(ParentAccessRequestReason::kChangeTimezone);
DismissWidget();
histogram_tester_.ExpectBucketCount(
ParentAccessView::kUMAParentAccessCodeUsage,
ParentAccessView::UMAUsage::kTimezoneChange, 1);

// The below usage depends on the session state.
GetSessionControllerClient()->SetSessionState(
session_manager::SessionState::ACTIVE);
ShowWidget(ParentAccessRequestReason::kChangeTime);
DismissWidget();
histogram_tester_.ExpectBucketCount(
ParentAccessView::kUMAParentAccessCodeUsage,
ParentAccessView::UMAUsage::kTimeChangeInSession, 1);

GetSessionControllerClient()->SetSessionState(
session_manager::SessionState::LOGIN_PRIMARY);
ShowWidget(ParentAccessRequestReason::kChangeTime);
DismissWidget();
histogram_tester_.ExpectBucketCount(
ParentAccessView::kUMAParentAccessCodeUsage,
ParentAccessView::UMAUsage::kTimeChangeLoginScreen, 1);

GetSessionControllerClient()->SetSessionState(
session_manager::SessionState::ACTIVE);
ShowWidget(ParentAccessRequestReason::kChangeTime);
DismissWidget();
histogram_tester_.ExpectBucketCount(
ParentAccessView::kUMAParentAccessCodeUsage,
ParentAccessView::UMAUsage::kTimeChangeInSession, 2);

histogram_tester_.ExpectTotalCount(
ParentAccessView::kUMAParentAccessCodeUsage, 5);
}

} // namespace ash
16 changes: 16 additions & 0 deletions tools/metrics/histograms/enums.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45956,6 +45956,22 @@ Called by update_net_trust_anchors.py.-->
<int value="9" label="Unknown range support from the response header."/>
</enum>

<enum name="ParentAccessCodeAction">
<summary>Action resulting from parent access code dialog</summary>
<int value="0" label="Validation success"/>
<int value="1" label="Validation error"/>
<int value="2" label="Validation canceled by user"/>
<int value="3" label="Get help user action"/>
</enum>

<enum name="ParentAccessCodeUsage">
<summary>Context in which parent access code was used</summary>
<int value="0" label="Time limits"/>
<int value="1" label="Time change on login screen"/>
<int value="2" label="Time change user session"/>
<int value="3" label="Timezone change"/>
</enum>

<enum name="ParentFrameKnown">
<int value="0" label="Parent Frame Not Known"/>
<int value="1" label="Parent Frame Known"/>
Expand Down
20 changes: 20 additions & 0 deletions tools/metrics/histograms/histograms.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137286,6 +137286,26 @@ should be kept until we use this API. -->
</summary>
</histogram>

<histogram name="Supervision.ParentAccessCode.Action"
enum="ParentAccessCodeAction" expires_after="M88">
<owner>agawronska@chromium.org</owner>
<owner>cros-families@google.com</owner>
<summary>
Action originated in parent access code dialog. Logged every time the action
happens.
</summary>
</histogram>

<histogram name="Supervision.ParentAccessCode.Usage"
enum="ParentAccessCodeUsage" expires_after="M88">
<owner>agawronska@chromium.org</owner>
<owner>cros-families@google.com</owner>
<summary>
The context in which parent access code was used. Logged every time the new
parent access dialog is shown.
</summary>
</histogram>

<histogram name="Supervision.StatusReport.Event"
enum="SupervisionStatusReportEvent" expires_after="M85">
<owner>escordeiro@google.com</owner>
Expand Down

0 comments on commit 8dc25af

Please sign in to comment.