Support onvoiceschanged for extension-based tts engines.

Support onvoiceschanged for extension-based tts engines.

TEST=Observe an onvoiceschanged event handler gets called when...
- built-in extension tts engine loads.
- webstore-based extension engine is enabled/disabled.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=282471

[dmazzoni, 2014-07-07 15:37:26]

https://codereview.chromium.org/361363003/diff/60001/chrome/browser/speech/tt...
File chrome/browser/speech/tts_controller.cc (right):

https://codereview.chromium.org/361363003/diff/60001/chrome/browser/speech/tt...
chrome/browser/speech/tts_controller.cc:162:
TtsEngineExtensionObserver::GetInstance(utterance->profile());
Is there a better place for this?

1) It feels like we should try to do this on more than just "Speak"

2) We're trying to move TtsController into content so I'd like to eliminate
direct dependencies on extension-related code. Can this be abstracted?

https://codereview.chromium.org/361363003/diff/60001/chrome/browser/speech/tt...
File chrome/browser/speech/tts_engine_extension_observer.cc (right):

https://codereview.chromium.org/361363003/diff/60001/chrome/browser/speech/tt...
chrome/browser/speech/tts_engine_extension_observer.cc:125:
TtsController::GetInstance()->VoicesChanged();
Should this be before RetrySpeaking...?

https://codereview.chromium.org/361363003/diff/60001/chrome/browser/speech/tt...
File chrome/browser/speech/tts_engine_extension_observer.h (right):

https://codereview.chromium.org/361363003/diff/60001/chrome/browser/speech/tt...
chrome/browser/speech/tts_engine_extension_observer.h:5: #ifndef
CHROME_BROWSER_SPEECH_TTS_ENGINE_EXTENSION_OBSERVER_H_
Move this into the extension_api directory. I'm trying to move most of the code
in chrome/browser/speech into content.

https://codereview.chromium.org/361363003/diff/60001/chrome/browser/speech/tt...
chrome/browser/speech/tts_engine_extension_observer.h:27: bool
SawExtensionLoad(const std::string& extension_id, bool update);
Is it possible for the extension to load before the instance of this class for a
given profile gets created?

[David Tseng, 2014-07-08 23:43:31]

https://codereview.chromium.org/361363003/diff/60001/chrome/browser/speech/tt...
File chrome/browser/speech/tts_controller.cc (right):

https://codereview.chromium.org/361363003/diff/60001/chrome/browser/speech/tt...
chrome/browser/speech/tts_controller.cc:162:
TtsEngineExtensionObserver::GetInstance(utterance->profile());
On 2014/07/07 15:37:26, dmazzoni wrote:
> Is there a better place for this?
> 
> 1) It feels like we should try to do this on more than just "Speak"
> 
> 2) We're trying to move TtsController into content so I'd like to eliminate
> direct dependencies on extension-related code. Can this be abstracted?

I've moved this call. Should address some of your other concerns I think. Let me
know if that looks like a good change and I think I can then clean up the
observer class a bit more (e.g. no longer a service).

https://codereview.chromium.org/361363003/diff/60001/chrome/browser/speech/tt...
File chrome/browser/speech/tts_engine_extension_observer.cc (right):

https://codereview.chromium.org/361363003/diff/60001/chrome/browser/speech/tt...
chrome/browser/speech/tts_engine_extension_observer.cc:125:
TtsController::GetInstance()->VoicesChanged();
On 2014/07/07 15:37:26, dmazzoni wrote:
> Should this be before RetrySpeaking...?

I didn't see a need since both calls are async. A caller wouldn't be able to
react to the voice change either way I think (at least for the queued up
utterances).

https://codereview.chromium.org/361363003/diff/60001/chrome/browser/speech/tt...
File chrome/browser/speech/tts_engine_extension_observer.h (right):

https://codereview.chromium.org/361363003/diff/60001/chrome/browser/speech/tt...
chrome/browser/speech/tts_engine_extension_observer.h:27: bool
SawExtensionLoad(const std::string& extension_id, bool update);
On 2014/07/07 15:37:26, dmazzoni wrote:
> Is it possible for the extension to load before the instance of this class for
a
> given profile gets created?

See the new change; I think TtsApi gets initialized pretty early on (as a
BrowserContextKeyedService), so shouldn't be a problem anymore I think. Let me
know if that looks right to you.

[dmazzoni, 2014-07-09 15:24:08]

Thanks, that works.

Also note this change, which will eliminate more dependencies from TtsController
on extensions: https://codereview.chromium.org/188633003/

[David Tseng, 2014-07-09 16:47:31]

On Wed, Jul 9, 2014 at 8:24 AM, <dmazzoni@chromium.org> wrote:

> Thanks, that works.
>
> Also note this change, which will eliminate more dependencies from
> TtsController
> on extensions: https://codereview.chromium.org/188633003/
>
> Great.
I think the cl's ready for another look.

>
> https://codereview.chromium.org/361363003/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[dmazzoni, 2014-07-09 16:54:17]

lgtm

[commit-bot: I haz the power, 2014-07-09 16:56:09]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dtseng@chromium.org/361363003/100001

[commit-bot: I haz the power, 2014-07-09 17:48:30]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_aosp on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/9...)
  android_chromium_gn_compile_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_chromium_gn_c...)
  android_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_clang_dbg/bui...)
  android_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg/builds/20...)
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)
  ios_dbg_simulator on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...)
  ios_rel_device on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...)
  ios_rel_device_ninja on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...)
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_chromeos_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)
  linux_chromium_gn_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_gn_rel...)
  linux_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_rel/bu...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  win8_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win8_chromium_rel/bui...)
  win_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...)
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)
  win_chromium_x64_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/26542)

[commit-bot: I haz the power, 2014-07-09 17:49:54]

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

[commit-bot: I haz the power, 2014-07-09 17:49:55]

Try jobs failed on following builders:
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/26571)

[David Tseng, 2014-07-09 18:27:26]

The CQ bit was checked by dtseng@chromium.org

[commit-bot: I haz the power, 2014-07-09 18:29:43]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dtseng@chromium.org/361363003/120001

[commit-bot: I haz the power, 2014-07-09 21:45:33]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
  linux_chromium_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_clang_...)
  mac_chromium_compile_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...)
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/26668)

[commit-bot: I haz the power, 2014-07-09 21:58:31]

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

[commit-bot: I haz the power, 2014-07-09 21:58:33]

Try jobs failed on following builders:
  linux_chromium_chromeos_clang_dbg on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)

[David Tseng, 2014-07-10 17:25:23]

The CQ bit was checked by dtseng@chromium.org

[commit-bot: I haz the power, 2014-07-10 17:26:45]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/dtseng@chromium.org/361363003/140001

[commit-bot: I haz the power, 2014-07-10 20:58:19]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-07-10 23:45:30]

Change committed as 282471