Revert "[a11y] Adds callback to notify screen reader when TTS engine connects." This reverts commit 9e7e8382f1a052caace6c38ba7546685b323f1d6. Reason for revert: fxb/64156 Original change's description: > [a11y] Adds callback to notify screen reader when TTS engine connects. > > This change adds a mechanism for the screen reader to be notified when > the TTS engine is ready to produce output. > > TEST: fx test a11y_lib_tests; fx test a11y_tests > Change-Id: I03e501ce1e99ec88ee02cd6cd2ffad9c90612b15 > Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/445219 > Reviewed-by: Lucas Radaelli <lucasradaelli@google.com> > Testability-Review: Lucas Radaelli <lucasradaelli@google.com> > Commit-Queue: Alexander Brusher <abrusher@google.com> TBR=abrusher@google.com,lucasradaelli@google.com # Not skipping CQ checks because original CL landed > 1 day ago. Change-Id: I94a0059e6e5159f7a0bb9e824bf6aaa062d7cd3d Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/450195 Reviewed-by: Alexander Brusher <abrusher@google.com> Commit-Queue: Alexander Brusher <abrusher@google.com>
diff --git a/src/ui/a11y/bin/a11y_manager/app.cc b/src/ui/a11y/bin/a11y_manager/app.cc index b324ab9b..58eec612 100644 --- a/src/ui/a11y/bin/a11y_manager/app.cc +++ b/src/ui/a11y/bin/a11y_manager/app.cc
@@ -22,8 +22,7 @@ color_transform_manager_(color_transform_manager), gesture_listener_registry_(gesture_listener_registry), inspect_node_(std::move(inspect_node)), - inspect_property_intl_property_provider_disconnected_( - inspect_node_.CreateBool(kIntlPropertyProviderDisconnectedInspectName, false)) { + inspect_property_intl_property_provider_disconnected_(inspect_node_.CreateBool(kIntlPropertyProviderDisconnectedInspectName, false)) { FX_DCHECK(context); FX_DCHECK(view_manager); FX_DCHECK(tts_manager); @@ -245,7 +244,7 @@ auto screen_reader_context = std::make_unique<a11y::ScreenReaderContext>( std::move(a11y_focus_manager), tts_manager_, locale_id); auto screen_reader = std::make_unique<a11y::ScreenReader>( - std::move(screen_reader_context), view_manager_, gesture_listener_registry_, tts_manager_); + std::move(screen_reader_context), view_manager_, gesture_listener_registry_); view_manager_->GetSemanticsEventManager()->Register( screen_reader->GetSemanticsEventListenerWeakPtr()); return screen_reader;
diff --git a/src/ui/a11y/lib/screen_reader/screen_reader.cc b/src/ui/a11y/lib/screen_reader/screen_reader.cc index 920d89a..5b71cbd 100644 --- a/src/ui/a11y/lib/screen_reader/screen_reader.cc +++ b/src/ui/a11y/lib/screen_reader/screen_reader.cc
@@ -90,15 +90,13 @@ ScreenReader::ScreenReader(std::unique_ptr<ScreenReaderContext> context, SemanticsSource* semantics_source, - GestureListenerRegistry* gesture_listener_registry, - TtsManager* tts_manager) - : ScreenReader(std::move(context), semantics_source, gesture_listener_registry, tts_manager, + GestureListenerRegistry* gesture_listener_registry) + : ScreenReader(std::move(context), semantics_source, gesture_listener_registry, std::make_unique<ScreenReaderActionRegistryImpl>()) {} ScreenReader::ScreenReader(std::unique_ptr<ScreenReaderContext> context, SemanticsSource* semantics_source, GestureListenerRegistry* gesture_listener_registry, - TtsManager* tts_manager, std::unique_ptr<ScreenReaderActionRegistry> action_registry) : context_(std::move(context)), gesture_listener_registry_(gesture_listener_registry), @@ -107,10 +105,7 @@ action_context_ = std::make_unique<ScreenReaderAction::ActionContext>(); action_context_->semantics_source = semantics_source; InitializeActions(); - FX_DCHECK(tts_manager); - // TODO(fxb/59693): Only vocalize message on reboot if user-initiated. - tts_manager->RegisterTTSEngineReadyCallback( - [this]() { SpeakMessage(fuchsia::intl::l10n::MessageIds::SCREEN_READER_ON_HINT); }); + SpeakMessage(fuchsia::intl::l10n::MessageIds::SCREEN_READER_ON_HINT); context_->speaker()->set_epitaph(fuchsia::intl::l10n::MessageIds::SCREEN_READER_OFF_HINT); }
diff --git a/src/ui/a11y/lib/screen_reader/screen_reader.h b/src/ui/a11y/lib/screen_reader/screen_reader.h index 3f18621..d05d321 100644 --- a/src/ui/a11y/lib/screen_reader/screen_reader.h +++ b/src/ui/a11y/lib/screen_reader/screen_reader.h
@@ -37,10 +37,10 @@ // outlive screen reader. A11y App is responsible for creating these pointers along with Screen // Reader object. ScreenReader(std::unique_ptr<ScreenReaderContext> context, SemanticsSource* semantics_source, - GestureListenerRegistry* gesture_listener_registry, TtsManager* tts_manager); + GestureListenerRegistry* gesture_listener_registry); // Same as above, but accepts a custom |action_registry|. ScreenReader(std::unique_ptr<ScreenReaderContext> context, SemanticsSource* semantics_source, - GestureListenerRegistry* gesture_listener_registry, TtsManager* tts_manager, + GestureListenerRegistry* gesture_listener_registry, std::unique_ptr<ScreenReaderActionRegistry> action_registry); ~ScreenReader();
diff --git a/src/ui/a11y/lib/screen_reader/tests/BUILD.gn b/src/ui/a11y/lib/screen_reader/tests/BUILD.gn index 137f628..417ab52 100644 --- a/src/ui/a11y/lib/screen_reader/tests/BUILD.gn +++ b/src/ui/a11y/lib/screen_reader/tests/BUILD.gn
@@ -44,7 +44,6 @@ "//src/ui/a11y/lib/semantics/tests/mocks", "//src/ui/a11y/lib/testing:input", "//src/ui/a11y/lib/tts", - "//src/ui/a11y/lib/tts/tests/mocks", "//src/ui/a11y/lib/util", "//src/ui/a11y/lib/view", "//src/ui/a11y/lib/view/tests/mocks",
diff --git a/src/ui/a11y/lib/screen_reader/tests/screen_reader_unittest.cc b/src/ui/a11y/lib/screen_reader/tests/screen_reader_unittest.cc index 89c62f8..f7c125e9 100644 --- a/src/ui/a11y/lib/screen_reader/tests/screen_reader_unittest.cc +++ b/src/ui/a11y/lib/screen_reader/tests/screen_reader_unittest.cc
@@ -29,7 +29,6 @@ #include "src/ui/a11y/lib/semantics/tests/mocks/mock_semantic_tree_service_factory.h" #include "src/ui/a11y/lib/semantics/tests/mocks/mock_semantics_event_manager.h" #include "src/ui/a11y/lib/testing/input.h" -#include "src/ui/a11y/lib/tts/tests/mocks/mock_tts_manager.h" #include "src/ui/a11y/lib/tts/tts_manager.h" #include "src/ui/a11y/lib/util/util.h" #include "src/ui/a11y/lib/view/tests/mocks/mock_view_semantics.h" @@ -90,10 +89,9 @@ mock_speaker_ptr_(context_ptr_->mock_speaker_ptr()), mock_action_registry_(std::make_unique<MockScreenReaderActionRegistryImpl>()), mock_action_registry_ptr_(mock_action_registry_.get()), - mock_tts_manager_(std::make_unique<MockTtsManager>(context_provider_.context())), - screen_reader_(std::make_unique<a11y::ScreenReader>( - std::move(context_), &view_manager_, &gesture_listener_registry_, - mock_tts_manager_.get(), std::move(mock_action_registry_))), + screen_reader_(std::make_unique<a11y::ScreenReader>(std::move(context_), &view_manager_, + &gesture_listener_registry_, + std::move(mock_action_registry_))), semantic_provider_(&view_manager_) { screen_reader_->BindGestures(&mock_gesture_handler_); gesture_listener_registry_.Register(mock_gesture_listener_.NewBinding(), []() {}); @@ -118,7 +116,6 @@ MockScreenReaderContext::MockSpeaker* mock_speaker_ptr_; std::unique_ptr<MockScreenReaderActionRegistryImpl> mock_action_registry_; MockScreenReaderActionRegistryImpl* mock_action_registry_ptr_; - std::unique_ptr<MockTtsManager> mock_tts_manager_; std::unique_ptr<a11y::ScreenReader> screen_reader_; MockSemanticProvider semantic_provider_; }; // namespace @@ -176,15 +173,6 @@ } TEST_F(ScreenReaderTest, ScreenReaderSpeaksWhenItTurnsOnAndOff) { - // No output should be spoken until the tts engine is connected. - EXPECT_TRUE(mock_speaker_ptr_->message_ids().empty()); - - // Connect the tts engine. - fidl::InterfaceHandle<fuchsia::accessibility::tts::Engine> engine_handle; - mock_tts_manager_->RegisterEngine( - std::move(engine_handle), - [](fuchsia::accessibility::tts::EngineRegistry_RegisterEngine_Result result) {}); - // The screen reader object has already been initialized, check if it announced it: EXPECT_EQ(mock_speaker_ptr_->message_ids().size(), 1u); EXPECT_EQ(mock_speaker_ptr_->message_ids()[0],
diff --git a/src/ui/a11y/lib/screen_reader/tests/speaker_unittest.cc b/src/ui/a11y/lib/screen_reader/tests/speaker_unittest.cc index c78aa3e7..f7538bb 100644 --- a/src/ui/a11y/lib/screen_reader/tests/speaker_unittest.cc +++ b/src/ui/a11y/lib/screen_reader/tests/speaker_unittest.cc
@@ -28,7 +28,6 @@ executor_(async_get_default_dispatcher()) { fidl::InterfaceHandle<fuchsia::accessibility::tts::Engine> engine_handle = mock_tts_engine_.GetHandle(); - tts_manager_.RegisterTTSEngineReadyCallback([]() {}); tts_manager_.RegisterEngine( std::move(engine_handle), [](fuchsia::accessibility::tts::EngineRegistry_RegisterEngine_Result result) {
diff --git a/src/ui/a11y/lib/tts/tests/mocks/BUILD.gn b/src/ui/a11y/lib/tts/tests/mocks/BUILD.gn deleted file mode 100644 index e5424256..0000000 --- a/src/ui/a11y/lib/tts/tests/mocks/BUILD.gn +++ /dev/null
@@ -1,22 +0,0 @@ -# Copyright 2020 The Fuchsia Authors. All rights reserved. -# Use of this source code is governed by a BSD-style license that can be -# found in the LICENSE file. - -source_set("mocks") { - testonly = true - - sources = [ - "mock_tts_manager.cc", - "mock_tts_manager.h", - ] - - deps = [ - "//garnet/public/lib/gtest", - "//sdk/fidl/fuchsia.accessibility.tts", - "//sdk/fidl/fuchsia.logger", - "//sdk/lib/sys/cpp/testing:integration", - "//sdk/lib/syslog/cpp", - "//src/lib/fxl/test:gtest_main", - "//src/ui/a11y/lib/tts", - ] -}
diff --git a/src/ui/a11y/lib/tts/tests/mocks/mock_tts_manager.cc b/src/ui/a11y/lib/tts/tests/mocks/mock_tts_manager.cc deleted file mode 100644 index 922334d..0000000 --- a/src/ui/a11y/lib/tts/tests/mocks/mock_tts_manager.cc +++ /dev/null
@@ -1,47 +0,0 @@ -// Copyright 2020 The Fuchsia 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 "src/ui/a11y/lib/tts/tests/mocks/mock_tts_manager.h" - -namespace accessibility_test { - -void MockTtsManager::OpenEngine( - fidl::InterfaceRequest<fuchsia::accessibility::tts::Engine> engine_request, - a11y::TtsManager::OpenEngineCallback callback) { - fuchsia::accessibility::tts::TtsManager_OpenEngine_Result result; - if (engine_in_use_) { - result.set_err(fuchsia::accessibility::tts::Error::BUSY); - } else { - result.set_response(fuchsia::accessibility::tts::TtsManager_OpenEngine_Response{}); - } - callback(std::move(result)); - engine_in_use_ = true; -} - -void MockTtsManager::SetEngineInUse(bool engine_in_use) { engine_in_use_ = engine_in_use; } - -void MockTtsManager::RegisterEngine( - fidl::InterfaceHandle<fuchsia::accessibility::tts::Engine> engine, - a11y::TtsManager::RegisterEngineCallback callback) { - fuchsia::accessibility::tts::EngineRegistry_RegisterEngine_Result result; - if (!engine_registered_) { - result.set_err(fuchsia::accessibility::tts::Error::BUSY); - } else { - result.set_response(fuchsia::accessibility::tts::EngineRegistry_RegisterEngine_Response{}); - } - callback(std::move(result)); - tts_engine_ready_callback_(); - engine_registered_ = true; -} - -void MockTtsManager::SetEngineRegisered(bool engine_registered) { - engine_registered_ = engine_registered; -} - -void MockTtsManager::RegisterTTSEngineReadyCallback( - a11y::TtsManager::TTSEngineReadyCallback callback) { - tts_engine_ready_callback_ = std::move(callback); -} - -} // namespace accessibility_test
diff --git a/src/ui/a11y/lib/tts/tests/mocks/mock_tts_manager.h b/src/ui/a11y/lib/tts/tests/mocks/mock_tts_manager.h deleted file mode 100644 index e933d01..0000000 --- a/src/ui/a11y/lib/tts/tests/mocks/mock_tts_manager.h +++ /dev/null
@@ -1,50 +0,0 @@ -// Copyright 2020 The Fuchsia 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 SRC_UI_A11Y_LIB_TTS_TESTS_MOCKS_MOCK_TTS_MANAGER_H_ -#define SRC_UI_A11Y_LIB_TTS_TESTS_MOCKS_MOCK_TTS_MANAGER_H_ - -#include <map> -#include <optional> - -#include "src/ui/a11y/lib/tts/tts_manager.h" - -namespace accessibility_test { - -class MockTtsManager : public a11y::TtsManager { - public: - explicit MockTtsManager(sys::ComponentContext* context) : TtsManager(context) {} - ~MockTtsManager() override = default; - - // |a11y::TtsManager| - void OpenEngine(fidl::InterfaceRequest<fuchsia::accessibility::tts::Engine> engine_request, - OpenEngineCallback callback) override; - - // Sets value of engine_in_use_. - void SetEngineInUse(bool engine_in_use); - - // |a11y::TtsManager| - void RegisterEngine(fidl::InterfaceHandle<fuchsia::accessibility::tts::Engine> engine, - RegisterEngineCallback callback) override; - - // Sets value of engine_registered_. - void SetEngineRegisered(bool engine_registered); - - // |a11y::TtsManager| - void RegisterTTSEngineReadyCallback(a11y::TtsManager::TTSEngineReadyCallback callback) override; - - private: - // Callback invoked during |RegisterEngine()|. - TTSEngineReadyCallback tts_engine_ready_callback_; - - // Indicates whether an engine has been registered. - bool engine_registered_ = false; - - // Indicates whether a speaker is using the registered engine. - bool engine_in_use_ = false; -}; - -} // namespace accessibility_test - -#endif // SRC_UI_A11Y_LIB_TTS_TESTS_MOCKS_MOCK_TTS_MANAGER_H_
diff --git a/src/ui/a11y/lib/tts/tests/tts_manager_unittest.cc b/src/ui/a11y/lib/tts/tests/tts_manager_unittest.cc index 6b72b90..9eb6289 100644 --- a/src/ui/a11y/lib/tts/tests/tts_manager_unittest.cc +++ b/src/ui/a11y/lib/tts/tests/tts_manager_unittest.cc
@@ -78,7 +78,6 @@ void SetUp() override { startup_context_ = sys::ComponentContext::CreateAndServeOutgoingDirectory(); tts_manager_ = std::make_unique<TtsManager>(startup_context_.get()); - tts_manager_->RegisterTTSEngineReadyCallback([]() {}); } std::unique_ptr<sys::ComponentContext> startup_context_; @@ -237,67 +236,5 @@ EXPECT_EQ(fake_engine_new.ExamineUtterances()[0].message(), "hello world new"); } -TEST_F(TtsManagerTest, RegisterSpeakerThenEngine) { - bool callback_invoked = false; - tts_manager_->RegisterTTSEngineReadyCallback([&callback_invoked]() { callback_invoked = true; }); - - // Register speaker. - fuchsia::accessibility::tts::EnginePtr speaker_1; - tts_manager_->OpenEngine(speaker_1.NewRequest(), - [](fuchsia::accessibility::tts::TtsManager_OpenEngine_Result result) { - EXPECT_TRUE(result.is_response()); - }); - RunLoopUntilIdle(); - - // Callback should not have been invoked yet since no engine is registered - // yet. - EXPECT_FALSE(callback_invoked); - - // Register engine. - FakeEngine fake_engine_1; - auto engine_handle_1 = fake_engine_1.GetHandle(); - tts_manager_->RegisterEngine( - std::move(engine_handle_1), - [](fuchsia::accessibility::tts::EngineRegistry_RegisterEngine_Result result) { - EXPECT_TRUE(result.is_response()); - }); - RunLoopUntilIdle(); - - // Callback should have been invoked now that both engine and speaker are - // connected. - EXPECT_TRUE(callback_invoked); -} - -TEST_F(TtsManagerTest, RegisterEngineThenSpeaker) { - bool callback_invoked = false; - tts_manager_->RegisterTTSEngineReadyCallback([&callback_invoked]() { callback_invoked = true; }); - - // Register engine. - FakeEngine fake_engine_1; - auto engine_handle_1 = fake_engine_1.GetHandle(); - tts_manager_->RegisterEngine( - std::move(engine_handle_1), - [](fuchsia::accessibility::tts::EngineRegistry_RegisterEngine_Result result) { - EXPECT_TRUE(result.is_response()); - }); - RunLoopUntilIdle(); - - // Callback should not have been invoked yet since no speaker is registered - // yet. - EXPECT_FALSE(callback_invoked); - - // Register speaker. - fuchsia::accessibility::tts::EnginePtr speaker_1; - tts_manager_->OpenEngine(speaker_1.NewRequest(), - [](fuchsia::accessibility::tts::TtsManager_OpenEngine_Result result) { - EXPECT_TRUE(result.is_response()); - }); - RunLoopUntilIdle(); - - // Callback should have been invoked now that both engine and speaker are - // connected. - EXPECT_TRUE(callback_invoked); -} - } // namespace } // namespace a11y
diff --git a/src/ui/a11y/lib/tts/tts_manager.cc b/src/ui/a11y/lib/tts/tts_manager.cc index 79307d1..f33882d 100644 --- a/src/ui/a11y/lib/tts/tts_manager.cc +++ b/src/ui/a11y/lib/tts/tts_manager.cc
@@ -29,7 +29,6 @@ result.set_response(fuchsia::accessibility::tts::TtsManager_OpenEngine_Response{}); } callback(std::move(result)); - CheckIfTtsEngineIsReadyAndRunCallback(); } void TtsManager::RegisterEngine(fidl::InterfaceHandle<fuchsia::accessibility::tts::Engine> engine, @@ -43,7 +42,6 @@ result.set_err(fuchsia::accessibility::tts::Error::BUSY); } callback(std::move(result)); - CheckIfTtsEngineIsReadyAndRunCallback(); } void TtsManager::Enqueue(fuchsia::accessibility::tts::Utterance utterance, @@ -57,20 +55,6 @@ } } -void TtsManager::CheckIfTtsEngineIsReadyAndRunCallback() { - if (!engine_binding_.is_bound() || !engine_) { - return; - } - - for (const auto& callback : tts_engine_ready_callbacks_) { - if (callback) { - callback(); - } - } - - tts_engine_ready_callbacks_.clear(); -} - void TtsManager::Speak(SpeakCallback callback) { fuchsia::accessibility::tts::Engine_Speak_Result result; if (!engine_) { @@ -89,7 +73,4 @@ } } -void TtsManager::RegisterTTSEngineReadyCallback(TTSEngineReadyCallback callback) { - tts_engine_ready_callbacks_.emplace_back(std::move(callback)); -} } // namespace a11y
diff --git a/src/ui/a11y/lib/tts/tts_manager.h b/src/ui/a11y/lib/tts/tts_manager.h index fd9b3cc..eaf648a9 100644 --- a/src/ui/a11y/lib/tts/tts_manager.h +++ b/src/ui/a11y/lib/tts/tts_manager.h
@@ -23,8 +23,6 @@ public fuchsia::accessibility::tts::EngineRegistry, public fuchsia::accessibility::tts::Engine { public: - using TTSEngineReadyCallback = fit::function<void()>; - // On initialization, this class exposes the services defined in // |fuchsia.accessibility.tts.(TtsManager|EngineRegistry|Engine)| explicit TtsManager(sys::ComponentContext* startup_context); @@ -38,10 +36,6 @@ void RegisterEngine(fidl::InterfaceHandle<fuchsia::accessibility::tts::Engine> engine, RegisterEngineCallback callback) override; - // Registers a callback that will be invoked once the TTS engine is ready to receive speak - // requests. - virtual void RegisterTTSEngineReadyCallback(TTSEngineReadyCallback callback); - private: // |fuchsia.accessibility.tts.Engine| void Enqueue(fuchsia::accessibility::tts::Utterance utterance, EnqueueCallback callback) override; @@ -52,10 +46,6 @@ // |fuchsia.accessibility.tts.Engine| void Cancel(CancelCallback callback) override; - // Executes TTS engine ready callback(s) if both engine and speaker are - // connected. - void CheckIfTtsEngineIsReadyAndRunCallback(); - // Bindings to services implemented by this class. fidl::BindingSet<fuchsia::accessibility::tts::TtsManager> manager_bindings_; fidl::BindingSet<fuchsia::accessibility::tts::EngineRegistry> registry_bindings_; @@ -64,9 +54,6 @@ // Registered engine with this Tts manager. For now, only one engine is // allowed to be registered at a time. fuchsia::accessibility::tts::EnginePtr engine_; - - // Callback that will be invoked once the TTS engine is ready to receive speak requests. - std::vector<TTSEngineReadyCallback> tts_engine_ready_callbacks_; }; } // namespace a11y