From c06cdada49ac036536cecb02d89947a1cf1d7b7f Mon Sep 17 00:00:00 2001 From: David Tseng Date: Fri, 20 Nov 2020 01:21:43 +0000 Subject: [PATCH] Rename TtsUtterance::*CanEnqueue to TtsUtterance::*ShouldClearQueue TtsController owns a queue of TtsUtterance objects. On each call to SpeakOrEnqueue, the queue is processed in some way: - if the utterance requests a clear, the queue is cleared, and the utterance is enqueued (maybe implicitly via immediate processing) - if the utterance doesn't request a clear, the utterance is simply enqueued If the controller isn't ready (e.g. built in voices are loading, platform is initializing), or if the controller is in a paused state, processing ends. Otherwise, in both of the above cases thereafter, the queue is immediately processed (e.g. the controller begins dequeueing and speaking utterances in fifo order). Prior to this change, the "CanEnqueue" naming became somewhat misleading when dealing with utterances especially in paused/not ready scenarios; issue fixed here: https://chromium-review.googlesource.com/c/chromium/src/+/2543374 This change follows up by adjusting naming accordingly. R=avi@chromium.org, dmazzoni@chromium.org TBR=seantopping@chromium.org Change-Id: I30881e22b14b774220af1f7364ee1050bdd789d6 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2550430 Commit-Queue: David Tseng Reviewed-by: Avi Drissman Cr-Commit-Position: refs/heads/master@{#829464} --- .../chromeos/input_method/tts_handler.cc | 2 +- .../speech/extension_api/tts_extension_api.cc | 2 +- .../tts/pause_speak_no_enqueue/test.js | 12 +++++-- .../extensions/api/tts/tts_extension_api.cc | 2 +- .../browser/speech/speech_synthesis_impl.cc | 2 +- content/browser/speech/tts_controller_impl.cc | 8 ++--- .../browser/speech/tts_controller_unittest.cc | 32 +++++++++---------- content/browser/speech/tts_utterance_impl.cc | 10 +++--- content/browser/speech/tts_utterance_impl.h | 6 ++-- content/public/browser/tts_controller.h | 9 +++--- content/public/browser/tts_utterance.h | 6 ++-- 11 files changed, 50 insertions(+), 41 deletions(-) diff --git a/chrome/browser/chromeos/input_method/tts_handler.cc b/chrome/browser/chromeos/input_method/tts_handler.cc index b6390ef5f82880..698493b251a15d 100644 --- a/chrome/browser/chromeos/input_method/tts_handler.cc +++ b/chrome/browser/chromeos/input_method/tts_handler.cc @@ -36,7 +36,7 @@ void TtsHandler::Speak(const std::string& text) { content::TtsUtterance::Create(profile_); utterance->SetText(text); utterance->SetEventDelegate(this); - utterance->SetCanEnqueue(true); + utterance->SetShouldClearQueue(false); auto* tts_controller = content::TtsController::GetInstance(); tts_controller->SpeakOrEnqueue(std::move(utterance)); diff --git a/chrome/browser/speech/extension_api/tts_extension_api.cc b/chrome/browser/speech/extension_api/tts_extension_api.cc index 3e55f8efc10c14..1d71a2e736f3b2 100644 --- a/chrome/browser/speech/extension_api/tts_extension_api.cc +++ b/chrome/browser/speech/extension_api/tts_extension_api.cc @@ -288,7 +288,7 @@ ExtensionFunction::ResponseAction TtsSpeakFunction::Run() { utterance->SetSrcUrl(source_url()); utterance->SetLang(lang); utterance->SetContinuousParameters(rate, pitch, volume); - utterance->SetCanEnqueue(can_enqueue); + utterance->SetShouldClearQueue(!can_enqueue); utterance->SetRequiredEventTypes(required_event_types); utterance->SetDesiredEventTypes(desired_event_types); utterance->SetEngineId(voice_extension_id); diff --git a/chrome/test/data/extensions/api_test/tts/pause_speak_no_enqueue/test.js b/chrome/test/data/extensions/api_test/tts/pause_speak_no_enqueue/test.js index af5f8b2de62d70..06c2db1071d893 100644 --- a/chrome/test/data/extensions/api_test/tts/pause_speak_no_enqueue/test.js +++ b/chrome/test/data/extensions/api_test/tts/pause_speak_no_enqueue/test.js @@ -6,11 +6,19 @@ // browser_tests.exe --gtest_filter="TtsApiTest.*" chrome.test.runTests([function testPauseCancel() { + let gotSecondSpeak = false; chrome.tts.pause(); - chrome.tts.speak('text 1', {'enqueue': true}); + chrome.tts.speak('text 1', { + 'enqueue': true, + 'onEvent': event => { + if (event.type == 'cancelled' && gotSecondSpeak) { + chrome.test.succeed(); + } + } + }); chrome.tts.speak('text 2', {'enqueue': false}, function() { chrome.test.assertNoLastError(); - chrome.test.succeed(); + gotSecondSpeak = true; }); chrome.tts.resume(); }]); diff --git a/chromecast/browser/extensions/api/tts/tts_extension_api.cc b/chromecast/browser/extensions/api/tts/tts_extension_api.cc index ac773d4da5fcbc..8e0e940386a66a 100644 --- a/chromecast/browser/extensions/api/tts/tts_extension_api.cc +++ b/chromecast/browser/extensions/api/tts/tts_extension_api.cc @@ -264,7 +264,7 @@ ExtensionFunction::ResponseAction TtsSpeakFunction::Run() { utterance->SetSrcUrl(source_url()); utterance->SetLang(lang); utterance->SetContinuousParameters(rate, pitch, volume); - utterance->SetCanEnqueue(can_enqueue); + utterance->SetShouldClearQueue(!can_enqueue); utterance->SetRequiredEventTypes(required_event_types); utterance->SetDesiredEventTypes(desired_event_types); utterance->SetEngineId(voice_extension_id); diff --git a/content/browser/speech/speech_synthesis_impl.cc b/content/browser/speech/speech_synthesis_impl.cc index e46103bcb660df..a3514be021554c 100644 --- a/content/browser/speech/speech_synthesis_impl.cc +++ b/content/browser/speech/speech_synthesis_impl.cc @@ -129,7 +129,7 @@ void SpeechSynthesisImpl::Speak( tts_utterance->SetText(utterance->text); tts_utterance->SetLang(utterance->lang); tts_utterance->SetVoiceName(utterance->voice); - tts_utterance->SetCanEnqueue(true); + tts_utterance->SetShouldClearQueue(false); tts_utterance->SetContinuousParameters(utterance->rate, utterance->pitch, utterance->volume); diff --git a/content/browser/speech/tts_controller_impl.cc b/content/browser/speech/tts_controller_impl.cc index 84967c5a82f074..4ed83ffaa39730 100644 --- a/content/browser/speech/tts_controller_impl.cc +++ b/content/browser/speech/tts_controller_impl.cc @@ -133,25 +133,23 @@ void TtsControllerImpl::SpeakOrEnqueue( // engine implementation. Every utterances are postponed until the platform // specific implementation is loaded to avoid racy behaviors. if (TtsPlatformLoading()) { - bool can_enqueue = utterance->GetCanEnqueue(); - if (!can_enqueue) + if (utterance->GetShouldClearQueue()) ClearUtteranceQueue(true); utterance_list_.emplace_back(std::move(utterance)); - return; } // If we're paused and we get an utterance that can't be queued, // flush the queue but stay in the paused state. - if (paused_ && !utterance->GetCanEnqueue()) { + if (paused_ && utterance->GetShouldClearQueue()) { Stop(); utterance_list_.emplace_back(std::move(utterance)); paused_ = true; return; } - if (paused_ || (IsSpeaking() && utterance->GetCanEnqueue())) { + if (paused_ || (IsSpeaking() && !utterance->GetShouldClearQueue())) { utterance_list_.emplace_back(std::move(utterance)); } else { Stop(); diff --git a/content/browser/speech/tts_controller_unittest.cc b/content/browser/speech/tts_controller_unittest.cc index b84fe3e07fe130..c1d7ae168da817 100644 --- a/content/browser/speech/tts_controller_unittest.cc +++ b/content/browser/speech/tts_controller_unittest.cc @@ -232,12 +232,12 @@ class TtsControllerTest : public testing::Test { TEST_F(TtsControllerTest, TestTtsControllerShutdown) { std::unique_ptr utterance1 = TtsUtterance::Create(); - utterance1->SetCanEnqueue(true); + utterance1->SetShouldClearQueue(false); utterance1->SetSrcId(1); controller()->SpeakOrEnqueue(std::move(utterance1)); std::unique_ptr utterance2 = TtsUtterance::Create(); - utterance2->SetCanEnqueue(true); + utterance2->SetShouldClearQueue(false); utterance2->SetSrcId(2); controller()->SpeakOrEnqueue(std::move(utterance2)); @@ -259,7 +259,7 @@ TEST_F(TtsControllerTest, TestBrowserContextRemoved) { std::unique_ptr utterance1 = TtsUtterance::Create(browser_context()); utterance1->SetEngineId("x"); - utterance1->SetCanEnqueue(true); + utterance1->SetShouldClearQueue(false); utterance1->SetSrcId(1); controller()->SpeakOrEnqueue(std::move(utterance1)); @@ -271,7 +271,7 @@ TEST_F(TtsControllerTest, TestBrowserContextRemoved) { std::unique_ptr utterance2 = TtsUtterance::Create(browser_context()); utterance2->SetEngineId("x"); - utterance2->SetCanEnqueue(true); + utterance2->SetShouldClearQueue(false); utterance2->SetSrcId(2); controller()->SpeakOrEnqueue(std::move(utterance2)); @@ -487,7 +487,7 @@ TEST_F(TtsControllerTest, StartsQueuedUtteranceWhenWebContentsDestroyed) { void* raw_utterance1 = utterance1.get(); std::unique_ptr utterance2 = CreateUtteranceImpl(web_contents2.get()); - utterance2->SetCanEnqueue(true); + utterance2->SetShouldClearQueue(false); void* raw_utterance2 = utterance2.get(); controller()->SpeakOrEnqueue(std::move(utterance1)); @@ -514,8 +514,8 @@ TEST_F(TtsControllerTest, StartsQueuedUtteranceWhenWebContentsDestroyed2) { std::unique_ptr utterance3 = CreateUtteranceImpl(web_contents2.get()); void* raw_utterance3 = utterance3.get(); - utterance2->SetCanEnqueue(true); - utterance3->SetCanEnqueue(true); + utterance2->SetShouldClearQueue(false); + utterance3->SetShouldClearQueue(false); controller()->SpeakOrEnqueue(std::move(utterance1)); controller()->SpeakOrEnqueue(std::move(utterance2)); @@ -566,7 +566,7 @@ TEST_F(TtsControllerTest, SkipsQueuedUtteranceFromHiddenWebContents) { const int utterance1_id = utterance1->GetId(); std::unique_ptr utterance2 = CreateUtteranceImpl(web_contents2.get()); - utterance2->SetCanEnqueue(true); + utterance2->SetShouldClearQueue(false); controller()->SpeakOrEnqueue(std::move(utterance1)); EXPECT_TRUE(TtsControllerCurrentUtterance()); @@ -600,7 +600,7 @@ TEST_F(TtsControllerTest, SpeakPauseResume) { std::unique_ptr web_contents = CreateWebContents(); std::unique_ptr utterance = CreateUtteranceImpl(web_contents.get()); - utterance->SetCanEnqueue(true); + utterance->SetShouldClearQueue(false); // Start speaking an utterance. controller()->SpeakOrEnqueue(std::move(utterance)); @@ -640,7 +640,7 @@ TEST_F(TtsControllerTest, SpeakWhenPaused) { std::unique_ptr web_contents = CreateWebContents(); std::unique_ptr utterance = CreateUtteranceImpl(web_contents.get()); - utterance->SetCanEnqueue(true); + utterance->SetShouldClearQueue(false); // Pause the controller. controller()->Pause(); @@ -676,7 +676,7 @@ TEST_F(TtsControllerTest, SpeakWhenPausedAndCannotEnqueueUtterance) { std::unique_ptr web_contents = CreateWebContents(); std::unique_ptr utterance1 = CreateUtteranceImpl(web_contents.get()); - utterance1->SetCanEnqueue(false); + utterance1->SetShouldClearQueue(true); // Pause the controller. controller()->Pause(); @@ -693,7 +693,7 @@ TEST_F(TtsControllerTest, SpeakWhenPausedAndCannotEnqueueUtterance) { // and the second utterance must be queued with the first also queued. std::unique_ptr utterance2 = CreateUtteranceImpl(web_contents.get()); - utterance2->SetCanEnqueue(true); + utterance2->SetShouldClearQueue(false); controller()->SpeakOrEnqueue(std::move(utterance2)); EXPECT_TRUE(controller()->IsPausedForTesting()); @@ -705,7 +705,7 @@ TEST_F(TtsControllerTest, SpeakWhenPausedAndCannotEnqueueUtterance) { // enqueue the new utterance. std::unique_ptr utterance3 = CreateUtteranceImpl(web_contents.get()); - utterance3->SetCanEnqueue(false); + utterance3->SetShouldClearQueue(true); controller()->SpeakOrEnqueue(std::move(utterance3)); EXPECT_TRUE(controller()->IsPausedForTesting()); @@ -722,7 +722,7 @@ TEST_F(TtsControllerTest, StopMustResumeController) { std::unique_ptr web_contents = CreateWebContents(); std::unique_ptr utterance = CreateUtteranceImpl(web_contents.get()); - utterance->SetCanEnqueue(true); + utterance->SetShouldClearQueue(false); // Speak an utterance while controller is paused. The utterance is queued. controller()->SpeakOrEnqueue(std::move(utterance)); @@ -747,7 +747,7 @@ TEST_F(TtsControllerTest, PauseAndStopMustResumeController) { std::unique_ptr web_contents = CreateWebContents(); std::unique_ptr utterance = CreateUtteranceImpl(web_contents.get()); - utterance->SetCanEnqueue(true); + utterance->SetShouldClearQueue(false); // Pause the controller. controller()->Pause(); @@ -796,7 +796,7 @@ TEST_F(TtsControllerTest, SpeakWhenLoading) { std::unique_ptr web_contents = CreateWebContents(); std::unique_ptr utterance = CreateUtteranceImpl(web_contents.get()); - utterance->SetCanEnqueue(true); + utterance->SetShouldClearQueue(false); // Speak an utterance while platform is loading, the utterance should be // queued. diff --git a/content/browser/speech/tts_utterance_impl.cc b/content/browser/speech/tts_utterance_impl.cc index 20d442a8959bf4..a30b37420f49c4 100644 --- a/content/browser/speech/tts_utterance_impl.cc +++ b/content/browser/speech/tts_utterance_impl.cc @@ -63,7 +63,7 @@ TtsUtteranceImpl::TtsUtteranceImpl(BrowserContext* browser_context, was_created_with_web_contents_(web_contents != nullptr), id_(next_utterance_id_++), src_id_(-1), - can_enqueue_(false), + should_clear_queue_(true), char_index_(0), finished_(false) { options_.reset(new base::DictionaryValue()); @@ -155,12 +155,12 @@ TtsUtteranceImpl::GetContinuousParameters() { return continuous_parameters_; } -void TtsUtteranceImpl::SetCanEnqueue(bool can_enqueue) { - can_enqueue_ = can_enqueue; +void TtsUtteranceImpl::SetShouldClearQueue(bool value) { + should_clear_queue_ = value; } -bool TtsUtteranceImpl::GetCanEnqueue() { - return can_enqueue_; +bool TtsUtteranceImpl::GetShouldClearQueue() { + return should_clear_queue_; } void TtsUtteranceImpl::SetRequiredEventTypes( diff --git a/content/browser/speech/tts_utterance_impl.h b/content/browser/speech/tts_utterance_impl.h index 2b54961acbd4ec..80f3e11761f560 100644 --- a/content/browser/speech/tts_utterance_impl.h +++ b/content/browser/speech/tts_utterance_impl.h @@ -62,8 +62,8 @@ class CONTENT_EXPORT TtsUtteranceImpl : public TtsUtterance, const double volume) override; const UtteranceContinuousParameters& GetContinuousParameters() override; - void SetCanEnqueue(bool can_enqueue) override; - bool GetCanEnqueue() override; + void SetShouldClearQueue(bool value) override; + bool GetShouldClearQueue() override; void SetRequiredEventTypes(const std::set& types) override; const std::set& GetRequiredEventTypes() override; @@ -123,7 +123,7 @@ class CONTENT_EXPORT TtsUtteranceImpl : public TtsUtterance, std::string voice_name_; std::string lang_; UtteranceContinuousParameters continuous_parameters_; - bool can_enqueue_; + bool should_clear_queue_; std::set required_event_types_; std::set desired_event_types_; diff --git a/content/public/browser/tts_controller.h b/content/public/browser/tts_controller.h index ced4805ea7689d..04934c66497a0c 100644 --- a/content/public/browser/tts_controller.h +++ b/content/public/browser/tts_controller.h @@ -87,10 +87,11 @@ class CONTENT_EXPORT TtsController { // Returns true if we're currently speaking an utterance. virtual bool IsSpeaking() = 0; - // Speak the given utterance. If the utterance's can_enqueue flag is true - // and another utterance is in progress, adds it to the end of the queue. - // Otherwise, interrupts any current utterance and speaks this one - // immediately. + // Speak the given utterance. If the utterance's should_flush_queue flag is + // true, clears the speech queue including the currently speaking utterance + // (if one exists), and starts processing the speech queue by speaking the new + // utterance immediately. Otherwise, enqueues the new utterance and triggers + // continued processing of the speech queue. virtual void SpeakOrEnqueue(std::unique_ptr utterance) = 0; // Stop all utterances and flush the queue. Implies leaving pause mode diff --git a/content/public/browser/tts_utterance.h b/content/public/browser/tts_utterance.h index 601574c511db62..774266a9c0cee5 100644 --- a/content/public/browser/tts_utterance.h +++ b/content/public/browser/tts_utterance.h @@ -107,8 +107,10 @@ class CONTENT_EXPORT TtsUtterance { const double volume) = 0; virtual const UtteranceContinuousParameters& GetContinuousParameters() = 0; - virtual void SetCanEnqueue(bool can_enqueue) = 0; - virtual bool GetCanEnqueue() = 0; + // Prior to processing this utterance, determines whether the utterance queue + // gets cleared. + virtual void SetShouldClearQueue(bool value) = 0; + virtual bool GetShouldClearQueue() = 0; virtual void SetRequiredEventTypes(const std::set& types) = 0; virtual const std::set& GetRequiredEventTypes() = 0;