diff --git a/ash/assistant/assistant_controller_impl.cc b/ash/assistant/assistant_controller_impl.cc index f3d988d6d449d..6a9175f861672 100644 --- a/ash/assistant/assistant_controller_impl.cc +++ b/ash/assistant/assistant_controller_impl.cc @@ -10,14 +10,11 @@ #include "ash/accessibility/accessibility_controller_impl.h" #include "ash/assistant/util/deep_link_util.h" #include "ash/capture_mode/capture_mode_controller.h" -#include "ash/constants/ash_features.h" #include "ash/public/cpp/android_intent_helper.h" #include "ash/public/cpp/new_window_delegate.h" -#include "ash/public/cpp/style/scoped_light_mode_as_default.h" #include "ash/public/mojom/assistant_volume_control.mojom.h" #include "ash/session/session_controller_impl.h" #include "ash/shell.h" -#include "ash/style/ash_color_provider.h" #include "base/bind.h" #include "base/memory/scoped_refptr.h" #include "chromeos/services/assistant/public/cpp/assistant_prefs.h" @@ -40,8 +37,6 @@ AssistantControllerImpl::AssistantControllerImpl() { // provide an opportunity to turn on/off A11Y features. Shell::Get()->accessibility_controller()->AddObserver(this); - color_mode_observer_.Observe(AshColorProvider::Get()); - NotifyConstructed(); } @@ -79,9 +74,6 @@ void AssistantControllerImpl::SetAssistant( OnAccessibilityStatusChanged(); - ScopedLightModeAsDefault scoped_light_mode_as_default; - OnColorModeChanged(AshColorProvider::Get()->IsDarkModeEnabled()); - if (assistant) { for (AssistantControllerObserver& observer : observers_) observer.OnAssistantReady(); @@ -277,13 +269,6 @@ void AssistantControllerImpl::OnAccessibilityStatusChanged() { Shell::Get()->accessibility_controller()->spoken_feedback().enabled()); } -void AssistantControllerImpl::OnColorModeChanged(bool dark_mode_enabled) { - if (!assistant_) - return; - - assistant_->OnColorModeChanged(dark_mode_enabled); -} - bool AssistantControllerImpl::IsAssistantReady() const { return !!assistant_; } diff --git a/ash/assistant/assistant_controller_impl.h b/ash/assistant/assistant_controller_impl.h index 1bdc4f50706e3..3467df8332f29 100644 --- a/ash/assistant/assistant_controller_impl.h +++ b/ash/assistant/assistant_controller_impl.h @@ -28,13 +28,10 @@ #include "ash/public/cpp/assistant/controller/assistant_controller.h" #include "ash/public/cpp/assistant/controller/assistant_controller_observer.h" #include "ash/public/cpp/image_downloader.h" -#include "ash/public/cpp/style/color_mode_observer.h" #include "ash/public/mojom/assistant_volume_control.mojom.h" -#include "ash/style/ash_color_provider.h" #include "base/macros.h" #include "base/memory/weak_ptr.h" #include "base/observer_list.h" -#include "base/scoped_observation.h" #include "chromeos/services/assistant/public/cpp/assistant_service.h" #include "components/prefs/pref_service.h" #include "mojo/public/cpp/bindings/pending_receiver.h" @@ -55,8 +52,7 @@ class ASH_EXPORT AssistantControllerImpl public mojom::AssistantVolumeControl, public CrasAudioHandler::AudioObserver, public AccessibilityObserver, - public AssistantInterfaceBinder, - public ColorModeObserver { + public AssistantInterfaceBinder { public: AssistantControllerImpl(); ~AssistantControllerImpl() override; @@ -102,9 +98,6 @@ class ASH_EXPORT AssistantControllerImpl // AccessibilityObserver: void OnAccessibilityStatusChanged() override; - // ColorModeObserver: - void OnColorModeChanged(bool dark_mode_enabled) override; - AssistantAlarmTimerControllerImpl* alarm_timer_controller() { return &assistant_alarm_timer_controller_; } @@ -153,8 +146,6 @@ class ASH_EXPORT AssistantControllerImpl assistant_volume_control_receiver_{this}; mojo::RemoteSet<mojom::VolumeObserver> volume_observers_; - // |assistant_| can be nullptr if libassistant creation is not yet completed, - // i.e. it cannot take a request. chromeos::assistant::Assistant* assistant_ = nullptr; // Assistant sub-controllers. @@ -171,9 +162,6 @@ class ASH_EXPORT AssistantControllerImpl AssistantViewDelegateImpl view_delegate_{this}; - base::ScopedObservation<AshColorProvider, ColorModeObserver> - color_mode_observer_{this}; - base::WeakPtrFactory<AssistantControllerImpl> weak_factory_{this}; DISALLOW_COPY_AND_ASSIGN(AssistantControllerImpl); diff --git a/ash/assistant/assistant_controller_impl_unittest.cc b/ash/assistant/assistant_controller_impl_unittest.cc index 6bafe71131102..eecc120cd3a9c 100644 --- a/ash/assistant/assistant_controller_impl_unittest.cc +++ b/ash/assistant/assistant_controller_impl_unittest.cc @@ -9,18 +9,10 @@ #include <string> #include "ash/assistant/test/assistant_ash_test_base.h" -#include "ash/assistant/test/test_assistant_service.h" #include "ash/assistant/util/deep_link_util.h" -#include "ash/constants/ash_features.h" -#include "ash/constants/ash_pref_names.h" #include "ash/public/cpp/assistant/controller/assistant_controller_observer.h" #include "ash/public/cpp/test/test_new_window_delegate.h" -#include "ash/session/session_controller_impl.h" -#include "ash/shell.h" -#include "ash/style/ash_color_provider.h" #include "base/scoped_observation.h" -#include "base/test/scoped_feature_list.h" -#include "chromeos/services/assistant/public/cpp/assistant_service.h" #include "testing/gmock/include/gmock/gmock.h" namespace ash { @@ -105,17 +97,10 @@ class AssistantControllerImplTest : public AssistantAshTestBase { const AssistantUiModel* ui_model() { return AssistantUiController::Get()->GetModel(); } - TestAssistantService* test_assistant_service() { - return &test_assistant_service_; - } private: MockNewWindowDelegate* new_window_delegate_; std::unique_ptr<TestNewWindowDelegateProvider> delegate_provider_; - - // AssistantService must outlive AssistantController as destructor can - // reference AssistantService. - TestAssistantService test_assistant_service_; }; } // namespace @@ -230,40 +215,4 @@ TEST_F(AssistantControllerImplTest, ClosesAssistantUiForFeedbackDeeplink) { ui_model()->RemoveObserver(&ui_model_observer_mock); } -// Make sure that AssistantControllerImpl sets dark mode = false even if the -// flag is off. SettingsController won't set options if dark mode bit is not -// set. -TEST_F(AssistantControllerImplTest, ColorModeIsSetWhenAssistantIsReadyFlagOff) { - ASSERT_FALSE(features::IsDarkLightModeEnabled()); - - controller()->SetAssistant(test_assistant_service()); - - ASSERT_TRUE(test_assistant_service()->dark_mode_enabled().has_value()); - EXPECT_FALSE(test_assistant_service()->dark_mode_enabled().value()); -} - -TEST_F(AssistantControllerImplTest, ColorModeIsUpdated) { - base::test::ScopedFeatureList feature_list; - feature_list.InitAndEnableFeature(features::kDarkLightMode); - - ASSERT_TRUE(features::IsDarkLightModeEnabled()); - - // AshColorProvider::IsDarkModeEnabled reports it's in dark mode if active - // pref service is not set. - AshColorProvider::Get()->OnActiveUserPrefServiceChanged( - Shell::Get()->session_controller()->GetPrimaryUserPrefService()); - ASSERT_FALSE(AshColorProvider::Get()->IsDarkModeEnabled()); - - controller()->SetAssistant(test_assistant_service()); - - ASSERT_TRUE(test_assistant_service()->dark_mode_enabled().has_value()); - EXPECT_FALSE(test_assistant_service()->dark_mode_enabled().value()); - - Shell::Get()->session_controller()->GetPrimaryUserPrefService()->SetBoolean( - prefs::kDarkModeEnabled, true); - - ASSERT_TRUE(test_assistant_service()->dark_mode_enabled().has_value()); - EXPECT_TRUE(test_assistant_service()->dark_mode_enabled().value()); -} - } // namespace ash diff --git a/ash/assistant/test/test_assistant_service.cc b/ash/assistant/test/test_assistant_service.cc index 334d89522a593..922d6eb69286f 100644 --- a/ash/assistant/test/test_assistant_service.cc +++ b/ash/assistant/test/test_assistant_service.cc @@ -266,10 +266,6 @@ void TestAssistantService::DismissNotification( void TestAssistantService::OnAccessibilityStatusChanged( bool spoken_feedback_enabled) {} -void TestAssistantService::OnColorModeChanged(bool dark_mode_enabled) { - dark_mode_enabled_ = dark_mode_enabled; -} - void TestAssistantService::SendAssistantFeedback( const chromeos::assistant::AssistantFeedback& feedback) {} diff --git a/ash/assistant/test/test_assistant_service.h b/ash/assistant/test/test_assistant_service.h index 1dff7dcb7facc..f36439909515d 100644 --- a/ash/assistant/test/test_assistant_service.h +++ b/ash/assistant/test/test_assistant_service.h @@ -105,7 +105,6 @@ class TestAssistantService : public chromeos::assistant::Assistant { void DismissNotification( const chromeos::assistant::AssistantNotification& notification) override; void OnAccessibilityStatusChanged(bool spoken_feedback_enabled) override; - void OnColorModeChanged(bool dark_mode_enabled) override; void SendAssistantFeedback( const chromeos::assistant::AssistantFeedback& feedback) override; void AddTimeToTimer(const std::string& id, base::TimeDelta duration) override; @@ -113,8 +112,6 @@ class TestAssistantService : public chromeos::assistant::Assistant { void RemoveAlarmOrTimer(const std::string& id) override; void ResumeTimer(const std::string& id) override; - absl::optional<bool> dark_mode_enabled() { return dark_mode_enabled_; } - private: void StartInteraction( chromeos::assistant::AssistantInteractionType type, @@ -127,8 +124,6 @@ class TestAssistantService : public chromeos::assistant::Assistant { std::unique_ptr<CurrentInteractionSubscriber> current_interaction_subscriber_; std::unique_ptr<InteractionResponse> interaction_response_; - absl::optional<bool> dark_mode_enabled_; - base::ObserverList<chromeos::assistant::AssistantInteractionSubscriber> interaction_subscribers_; bool running_active_interaction_ = false; diff --git a/chromeos/services/assistant/assistant_manager_service_impl.cc b/chromeos/services/assistant/assistant_manager_service_impl.cc index 066c164f037cb..539cac71c55eb 100644 --- a/chromeos/services/assistant/assistant_manager_service_impl.cc +++ b/chromeos/services/assistant/assistant_manager_service_impl.cc @@ -444,7 +444,6 @@ void AssistantManagerServiceImpl::InitAssistant( bootup_config->hotword_enabled = assistant_state()->hotword_enabled().value(); bootup_config->locale = assistant_state()->locale().value(); bootup_config->spoken_feedback_enabled = spoken_feedback_enabled_; - bootup_config->dark_mode_enabled = dark_mode_enabled_; service_controller().Initialize(std::move(bootup_config), BindURLLoaderFactory()); @@ -538,16 +537,6 @@ void AssistantManagerServiceImpl::OnAccessibilityStatusChanged( settings_controller().SetSpokenFeedbackEnabled(spoken_feedback_enabled_); } -void AssistantManagerServiceImpl::OnColorModeChanged(bool dark_mode_enabled) { - if (dark_mode_enabled_ == dark_mode_enabled) - return; - - dark_mode_enabled_ = dark_mode_enabled; - - if (IsServiceStarted()) - settings_controller().SetDarkModeEnabled(dark_mode_enabled_); -} - void AssistantManagerServiceImpl::OnDeviceAppsEnabled(bool enabled) { // The device apps state sync should only be sent after service is running. // Check state here to prevent timing issue when the service is restarting. diff --git a/chromeos/services/assistant/assistant_manager_service_impl.h b/chromeos/services/assistant/assistant_manager_service_impl.h index 1f83c9b0b0d0c..535270b41c759 100644 --- a/chromeos/services/assistant/assistant_manager_service_impl.h +++ b/chromeos/services/assistant/assistant_manager_service_impl.h @@ -148,7 +148,6 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AssistantManagerServiceImpl int action_index) override; void DismissNotification(const AssistantNotification& notification) override; void OnAccessibilityStatusChanged(bool spoken_feedback_enabled) override; - void OnColorModeChanged(bool dark_mode_enabled) override; void SendAssistantFeedback(const AssistantFeedback& feedback) override; void AddTimeToTimer(const std::string& id, base::TimeDelta duration) override; void PauseTimer(const std::string& id) override; @@ -251,7 +250,6 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) AssistantManagerServiceImpl state_observer_receiver_{this}; bool spoken_feedback_enabled_ = false; - bool dark_mode_enabled_ = false; base::TimeTicks started_time_; diff --git a/chromeos/services/assistant/assistant_manager_service_impl_unittest.cc b/chromeos/services/assistant/assistant_manager_service_impl_unittest.cc index 41f7f79048d99..38f692ffc42e4 100644 --- a/chromeos/services/assistant/assistant_manager_service_impl_unittest.cc +++ b/chromeos/services/assistant/assistant_manager_service_impl_unittest.cc @@ -202,13 +202,11 @@ class AssistantManagerServiceImplTest : public testing::Test { void RunUntilIdle() { // First ensure our mojom thread is finished. - FlushForTesting(); + background_thread().FlushForTesting(); // Then handle any callbacks. base::RunLoop().RunUntilIdle(); } - void FlushForTesting() { background_thread().FlushForTesting(); } - // Adds a state observer mock, and add the expectation for the fact that it // auto-fires the observer. void AddStateObserver(StateObserverMock* observer) { @@ -721,20 +719,5 @@ TEST_F(AssistantManagerServiceImplTest, mojom_mock.FlushForTesting(); } -TEST_F(AssistantManagerServiceImplTest, ShouldPropagateColorMode) { - ASSERT_FALSE(mojom_service_controller().dark_mode_enabled().has_value()); - - StartAndWaitForRunning(); - - ASSERT_TRUE(mojom_service_controller().dark_mode_enabled().has_value()); - EXPECT_FALSE(mojom_service_controller().dark_mode_enabled().value()); - - assistant_manager_service()->OnColorModeChanged(true); - FlushForTesting(); - - ASSERT_TRUE(mojom_service_controller().dark_mode_enabled().has_value()); - EXPECT_TRUE(mojom_service_controller().dark_mode_enabled().value()); -} - } // namespace assistant } // namespace chromeos diff --git a/chromeos/services/assistant/public/cpp/assistant_service.h b/chromeos/services/assistant/public/cpp/assistant_service.h index b31de35be740e..f44d39c15c87c 100644 --- a/chromeos/services/assistant/public/cpp/assistant_service.h +++ b/chromeos/services/assistant/public/cpp/assistant_service.h @@ -129,9 +129,6 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE_PUBLIC) Assistant { // accessibility status has changed, |spoken_feedback_enabled| may not have. virtual void OnAccessibilityStatusChanged(bool spoken_feedback_enabled) = 0; - // Invoked when color mode (dark/light mode) status changed. - virtual void OnColorModeChanged(bool dark_mode_enabled) = 0; - // Send Assistant feedback to Assistant server. virtual void SendAssistantFeedback(const AssistantFeedback& feedback) = 0; diff --git a/chromeos/services/assistant/test_support/fake_assistant_manager_service_impl.cc b/chromeos/services/assistant/test_support/fake_assistant_manager_service_impl.cc index 1c2521c2d7eaa..028564e90479f 100644 --- a/chromeos/services/assistant/test_support/fake_assistant_manager_service_impl.cc +++ b/chromeos/services/assistant/test_support/fake_assistant_manager_service_impl.cc @@ -110,9 +110,6 @@ void FakeAssistantManagerServiceImpl::DismissNotification( void FakeAssistantManagerServiceImpl::OnAccessibilityStatusChanged( bool spoken_feedback_enabled) {} -void FakeAssistantManagerServiceImpl::OnColorModeChanged( - bool dark_mode_enabled) {} - void FakeAssistantManagerServiceImpl::SendAssistantFeedback( const AssistantFeedback& feedback) {} diff --git a/chromeos/services/assistant/test_support/fake_assistant_manager_service_impl.h b/chromeos/services/assistant/test_support/fake_assistant_manager_service_impl.h index 913f87f0efaef..b0c59ea2aff1c 100644 --- a/chromeos/services/assistant/test_support/fake_assistant_manager_service_impl.h +++ b/chromeos/services/assistant/test_support/fake_assistant_manager_service_impl.h @@ -73,7 +73,6 @@ class COMPONENT_EXPORT(ASSISTANT_SERVICE) FakeAssistantManagerServiceImpl int action_index) override; void DismissNotification(const AssistantNotification& notification) override; void OnAccessibilityStatusChanged(bool spoken_feedback_enabled) override; - void OnColorModeChanged(bool dark_mode_enabled) override; void SendAssistantFeedback(const AssistantFeedback& feedback) override; void AddTimeToTimer(const std::string& id, base::TimeDelta duration) override; void PauseTimer(const std::string& id) override; diff --git a/chromeos/services/assistant/test_support/fake_service_controller.cc b/chromeos/services/assistant/test_support/fake_service_controller.cc index 5ceb8357f9da0..465b463d82967 100644 --- a/chromeos/services/assistant/test_support/fake_service_controller.cc +++ b/chromeos/services/assistant/test_support/fake_service_controller.cc @@ -81,7 +81,6 @@ void FakeServiceController::Initialize( authentication_tokens_ = std::move(libassistant_config_->authentication_tokens); - dark_mode_enabled_ = libassistant_config_->dark_mode_enabled; } void FakeServiceController::Start() { @@ -116,10 +115,6 @@ void FakeServiceController::SetAuthenticationTokens( authentication_tokens_ = std::move(tokens); } -void FakeServiceController::SetDarkModeEnabled(bool value) { - dark_mode_enabled_ = value; -} - void FakeServiceController::UpdateSettings(const std::string& settings, UpdateSettingsCallback callback) { // Callback must be called to satisfy the mojom contract. diff --git a/chromeos/services/assistant/test_support/fake_service_controller.h b/chromeos/services/assistant/test_support/fake_service_controller.h index 7b1a77508f359..9b42ab12fe5d4 100644 --- a/chromeos/services/assistant/test_support/fake_service_controller.h +++ b/chromeos/services/assistant/test_support/fake_service_controller.h @@ -8,8 +8,6 @@ #include <mutex> #include <string> -#include "third_party/abseil-cpp/absl/types/optional.h" - #include "base/memory/weak_ptr.h" #include "chromeos/services/libassistant/public/mojom/service_controller.mojom.h" #include "chromeos/services/libassistant/public/mojom/settings_controller.mojom.h" @@ -78,8 +76,6 @@ class FakeServiceController // True if ResetAllDataAndStop() was called. bool has_data_been_reset() const { return has_data_been_reset_; } - absl::optional<bool> dark_mode_enabled() const { return dark_mode_enabled_; } - private: // mojom::ServiceController implementation: void Initialize(chromeos::libassistant::mojom::BootupConfigPtr config, @@ -99,7 +95,6 @@ class FakeServiceController void SetListeningEnabled(bool value) override {} void SetLocale(const std::string& value) override {} void SetSpokenFeedbackEnabled(bool value) override {} - void SetDarkModeEnabled(bool value) override; void UpdateSettings(const std::string& settings, UpdateSettingsCallback callback) override; void GetSettings(const std::string& selector, @@ -121,8 +116,6 @@ class FakeServiceController std::vector<chromeos::libassistant::mojom::AuthenticationTokenPtr> authentication_tokens_; - absl::optional<bool> dark_mode_enabled_; - State state_ = State::kStopped; mojo::Receiver<chromeos::libassistant::mojom::ServiceController> service_receiver_{this}; diff --git a/chromeos/services/libassistant/public/mojom/service_controller.mojom b/chromeos/services/libassistant/public/mojom/service_controller.mojom index 50cf2b39e7ef3..783924f04fa47 100644 --- a/chromeos/services/libassistant/public/mojom/service_controller.mojom +++ b/chromeos/services/libassistant/public/mojom/service_controller.mojom @@ -63,6 +63,5 @@ struct BootupConfig { array<AuthenticationToken> authentication_tokens; string locale; bool spoken_feedback_enabled; - bool dark_mode_enabled; bool hotword_enabled; }; diff --git a/chromeos/services/libassistant/public/mojom/settings_controller.mojom b/chromeos/services/libassistant/public/mojom/settings_controller.mojom index 0fff34f3b2c21..9e77b7d583ed0 100644 --- a/chromeos/services/libassistant/public/mojom/settings_controller.mojom +++ b/chromeos/services/libassistant/public/mojom/settings_controller.mojom @@ -25,9 +25,6 @@ interface SettingsController { // Sets whether spoken feedback is enabled. SetSpokenFeedbackEnabled(bool value); - // Sets whether dark mode is enabled. - SetDarkModeEnabled(bool value); - // Sets whether hotword detection is enabled. SetHotwordEnabled(bool value); diff --git a/chromeos/services/libassistant/service_controller.cc b/chromeos/services/libassistant/service_controller.cc index 9c351d7fcb9cb..ac19e78f2e422 100644 --- a/chromeos/services/libassistant/service_controller.cc +++ b/chromeos/services/libassistant/service_controller.cc @@ -161,7 +161,6 @@ void ServiceController::Initialize( settings_controller_->SetHotwordEnabled(config->hotword_enabled); settings_controller_->SetSpokenFeedbackEnabled( config->spoken_feedback_enabled); - settings_controller_->SetDarkModeEnabled(config->dark_mode_enabled); CreateAndRegisterDeviceStateListener(); CreateAndRegisterChromiumApiDelegate(std::move(url_loader_factory)); diff --git a/chromeos/services/libassistant/service_controller_unittest.cc b/chromeos/services/libassistant/service_controller_unittest.cc index 30a6ee1b4e026..684aa45decab6 100644 --- a/chromeos/services/libassistant/service_controller_unittest.cc +++ b/chromeos/services/libassistant/service_controller_unittest.cc @@ -109,7 +109,6 @@ class SettingsControllerMock : public mojom::SettingsController { MOCK_METHOD(void, SetListeningEnabled, (bool value)); MOCK_METHOD(void, SetLocale, (const std::string& value)); MOCK_METHOD(void, SetSpokenFeedbackEnabled, (bool value)); - MOCK_METHOD(void, SetDarkModeEnabled, (bool value)); MOCK_METHOD(void, SetHotwordEnabled, (bool value)); MOCK_METHOD(void, GetSettings, diff --git a/chromeos/services/libassistant/settings_controller.cc b/chromeos/services/libassistant/settings_controller.cc index fd4a2fe872bdc..aff4b7a24103f 100644 --- a/chromeos/services/libassistant/settings_controller.cc +++ b/chromeos/services/libassistant/settings_controller.cc @@ -48,11 +48,9 @@ const char* LocaleOrDefault(const std::string& locale) { assistant_client::InternalOptions* WARN_UNUSED_RESULT CreateInternalOptions( assistant_client::AssistantManagerInternal* assistant_manager_internal, const std::string& locale, - bool spoken_feedback_enabled, - bool dark_mode_enabled) { + bool spoken_feedback_enabled) { auto* result = assistant_manager_internal->CreateDefaultInternalOptions(); - assistant::SetAssistantOptions(result, locale, spoken_feedback_enabled, - dark_mode_enabled); + assistant::SetAssistantOptions(result, locale, spoken_feedback_enabled); result->SetClientControlEnabled(assistant::features::IsRoutinesEnabled()); @@ -234,7 +232,7 @@ void SettingsController::SetAuthenticationTokens( void SettingsController::SetLocale(const std::string& value) { locale_ = LocaleOrDefault(value); - UpdateInternalOptions(locale_, spoken_feedback_enabled_, dark_mode_enabled_); + UpdateInternalOptions(locale_, spoken_feedback_enabled_); UpdateDeviceSettings(locale_, hotword_enabled_); } @@ -245,12 +243,7 @@ void SettingsController::SetListeningEnabled(bool value) { void SettingsController::SetSpokenFeedbackEnabled(bool value) { spoken_feedback_enabled_ = value; - UpdateInternalOptions(locale_, spoken_feedback_enabled_, dark_mode_enabled_); -} - -void SettingsController::SetDarkModeEnabled(bool value) { - dark_mode_enabled_ = value; - UpdateInternalOptions(locale_, spoken_feedback_enabled_, dark_mode_enabled_); + UpdateInternalOptions(locale_, spoken_feedback_enabled_); } void SettingsController::SetHotwordEnabled(bool value) { @@ -296,26 +289,20 @@ void SettingsController::UpdateAuthenticationTokens( void SettingsController::UpdateInternalOptions( const absl::optional<std::string>& locale, - absl::optional<bool> spoken_feedback_enabled, - absl::optional<bool> dark_mode_enabled) { + absl::optional<bool> spoken_feedback_enabled) { if (!assistant_manager_internal_) return; - // All of those bits should be ready if |assistant_manager_internal_| is not - // nullptr. ServiceController should set all of them before calling - // SettingsController::OnAssistantManagerCreated. - DCHECK(locale.has_value()); - DCHECK(spoken_feedback_enabled.has_value()); - DCHECK(dark_mode_enabled.has_value()); - if (locale.has_value()) assistant_manager_internal_->SetLocaleOverride(locale.value()); - assistant_manager_internal_->SetOptions( - *CreateInternalOptions(assistant_manager_internal_, locale.value(), - spoken_feedback_enabled.value(), - dark_mode_enabled.value()), - [](bool success) { DVLOG(2) << "set options: " << success; }); + if (locale.has_value() && spoken_feedback_enabled.has_value()) { + assistant_manager_internal_->SetOptions( + *CreateInternalOptions(assistant_manager_internal_, locale.value(), + spoken_feedback_enabled.value()), + [](bool success) { DVLOG(2) << "set options: " << success; }); + } + return; } void SettingsController::UpdateDeviceSettings( @@ -338,7 +325,7 @@ void SettingsController::OnAssistantClientCreated( // Note we do not enable the device settings updater here, as it requires // Libassistant to be started. UpdateAuthenticationTokens(authentication_tokens_); - UpdateInternalOptions(locale_, spoken_feedback_enabled_, dark_mode_enabled_); + UpdateInternalOptions(locale_, spoken_feedback_enabled_); UpdateListeningEnabled(listening_enabled_); } diff --git a/chromeos/services/libassistant/settings_controller.h b/chromeos/services/libassistant/settings_controller.h index d8072e69db478..730b237999982 100644 --- a/chromeos/services/libassistant/settings_controller.h +++ b/chromeos/services/libassistant/settings_controller.h @@ -38,7 +38,6 @@ class SettingsController : public AssistantClientObserver, void SetListeningEnabled(bool value) override; void SetLocale(const std::string& value) override; void SetSpokenFeedbackEnabled(bool value) override; - void SetDarkModeEnabled(bool value) override; void SetHotwordEnabled(bool value) override; void GetSettings(const std::string& selector, bool include_header, @@ -60,8 +59,7 @@ class SettingsController : public AssistantClientObserver, void UpdateAuthenticationTokens( const absl::optional<std::vector<mojom::AuthenticationTokenPtr>>& tokens); void UpdateInternalOptions(const absl::optional<std::string>& locale, - absl::optional<bool> spoken_feedback_enabled, - absl::optional<bool> dark_mode_enabled); + absl::optional<bool> spoken_feedback_enabled); void UpdateDeviceSettings(const absl::optional<std::string>& locale, absl::optional<bool> hotword_enabled); @@ -80,7 +78,6 @@ class SettingsController : public AssistantClientObserver, absl::optional<bool> hotword_enabled_; absl::optional<bool> spoken_feedback_enabled_; - absl::optional<bool> dark_mode_enabled_; absl::optional<bool> listening_enabled_; absl::optional<std::string> locale_; absl::optional<std::vector<mojom::AuthenticationTokenPtr>> diff --git a/chromeos/services/libassistant/settings_controller_unittest.cc b/chromeos/services/libassistant/settings_controller_unittest.cc index f1d80023ccefc..0ba604e7cdb9a 100644 --- a/chromeos/services/libassistant/settings_controller_unittest.cc +++ b/chromeos/services/libassistant/settings_controller_unittest.cc @@ -132,7 +132,6 @@ TEST_F(AssistantSettingsControllerTest, controller().SetListeningEnabled(true); controller().SetLocale("locale"); controller().SetSpokenFeedbackEnabled(true); - controller().SetDarkModeEnabled(false); } TEST_F(AssistantSettingsControllerTest, @@ -145,7 +144,6 @@ TEST_F(AssistantSettingsControllerTest, controller().SetListeningEnabled(true); controller().SetLocale("locale"); controller().SetSpokenFeedbackEnabled(true); - controller().SetDarkModeEnabled(false); } TEST_F(AssistantSettingsControllerTest, @@ -154,7 +152,6 @@ TEST_F(AssistantSettingsControllerTest, controller().SetHotwordEnabled(true); controller().SetLocale("locale"); controller().SetSpokenFeedbackEnabled(true); - controller().SetDarkModeEnabled(true); DestroyLibassistant(); @@ -194,7 +191,6 @@ TEST_F(AssistantSettingsControllerTest, EXPECT_NO_CALLS(assistant_manager_internal_mock(), SetOptions); controller().SetSpokenFeedbackEnabled(true); - controller().SetDarkModeEnabled(false); } TEST_F(AssistantSettingsControllerTest, @@ -205,25 +201,12 @@ TEST_F(AssistantSettingsControllerTest, EXPECT_NO_CALLS(assistant_manager_internal_mock(), SetOptions); controller().SetLocale("locale"); - controller().SetDarkModeEnabled(false); -} - -TEST_F(AssistantSettingsControllerTest, - ShouldNotSetInternalOptionsWhenDarkModeEnabledIsNotSet) { - IGNORE_CALLS(assistant_manager_internal_mock(), SetLocaleOverride); - CreateLibassistant(); - - EXPECT_NO_CALLS(assistant_manager_internal_mock(), SetOptions); - - controller().SetLocale("locale"); - controller().SetSpokenFeedbackEnabled(true); } TEST_F(AssistantSettingsControllerTest, ShouldSetInternalOptionsWhenLocaleIsUpdated) { IGNORE_CALLS(assistant_manager_internal_mock(), SetLocaleOverride); controller().SetSpokenFeedbackEnabled(true); - controller().SetDarkModeEnabled(false); CreateLibassistant(); EXPECT_CALL(assistant_manager_internal_mock(), SetOptions); @@ -235,7 +218,6 @@ TEST_F(AssistantSettingsControllerTest, ShouldSetInternalOptionsWhenSpokenFeedbackEnabledIsUpdated) { IGNORE_CALLS(assistant_manager_internal_mock(), SetLocaleOverride); controller().SetLocale("locale"); - controller().SetDarkModeEnabled(false); CreateLibassistant(); EXPECT_CALL(assistant_manager_internal_mock(), SetOptions); @@ -243,23 +225,10 @@ TEST_F(AssistantSettingsControllerTest, controller().SetSpokenFeedbackEnabled(true); } -TEST_F(AssistantSettingsControllerTest, - ShouldSetInternalOptionsWhenDarkModeEnabledIsUpdated) { - IGNORE_CALLS(assistant_manager_internal_mock(), SetLocaleOverride); - controller().SetLocale("locale"); - controller().SetSpokenFeedbackEnabled(true); - CreateLibassistant(); - - EXPECT_CALL(assistant_manager_internal_mock(), SetOptions); - - controller().SetDarkModeEnabled(false); -} - TEST_F(AssistantSettingsControllerTest, ShouldSetInternalOptionsAndLocaleWhenLibassistantIsCreated) { controller().SetLocale("locale"); controller().SetSpokenFeedbackEnabled(true); - controller().SetDarkModeEnabled(false); EXPECT_CALL(assistant_manager_internal_mock(), SetLocaleOverride); EXPECT_CALL(assistant_manager_internal_mock(), SetOptions);