Commit 45d37acc authored by Jia's avatar Jia Committed by Commit Bot

[On-device adaptive brightness] Log UserAdjustment counts to UMA.

We split user adjustments into two types: with or without prior model
adjustments.

Bug: 881215
Change-Id: I34c6084ef96cfcf81f97c0825951a340934c4557
Reviewed-on: https://chromium-review.googlesource.com/c/1312179
Commit-Queue: Jia Meng <jiameng@chromium.org>
Reviewed-by: default avatarDan Erat <derat@chromium.org>
Reviewed-by: default avatarSteven Holte <holte@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605250}
parent ff62446d
...@@ -8,6 +8,7 @@ ...@@ -8,6 +8,7 @@
#include "base/logging.h" #include "base/logging.h"
#include "base/memory/ptr_util.h" #include "base/memory/ptr_util.h"
#include "base/metrics/field_trial_params.h" #include "base/metrics/field_trial_params.h"
#include "base/metrics/histogram_macros.h"
#include "base/time/default_tick_clock.h" #include "base/time/default_tick_clock.h"
#include "chrome/browser/chromeos/power/auto_screen_brightness/utils.h" #include "chrome/browser/chromeos/power/auto_screen_brightness/utils.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
...@@ -88,6 +89,11 @@ void Adapter::OnUserBrightnessChanged(double old_brightness_percent, ...@@ -88,6 +89,11 @@ void Adapter::OnUserBrightnessChanged(double old_brightness_percent,
double new_brightness_percent) {} double new_brightness_percent) {}
void Adapter::OnUserBrightnessChangeRequested() { void Adapter::OnUserBrightnessChangeRequested() {
UMA_HISTOGRAM_ENUMERATION("AutoScreenBrightness.UserAdjustment",
latest_brightness_change_time_.is_null()
? UserAdjustment::kNoPriorModelAdjustment
: UserAdjustment::kWithPriorModelAdjustment);
// This will disable |adapter_status_| so that the model will not make any // This will disable |adapter_status_| so that the model will not make any
// brightness adjustment. // brightness adjustment.
adapter_status_ = Status::kDisabled; adapter_status_ = Status::kDisabled;
......
...@@ -47,6 +47,14 @@ class Adapter : public AlsReader::Observer, ...@@ -47,6 +47,14 @@ class Adapter : public AlsReader::Observer,
kMaxValue = kLatest kMaxValue = kLatest
}; };
// These values are persisted to logs. Entries should not be renumbered and
// numeric values should never be reused.
enum class UserAdjustment {
kNoPriorModelAdjustment = 0,
kWithPriorModelAdjustment = 1,
kMaxValue = kWithPriorModelAdjustment
};
// TODO(jiameng): we currently use past 2 seconds of ambient values to // TODO(jiameng): we currently use past 2 seconds of ambient values to
// calculate average ambient when we predict optimal brightness. This is // calculate average ambient when we predict optimal brightness. This is
// shorter than the duration used for training data (10 seconds), because it's // shorter than the duration used for training data (10 seconds), because it's
......
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include "ash/public/cpp/ash_pref_names.h" #include "ash/public/cpp/ash_pref_names.h"
#include "base/memory/ptr_util.h" #include "base/memory/ptr_util.h"
#include "base/task/task_scheduler/task_scheduler.h" #include "base/task/task_scheduler/task_scheduler.h"
#include "base/test/metrics/histogram_tester.h"
#include "base/test/scoped_feature_list.h" #include "base/test/scoped_feature_list.h"
#include "base/test/scoped_task_environment.h" #include "base/test/scoped_task_environment.h"
#include "base/threading/sequenced_task_runner_handle.h" #include "base/threading/sequenced_task_runner_handle.h"
...@@ -34,6 +35,14 @@ namespace auto_screen_brightness { ...@@ -34,6 +35,14 @@ namespace auto_screen_brightness {
namespace { namespace {
using SamplesCounts = std::vector<
std::pair<base::HistogramBase::Sample, base::HistogramBase::Count>>;
constexpr auto kNoPriorModelAdjustment =
static_cast<int>(Adapter::UserAdjustment::kNoPriorModelAdjustment);
constexpr auto kWithPriorModelAdjustment =
static_cast<int>(Adapter::UserAdjustment::kWithPriorModelAdjustment);
// Testing modeller. // Testing modeller.
class FakeModeller : public Modeller { class FakeModeller : public Modeller {
public: public:
...@@ -196,10 +205,25 @@ class AdapterTest : public testing::Test { ...@@ -196,10 +205,25 @@ class AdapterTest : public testing::Test {
scoped_task_environment_.RunUntilIdle(); scoped_task_environment_.RunUntilIdle();
} }
void VerifyHistogramCounts(
const std::map<std::string, SamplesCounts>& expected_values) {
for (const auto& histogram_samples : expected_values) {
const std::string& histogram_name = histogram_samples.first;
const SamplesCounts& samples_counts = histogram_samples.second;
for (const auto& sample_count : samples_counts) {
const base::HistogramBase::Sample& sample = sample_count.first;
const base::HistogramBase::Count& count = sample_count.second;
histogram_tester_.ExpectUniqueSample(histogram_name, sample, count);
}
}
}
protected: protected:
base::test::ScopedTaskEnvironment scoped_task_environment_; base::test::ScopedTaskEnvironment scoped_task_environment_;
content::TestBrowserThreadBundle thread_bundle_; content::TestBrowserThreadBundle thread_bundle_;
base::HistogramTester histogram_tester_;
TestObserver test_observer_; TestObserver test_observer_;
base::ScopedTempDir temp_dir_; base::ScopedTempDir temp_dir_;
...@@ -385,6 +409,44 @@ TEST_F(AdapterTest, SequenceOfBrightnessUpdatesWithDefaultParams) { ...@@ -385,6 +409,44 @@ TEST_F(AdapterTest, SequenceOfBrightnessUpdatesWithDefaultParams) {
fake_brightness_monitor_.ReportUserBrightnessChangeRequested(); fake_brightness_monitor_.ReportUserBrightnessChangeRequested();
scoped_task_environment_.RunUntilIdle(); scoped_task_environment_.RunUntilIdle();
EXPECT_EQ(adapter_->GetStatusForTesting(), Adapter::Status::kDisabled); EXPECT_EQ(adapter_->GetStatusForTesting(), Adapter::Status::kDisabled);
VerifyHistogramCounts({{"AutoScreenBrightness.UserAdjustment",
{{kWithPriorModelAdjustment, 1}}}});
// Another user manual adjustment came in.
fake_brightness_monitor_.ReportUserBrightnessChangeRequested();
scoped_task_environment_.RunUntilIdle();
EXPECT_EQ(adapter_->GetStatusForTesting(), Adapter::Status::kDisabled);
VerifyHistogramCounts({{"AutoScreenBrightness.UserAdjustment",
{{kWithPriorModelAdjustment, 2}}}});
}
TEST_F(AdapterTest, UserBrightnessRequestBeforeAnyModelUpdate) {
Init(AlsReader::AlsInitStatus::kSuccess, BrightnessMonitor::Status::kSuccess,
global_curve_, personal_curve_, default_params_);
EXPECT_EQ(adapter_->GetStatusForTesting(), Adapter::Status::kSuccess);
EXPECT_TRUE(adapter_->GetGlobalCurveForTesting());
EXPECT_EQ(*adapter_->GetGlobalCurveForTesting(), *global_curve_);
EXPECT_TRUE(adapter_->GetPersonalCurveForTesting());
EXPECT_EQ(*adapter_->GetPersonalCurveForTesting(), *personal_curve_);
// Adapter is disabled after a user manual adjustment.
fake_brightness_monitor_.ReportUserBrightnessChangeRequested();
scoped_task_environment_.RunUntilIdle();
EXPECT_EQ(adapter_->GetStatusForTesting(), Adapter::Status::kDisabled);
VerifyHistogramCounts({{"AutoScreenBrightness.UserAdjustment",
{{kNoPriorModelAdjustment, 1}}}});
// Another user manual adjustment came in.
fake_brightness_monitor_.ReportUserBrightnessChangeRequested();
scoped_task_environment_.RunUntilIdle();
EXPECT_EQ(adapter_->GetStatusForTesting(), Adapter::Status::kDisabled);
VerifyHistogramCounts({{"AutoScreenBrightness.UserAdjustment",
{{kNoPriorModelAdjustment, 2}}}});
} }
TEST_F(AdapterTest, BrightnessLuxThresholds) { TEST_F(AdapterTest, BrightnessLuxThresholds) {
......
...@@ -3243,6 +3243,11 @@ uploading your change for review. These are checked by presubmit scripts. ...@@ -3243,6 +3243,11 @@ uploading your change for review. These are checked by presubmit scripts.
<int value="2" label="dual source"/> <int value="2" label="dual source"/>
</enum> </enum>
<enum name="AutoScreenBrightnessUserAdjustment">
<int value="0" label="NoPriorModelAdjustment"/>
<int value="1" label="WithPriorModelAdjustment"/>
</enum>
<enum name="AutoSigninFirstRun"> <enum name="AutoSigninFirstRun">
<int value="0" label="No action"/> <int value="0" label="No action"/>
<int value="1" label="Turn off"/> <int value="1" label="Turn off"/>
...@@ -8140,6 +8140,12 @@ uploading your change for review. ...@@ -8140,6 +8140,12 @@ uploading your change for review.
<summary>Whether the user changed autofilled field.</summary> <summary>Whether the user changed autofilled field.</summary>
</histogram> </histogram>
<histogram name="AutoScreenBrightness.UserAdjustment"
enum="AutoScreenBrightnessUserAdjustment">
<owner>jiameng@chromium.org</owner>
<summary>Type of user manual screen brightness adjustment.</summary>
</histogram>
<histogram base="true" name="BackgroundFetch.EventDispatchFailure.Dispatch" <histogram base="true" name="BackgroundFetch.EventDispatchFailure.Dispatch"
enum="ServiceWorkerStatusCode"> enum="ServiceWorkerStatusCode">
<!-- Name completed by histogram_suffixes name="BackgroundFetchEvents" --> <!-- Name completed by histogram_suffixes name="BackgroundFetchEvents" -->
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