Start removing support for the experimental x-webkit-speech API

Start removing support for the experimental x-webkit-speech API

The API has been considered deprecated for some time, and was disabled
in https://src.chromium.org/viewvc/blink?revision=171373&view=revision

BUG=223198
TBR=jam@chromium.org

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

[hans, 2014-05-01 18:30:54]

tommi, tommyw: please take a look.
msw, primiano: FYI.

This should cover most of the Chromium side. MockWebSpeechInputController will
need to be untangled separately.

[tommi (sloooow) - chröme, 2014-05-01 18:53:59]

Lgtm! Nice :-)
Tommy is ooo right now so no need to wait.

[hans, 2014-05-01 21:32:45]

Owners, please take a look.

jam: content/ and chrome/
jar: tools/metrics/

[msw, 2014-05-02 00:05:09]

Driveby nits and comments.

Can you also remove the ENABLE_INPUT_SPEECH flag?
( build/common.gypi, build/config/BUILD.gn, more? )

https://codereview.chromium.org/260903010/diff/20001/chrome/app/chrome_comman...
File chrome/app/chrome_command_ids.h (right):

https://codereview.chromium.org/260903010/diff/20001/chrome/app/chrome_comman...
chrome/app/chrome_command_ids.h:348: #define IDC_MEDIA_STREAM_DEVICE_STATUS_TRAY
51300
nit: should these subsequent commands have reassigned values?

https://codereview.chromium.org/260903010/diff/20001/chrome/browser/renderer_...
File chrome/browser/renderer_context_menu/render_view_context_menu.cc (right):

https://codereview.chromium.org/260903010/diff/20001/chrome/browser/renderer_...
chrome/browser/renderer_context_menu/render_view_context_menu.cc:202: { 52,
IDC_CONTENT_CONTEXT_OPENLINKWITH },
nit: should these subsequent commands have reassigned values?

https://codereview.chromium.org/260903010/diff/20001/chrome/browser/ui/browse...
File chrome/browser/ui/browser_commands.cc (right):

https://codereview.chromium.org/260903010/diff/20001/chrome/browser/ui/browse...
chrome/browser/ui/browser_commands.cc:1063: WebContents* web_contents =
nit: this could be inlined below.

https://codereview.chromium.org/260903010/diff/20001/content/browser/speech/i...
File content/browser/speech/input_tag_speech_browsertest.cc (left):

https://codereview.chromium.org/260903010/diff/20001/content/browser/speech/i...
content/browser/speech/input_tag_speech_browsertest.cc:111:
RunSpeechRecognitionTest("basic_recognition.html");
Can you now also remove content/test/data/speech/basic_recognition.html?
Perhaps also grammar_attribute.html and web_speech_recognition.html?

https://codereview.chromium.org/260903010/diff/20001/content/browser/speech/s...
File content/browser/speech/speech_recognition_dispatcher_host.cc (right):

https://codereview.chromium.org/260903010/diff/20001/content/browser/speech/s...
content/browser/speech/speech_recognition_dispatcher_host.cc:109: //
input_tag_dispatcher_host.cc
nit: this is no longer true, please update the comment.

https://codereview.chromium.org/260903010/diff/20001/content/browser/speech/s...
content/browser/speech/speech_recognition_dispatcher_host.cc:114:
FilterProfanities(render_process_id_);
nit: This is the last user of FilterProfanities and the underlying pref. I
assume it's still needed here, but want to make sure.

https://codereview.chromium.org/260903010/diff/20001/content/public/browser/s...
File content/public/browser/speech_recognition_session_config.h (right):

https://codereview.chromium.org/260903010/diff/20001/content/public/browser/s...
content/public/browser/speech_recognition_session_config.h:25: bool
is_legacy_api;  // TODO(hans): The legacy API is dead; remove this flag.
nit: point to the bug.

https://codereview.chromium.org/260903010/diff/20001/tools/metrics/histograms...
File tools/metrics/histograms/histograms.xml (left):

https://codereview.chromium.org/260903010/diff/20001/tools/metrics/histograms...
tools/metrics/histograms/histograms.xml:39953: <int value="52"
label="IDC_CONTENT_CONTEXT_OPENLINKWITH"/>
nit: should these subsequent commands have reassigned values?

[hans, 2014-05-02 02:36:14]

On 2014/05/02 00:05:09, msw wrote:
> Driveby nits and comments.

Thanks for the comments!

> Can you also remove the ENABLE_INPUT_SPEECH flag?
> ( build/common.gypi, build/config/BUILD.gn, more? )

