Adding a serviceURI param to SpeechRecognition

Adding a serviceURI param to SpeechRecognition so users can pass the param over to the speech side.

The speech API supports serviceURI param in SpeechRecognition. 
https://dvcs.w3.org/hg/speech-api/raw-file/tip/speechapi.html#dfn-serviceuri

BUG=480516
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=194498

[kirtia, 2015-04-23 17:59:30]

kirtia@chromium.org changed reviewers:
+ sigbjornf@opera.com

[sof, 2015-04-23 18:52:45]

The extension looks fine really, just details remain.

https://codereview.chromium.org/1034833002/diff/1/Source/modules/speech/Speec...
File Source/modules/speech/SpeechRecognition.idl (right):

https://codereview.chromium.org/1034833002/diff/1/Source/modules/speech/Speec...
Source/modules/speech/SpeechRecognition.idl:35: attribute DOMString serviceURI;
As this exposes a new property on SpeechRecognition, you'll need to update the
expected results on the LayoutTests/webexposed/ tests that enumerate these.

https://codereview.chromium.org/1034833002/diff/1/Source/modules/speech/Speec...
File Source/modules/speech/SpeechRecognitionClient.h (right):

https://codereview.chromium.org/1034833002/diff/1/Source/modules/speech/Speec...
Source/modules/speech/SpeechRecognitionClient.h:40: virtual void
start(SpeechRecognition*, const SpeechGrammarList*, const String& lang, const
String& serviceURI, bool continuous, bool interimResults, unsigned long
maxAlternatives, MediaStreamTrack* audioTrack) = 0;
indentation is 4 spaces.

https://codereview.chromium.org/1034833002/diff/1/Source/modules/speech/Speec...
File Source/modules/speech/SpeechRecognitionController.h (right):

https://codereview.chromium.org/1034833002/diff/1/Source/modules/speech/Speec...
Source/modules/speech/SpeechRecognitionController.h:44:
m_client->start(recognition, grammars, lang, serviceURI, continuous,
interimResults, maxAlternatives, audioTrack);
Indentation (editor set up for 2, rather than 4? :) )

https://codereview.chromium.org/1034833002/diff/1/Source/web/SpeechRecognitio...
File Source/web/SpeechRecognitionClientProxy.cpp (right):

https://codereview.chromium.org/1034833002/diff/1/Source/web/SpeechRecognitio...
Source/web/SpeechRecognitionClientProxy.cpp:58: void
SpeechRecognitionClientProxy::start(SpeechRecognition* recognition, const
SpeechGrammarList* grammarList, const String& lang, const String& serviceURI,
bool continuous, bool interimResults, unsigned long maxAlternatives,
MediaStreamTrack* audioTrack)
Indentation.

https://codereview.chromium.org/1034833002/diff/1/public/web/WebSpeechRecogni...
File public/web/WebSpeechRecognitionParams.h (right):

https://codereview.chromium.org/1034833002/diff/1/public/web/WebSpeechRecogni...
public/web/WebSpeechRecognitionParams.h:41: WebSpeechRecognitionParams(const
WebVector<WebSpeechGrammar>& grammars, const WebString& language, const
WebString& serviceURI, bool continuous, bool interimResults, unsigned long
maxAlternatives, const WebMediaStreamTrack& audioTrack, const WebSecurityOrigin&
origin)
Indentation.

[kirtia, 2015-04-24 20:44:38]

Thanks a lot for taking a look!

Please let me know if the new patch looks reasonable. :)

https://codereview.chromium.org/1034833002/diff/1/Source/modules/speech/Speec...
File Source/modules/speech/SpeechRecognition.idl (right):

https://codereview.chromium.org/1034833002/diff/1/Source/modules/speech/Speec...
Source/modules/speech/SpeechRecognition.idl:35: attribute DOMString serviceURI;
On 2015/04/23 18:52:44, sof wrote:
> As this exposes a new property on SpeechRecognition, you'll need to update the
> expected results on the LayoutTests/webexposed/ tests that enumerate these.

I have changed the LayoutTests expected file for one test. Hopefully, we are
okay now. :-) Please let me know if I am missing something else. :-)

https://codereview.chromium.org/1034833002/diff/1/Source/modules/speech/Speec...
File Source/modules/speech/SpeechRecognitionClient.h (right):

https://codereview.chromium.org/1034833002/diff/1/Source/modules/speech/Speec...
Source/modules/speech/SpeechRecognitionClient.h:40: virtual void
start(SpeechRecognition*, const SpeechGrammarList*, const String& lang, const
String& serviceURI, bool continuous, bool interimResults, unsigned long
maxAlternatives, MediaStreamTrack* audioTrack) = 0;
On 2015/04/23 18:52:45, sof wrote:
> indentation is 4 spaces.

Done.

