Allows endCallbacks in tts to queue up speech in the speech queue and simplify speech output logic.

Allows endCallbacks in tts to queue up speech in the speech queue and simplify speech output logic.

- the speech queue was clearing the current utterance _after_ executing end callbacks; end callbacks themselves could have generated new utterances, so this cleared the new utterance rather than the previous "current" ones.
- remove overriding of onEvent by output.js.
- update continous read test.

TEST=chromevox_tests --run-manual passes.
BUG=461824
Committed: https://crrev.com/dfb97c2fe170e638d7cdf40e769a350e6cecf10c
Cr-Commit-Position: refs/heads/master@{#318061}

[David Tseng, 2015-02-24 22:46:14]

dtseng@chromium.org changed reviewers:
+ dmazzoni@chromium.org

[dmazzoni, 2015-02-24 22:59:07]

https://codereview.chromium.org/955643006/diff/1/chrome/browser/resources/chr...
File chrome/browser/resources/chromeos/chromevox/host/chrome/tts_background.js
(left):

https://codereview.chromium.org/955643006/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/host/chrome/tts_background.js:305:
this.cancelUtterance_(this.currentUtterance_);
Are you totally sure this won't break ChromeVox classic?

https://codereview.chromium.org/955643006/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/host/chrome/tts_background.js:460:
utterance.properties['endCallback']();
Does the endCallback not get called when speech is cancelled now?

[David Tseng, 2015-02-24 23:17:56]

https://codereview.chromium.org/955643006/diff/1/chrome/browser/resources/chr...
File chrome/browser/resources/chromeos/chromevox/host/chrome/tts_background.js
(left):

https://codereview.chromium.org/955643006/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/host/chrome/tts_background.js:305:
this.cancelUtterance_(this.currentUtterance_);
On 2015/02/24 22:59:07, dmazzoni wrote:
> Are you totally sure this won't break ChromeVox classic?

I'm pretty sure; I'm not clear on why we kept the previous behavior (which I
assume you persisted) where we called end callbacks when canceling or
interrupting an utterance.

I tested manually with Classic:
- continuous read
- reading of multi element utterances (e.g. a single line).
- gmail (api_implementation.js usage of callbacks).

A somewhat related issue I've recently seen is double speaking of some
utterances which may or may not be related to this.

https://codereview.chromium.org/955643006/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/host/chrome/tts_background.js:460:
utterance.properties['endCallback']();
On 2015/02/24 22:59:07, dmazzoni wrote:
> Does the endCallback not get called when speech is cancelled now?

Why were we expecting this before? End callbacks usually come as a result of
navigation utterances chunking up (e.g. multiple elements on a line). Cancelling
seems like it should stop the whole utterance chain.

[dmazzoni, 2015-02-24 23:35:09]

https://codereview.chromium.org/955643006/diff/1/chrome/browser/resources/chr...
File chrome/browser/resources/chromeos/chromevox/host/chrome/tts_background.js
(left):

https://codereview.chromium.org/955643006/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/host/chrome/tts_background.js:460:
utterance.properties['endCallback']();
On 2015/02/24 23:17:56, David Tseng wrote:
> On 2015/02/24 22:59:07, dmazzoni wrote:
> > Does the endCallback not get called when speech is cancelled now?
> 
> Why were we expecting this before? End callbacks usually come as a result of
> navigation utterances chunking up (e.g. multiple elements on a line).
Cancelling
> seems like it should stop the whole utterance chain.

One issue is leaking resources. Every time you call speak() from a content
script and have a callback, we add a mapping from the id to the callback closure
to a big mapping in tts.js, and clean it up when we get the response.

[David Tseng, 2015-02-25 00:51:17]

https://codereview.chromium.org/955643006/diff/1/chrome/browser/resources/chr...
File chrome/browser/resources/chromeos/chromevox/host/chrome/tts_background.js
(left):

https://codereview.chromium.org/955643006/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/host/chrome/tts_background.js:460:
utterance.properties['endCallback']();
On 2015/02/24 23:35:09, dmazzoni wrote:
> On 2015/02/24 23:17:56, David Tseng wrote:
> > On 2015/02/24 22:59:07, dmazzoni wrote:
> > > Does the endCallback not get called when speech is cancelled now?
> > 
> > Why were we expecting this before? End callbacks usually come as a result of
> > navigation utterances chunking up (e.g. multiple elements on a line).
> Cancelling
> > seems like it should stop the whole utterance chain.
> 
> One issue is leaking resources. Every time you call speak() from a content
> script and have a callback, we add a mapping from the id to the callback
closure
> to a big mapping in tts.js, and clean it up when we get the response.

Added a dictionary attribute to distinguish from the cleanup case vs the
invokation + cleanup case.

[dmazzoni, 2015-02-25 01:02:21]

lgtm

https://codereview.chromium.org/955643006/diff/20001/chrome/browser/resources...
File
chrome/browser/resources/chromeos/chromevox/chromevox/background/background.js
(right):

https://codereview.chromium.org/955643006/diff/20001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/chromevox/background/background.js:386:
cleanupOnly: opt_cleanupOnly,
keep the syntax for object key ids consistent - there should be quotes around
cleanupOnly (like 'message' and 'id') - maybe to stress that these are
serialized, or out of paranoia over Closure renaming

https://codereview.chromium.org/955643006/diff/20001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/chromevox/background/background.js:393:
cleanupOnly: opt_cleanupOnly,
same

[David Tseng, 2015-02-25 15:56:51]

https://codereview.chromium.org/955643006/diff/20001/chrome/browser/resources...
File
chrome/browser/resources/chromeos/chromevox/chromevox/background/background.js
(right):

https://codereview.chromium.org/955643006/diff/20001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/chromevox/background/background.js:386:
cleanupOnly: opt_cleanupOnly,
On 2015/02/25 01:02:21, dmazzoni wrote:
> keep the syntax for object key ids consistent - there should be quotes around
> cleanupOnly (like 'message' and 'id') - maybe to stress that these are
> serialized, or out of paranoia over Closure renaming

Yeah, you're right...that's consistent and distinguishes JSON vs a js Object.
Done.
Note that I've omitted quotes in ChromeVox Next code.

https://codereview.chromium.org/955643006/diff/20001/chrome/browser/resources...
chrome/browser/resources/chromeos/chromevox/chromevox/background/background.js:393:
cleanupOnly: opt_cleanupOnly,
On 2015/02/25 01:02:21, dmazzoni wrote:
> same

Done.

[David Tseng, 2015-02-25 16:03:37]

The CQ bit was checked by dtseng@chromium.org

[David Tseng, 2015-02-25 16:03:37]

The patchset sent to the CQ was uploaded after l-g-t-m from
dmazzoni@chromium.org
Link to the patchset: https://codereview.chromium.org/955643006/#ps40001 (title:
" ")

[commit-bot: I haz the power, 2015-02-25 16:04:11]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/955643006/40001

[commit-bot: I haz the power, 2015-02-25 16:38:52]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-02-25 16:39:33]

Patchset 3 (id:??) landed as
https://crrev.com/dfb97c2fe170e638d7cdf40e769a350e6cecf10c
Cr-Commit-Position: refs/heads/master@{#318061}