Not yet. It's used by the content_shell test runner. I'm removing that stuff in
https://codereview.chromium.org/268723002/, and then I'll go after
ENABLE_INPUT_SPEECH, and also see if the 'input_speech' GYP flag can be killed
(though I'm worried it might be conflated with the Web Speech API a little).

[hans, 2014-05-02 02:36:27]

https://codereview.chromium.org/260903010/diff/20001/chrome/app/chrome_comman...
File chrome/app/chrome_command_ids.h (right):

https://codereview.chromium.org/260903010/diff/20001/chrome/app/chrome_comman...
chrome/app/chrome_command_ids.h:348: #define IDC_MEDIA_STREAM_DEVICE_STATUS_TRAY
51300
On 2014/05/02 00:05:10, msw wrote:
> nit: should these subsequent commands have reassigned values?

I'm not familiar with this file, but the last commit that removed entries here
(crrev.com/251439) didn't do any renumbering (note the hole around IDC_SEARCH).

https://codereview.chromium.org/260903010/diff/20001/chrome/browser/renderer_...
File chrome/browser/renderer_context_menu/render_view_context_menu.cc (right):

https://codereview.chromium.org/260903010/diff/20001/chrome/browser/renderer_...
chrome/browser/renderer_context_menu/render_view_context_menu.cc:202: { 52,
IDC_CONTENT_CONTEXT_OPENLINKWITH },
On 2014/05/02 00:05:10, msw wrote:
> nit: should these subsequent commands have reassigned values?

The comment above says the mapping must never change.

https://codereview.chromium.org/260903010/diff/20001/chrome/browser/ui/browse...
File chrome/browser/ui/browser_commands.cc (right):

https://codereview.chromium.org/260903010/diff/20001/chrome/browser/ui/browse...
chrome/browser/ui/browser_commands.cc:1063: WebContents* web_contents =
On 2014/05/02 00:05:10, msw wrote:
> nit: this could be inlined below.

Done.

https://codereview.chromium.org/260903010/diff/20001/content/browser/speech/i...
File content/browser/speech/input_tag_speech_browsertest.cc (left):

https://codereview.chromium.org/260903010/diff/20001/content/browser/speech/i...
content/browser/speech/input_tag_speech_browsertest.cc:111:
RunSpeechRecognitionTest("basic_recognition.html");
On 2014/05/02 00:05:10, msw wrote:
> Can you now also remove content/test/data/speech/basic_recognition.html?
> Perhaps also grammar_attribute.html and web_speech_recognition.html?

Nice catch! web_speech_recognition.html is for the Web Speech API, but the other
two can go.

https://codereview.chromium.org/260903010/diff/20001/content/browser/speech/s...
File content/browser/speech/speech_recognition_dispatcher_host.cc (right):

https://codereview.chromium.org/260903010/diff/20001/content/browser/speech/s...
content/browser/speech/speech_recognition_dispatcher_host.cc:109: //
input_tag_dispatcher_host.cc
On 2014/05/02 00:05:10, msw wrote:
> nit: this is no longer true, please update the comment.

Done.

https://codereview.chromium.org/260903010/diff/20001/content/browser/speech/s...
content/browser/speech/speech_recognition_dispatcher_host.cc:114:
FilterProfanities(render_process_id_);
On 2014/05/02 00:05:10, msw wrote:
> nit: This is the last user of FilterProfanities and the underlying pref. I
> assume it's still needed here, but want to make sure.

Yeah, I want to come back to this pref in a follow-up. It's still used for
requests sent by Web Speech API, but it seems the only way to set the preference
was by right-clicking a x-webkit-speech input element which is no longer
possible.

https://codereview.chromium.org/260903010/diff/20001/content/public/browser/s...
File content/public/browser/speech_recognition_session_config.h (right):

https://codereview.chromium.org/260903010/diff/20001/content/public/browser/s...
content/public/browser/speech_recognition_session_config.h:25: bool
is_legacy_api;  // TODO(hans): The legacy API is dead; remove this flag.
On 2014/05/02 00:05:10, msw wrote:
> nit: point to the bug.

Done.

https://codereview.chromium.org/260903010/diff/20001/tools/metrics/histograms...
File tools/metrics/histograms/histograms.xml (left):

https://codereview.chromium.org/260903010/diff/20001/tools/metrics/histograms...
tools/metrics/histograms/histograms.xml:39953: <int value="52"
label="IDC_CONTENT_CONTEXT_OPENLINKWITH"/>
On 2014/05/02 00:05:10, msw wrote:
> nit: should these subsequent commands have reassigned values?

These are some kind of UMA numbers. For what I understand, they should never be
reused.

[msw, 2014-05-02 02:55:50]

lgtm, thanks for the big, ongoing cleanup!

[hans, 2014-05-02 17:06:30]

On 2014/05/02 02:55:50, msw wrote:
> lgtm, thanks for the big, ongoing cleanup!

Thanks for looking!

jam, jar: ping for owners review (content/ and tools/metrics/, respectively).

[jar (doing other things), 2014-05-02 17:33:25]

I'd suggest not change the histograms.xml file.

There is probably no real reason to remove the enum entries in the xml file. 
Leaving them intact will allows folks to look at old histograms.

Generally, all we ever do is <obsolete> old histograms (which then means that a
check-box click is needed to be able to see them on current dashboards).  This
approach also keeps folks from redefining things, that collide with data from
old clients, and confuse the heck out of analysis.

[hans, 2014-05-02 17:43:27]

On 2014/05/02 17:33:25, jar wrote:
> I'd suggest not change the histograms.xml file.

Done.

[jam, 2014-05-02 18:45:33]

lgtm

[hans, 2014-05-02 18:48:15]

The CQ bit was checked by hans@chromium.org

[commit-bot: I haz the power, 2014-05-02 18:48:25]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/hans@chromium.org/260903010/60001

[commit-bot: I haz the power, 2014-05-02 19:01:15]

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

[commit-bot: I haz the power, 2014-05-02 19:01:15]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium

[hans, 2014-05-02 19:47:07]

On 2014/05/02 19:01:15, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   chromium_presubmit on tryserver.chromium


> ** Presubmit ERRORS **
> Missing LGTM from an OWNER for these files:
>     content/common/frame_messages.h
>     content/common/speech_recognition_messages.h

Wat? I'm pretty sure jam's lgtm is good enough for those.

[hans, 2014-05-02 20:41:22]

The CQ bit was checked by hans@chromium.org

[commit-bot: I haz the power, 2014-05-02 20:41:41]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/hans@chromium.org/260903010/60001

[commit-bot: I haz the power, 2014-05-02 22:05:45]

Change committed as 267937