https://codereview.chromium.org/1034833002/diff/1/Source/modules/speech/Speec...
File Source/modules/speech/SpeechRecognitionController.h (right):

https://codereview.chromium.org/1034833002/diff/1/Source/modules/speech/Speec...
Source/modules/speech/SpeechRecognitionController.h:44:
m_client->start(recognition, grammars, lang, serviceURI, continuous,
interimResults, maxAlternatives, audioTrack);
On 2015/04/23 18:52:45, sof wrote:
> Indentation (editor set up for 2, rather than 4? :) )

Done.

https://codereview.chromium.org/1034833002/diff/1/Source/web/SpeechRecognitio...
File Source/web/SpeechRecognitionClientProxy.cpp (right):

https://codereview.chromium.org/1034833002/diff/1/Source/web/SpeechRecognitio...
Source/web/SpeechRecognitionClientProxy.cpp:58: void
SpeechRecognitionClientProxy::start(SpeechRecognition* recognition, const
SpeechGrammarList* grammarList, const String& lang, const String& serviceURI,
bool continuous, bool interimResults, unsigned long maxAlternatives,
MediaStreamTrack* audioTrack)
On 2015/04/23 18:52:45, sof wrote:
> Indentation.

Done.

https://codereview.chromium.org/1034833002/diff/1/public/web/WebSpeechRecogni...
File public/web/WebSpeechRecognitionParams.h (right):

https://codereview.chromium.org/1034833002/diff/1/public/web/WebSpeechRecogni...
public/web/WebSpeechRecognitionParams.h:41: WebSpeechRecognitionParams(const
WebVector<WebSpeechGrammar>& grammars, const WebString& language, const
WebString& serviceURI, bool continuous, bool interimResults, unsigned long
maxAlternatives, const WebMediaStreamTrack& audioTrack, const WebSecurityOrigin&
origin)
On 2015/04/23 18:52:45, sof wrote:
> Indentation.

Done.

[dmazzoni, 2015-04-24 21:49:49]

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

[dmazzoni, 2015-04-24 21:49:49]

You need to update this expectation too:

LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt

I'll kick off try jobs for you again.

[kirtia, 2015-04-24 22:00:04]

On 2015/04/24 21:49:49, dmazzoni wrote:
> You need to update this expectation too:
> 
> LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt
> 
> I'll kick off try jobs for you again.

I am really sorry, I didn't realize that. The layout tests weirdly gave me a
pass on everything, I might have misread it.

Sorry about that. The test has been added. Thanks much!

[sof, 2015-04-25 06:30:50]

On 2015/04/24 22:00:04, kirtia wrote:
> On 2015/04/24 21:49:49, dmazzoni wrote:
> > You need to update this expectation too:
> > 
> > LayoutTests/virtual/stable/webexposed/global-interface-listing-expected.txt
> > 
> > I'll kick off try jobs for you again.
> 
> I am really sorry, I didn't realize that. The layout tests weirdly gave me a
> pass on everything, I might have misread it.
> 
> Sorry about that. The test has been added. Thanks much!

Looks green & ready now, dmazzoni to approve as owner.

[dmazzoni, 2015-04-25 06:36:24]

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

[dmazzoni, 2015-04-25 06:36:24]

lgtm, but we still need an owner

There doesn't appear to be an owners file in modules/speech...adding mkwst

[Mike West, 2015-04-26 08:01:41]

LGTM.

We should really find an owner for this directory, but it doesn't look like it's
been touched much since the fork beyond Oilpan-related refactorings (e.g.
Sigbjorn, Kentaro-san are right at the top of the shortlog). Hrm.

dmazzoni@, would you be willing to step in (in a future CL)?

[dmazzoni, 2015-04-27 07:42:47]

The CQ bit was checked by dmazzoni@chromium.org

[commit-bot: I haz the power, 2015-04-27 07:42:51]

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

[dmazzoni, 2015-04-27 07:44:20]

I'm happy to be an owner of this directory, but non-trivial changes will
need someone more familiar with speech recognition to take a look too. I
think there are a few people who know speech recognition but just aren't
blink committers, so I can delegate to them to do an initial review.

On Sun, Apr 26, 2015 at 1:01 AM, <mkwst@chromium.org> wrote:

> LGTM.
>
> We should really find an owner for this directory, but it doesn't look
> like it's
> been touched much since the fork beyond Oilpan-related refactorings (e.g.
> Sigbjorn, Kentaro-san are right at the top of the shortlog). Hrm.
>
> dmazzoni@, would you be willing to step in (in a future CL)?
>
> https://codereview.chromium.org/1034833002/
>

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

[commit-bot: I haz the power, 2015-04-27 08:53:28]

Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=194498

[kirtia, 2015-06-25 17:23:28]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/1211973002/ by kirtia@chromium.org.

The reason for reverting is: Reverting this patch since we will not be
supporting serviceURI param. .