Commit 56373214 authored by Zentaro Kavanagh's avatar Zentaro Kavanagh Committed by Commit Bot

Ambient EQ: Only gate on feature, don't check sensor

- This disables the feature by default
- Updates tests based on changed feature state
- Fixes a race condition checking the sensor status via powerd
- Support for ambient EQ is now going to be gated on a cros config value
  because some devices have the sensor but can't support the feature
- The cros config value will enable the feature only on explicitly
  supported devices
- This is a partial revert of [1] which can't be cleanly reverted
  because the pref value on unsupported devices may have already
  been set

[1] - https://chromium-review.googlesource.com/c/chromium/src/+/1974815

BUG=1034068,1036038,1036546
TEST=NightLightTest.*,manual

Change-Id: If51ec1b6a8ae787fb64b89f2f536eec0087b09af
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1992183Reviewed-by: default avatarSteven Bennetts <stevenjb@chromium.org>
Reviewed-by: default avatarAhmed Fakhry <afakhry@chromium.org>
Reviewed-by: default avatarBailey Berro <baileyberro@chromium.org>
Commit-Queue: Zentaro Kavanagh <zentaro@chromium.org>
Cr-Commit-Position: refs/heads/master@{#730247}
parent 4b68bd23
...@@ -13,7 +13,7 @@ namespace ash { ...@@ -13,7 +13,7 @@ namespace ash {
namespace features { namespace features {
const base::Feature kAllowAmbientEQ{"AllowAmbientEQ", const base::Feature kAllowAmbientEQ{"AllowAmbientEQ",
base::FEATURE_ENABLED_BY_DEFAULT}; base::FEATURE_DISABLED_BY_DEFAULT};
const base::Feature kAutoNightLight{"AutoNightLight", const base::Feature kAutoNightLight{"AutoNightLight",
base::FEATURE_DISABLED_BY_DEFAULT}; base::FEATURE_DISABLED_BY_DEFAULT};
......
...@@ -568,10 +568,7 @@ void NightLightControllerImpl::SetAmbientColorEnabled(bool enabled) { ...@@ -568,10 +568,7 @@ void NightLightControllerImpl::SetAmbientColorEnabled(bool enabled) {
} }
bool NightLightControllerImpl::GetAmbientColorEnabled() const { bool NightLightControllerImpl::GetAmbientColorEnabled() const {
const bool ambient_eq_supported = return features::IsAllowAmbientEQEnabled() && active_user_pref_service_ &&
features::IsAllowAmbientEQEnabled() &&
chromeos::PowerManagerClient::Get()->SupportsAmbientColor();
return ambient_eq_supported && active_user_pref_service_ &&
active_user_pref_service_->GetBoolean(prefs::kAmbientColorEnabled); active_user_pref_service_->GetBoolean(prefs::kAmbientColorEnabled);
} }
......
...@@ -28,7 +28,6 @@ ...@@ -28,7 +28,6 @@
#include "base/optional.h" #include "base/optional.h"
#include "base/strings/pattern.h" #include "base/strings/pattern.h"
#include "base/test/scoped_feature_list.h" #include "base/test/scoped_feature_list.h"
#include "chromeos/dbus/power/fake_power_manager_client.h"
#include "components/prefs/pref_service.h" #include "components/prefs/pref_service.h"
#include "ui/compositor/layer.h" #include "ui/compositor/layer.h"
#include "ui/display/fake/fake_display_snapshot.h" #include "ui/display/fake/fake_display_snapshot.h"
...@@ -196,7 +195,6 @@ class NightLightTest : public NoSessionAshTestBase { ...@@ -196,7 +195,6 @@ class NightLightTest : public NoSessionAshTestBase {
// Start with ambient color pref disabled. // Start with ambient color pref disabled.
SetAmbientColorPrefEnabled(false); SetAmbientColorPrefEnabled(false);
SetAmbientColorSupported(false);
} }
void CreateTestUserSessions() { void CreateTestUserSessions() {
...@@ -219,12 +217,6 @@ class NightLightTest : public NoSessionAshTestBase { ...@@ -219,12 +217,6 @@ class NightLightTest : public NoSessionAshTestBase {
GetController()->SetAmbientColorEnabled(enabled); GetController()->SetAmbientColorEnabled(enabled);
} }
void SetAmbientColorSupported(bool supported) {
static_cast<chromeos::FakePowerManagerClient*>(
chromeos::PowerManagerClient::Get())
->set_supports_ambient_color(supported);
}
// Simulate powerd sending multiple times an ambient temperature of // Simulate powerd sending multiple times an ambient temperature of
// |powerd_temperature|. The remapped ambient temperature should eventually // |powerd_temperature|. The remapped ambient temperature should eventually
// reach |target_remapped_temperature|. // reach |target_remapped_temperature|.
...@@ -999,52 +991,31 @@ TEST_F(NightLightTest, TestCustomScheduleInvertedStartAndEndTimesCase3) { ...@@ -999,52 +991,31 @@ TEST_F(NightLightTest, TestCustomScheduleInvertedStartAndEndTimesCase3) {
controller->timer()->GetCurrentDelay()); controller->timer()->GetCurrentDelay());
} }
TEST_F(NightLightTest, TestAmbientLightEnabledSetting) { TEST_F(NightLightTest, TestAmbientLightEnabledSetting_FeatureOn) {
// Feature enabled, Device not supported, Pref disabled -> disabled base::test::ScopedFeatureList features;
SetAmbientColorPrefEnabled(false); features.InitAndEnableFeature(features::kAllowAmbientEQ);
SetAmbientColorSupported(false);
EXPECT_FALSE(GetController()->GetAmbientColorEnabled());
// Feature enabled, Device not supported, Pref enabled -> disabled // Feature enabled, Pref disabled -> disabled
SetAmbientColorPrefEnabled(true);
SetAmbientColorSupported(false);
EXPECT_FALSE(GetController()->GetAmbientColorEnabled());
// Feature enabled, Device not supported, Pref enabled -> disabled
SetAmbientColorPrefEnabled(false); SetAmbientColorPrefEnabled(false);
SetAmbientColorSupported(true);
EXPECT_FALSE(GetController()->GetAmbientColorEnabled()); EXPECT_FALSE(GetController()->GetAmbientColorEnabled());
// Feature enabled, Device supported, Pref enabled -> enabled // Feature enabled, Pref enabled -> enabled
SetAmbientColorPrefEnabled(true); SetAmbientColorPrefEnabled(true);
SetAmbientColorSupported(true);
EXPECT_TRUE(GetController()->GetAmbientColorEnabled()); EXPECT_TRUE(GetController()->GetAmbientColorEnabled());
}
TEST_F(NightLightTest, TestAmbientLightEnabledSetting_FeatureOff) {
// With the feature disabled it should always be disabled. // With the feature disabled it should always be disabled.
{ base::test::ScopedFeatureList features;
base::test::ScopedFeatureList features; features.InitAndDisableFeature(features::kAllowAmbientEQ);
features.InitAndDisableFeature(features::kAllowAmbientEQ);
// Feature disabled, Device not supported, Pref disabled -> disabled
SetAmbientColorPrefEnabled(false);
SetAmbientColorSupported(false);
EXPECT_FALSE(GetController()->GetAmbientColorEnabled());
// Feature disabled, Device not supported, Pref enabled -> disabled
SetAmbientColorPrefEnabled(true);
SetAmbientColorSupported(false);
EXPECT_FALSE(GetController()->GetAmbientColorEnabled());
// Feature disabled, Device not supported, Pref enabled -> disabled // Feature disabled, Pref disabled -> disabled
SetAmbientColorPrefEnabled(false); SetAmbientColorPrefEnabled(false);
SetAmbientColorSupported(true); EXPECT_FALSE(GetController()->GetAmbientColorEnabled());
EXPECT_FALSE(GetController()->GetAmbientColorEnabled());
// Feature disabled, Device supported, Pref enabled -> disabled // Feature disabled, Pref enabled -> disabled
SetAmbientColorPrefEnabled(true); SetAmbientColorPrefEnabled(true);
SetAmbientColorSupported(true); EXPECT_FALSE(GetController()->GetAmbientColorEnabled());
EXPECT_FALSE(GetController()->GetAmbientColorEnabled());
}
} }
TEST_F(NightLightTest, TestAmbientLightRemappingTemperature) { TEST_F(NightLightTest, TestAmbientLightRemappingTemperature) {
...@@ -1082,9 +1053,10 @@ TEST_F(NightLightTest, TestAmbientLightRemappingTemperature) { ...@@ -1082,9 +1053,10 @@ TEST_F(NightLightTest, TestAmbientLightRemappingTemperature) {
} }
TEST_F(NightLightTest, TestAmbientColorMatrix) { TEST_F(NightLightTest, TestAmbientColorMatrix) {
base::test::ScopedFeatureList features;
features.InitAndEnableFeature(features::kAllowAmbientEQ);
SetNightLightEnabled(false); SetNightLightEnabled(false);
SetAmbientColorPrefEnabled(true); SetAmbientColorPrefEnabled(true);
SetAmbientColorSupported(true);
auto scaling_factors = GetAllDisplaysCompositorsRGBScaleFactors(); auto scaling_factors = GetAllDisplaysCompositorsRGBScaleFactors();
// If no temperature is set, we expect 1.0 for each scaling factor. // If no temperature is set, we expect 1.0 for each scaling factor.
for (const gfx::Vector3dF& rgb : scaling_factors) { for (const gfx::Vector3dF& rgb : scaling_factors) {
...@@ -1113,12 +1085,14 @@ TEST_F(NightLightTest, TestAmbientColorMatrix) { ...@@ -1113,12 +1085,14 @@ TEST_F(NightLightTest, TestAmbientColorMatrix) {
} }
TEST_F(NightLightTest, TestNightLightAndAmbientColorInteraction) { TEST_F(NightLightTest, TestNightLightAndAmbientColorInteraction) {
base::test::ScopedFeatureList features;
features.InitAndEnableFeature(features::kAllowAmbientEQ);
SetNightLightEnabled(true); SetNightLightEnabled(true);
auto night_light_rgb = GetAllDisplaysCompositorsRGBScaleFactors().front(); auto night_light_rgb = GetAllDisplaysCompositorsRGBScaleFactors().front();
SetAmbientColorPrefEnabled(true); SetAmbientColorPrefEnabled(true);
SetAmbientColorSupported(true);
auto night_light_and_ambient_rgb = auto night_light_and_ambient_rgb =
GetAllDisplaysCompositorsRGBScaleFactors().front(); GetAllDisplaysCompositorsRGBScaleFactors().front();
......
...@@ -99,7 +99,6 @@ ...@@ -99,7 +99,6 @@
#include "chrome/common/webui_url_constants.h" #include "chrome/common/webui_url_constants.h"
#include "chromeos/constants/chromeos_features.h" #include "chromeos/constants/chromeos_features.h"
#include "chromeos/constants/chromeos_switches.h" #include "chromeos/constants/chromeos_switches.h"
#include "chromeos/dbus/power/power_manager_client.h"
#include "chromeos/services/assistant/public/features.h" #include "chromeos/services/assistant/public/features.h"
#include "chromeos/services/multidevice_setup/public/cpp/url_provider.h" #include "chromeos/services/multidevice_setup/public/cpp/url_provider.h"
#include "chromeos/strings/grit/chromeos_strings.h" #include "chromeos/strings/grit/chromeos_strings.h"
...@@ -1091,10 +1090,8 @@ void AddDeviceStrings(content::WebUIDataSource* html_source) { ...@@ -1091,10 +1090,8 @@ void AddDeviceStrings(content::WebUIDataSource* html_source) {
html_source->AddBoolean("listAllDisplayModes", html_source->AddBoolean("listAllDisplayModes",
display::features::IsListAllDisplayModesEnabled()); display::features::IsListAllDisplayModesEnabled());
const bool ambient_eq_supported = html_source->AddBoolean("deviceSupportsAmbientColor",
ash::features::IsAllowAmbientEQEnabled() && ash::features::IsAllowAmbientEQEnabled());
chromeos::PowerManagerClient::Get()->SupportsAmbientColor();
html_source->AddBoolean("deviceSupportsAmbientColor", ambient_eq_supported);
html_source->AddBoolean( html_source->AddBoolean(
"enableTouchCalibrationSetting", "enableTouchCalibrationSetting",
......
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