If user had consented for metrics reporting, send speech input request origin to the server.

If user had consented for metrics reporting, send speech input request origin to the server.
This is the chromium side of the webkit patch https://bugs.webkit.org/show_bug.cgi?id=52718.

Suggested reviewer split:
wtc@ - the 2 url_fetcher.* files
hans@ - rest of the files

I needed to add a URLFetcher::set_referrer() method to send the origin url in the Referer header.

Also I had to create a new IPC params struct for startRecognition since the
number of parameters exceed what is provided by the macros. And in the process I also moved
the speech input IPC messages to their own source files.

BUG=none
TEST=No change in functionality except additional debug info sent to server.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=71949

[jam, 2011-01-19 18:09:52]

yay for moving messages out of render_messages :)  I don't
see speech_input_messages.h in this change though

On Wed, Jan 19, 2011 at 9:31 AM, <satish@chromium.org> wrote:

> Reviewers: hans, wtc,
>
> Description:
> If user had consented for metrics reporting, send speech input request
> origin to
> the server.
> This is the chromium side of the webkit patch
> https://bugs.webkit.org/show_bug.cgi?id=52718.
>
> Suggested reviewer split:
> wtc@ - the 2 url_fetcher.* files
> hans@ - rest of the files
>
> I needed to add a URLFetcher::set_referrer() method to send the origin url
> in
> the Referer header.
>
> Also I had to create a new IPC params struct for startRecognition since the
> number of parameters exceed what is provided by the macros. And in the
> process I
> also moved
> the speech input IPC messages to their own source files.
>
> BUG=none
> TEST=No change in functionality except additional debug info sent to
> server.
>
> Please review this at http://codereview.chromium.org/6308009/
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src
>
> Affected files:
>  M chrome/browser/speech/speech_input_dispatcher_host.h
>  M chrome/browser/speech/speech_input_dispatcher_host.cc
>  M chrome/browser/speech/speech_input_manager.h
>  M chrome/browser/speech/speech_input_manager.cc
>  M chrome/browser/speech/speech_recognition_request.h
>  M chrome/browser/speech/speech_recognition_request.cc
>  M chrome/browser/speech/speech_recognizer.h
>  M chrome/browser/speech/speech_recognizer.cc
>  M chrome/chrome_common.gypi
>  M chrome/common/net/url_fetcher.h
>  M chrome/common/net/url_fetcher.cc
>  M chrome/common/render_messages.h
>  M chrome/common/render_messages.cc
>  M chrome/common/render_messages_internal.h
>  M chrome/renderer/speech_input_dispatcher.h
>  M chrome/renderer/speech_input_dispatcher.cc
>  M ipc/ipc_message_utils.h
>
>
>

[Satish, 2011-01-19 20:13:08]

oops, added now.

[jam, 2011-01-19 20:29:23]

http://codereview.chromium.org/6308009/diff/4001/chrome/common/speech_input_m...
File chrome/common/speech_input_messages.cc (right):

http://codereview.chromium.org/6308009/diff/4001/chrome/common/speech_input_m...
chrome/common/speech_input_messages.cc:8: #include
"chrome/common/speech_input_messages.h"
the above two lines should be after all other includes.  that's the style for
X_messages.cc, since it avoids compile errors.  In this case,
speech_input_result.h is already included by speech_input_messages.h, so you can
take it out from here (if it wasn't included in the header, you'd get a compile
error if this wasn't before #IPC_MESSAGE_IMPL)

http://codereview.chromium.org/6308009/diff/4001/chrome/common/speech_input_m...
File chrome/common/speech_input_messages.h (right):

http://codereview.chromium.org/6308009/diff/4001/chrome/common/speech_input_m...
chrome/common/speech_input_messages.h:65: int /* request id */)
nit: the usual style for parameter names inside the comments is to name it just
like the dispatcher function names is parameters.  this is useful in case in the
future we change how IPC messages are declared (i.e. IDLs) and we want to use a
script to generate the IDLs.

[wtc, 2011-01-19 22:22:02]

LGTM on url_fetcher.{h,cc}.

http://codereview.chromium.org/6308009/diff/4001/chrome/common/net/url_fetcher.h
File chrome/common/net/url_fetcher.h (right):

http://codereview.chromium.org/6308009/diff/4001/chrome/common/net/url_fetche...
chrome/common/net/url_fetcher.h:142: // The referrer URL for the request, to be
set before Start() is called.
Nit: I suggest you copy the same sentence
    Must be called before the request is started.
that's used in other setters (line 136, line 145, line
149, etc.).

[hans, 2011-01-20 09:29:29]

lgtm

http://codereview.chromium.org/6308009/diff/4001/chrome/common/speech_input_m...
File chrome/common/speech_input_messages.h (right):

http://codereview.chromium.org/6308009/diff/4001/chrome/common/speech_input_m...
chrome/common/speech_input_messages.h:69: // speech recognizer. If speech
recognition is not happening nor or is
"nor or" ?

[Satish, 2011-01-20 11:04:11]

Thanks for the quick reviews. I have addressed all comments, will submit
shortly.

http://codereview.chromium.org/6308009/diff/4001/chrome/common/net/url_fetcher.h
File chrome/common/net/url_fetcher.h (right):

http://codereview.chromium.org/6308009/diff/4001/chrome/common/net/url_fetche...
chrome/common/net/url_fetcher.h:142: // The referrer URL for the request, to be
set before Start() is called.
On 2011/01/19 22:22:02, wtc wrote:
> Nit: I suggest you copy the same sentence
>     Must be called before the request is started.
> that's used in other setters (line 136, line 145, line
> 149, etc.).

Done.

http://codereview.chromium.org/6308009/diff/4001/chrome/common/speech_input_m...
File chrome/common/speech_input_messages.cc (right):

http://codereview.chromium.org/6308009/diff/4001/chrome/common/speech_input_m...
chrome/common/speech_input_messages.cc:8: #include
"chrome/common/speech_input_messages.h"
On 2011/01/19 20:29:23, John Abd-El-Malek wrote:
> the above two lines should be after all other includes.  that's the style for
> X_messages.cc, since it avoids compile errors.  In this case,
> speech_input_result.h is already included by speech_input_messages.h, so you
can
> take it out from here (if it wasn't included in the header, you'd get a
compile
> error if this wasn't before #IPC_MESSAGE_IMPL)

Removed speech_input_result.h as suggested

http://codereview.chromium.org/6308009/diff/4001/chrome/common/speech_input_m...
File chrome/common/speech_input_messages.h (right):

http://codereview.chromium.org/6308009/diff/4001/chrome/common/speech_input_m...
chrome/common/speech_input_messages.h:65: int /* request id */)
On 2011/01/19 20:29:23, John Abd-El-Malek wrote:
> nit: the usual style for parameter names inside the comments is to name it
just
> like the dispatcher function names is parameters.  this is useful in case in
the
> future we change how IPC messages are declared (i.e. IDLs) and we want to use
a
> script to generate the IDLs.

Changed to 'request_id' in all these declarations

http://codereview.chromium.org/6308009/diff/4001/chrome/common/speech_input_m...
chrome/common/speech_input_messages.h:69: // speech recognizer. If speech
recognition is not happening nor or is
On 2011/01/20 09:29:29, hans wrote:
> "nor or" ?

Removed 'nor'