diff options
author | bruthig <bruthig@chromium.org> | 2015-04-27 11:45:39 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-04-27 18:46:18 +0000 |
commit | d89b76b49964e9b764fc93b2e8473e5120b74bf2 (patch) | |
tree | a9a5a5ed08449437d2b490bf70cd680f929a3ff3 /ash | |
parent | dfd59a02cb534d20419dd62af462e47865bd7747 (diff) | |
download | chromium_src-d89b76b49964e9b764fc93b2e8473e5120b74bf2.zip chromium_src-d89b76b49964e9b764fc93b2e8473e5120b74bf2.tar.gz chromium_src-d89b76b49964e9b764fc93b2e8473e5120b74bf2.tar.bz2 |
Changed when the UMA metric Ash.NumberOfVisibleWindowsInPrimaryDisplay is recorded.
Ash.NumberOfVisibleWindowsInPrimaryDisplay was incorrectly being recorded at the
login and lock screen.
TEST=UserMetricsRecorderTest.VerifyIsUserInActiveDesktopEnvironmentValues
TEST=UserMetricsRecorderTest.VerifyStatsRecordedWhenUserNotInActiveDesktopEnvironment
TEST=UserMetricsRecorderTest.VerifyStatsRecordedWhenUserInActiveDesktopEnvironment
TEST=UserMetricsRecorderTest.VerifyStatsRecordedByRecordPeriodicMetrics
BUG=477538
Review URL: https://codereview.chromium.org/1093483002
Cr-Commit-Position: refs/heads/master@{#327088}
Diffstat (limited to 'ash')
-rw-r--r-- | ash/ash.gyp | 3 | ||||
-rw-r--r-- | ash/metrics/user_metrics_recorder.cc | 60 | ||||
-rw-r--r-- | ash/metrics/user_metrics_recorder.h | 32 | ||||
-rw-r--r-- | ash/metrics/user_metrics_recorder_unittest.cc | 162 | ||||
-rw-r--r-- | ash/test/user_metrics_recorder_test_api.cc | 26 | ||||
-rw-r--r-- | ash/test/user_metrics_recorder_test_api.h | 37 |
6 files changed, 313 insertions, 7 deletions
diff --git a/ash/ash.gyp b/ash/ash.gyp index 0ade666..ff45e6c 100644 --- a/ash/ash.gyp +++ b/ash/ash.gyp @@ -717,6 +717,8 @@ 'test/test_volume_control_delegate.h', 'test/ui_controls_factory_ash.cc', 'test/ui_controls_factory_ash.h', + 'test/user_metrics_recorder_test_api.cc', + 'test/user_metrics_recorder_test_api.h', 'test/virtual_keyboard_test_helper.cc', 'test/virtual_keyboard_test_helper.h', ], @@ -791,6 +793,7 @@ 'keyboard_overlay/keyboard_overlay_delegate_unittest.cc', 'keyboard_overlay/keyboard_overlay_view_unittest.cc', 'magnifier/magnification_controller_unittest.cc', + 'metrics/user_metrics_recorder_unittest.cc', 'popup_message_unittest.cc', 'root_window_controller_unittest.cc', 'screen_util_unittest.cc', diff --git a/ash/metrics/user_metrics_recorder.cc b/ash/metrics/user_metrics_recorder.cc index 89278e4..f7e1f42 100644 --- a/ash/metrics/user_metrics_recorder.cc +++ b/ash/metrics/user_metrics_recorder.cc @@ -4,11 +4,13 @@ #include "ash/metrics/user_metrics_recorder.h" +#include "ash/session/session_state_delegate.h" #include "ash/shelf/shelf_layout_manager.h" #include "ash/shelf/shelf_view.h" #include "ash/shelf/shelf_widget.h" #include "ash/shell.h" #include "ash/shell_window_ids.h" +#include "ash/system/tray/system_tray_delegate.h" #include "ash/wm/window_state.h" #include "base/metrics/histogram.h" #include "base/metrics/user_metrics.h" @@ -64,6 +66,31 @@ ActiveWindowStateType GetActiveWindowState() { return active_window_state_type; } +// Returns true if kiosk mode is active. +bool IsKioskModeActive() { + return Shell::GetInstance()->system_tray_delegate()->GetUserLoginStatus() == + user::LOGGED_IN_KIOSK_APP; +} + +// Returns true if there is an active user and their session isn't currently +// locked. +bool IsUserActive() { + switch (Shell::GetInstance()->system_tray_delegate()->GetUserLoginStatus()) { + case user::LOGGED_IN_NONE: + case user::LOGGED_IN_LOCKED: + return false; + case user::LOGGED_IN_USER: + case user::LOGGED_IN_OWNER: + case user::LOGGED_IN_GUEST: + case user::LOGGED_IN_PUBLIC: + case user::LOGGED_IN_SUPERVISED: + case user::LOGGED_IN_KIOSK_APP: + return true; + } + NOTREACHED(); + return false; +} + // Array of window container ids that contain visible windows to be counted for // UMA statistics. Note the containers are ordered from top most visible // container to the lowest to allow the |GetNumVisibleWindows| method to short @@ -124,10 +151,12 @@ int GetNumVisibleWindowsInPrimaryDisplay() { } // namespace UserMetricsRecorder::UserMetricsRecorder() { - timer_.Start(FROM_HERE, - base::TimeDelta::FromSeconds(kAshPeriodicMetricsTimeInSeconds), - this, - &UserMetricsRecorder::RecordPeriodicMetrics); + StartTimer(); +} + +UserMetricsRecorder::UserMetricsRecorder(bool record_periodic_metrics) { + if (record_periodic_metrics) + StartTimer(); } UserMetricsRecorder::~UserMetricsRecorder() { @@ -525,7 +554,10 @@ void UserMetricsRecorder::RecordUserMetricsAction(UserMetricsAction action) { void UserMetricsRecorder::RecordPeriodicMetrics() { ShelfLayoutManager* manager = ShelfLayoutManager::ForShelf(Shell::GetPrimaryRootWindow()); + // TODO(bruthig): Investigating whether the check for |manager| is necessary + // and add tests if it is. if (manager) { + // TODO(bruthig): Consider tracking the time spent in each alignment. UMA_HISTOGRAM_ENUMERATION("Ash.ShelfAlignmentOverTime", manager->SelectValueForShelfAlignment( SHELF_ALIGNMENT_UMA_ENUM_VALUE_BOTTOM, @@ -535,12 +567,28 @@ void UserMetricsRecorder::RecordPeriodicMetrics() { SHELF_ALIGNMENT_UMA_ENUM_VALUE_COUNT); } - UMA_HISTOGRAM_COUNTS_100("Ash.NumberOfVisibleWindowsInPrimaryDisplay", - GetNumVisibleWindowsInPrimaryDisplay()); + if (IsUserInActiveDesktopEnvironment()) { + UMA_HISTOGRAM_COUNTS_100("Ash.NumberOfVisibleWindowsInPrimaryDisplay", + GetNumVisibleWindowsInPrimaryDisplay()); + } + // TODO(bruthig): Find out if this should only be logged when the user is + // active. + // TODO(bruthig): Consider tracking how long a particular type of window is + // active at a time. UMA_HISTOGRAM_ENUMERATION("Ash.ActiveWindowShowTypeOverTime", GetActiveWindowState(), ACTIVE_WINDOW_STATE_TYPE_COUNT); } +bool UserMetricsRecorder::IsUserInActiveDesktopEnvironment() const { + return IsUserActive() && !IsKioskModeActive(); +} + +void UserMetricsRecorder::StartTimer() { + timer_.Start(FROM_HERE, + base::TimeDelta::FromSeconds(kAshPeriodicMetricsTimeInSeconds), + this, &UserMetricsRecorder::RecordPeriodicMetrics); +} + } // namespace ash diff --git a/ash/metrics/user_metrics_recorder.h b/ash/metrics/user_metrics_recorder.h index 0986206..a2f5fb1 100644 --- a/ash/metrics/user_metrics_recorder.h +++ b/ash/metrics/user_metrics_recorder.h @@ -10,6 +10,10 @@ namespace ash { +namespace test { +class UserMetricsRecorderTestAPI; +} + enum UserMetricsAction { UMA_ACCEL_EXIT_FIRST_Q, UMA_ACCEL_EXIT_SECOND_Q, @@ -129,14 +133,40 @@ enum UserMetricsAction { // (RecordUserMetricsAction) are passed through the UserMetricsRecorder. class ASH_EXPORT UserMetricsRecorder { public: + // Creates a UserMetricsRecorder that records metrics periodically. Equivalent + // to calling UserMetricsRecorder(true). UserMetricsRecorder(); - ~UserMetricsRecorder(); + virtual ~UserMetricsRecorder(); + + // Records an Ash owned user action. void RecordUserMetricsAction(ash::UserMetricsAction action); + private: + friend class test::UserMetricsRecorderTestAPI; + + // Creates a UserMetricsRecorder and will only record periodic metrics if + // |record_periodic_metrics| is true. This is used by tests that do not want + // the timer to be started. + // TODO(bruthig): Add a constructor that accepts a base::RepeatingTimer so + // that tests can inject a test double that can be controlled by the test. The + // missing piece is a suitable base::RepeatingTimer test double. + explicit UserMetricsRecorder(bool record_periodic_metrics); + + // Records UMA metrics. Invoked periodically by the |timer_|. void RecordPeriodicMetrics(); + // Returns true if the user's session is active and they are in a desktop + // environment. + bool IsUserInActiveDesktopEnvironment() const; + + // Starts the |timer_| and binds it to |RecordPeriodicMetrics|. + void StartTimer(); + + // The periodic timer that triggers metrics to be recorded. base::RepeatingTimer<UserMetricsRecorder> timer_; + + DISALLOW_COPY_AND_ASSIGN(UserMetricsRecorder); }; } // namespace ash diff --git a/ash/metrics/user_metrics_recorder_unittest.cc b/ash/metrics/user_metrics_recorder_unittest.cc new file mode 100644 index 0000000..04cbd1c --- /dev/null +++ b/ash/metrics/user_metrics_recorder_unittest.cc @@ -0,0 +1,162 @@ +// Copyright 2015 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 "ash/metrics/user_metrics_recorder.h" + +#include "ash/shell.h" +#include "ash/system/user/login_status.h" +#include "ash/test/ash_test_base.h" +#include "ash/test/test_system_tray_delegate.h" +#include "ash/test/user_metrics_recorder_test_api.h" +#include "base/memory/scoped_ptr.h" +#include "base/test/histogram_tester.h" +#include "ui/aura/window.h" + +namespace ash { +namespace { + +const char kAsh_NumberOfVisibleWindowsInPrimaryDisplay[] = + "Ash.NumberOfVisibleWindowsInPrimaryDisplay"; + +const char kAsh_ActiveWindowShowTypeOverTime[] = + "Ash.ActiveWindowShowTypeOverTime"; + +} // namespace + +// Test fixture for the UserMetricsRecorder class. +class UserMetricsRecorderTest : public test::AshTestBase { + public: + UserMetricsRecorderTest(); + ~UserMetricsRecorderTest() override; + + // test::AshTestBase: + void SetUp() override; + void TearDown() override; + + // Sets the user login status. + void SetLoginStatus(user::LoginStatus login_status); + + // Sets the current user session to be active or inactive in a desktop + // environment. + void SetUserInActiveDesktopEnvironment(bool is_active); + + test::UserMetricsRecorderTestAPI* user_metrics_recorder_test_api() { + return user_metrics_recorder_test_api_.get(); + } + + base::HistogramTester& histograms() { return histograms_; } + + private: + // Test API to access private members of the test target. + scoped_ptr<test::UserMetricsRecorderTestAPI> user_metrics_recorder_test_api_; + + // Histogram value verifier. + base::HistogramTester histograms_; + + // The active SystemTrayDelegate. Not owned. + test::TestSystemTrayDelegate* test_system_tray_delegate_; + + DISALLOW_COPY_AND_ASSIGN(UserMetricsRecorderTest); +}; + +UserMetricsRecorderTest::UserMetricsRecorderTest() { +} + +UserMetricsRecorderTest::~UserMetricsRecorderTest() { +} + +void UserMetricsRecorderTest::SetUp() { + test::AshTestBase::SetUp(); + user_metrics_recorder_test_api_.reset(new test::UserMetricsRecorderTestAPI()); + test_system_tray_delegate_ = GetSystemTrayDelegate(); +} + +void UserMetricsRecorderTest::TearDown() { + test_system_tray_delegate_ = nullptr; + test::AshTestBase::TearDown(); +} + +void UserMetricsRecorderTest::SetLoginStatus(user::LoginStatus login_status) { + test_system_tray_delegate_->SetLoginStatus(login_status); +} + +void UserMetricsRecorderTest::SetUserInActiveDesktopEnvironment( + bool is_active) { + if (is_active) { + SetLoginStatus(user::LOGGED_IN_USER); + ASSERT_TRUE( + user_metrics_recorder_test_api()->IsUserInActiveDesktopEnvironment()); + } else { + SetLoginStatus(user::LOGGED_IN_LOCKED); + ASSERT_FALSE( + user_metrics_recorder_test_api()->IsUserInActiveDesktopEnvironment()); + } +} + +// Verifies the return value of IsUserInActiveDesktopEnvironment() for the +// different login status values. +TEST_F(UserMetricsRecorderTest, VerifyIsUserInActiveDesktopEnvironmentValues) { + SetLoginStatus(user::LOGGED_IN_NONE); + EXPECT_FALSE( + user_metrics_recorder_test_api()->IsUserInActiveDesktopEnvironment()); + + SetLoginStatus(user::LOGGED_IN_LOCKED); + EXPECT_FALSE( + user_metrics_recorder_test_api()->IsUserInActiveDesktopEnvironment()); + + SetLoginStatus(user::LOGGED_IN_USER); + EXPECT_TRUE( + user_metrics_recorder_test_api()->IsUserInActiveDesktopEnvironment()); + + SetLoginStatus(user::LOGGED_IN_OWNER); + EXPECT_TRUE( + user_metrics_recorder_test_api()->IsUserInActiveDesktopEnvironment()); + + SetLoginStatus(user::LOGGED_IN_GUEST); + EXPECT_TRUE( + user_metrics_recorder_test_api()->IsUserInActiveDesktopEnvironment()); + + SetLoginStatus(user::LOGGED_IN_PUBLIC); + EXPECT_TRUE( + user_metrics_recorder_test_api()->IsUserInActiveDesktopEnvironment()); + + SetLoginStatus(user::LOGGED_IN_SUPERVISED); + EXPECT_TRUE( + user_metrics_recorder_test_api()->IsUserInActiveDesktopEnvironment()); + + SetLoginStatus(user::LOGGED_IN_KIOSK_APP); + EXPECT_FALSE( + user_metrics_recorder_test_api()->IsUserInActiveDesktopEnvironment()); +} + +// Verifies that the IsUserInActiveDesktopEnvironment() dependent stats are not +// recorded when a user is not active in a desktop environment. +TEST_F(UserMetricsRecorderTest, + VerifyStatsRecordedWhenUserNotInActiveDesktopEnvironment) { + SetUserInActiveDesktopEnvironment(false); + user_metrics_recorder_test_api()->RecordPeriodicMetrics(); + + histograms().ExpectTotalCount(kAsh_NumberOfVisibleWindowsInPrimaryDisplay, 0); +} + +// Verifies that the IsUserInActiveDesktopEnvironment() dependent stats are +// recorded when a user is active in a desktop environment. +TEST_F(UserMetricsRecorderTest, + VerifyStatsRecordedWhenUserInActiveDesktopEnvironment) { + SetUserInActiveDesktopEnvironment(true); + user_metrics_recorder_test_api()->RecordPeriodicMetrics(); + + histograms().ExpectTotalCount(kAsh_NumberOfVisibleWindowsInPrimaryDisplay, 1); +} + +// Verifies recording of stats which are always recorded by +// RecordPeriodicMetrics. +TEST_F(UserMetricsRecorderTest, VerifyStatsRecordedByRecordPeriodicMetrics) { + SetUserInActiveDesktopEnvironment(true); + user_metrics_recorder_test_api()->RecordPeriodicMetrics(); + + histograms().ExpectTotalCount(kAsh_ActiveWindowShowTypeOverTime, 1); +} + +} // namespace ash diff --git a/ash/test/user_metrics_recorder_test_api.cc b/ash/test/user_metrics_recorder_test_api.cc new file mode 100644 index 0000000..b9a5eac --- /dev/null +++ b/ash/test/user_metrics_recorder_test_api.cc @@ -0,0 +1,26 @@ +// Copyright 2015 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 "ash/test/user_metrics_recorder_test_api.h" + +namespace ash { +namespace test { + +UserMetricsRecorderTestAPI::UserMetricsRecorderTestAPI() + : user_metrics_recorder_(false) { +} + +UserMetricsRecorderTestAPI::~UserMetricsRecorderTestAPI() { +} + +void UserMetricsRecorderTestAPI::RecordPeriodicMetrics() { + user_metrics_recorder_.RecordPeriodicMetrics(); +} + +bool UserMetricsRecorderTestAPI::IsUserInActiveDesktopEnvironment() const { + return user_metrics_recorder_.IsUserInActiveDesktopEnvironment(); +} + +} // namespace test +} // namespace ash diff --git a/ash/test/user_metrics_recorder_test_api.h b/ash/test/user_metrics_recorder_test_api.h new file mode 100644 index 0000000..e52ef0b --- /dev/null +++ b/ash/test/user_metrics_recorder_test_api.h @@ -0,0 +1,37 @@ +// Copyright 2015 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 ASH_TEST_USER_METRICS_RECORDER_TEST_API_H_ +#define ASH_TEST_USER_METRICS_RECORDER_TEST_API_H_ + +#include "ash/metrics/user_metrics_recorder.h" +#include "base/macros.h" +#include "base/memory/scoped_ptr.h" + +namespace ash { +namespace test { + +// Test API to access internals of the UserMetricsRecorder class. +class UserMetricsRecorderTestAPI { + public: + UserMetricsRecorderTestAPI(); + ~UserMetricsRecorderTestAPI(); + + // Accessor to UserMetricsRecorder::RecordPeriodicMetrics(). + void RecordPeriodicMetrics(); + + // Accessor to UserMetricsRecorder::IsUserInActiveDesktopEnvironment(). + bool IsUserInActiveDesktopEnvironment() const; + + private: + // The UserMetricsRecorder that |this| is providing internal access to. + UserMetricsRecorder user_metrics_recorder_; + + DISALLOW_COPY_AND_ASSIGN(UserMetricsRecorderTestAPI); +}; + +} // namespace test +} // namespace ash + +#endif // ASH_TEST_USER_METRICS_RECORDER_TEST_API_H_ |