Use an enum for ChromeVox queue mode.

Use an enum for ChromeVox queue mode.

There were lots and lots of lazy uses of this API function!

BUG=none

Committed: https://crrev.com/f7803cb1efe53a363854da07b058da664601eba2
Cr-Commit-Position: refs/heads/master@{#297543}

[dmazzoni, 2014-09-26 20:54:09]

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

[dmazzoni, 2014-09-26 20:54:10]

Again, whoever wants to look at this first.

This one probably needs more scrutiny to make sure I didn't accidentally change
something.

[David Tseng, 2014-09-26 22:29:23]

lgtm

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
File chrome/browser/resources/chromeos/chromevox/host/interface/tts_interface.js
(right):

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/host/interface/tts_interface.js:57:
* @param {cvox.QueueMode} queueMode The queue mode to use for speaking.
I pretty much just looked at this file. I assume Closure compiler correctly
enforces this now (so that speak('foo', 0) is no longer acceptable).

[Peter Lundblad, 2014-09-29 13:51:04]

lgtm

lgtm with comments.  Nice cleanup!

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
File
chrome/browser/resources/chromeos/chromevox/chromevox/background/accessibility_api_handler.js
(right):

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/chromevox/background/accessibility_api_handler.js:109:
* @type {cvox.QueueMode}
nit: should goog.require cvox.QueueMode.

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
File
chrome/browser/resources/chromeos/chromevox/chromevox/background/background.js
(right):

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/chromevox/background/background.js:143:
this.tts.speak(cvox.ChromeVox.msgs.getMsg('chromevox_inactive'),
nit: this file should goog.require cvox.QueueMode.

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/chromevox/background/background.js:347:
cvox.QueueMode.FLUSH);
This changes from QUEUE to flush, I assume that's intentional.

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
File chrome/browser/resources/chromeos/chromevox/chromevox/background/options.js
(right):

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/chromevox/background/options.js:559:
* @param {cvox.QueueMode=} queueMode The queue mode to use.
Should we make this required for consistency?

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
File
chrome/browser/resources/chromeos/chromevox/chromevox/injected/api_implementation.js
(right):

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/chromevox/injected/api_implementation.js:265:
if (opt_queueMode == undefined) {
Do you think it is safe to fix this == to a === while here?

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
File
chrome/browser/resources/chromeos/chromevox/chromevox/injected/live_regions.js
(right):

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/chromevox/injected/live_regions.js:91:
if (queueMode == undefined) {
This shouldn't happen if the compiler works correctly.

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
File
chrome/browser/resources/chromeos/chromevox/chromevox/injected/pdf_processor.js
(right):

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/chromevox/injected/pdf_processor.js:34:
'Unable to access copy-protected PDF. Skipping.',
I know this is not the purpose of this CL, but since you touch this
call anyway, perhaps internationalize this string. Your call.

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
File
chrome/browser/resources/chromeos/chromevox/chromevox/injected/ui/search_widget.js
(right):

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/chromevox/injected/ui/search_widget.js:324:
cvox.ChromeVox.tts.speak('Ignoring case.', cvox.QueueMode.FLUSH, null);
i18n if you don't mind.

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
File chrome/browser/resources/chromeos/chromevox/common/nav_description.js
(right):

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/common/nav_description.js:95: *
@param {function()=} endCallback Function called when this ends speaking.
nit: optional parameters should have opt_ prepended to their names.

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
File chrome/browser/resources/chromeos/chromevox/extensions/searchvox/search.js
(right):

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/extensions/searchvox/search.js:191:
cvox.QueueMode.QUEUE);
i18n

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
File chrome/browser/resources/chromeos/chromevox/host/chrome/tts.js (right):

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/host/chrome/tts.js:110: * @param
{cvox.QueueMode=} queueMode The queue mode.
I think this can be converted to required now.

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

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/host/chrome/tts_background.js:221:
if (queueMode === undefined) {
This is redundant now that queueMode is required.

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/host/chrome/tts_background.js:221:
if (queueMode === undefined) {
This is now redundant.

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
File chrome/browser/resources/chromeos/chromevox/host/interface/tts_interface.js
(right):

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/host/interface/tts_interface.js:12:
goog.provide('cvox.QueueMode');
In basically all other files, you should add a corresponding goog.require.

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/host/interface/tts_interface.js:17:
* Queue modes for calls to speak().
nit: Qualify speak, i.e. {@code cvox.TtsInterface.speak} since this is
its own object outside of TtsInterface.

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/host/interface/tts_interface.js:26:
FLUSH: 0,
Would be nice to have the jsdoc inside the {} pair above each enum value.

[dmazzoni, 2014-09-30 22:01:11]

The CQ bit was checked by dmazzoni@chromium.org

[dmazzoni, 2014-09-30 22:01:20]

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
File
chrome/browser/resources/chromeos/chromevox/chromevox/background/accessibility_api_handler.js
(right):

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/chromevox/background/accessibility_api_handler.js:109:
* @type {cvox.QueueMode}
On 2014/09/29 13:51:03, Peter Lundblad wrote:
> nit: should goog.require cvox.QueueMode.

Done.

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
File
chrome/browser/resources/chromeos/chromevox/chromevox/background/background.js
(right):

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/chromevox/background/background.js:143:
this.tts.speak(cvox.ChromeVox.msgs.getMsg('chromevox_inactive'),
On 2014/09/29 13:51:03, Peter Lundblad wrote:
> nit: this file should goog.require cvox.QueueMode.

Done.

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/chromevox/background/background.js:347:
cvox.QueueMode.FLUSH);
On 2014/09/29 13:51:03, Peter Lundblad wrote:
> This changes from QUEUE to flush, I assume that's intentional.

Acknowledged.

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
File chrome/browser/resources/chromeos/chromevox/chromevox/background/options.js
(right):

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/chromevox/background/options.js:559:
* @param {cvox.QueueMode=} queueMode The queue mode to use.
On 2014/09/29 13:51:03, Peter Lundblad wrote:
> Should we make this required for consistency?

Done.

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
File
chrome/browser/resources/chromeos/chromevox/chromevox/injected/api_implementation.js
(right):

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/chromevox/injected/api_implementation.js:265:
if (opt_queueMode == undefined) {
On 2014/09/29 13:51:03, Peter Lundblad wrote:
> Do you think it is safe to fix this == to a === while here?

Hmmm, I'd prefer to make sure we handle "null" correctly too, since this file
might get speech requests from the API. Elsewhere that'd be fine with me.

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
File
chrome/browser/resources/chromeos/chromevox/chromevox/injected/live_regions.js
(right):

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/chromevox/injected/live_regions.js:91:
if (queueMode == undefined) {
On 2014/09/29 13:51:03, Peter Lundblad wrote:
> This shouldn't happen if the compiler works correctly.

Done.

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
File
chrome/browser/resources/chromeos/chromevox/chromevox/injected/pdf_processor.js
(right):

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/chromevox/injected/pdf_processor.js:34:
'Unable to access copy-protected PDF. Skipping.',
On 2014/09/29 13:51:04, Peter Lundblad wrote:
> I know this is not the purpose of this CL, but since you touch this
> call anyway, perhaps internationalize this string. Your call.

Done.

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
File
chrome/browser/resources/chromeos/chromevox/chromevox/injected/ui/search_widget.js
(right):

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/chromevox/injected/ui/search_widget.js:324:
cvox.ChromeVox.tts.speak('Ignoring case.', cvox.QueueMode.FLUSH, null);
On 2014/09/29 13:51:04, Peter Lundblad wrote:
> i18n if you don't mind.

Done.

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
File chrome/browser/resources/chromeos/chromevox/common/nav_description.js
(right):

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/common/nav_description.js:95: *
@param {function()=} endCallback Function called when this ends speaking.
On 2014/09/29 13:51:04, Peter Lundblad wrote:
> nit: optional parameters should have opt_ prepended to their names.

Done.

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
File chrome/browser/resources/chromeos/chromevox/extensions/searchvox/search.js
(right):

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/extensions/searchvox/search.js:191:
cvox.QueueMode.QUEUE);
On 2014/09/29 13:51:04, Peter Lundblad wrote:
> i18n

I'm going to skip it for anything in extensions/

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
File chrome/browser/resources/chromeos/chromevox/host/chrome/tts.js (right):

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/host/chrome/tts.js:110: * @param
{cvox.QueueMode=} queueMode The queue mode.
On 2014/09/29 13:51:04, Peter Lundblad wrote:
> I think this can be converted to required now.

Done.

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

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/host/chrome/tts_background.js:221:
if (queueMode === undefined) {
On 2014/09/29 13:51:04, Peter Lundblad wrote:
> This is redundant now that queueMode is required.

Done.

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
File chrome/browser/resources/chromeos/chromevox/host/interface/tts_interface.js
(right):

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/host/interface/tts_interface.js:12:
goog.provide('cvox.QueueMode');
On 2014/09/29 13:51:04, Peter Lundblad wrote:
> In basically all other files, you should add a corresponding goog.require.

Done.

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/host/interface/tts_interface.js:17:
* Queue modes for calls to speak().
On 2014/09/29 13:51:04, Peter Lundblad wrote:
> nit: Qualify speak, i.e. {@code cvox.TtsInterface.speak} since this is
> its own object outside of TtsInterface.

Done.

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/host/interface/tts_interface.js:26:
FLUSH: 0,
On 2014/09/29 13:51:04, Peter Lundblad wrote:
> Would be nice to have the jsdoc inside the {} pair above each enum value.

Done.

https://codereview.chromium.org/604423002/diff/1/chrome/browser/resources/chr...
chrome/browser/resources/chromeos/chromevox/host/interface/tts_interface.js:57:
* @param {cvox.QueueMode} queueMode The queue mode to use for speaking.
On 2014/09/26 22:29:23, David Tseng wrote:
> I pretty much just looked at this file. I assume Closure compiler correctly
> enforces this now (so that speak('foo', 0) is no longer acceptable).

Acknowledged.

[commit-bot: I haz the power, 2014-09-30 22:01:52]

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

[commit-bot: I haz the power, 2014-09-30 22:40:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-30 22:40:17]

Failed to apply patch for
chrome/browser/resources/chromeos/chromevox/chromevox/injected/live_regions_deprecated.js:
While running git apply --index -3 -p1;
  error:
chrome/browser/resources/chromeos/chromevox/chromevox/injected/live_regions_deprecated.js:
does not exist in index

Patch:      
chrome/browser/resources/chromeos/chromevox/chromevox/injected/live_regions_deprecated.js
Index:
chrome/browser/resources/chromeos/chromevox/chromevox/injected/live_regions_deprecated.js
diff --git
a/chrome/browser/resources/chromeos/chromevox/chromevox/injected/live_regions_deprecated.js
b/chrome/browser/resources/chromeos/chromevox/chromevox/injected/live_regions_deprecated.js
index
034be6a39ef6f2d3ed5dbf46c0557f47b0e03667..8da75b3651402fa89a9267ac03e330947e0cb37a
100644
---
a/chrome/browser/resources/chromeos/chromevox/chromevox/injected/live_regions_deprecated.js
+++
b/chrome/browser/resources/chromeos/chromevox/chromevox/injected/live_regions_deprecated.js
@@ -59,7 +59,7 @@ cvox.LiveRegionsDeprecated.INITIAL_SILENCE_MS = 2000;
  */
 cvox.LiveRegionsDeprecated.init = function(pageLoadTime, queueMode,
disableSpeak) {
   if (queueMode == undefined) {
-    queueMode = cvox.AbstractTts.QUEUE_MODE_FLUSH;
+    queueMode = cvox.QueueMode.FLUSH;
   }
 
   cvox.LiveRegionsDeprecated.pageLoadTime = pageLoadTime;
@@ -70,7 +70,7 @@ cvox.LiveRegionsDeprecated.init = function(pageLoadTime,
queueMode, disableSpeak
     if (cvox.LiveRegionsDeprecated.updateLiveRegion(regions[i], queueMode,
                                           disableSpeak)) {
       anyRegionsAnnounced = true;
-      queueMode = cvox.AbstractTts.QUEUE_MODE_QUEUE;
+      queueMode = cvox.QueueMode.QUEUE;
     }
   }
 
@@ -188,11 +188,11 @@ cvox.LiveRegionsDeprecated.updateLiveRegion =
function(region, queueMode, disabl
   // Announce the changes with the appropriate politeness level.
   var live = cvox.AriaUtil.getAriaLive(region);
   if (live == 'polite') {
-    queueMode = cvox.AbstractTts.QUEUE_MODE_QUEUE;
+    queueMode = cvox.QueueMode.QUEUE;
   }
   for (i = 0; i < messages.length; i++) {
     messages[i].speak(queueMode);
-    queueMode = cvox.AbstractTts.QUEUE_MODE_QUEUE;
+    queueMode = cvox.QueueMode.QUEUE;
   }
 
   return true;

[dmazzoni, 2014-09-30 22:54:14]

The CQ bit was checked by dmazzoni@chromium.org

[commit-bot: I haz the power, 2014-09-30 22:55:41]

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

[commit-bot: I haz the power, 2014-09-30 23:35:36]

Committed patchset #3 (id:40001) as 77843d97b1dc0973224850a452a502ec577da8d6

[commit-bot: I haz the power, 2014-09-30 23:36:16]

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