Commit c3123cb9 authored by Katie D's avatar Katie D Committed by Commit Bot

Fix race condition where speech was canceled after starting.

This occured when a user used seach+s to start speech while speech
was already in progress. The change makes sure that chrome.tts.stop
is called before the 'isSpeaking' callback is fired, to make sure
that it doesn't get called late.

This may cause a few cancel events to not be logged if the tts engine
responds very quickly, but based on some manual testing it appears
that events should be fine because the tts engine has some latency.

I'm not sure how to write tests for this because it requires a lot
of timing and a real TTS engine. Open to suggestions. Works well based
on local testing, repeatedly using search+s and search+mouse to
to cancel and restart speech.

Bug: 819926
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I9b59caa4d69be1c0a8ae14af6c850113ff140360
Reviewed-on: https://chromium-review.googlesource.com/956226Reviewed-by: default avatarDavid Tseng <dtseng@chromium.org>
Commit-Queue: Katie Dektar <katie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541943}
parent 5b621734
...@@ -202,7 +202,7 @@ SelectToSpeak.prototype = { ...@@ -202,7 +202,7 @@ SelectToSpeak.prototype = {
this.trackingMouse_ = true; this.trackingMouse_ = true;
this.didTrackMouse_ = true; this.didTrackMouse_ = true;
this.mouseStart_ = {x: evt.screenX, y: evt.screenY}; this.mouseStart_ = {x: evt.screenX, y: evt.screenY};
chrome.tts.stop(); this.cancelIfSpeaking_();
// Fire a hit test event on click to warm up the cache. // Fire a hit test event on click to warm up the cache.
this.desktop_.hitTest(evt.screenX, evt.screenY, EventType.MOUSE_PRESSED); this.desktop_.hitTest(evt.screenX, evt.screenY, EventType.MOUSE_PRESSED);
...@@ -345,7 +345,7 @@ SelectToSpeak.prototype = { ...@@ -345,7 +345,7 @@ SelectToSpeak.prototype = {
if (this.isSelectionKeyDown_ && this.keysPressedTogether_.size == 2 && if (this.isSelectionKeyDown_ && this.keysPressedTogether_.size == 2 &&
this.keysPressedTogether_.has(evt.keyCode) && this.keysPressedTogether_.has(evt.keyCode) &&
this.keysPressedTogether_.has(SelectToSpeak.SEARCH_KEY_CODE)) { this.keysPressedTogether_.has(SelectToSpeak.SEARCH_KEY_CODE)) {
chrome.tts.isSpeaking(this.cancelIfSpeaking_.bind(this)); this.cancelIfSpeaking_();
chrome.automation.getFocus(this.requestSpeakSelectedText_.bind(this)); chrome.automation.getFocus(this.requestSpeakSelectedText_.bind(this));
} }
this.isSelectionKeyDown_ = false; this.isSelectionKeyDown_ = false;
...@@ -367,7 +367,7 @@ SelectToSpeak.prototype = { ...@@ -367,7 +367,7 @@ SelectToSpeak.prototype = {
this.keysPressedTogether_.has(evt.keyCode) && this.keysPressedTogether_.has(evt.keyCode) &&
this.keysPressedTogether_.size == 1) { this.keysPressedTogether_.size == 1) {
this.trackingMouse_ = false; this.trackingMouse_ = false;
chrome.tts.isSpeaking(this.cancelIfSpeaking_.bind(this)); this.cancelIfSpeaking_();
} }
this.keysCurrentlyDown_.delete(evt.keyCode); this.keysCurrentlyDown_.delete(evt.keyCode);
...@@ -750,7 +750,7 @@ SelectToSpeak.prototype = { ...@@ -750,7 +750,7 @@ SelectToSpeak.prototype = {
* Prepares for speech. Call once before chrome.tts.speak is called. * Prepares for speech. Call once before chrome.tts.speak is called.
*/ */
prepareForSpeech_: function() { prepareForSpeech_: function() {
chrome.tts.stop(); this.cancelIfSpeaking_();
if (this.intervalRef_ !== undefined) { if (this.intervalRef_ !== undefined) {
clearInterval(this.intervalRef_); clearInterval(this.intervalRef_);
} }
...@@ -794,11 +794,22 @@ SelectToSpeak.prototype = { ...@@ -794,11 +794,22 @@ SelectToSpeak.prototype = {
}, },
/** /**
* Cancels the current speech queue if speech is in progress. * Cancels the current speech queue after doing a callback to
* record a cancel event if speech was in progress. We must cancel
* before the callback (rather than in it) to avoid race conditions
* where cancel is called twice.
*/
cancelIfSpeaking_: function() {
chrome.tts.isSpeaking(this.recordCancelIfSpeaking_.bind(this));
this.stopAll_();
},
/**
* Records a cancel event if speech was in progress.
* @param {boolean} speaking Whether speech was in progress
*/ */
cancelIfSpeaking_: function(speaking) { recordCancelIfSpeaking_: function(speaking) {
if (speaking) { if (speaking) {
this.stopAll_();
this.recordCancelEvent_(); this.recordCancelEvent_();
} }
}, },
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment