Misc fix / cleanup to c/b/s/tts_controller_impl.cc

- in SpeakOrEnqueue, for tts platforms that were not yet ready, we were
  enqueueing the utterance then immediately clearing it if the utterance was
  !CanBeEnqueued. We should clear the queue, then add the utterance to be spoken
  later.
- cleanup for style/readability

R=dmazzoni@chromium.org

Change-Id: Ifb2311792c6dca042181871797de8aa0c67ba261
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2543374
Commit-Queue: David Tseng <dtseng@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#829254}

chrome/browser/speech/extension_api/tts_extension_apitest.cc

@@ -452,9 +452,11 @@ IN_PROC_BROWSER_TEST_F(TtsApiTest, PlatformPauseResume) {
 
 IN_PROC_BROWSER_TEST_F(TtsApiTest, PlatformPauseSpeakNoEnqueue) {
   // While paused, one utterance is enqueued, and then a second utterance that
-  // cannot be enqueued cancels both.
+  // cannot be enqueued cancels only the first.
   InSequence s;
   EXPECT_CALL(mock_platform_impl_, StopSpeaking()).WillOnce(Return(true));
+  EXPECT_CALL(mock_platform_impl_, DoSpeak(_, "text 2", _, _, _));
+  EXPECT_CALL(mock_platform_impl_, StopSpeaking()).WillOnce(Return(false));
   ASSERT_TRUE(RunExtensionTest("tts/pause_speak_no_enqueue")) << message_;
 }
 

chrome/test/data/extensions/api_test/tts/pause_speak_no_enqueue/test.js

@@ -5,38 +5,12 @@
 // TTS api test for Chrome.
 // browser_tests.exe --gtest_filter="TtsApiTest.*"
 
-chrome.test.runTests([
-  function testPauseCancel() {
-    var callbacks = 0;
-    chrome.tts.pause();
-    chrome.tts.speak(
-        'text 1',
-        {
-         'enqueue': true,
-         'onEvent': function(event) {
-            chrome.test.assertEq('cancelled', event.type);
-            callbacks++;
-         }
-        },
-        function() {
-          chrome.test.assertNoLastError();
-          callbacks++;
-        });
-    chrome.tts.speak(
-        'text 2',
-        {
-         'enqueue': false,
-         'onEvent': function(event) {
-           chrome.test.assertEq('cancelled', event.type);
-           callbacks++;
-           if (callbacks == 4) {
-             chrome.test.succeed();
-           }
-         }
-        },
-        function() {
-          chrome.test.assertNoLastError();
-          callbacks++;
-        });
-  }
-]);
+chrome.test.runTests([function testPauseCancel() {
+  chrome.tts.pause();
+  chrome.tts.speak('text 1', {'enqueue': true});
+  chrome.tts.speak('text 2', {'enqueue': false}, function() {
+    chrome.test.assertNoLastError();
+    chrome.test.succeed();
+  });
+  chrome.tts.resume();
+}]);

content/browser/speech/tts_controller_impl.cc

@@ -134,17 +134,19 @@ void TtsControllerImpl::SpeakOrEnqueue(
   // specific implementation is loaded to avoid racy behaviors.
   if (TtsPlatformLoading()) {
     bool can_enqueue = utterance->GetCanEnqueue();
-    utterance_list_.emplace_back(std::move(utterance));
     if (!can_enqueue)
       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()) {
-    utterance_list_.emplace_back(std::move(utterance));
     Stop();
+    utterance_list_.emplace_back(std::move(utterance));
     paused_ = true;
     return;
   }
@@ -539,12 +541,15 @@ void TtsControllerImpl::ClearUtteranceQueue(bool send_events) {
 }
 
 void TtsControllerImpl::FinishCurrentUtterance() {
-  if (current_utterance_) {
-    if (!current_utterance_->IsFinished())
-      current_utterance_->OnTtsEvent(TTS_EVENT_INTERRUPTED, kInvalidCharIndex,
-                                     kInvalidLength, std::string());
-    SetCurrentUtterance(nullptr);
+  if (!current_utterance_)
+    return;
+
+  if (!current_utterance_->IsFinished()) {
+    current_utterance_->OnTtsEvent(TTS_EVENT_INTERRUPTED, kInvalidCharIndex,
+                                   kInvalidLength, std::string());
   }
+
+  SetCurrentUtterance(nullptr);
 }
 
 void TtsControllerImpl::SpeakNextUtterance() {

content/browser/speech/tts_controller_unittest.cc

@@ -682,31 +682,35 @@ TEST_F(TtsControllerTest, SpeakWhenPausedAndCannotEnqueueUtterance) {
   controller()->Pause();
   EXPECT_TRUE(controller()->IsPausedForTesting());
 
-  // Speak an utterance while controller is paused. The utterance cannot be
-  // queued and should be dropped.
+  // Speak an utterance while controller is paused. The utterance clears the
+  // queue and is enqueued itself.
   controller()->SpeakOrEnqueue(std::move(utterance1));
-  EXPECT_TRUE(IsUtteranceListEmpty());
+  EXPECT_FALSE(IsUtteranceListEmpty());
+  EXPECT_EQ(1, controller()->QueueSize());
   EXPECT_FALSE(TtsControllerCurrentUtterance());
 
   // Speak an utterance that can be queued. The controller should stay paused
-  // and the second utterance must be queued.
+  // and the second utterance must be queued with the first also queued.
   std::unique_ptr<TtsUtteranceImpl> utterance2 =
       CreateUtteranceImpl(web_contents.get());
   utterance2->SetCanEnqueue(true);
 
   controller()->SpeakOrEnqueue(std::move(utterance2));
   EXPECT_TRUE(controller()->IsPausedForTesting());
+  EXPECT_EQ(2, controller()->QueueSize());
   EXPECT_FALSE(IsUtteranceListEmpty());
   EXPECT_FALSE(TtsControllerCurrentUtterance());
 
-  // Speak an utterance that cannot be queued should clear the queue.
+  // Speak an utterance that cannot be queued should clear the queue, then
+  // enqueue the new utterance.
   std::unique_ptr<TtsUtteranceImpl> utterance3 =
       CreateUtteranceImpl(web_contents.get());
   utterance3->SetCanEnqueue(false);
 
   controller()->SpeakOrEnqueue(std::move(utterance3));
   EXPECT_TRUE(controller()->IsPausedForTesting());
-  EXPECT_TRUE(IsUtteranceListEmpty());
+  EXPECT_FALSE(IsUtteranceListEmpty());
+  EXPECT_EQ(1, controller()->QueueSize());
   EXPECT_FALSE(TtsControllerCurrentUtterance());
 
   // Resume the controller.