Skip to content

Commit

Permalink
Fix potential hang in TtsControllerImpl
Browse files Browse the repository at this point in the history
In the attached bug, speech is triggered in a startup, fresh imaged
environment.

To replicate this,
`rm -rf /home/chronos/*`, then trigger some tts after boot.

The browser process hangs, e.g.
gdb attach <browser_pid>
...
bt

...
#5  0x000058e02342da8f in TtsExtensionEngine::GetVoices(content::BrowserContext*, std::__1::vector<content::VoiceData, std::__1::allocator<content::VoiceData> >*) ()
#6  0x000058e021612615 in content::TtsControllerImpl::SpeakNow(std::__1::unique_ptr<content::TtsUtterance, std::__1::default_delete<content::TtsUtterance> >) ()
#7  0x000058e021612e8f in content::TtsControllerImpl::SpeakNextUtterance() ()
#8  0x000058e01ff769d4 in content::TtsControllerImpl::VoicesChanged() ()
#9  0x000058e02342eeb4 in ExtensionTtsEngineUpdateVoicesFunction::Run() ()
#10 0x000058e02185ebe2 in ExtensionFunction::RunWithValidation() ()
#11 0x000058e021860c5d in extensions::ExtensionFunctionDispatcher::DispatchWithCallbackInternal(ExtensionHostMsg_Request_Params const&, content::RenderFrameHost*, int, base::RepeatingCallback<void (ExtensionFunction::ResponseType, base::ListValue const&, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&)> const&) ()
#12 0x000058e021860773 in extensions::ExtensionFunctionDispatcher::Dispatch(ExtensionHostMsg_Request_Params const&, content::RenderFrameHost*, int) ()
#13 0x000058e0218818ea in bool IPC::MessageT<ExtensionHostMsg_Request_Meta, std::__1::tuple<ExtensionHostMsg_Request_Params>, void>::Dispatch<extensions::ExtensionWebContentsObserver, extensions::ExtensionWebContentsObserver, content::RenderFrameHost, void (extensions::ExtensionWebContentsObserver::*)(content::RenderFrameHost*, ExtensionHostMsg_Request_Params const&)>(IPC::Message const*, extensions::ExtensionWebContentsObserver*, extensions::ExtensionWebContentsObserver*, content::RenderFrameHost*, void (extensions::ExtensionWebContentsObserver::*)(content::RenderFrameHost*, ExtensionHostMsg_Request_Params const&)) ()

The hang:
TtsControllerImpl::SpeakNextUtterance ->
TtsControllerImpl::SpeakNow ->
TtsControllerImpl::OnSpeakFinished ->
enqueues the utterance back onto |utterance_list_| on failure.

which occurs all while(... !utterance_list_.empty()), leading to a
browser hang.

Solution:
The above utterance fails immediately because the voice is not yet
ready.  We fix this by never returning the voice in the first place, in
TtsControllerImpl::GetVoices.

Therefore, we never get to the point where an utterance matches against
a not yet initialized voice, leading to an attempt to speak, then to an
immediate failure, next to a re-enqueue, and finally a hang.

Bug: 1161107
Change-Id: I2d88c0c6e86bf79ad9ac5dc5240825848daa40ae
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2642746
Commit-Queue: David Tseng <[email protected]>
Reviewed-by: Dominic Mazzoni <[email protected]>
Cr-Commit-Position: refs/heads/master@{#845861}
  • Loading branch information
dtsengchromium authored and Chromium LUCI CQ committed Jan 22, 2021
1 parent ec183d3 commit b34ba0a
Showing 1 changed file with 10 additions and 1 deletion.
11 changes: 10 additions & 1 deletion content/browser/speech/tts_controller_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -303,8 +303,10 @@ void TtsControllerImpl::GetVoices(BrowserContext* browser_context,
if (TtsPlatformReady())
tts_platform->GetVoices(out_voices);

if (browser_context && engine_delegate_)
if (browser_context && engine_delegate_ &&
engine_delegate_->IsBuiltInTtsEngineInitialized(browser_context)) {
engine_delegate_->GetVoices(browser_context, out_voices);
}
}

bool TtsControllerImpl::IsSpeaking() {
Expand Down Expand Up @@ -521,6 +523,8 @@ void TtsControllerImpl::OnSpeakFinished(int utterance_id, bool success) {
// the browser has built-in TTS that isn't loaded yet.
if (GetTtsPlatform()->LoadBuiltInTtsEngine(
current_utterance_->GetBrowserContext())) {
// Careful here; we're adding this utterance back to the queue, which can
// lead to hangs processing speech. See SpeakNextUtterance.
utterance_list_.emplace_back(std::move(current_utterance_));
return;
}
Expand Down Expand Up @@ -562,14 +566,19 @@ void TtsControllerImpl::SpeakNextUtterance() {

// Start speaking the next utterance in the queue. Keep trying in case
// one fails but there are still more in the queue to try.
TtsUtterance* previous_utterance = nullptr;
while (!utterance_list_.empty() && !current_utterance_) {
std::unique_ptr<TtsUtterance> utterance =
std::move(utterance_list_.front());
utterance_list_.pop_front();
DCHECK(previous_utterance != utterance.get());

if (ShouldSpeakUtterance(utterance.get()))
SpeakNow(std::move(utterance));
else
utterance->Finish();

previous_utterance = utterance.get();
}
}

Expand Down

0 comments on commit b34ba0a

Please sign in to comment.