Language tag for speech prepatch

This is the first CL in a 4-sided patch to add language attribute support to speech input. The webkit change is at https://bugs.webkit.org/show_bug.cgi?id=47089 and will be submitted after this CL. Once the webkit change has been rolled into chromium DEPS the remaining chromium changes will be submitted by http://codereview.chromium.org/3595018/show . A last webkit patch will be submitted too to remove temporary fixes introduced to avoid test_shell breaking: https://bugs.webkit.org/show_bug.cgi?id=47420 .

This patch implements the incoming changes in the WebKit::SpeechInputDispatcher interface.

BUG=53598
TEST=none

[Leandro Graciá Gil, 2010-10-04 14:37:53]

Let's go with this before submitting the WebKit patch.

[Satish, 2010-10-04 15:15:35]

On 2010/10/04 14:37:53, Leandro Graciá Gil wrote:
> Let's go with this before submitting the WebKit patch.

Can you point to the webkit patch in this CL's description? And mention what the
change is in detail in place of the current single line explanation.. something
like 'This is the first CL in a 3-sided patch to add language attribute support
to speech input. The webkit change is at <insert-webkit-bug-url> and will be
submitted after this CL. Once the webkit change has been rolled into chromium
DEPS the remaining chromium changes will be submitted (or if those changes are
also ready now, insert that CL url here).

[Satish, 2010-10-04 15:16:47]

http://codereview.chromium.org/3615005/diff/1/2
File chrome/renderer/speech_input_dispatcher.cc (right):

http://codereview.chromium.org/3615005/diff/1/2#newcode36
chrome/renderer/speech_input_dispatcher.cc:36: // FIXME: preparing for a
two-sided patch.
remove this comment as the header explains about this function

http://codereview.chromium.org/3615005/diff/1/3
File chrome/renderer/speech_input_dispatcher.h (right):

http://codereview.chromium.org/3615005/diff/1/3#newcode32
chrome/renderer/speech_input_dispatcher.h:32: // FIXME: preparing for a
two-sided patch.
Can you upload the webkit patch and add a link to that here, so it is clear
which patch you are referring to?

Also, could you replace FIXME with TODO(leandro)? I see TODO is generally used
more in chromium than FIXME and I typically find it more useful as it gives the
name of the person to contact.

http://codereview.chromium.org/3615005/diff/1/3#newcode33
chrome/renderer/speech_input_dispatcher.h:33: bool startRecognition(int
request_id, const WebKit::WebCString& language_tag,
should you be forward-declaring WebCString at the top of this file?

[Satish, 2010-10-04 15:18:08]

Also please use BUG=53598 for all major speech input CLs.

[Leandro Graciá Gil, 2010-10-06 15:38:50]

Code review fixes and description update.

http://codereview.chromium.org/3615005/diff/1/2
File chrome/renderer/speech_input_dispatcher.cc (right):

http://codereview.chromium.org/3615005/diff/1/2#newcode36
chrome/renderer/speech_input_dispatcher.cc:36: // FIXME: preparing for a
two-sided patch.
On 2010/10/04 15:16:47, Satish wrote:
> remove this comment as the header explains about this function

Done.

http://codereview.chromium.org/3615005/diff/1/3
File chrome/renderer/speech_input_dispatcher.h (right):

http://codereview.chromium.org/3615005/diff/1/3#newcode32
chrome/renderer/speech_input_dispatcher.h:32: // FIXME: preparing for a
two-sided patch.
On 2010/10/04 15:16:47, Satish wrote:
> Can you upload the webkit patch and add a link to that here, so it is clear
> which patch you are referring to?
> 
> Also, could you replace FIXME with TODO(leandro)? I see TODO is generally used
> more in chromium than FIXME and I typically find it more useful as it gives
the
> name of the person to contact.

Done.

http://codereview.chromium.org/3615005/diff/1/3#newcode33
chrome/renderer/speech_input_dispatcher.h:33: bool startRecognition(int
request_id, const WebKit::WebCString& language_tag,
On 2010/10/04 15:16:47, Satish wrote:
> should you be forward-declaring WebCString at the top of this file?

It's already being done in its included header
third_party/WebKit/WebKit/chromium/public/WebSpeechInputController.h

[Leandro Graciá Gil, 2010-10-06 16:50:16]

Compile error fixed.

http://codereview.chromium.org/3615005/diff/1/3
File chrome/renderer/speech_input_dispatcher.h (right):

http://codereview.chromium.org/3615005/diff/1/3#newcode33
chrome/renderer/speech_input_dispatcher.h:33: bool startRecognition(int
request_id, const WebKit::WebCString& language_tag,
On 2010/10/06 15:38:51, Leandro Graciá Gil wrote:
> On 2010/10/04 15:16:47, Satish wrote:
> > should you be forward-declaring WebCString at the top of this file?
> 
> It's already being done in its included header
> third_party/WebKit/WebKit/chromium/public/WebSpeechInputController.h

You were right. It seems that for some reason some sources are not getting the
forward declaration from that include. Strange but apparently true.

[Leandro Graciá Gil, 2010-10-06 16:53:17]

Adding jorlow for review.

[Leandro Graciá Gil, 2010-10-07 19:50:42]

On 2010/10/06 16:53:17, Leandro Graciá Gil wrote:
> Adding jorlow for review.

Adapting to recent changes in the WebKit side of the patch.

[Satish, 2010-10-08 10:29:40]

LGTM

http://codereview.chromium.org/3615005/diff/16001/17002
File chrome/renderer/speech_input_dispatcher.h (right):

http://codereview.chromium.org/3615005/diff/16001/17002#newcode33
chrome/renderer/speech_input_dispatcher.h:33: // TODO(leandro): this is a
temporary fix for a two-sided patch.
isn't this a 3-sided patch (first in chromium, then in webkit, last in chromium
again) ?

[Leandro Graciá Gil, 2010-10-08 12:30:35]

Updated issue description with more details about the 4-sided patch.

http://codereview.chromium.org/3615005/diff/16001/17002
File chrome/renderer/speech_input_dispatcher.h (right):

http://codereview.chromium.org/3615005/diff/16001/17002#newcode33
chrome/renderer/speech_input_dispatcher.h:33: // TODO(leandro): this is a
temporary fix for a two-sided patch.
On 2010/10/08 10:29:40, Satish wrote:
> isn't this a 3-sided patch (first in chromium, then in webkit, last in
chromium
> again) ?

You got a point. I thought that 2-sided only meant chromium and webkit. If
that's the case, it's indeed a 4-sided patch as both chromium and webkit have
temporary fixes to avoid breaking: chromium because of the API changes and
webkit because of the test_shell.

[Satish, 2010-10-13 16:01:39]

LGTM