Remove setVoiceSearchSupported part of EmbeddedSearch SearchBox API.

Remove setVoiceSearchSupported part of EmbeddedSearch SearchBox API.

also remove the mic search view from the location bar. The NTP "omnibox" still has voice search but that's unrelated.

BUG=522168
TBR=oshima@chromium.org

Committed: https://crrev.com/e3168e1578dc4bbc928d86976a33db284ede2164
Cr-Commit-Position: refs/heads/master@{#358641}

[Evan Stade, 2015-11-06 23:09:55]

estade@chromium.org changed reviewers:
+ msw@chromium.org, oshima@chromium.org, samarth@chromium.org

[Evan Stade, 2015-11-06 23:09:55]

+samarth for search/ and searchbox/
+msw for the rest of c/b/ui
+oshima for c/a/theme

[msw, 2015-11-06 23:59:23]

It's hard to distinguish genuinely dead code here from the still-used NTP
"Search by voice" icon, etc. Can you clarify the difference in the CL
description and add a reviewer familiar with the latest status there?

https://codereview.chromium.org/1428423002/diff/1/chrome/browser/ui/cocoa/loc...
File chrome/browser/ui/cocoa/location_bar/mic_search_decoration.h (left):

https://codereview.chromium.org/1428423002/diff/1/chrome/browser/ui/cocoa/loc...
chrome/browser/ui/cocoa/location_bar/mic_search_decoration.h:1: // Copyright
2013 The Chromium Authors. All rights reserved.
Update chrome/chrome_browser_ui.gypi

https://codereview.chromium.org/1428423002/diff/1/chrome/browser/ui/search/se...
File chrome/browser/ui/search/search_ipc_router.h (right):

https://codereview.chromium.org/1428423002/diff/1/chrome/browser/ui/search/se...
chrome/browser/ui/search/search_ipc_router.h:117: virtual bool
ShouldSendToggleVoiceSearch() = 0;
Should this also be removed?

https://codereview.chromium.org/1428423002/diff/1/chrome/browser/ui/search/se...
chrome/browser/ui/search/search_ipc_router.h:167: void ToggleVoiceSearch();
Should this also be removed?

https://codereview.chromium.org/1428423002/diff/1/chrome/browser/ui/search/se...
File chrome/browser/ui/search/search_model.cc (right):

https://codereview.chromium.org/1428423002/diff/1/chrome/browser/ui/search/se...
chrome/browser/ui/search/search_model.cc:11: :
instant_support(INSTANT_SUPPORT_UNKNOWN) {
nit: fits on line above

https://codereview.chromium.org/1428423002/diff/1/chrome/browser/ui/search/se...
File chrome/browser/ui/search/search_model.h (right):

https://codereview.chromium.org/1428423002/diff/1/chrome/browser/ui/search/se...
chrome/browser/ui/search/search_model.h:28: InstantSupportState
instant_support);
nit: fits on line above

https://codereview.chromium.org/1428423002/diff/1/chrome/browser/ui/search/se...
File chrome/browser/ui/search/search_tab_helper.h (right):

https://codereview.chromium.org/1428423002/diff/1/chrome/browser/ui/search/se...
chrome/browser/ui/search/search_tab_helper.h:94: void ToggleVoiceSearch();
Should this also be removed?

https://codereview.chromium.org/1428423002/diff/1/chrome/browser/ui/views/loc...
File chrome/browser/ui/views/location_bar/location_bar_view.cc (left):

https://codereview.chromium.org/1428423002/diff/1/chrome/browser/ui/views/loc...
chrome/browser/ui/views/location_bar/location_bar_view.cc:288:
l10n_util::GetStringUTF16(IDS_TOOLTIP_MIC_SEARCH));
Remove IDS_TOOLTIP_MIC_SEARCH and other uses?

https://codereview.chromium.org/1428423002/diff/1/chrome/browser/ui/views/loc...
chrome/browser/ui/views/location_bar/location_bar_view.cc:1349:
command_updater()->ExecuteCommand(IDC_TOGGLE_SPEECH_INPUT);
Remove IDC_TOGGLE_SPEECH_INPUT and uses?

https://codereview.chromium.org/1428423002/diff/1/chrome/browser/ui/views/loc...
File chrome/browser/ui/views/location_bar/location_bar_view.cc (right):

https://codereview.chromium.org/1428423002/diff/1/chrome/browser/ui/views/loc...
chrome/browser/ui/views/location_bar/location_bar_view.cc:38: #include
"chrome/browser/ui/view_ids.h"
nit: remove

https://codereview.chromium.org/1428423002/diff/1/chrome/browser/ui/views/loc...
chrome/browser/ui/views/location_bar/location_bar_view.cc:77: #include
"ui/base/l10n/l10n_util.h"
nit: remove

https://codereview.chromium.org/1428423002/diff/1/chrome/browser/ui/views/loc...
chrome/browser/ui/views/location_bar/location_bar_view.cc:95: #include
"ui/views/controls/button/image_button.h"
nit: remove

https://codereview.chromium.org/1428423002/diff/1/chrome/browser/ui/views/loc...
File chrome/browser/ui/views/location_bar/location_bar_view.h (right):

https://codereview.chromium.org/1428423002/diff/1/chrome/browser/ui/views/loc...
chrome/browser/ui/views/location_bar/location_bar_view.h:57: class ImageButton;
nit: remove

https://codereview.chromium.org/1428423002/diff/1/chrome/browser/ui/views/loc...
chrome/browser/ui/views/location_bar/location_bar_view.h:58: class ImageView;
nit: remove

https://codereview.chromium.org/1428423002/diff/1/chrome/renderer/searchbox/s...
File chrome/renderer/searchbox/searchbox.h (right):

https://codereview.chromium.org/1428423002/diff/1/chrome/renderer/searchbox/s...
chrome/renderer/searchbox/searchbox.h:167: void OnToggleVoiceSearch();
Remove this and the base decl?

https://codereview.chromium.org/1428423002/diff/1/chrome/renderer/searchbox/s...
File chrome/renderer/searchbox/searchbox_extension.cc (right):

https://codereview.chromium.org/1428423002/diff/1/chrome/renderer/searchbox/s...
chrome/renderer/searchbox/searchbox_extension.cc:627: void
SearchBoxExtension::DispatchToggleVoiceSearch(
Remove this and kDispatchToggleVoiceSearchScript?

[Evan Stade, 2015-11-07 00:43:03]

> It's hard to distinguish genuinely dead code here from the still-used NTP
> "Search by voice" icon, etc. Can you clarify the difference in the CL
> description and add a reviewer familiar with the latest status there?

I'm not sure what you would like me to add, can you suggest a new description?
The description is pretty specific about what API is being changed. The NTP
voice search uses getUserMedia AFAICT which is not the same API (I'm not sure
why it's important to mention that?). As far as a reviewer familiar with the
feature, I believe samarth@ provides that perspective.

https://codereview.chromium.org/1428423002/diff/1/chrome/browser/ui/cocoa/loc...
File chrome/browser/ui/cocoa/location_bar/mic_search_decoration.h (left):

https://codereview.chromium.org/1428423002/diff/1/chrome/browser/ui/cocoa/loc...
chrome/browser/ui/cocoa/location_bar/mic_search_decoration.h:1: // Copyright
2013 The Chromium Authors. All rights reserved.
On 2015/11/06 23:59:22, msw wrote:
> Update chrome/chrome_browser_ui.gypi

Done.

https://codereview.chromium.org/1428423002/diff/1/chrome/browser/ui/search/se...
File chrome/browser/ui/search/search_ipc_router.h (right):

https://codereview.chromium.org/1428423002/diff/1/chrome/browser/ui/search/se...
chrome/browser/ui/search/search_ipc_router.h:117: virtual bool
ShouldSendToggleVoiceSearch() = 0;
On 2015/11/06 23:59:22, msw wrote:
> Should this also be removed?

Done.

https://codereview.chromium.org/1428423002/diff/1/chrome/browser/ui/search/se...
chrome/browser/ui/search/search_ipc_router.h:167: void ToggleVoiceSearch();
On 2015/11/06 23:59:22, msw wrote:
> Should this also be removed?

Done.

https://codereview.chromium.org/1428423002/diff/1/chrome/browser/ui/search/se...
File chrome/browser/ui/search/search_tab_helper.h (right):

https://codereview.chromium.org/1428423002/diff/1/chrome/browser/ui/search/se...
chrome/browser/ui/search/search_tab_helper.h:94: void ToggleVoiceSearch();
On 2015/11/06 23:59:23, msw wrote:
> Should this also be removed?

Done.

https://codereview.chromium.org/1428423002/diff/1/chrome/browser/ui/views/loc...
File chrome/browser/ui/views/location_bar/location_bar_view.cc (left):

https://codereview.chromium.org/1428423002/diff/1/chrome/browser/ui/views/loc...
chrome/browser/ui/views/location_bar/location_bar_view.cc:288:
l10n_util::GetStringUTF16(IDS_TOOLTIP_MIC_SEARCH));
On 2015/11/06 23:59:23, msw wrote:
> Remove IDS_TOOLTIP_MIC_SEARCH and other uses?

no, this tooltip is used in the cros app list

https://codereview.chromium.org/1428423002/diff/1/chrome/browser/ui/views/loc...
File chrome/browser/ui/views/location_bar/location_bar_view.cc (right):

https://codereview.chromium.org/1428423002/diff/1/chrome/browser/ui/views/loc...
chrome/browser/ui/views/location_bar/location_bar_view.cc:38: #include
"chrome/browser/ui/view_ids.h"
On 2015/11/06 23:59:23, msw wrote:
> nit: remove

Done.

https://codereview.chromium.org/1428423002/diff/1/chrome/browser/ui/views/loc...
chrome/browser/ui/views/location_bar/location_bar_view.cc:77: #include
"ui/base/l10n/l10n_util.h"
On 2015/11/06 23:59:23, msw wrote:
> nit: remove

Done.

https://codereview.chromium.org/1428423002/diff/1/chrome/browser/ui/views/loc...
chrome/browser/ui/views/location_bar/location_bar_view.cc:95: #include
"ui/views/controls/button/image_button.h"
On 2015/11/06 23:59:23, msw wrote:
> nit: remove

Done.

https://codereview.chromium.org/1428423002/diff/1/chrome/browser/ui/views/loc...
File chrome/browser/ui/views/location_bar/location_bar_view.h (right):

https://codereview.chromium.org/1428423002/diff/1/chrome/browser/ui/views/loc...
chrome/browser/ui/views/location_bar/location_bar_view.h:57: class ImageButton;
On 2015/11/06 23:59:23, msw wrote:
> nit: remove

Done.

https://codereview.chromium.org/1428423002/diff/1/chrome/browser/ui/views/loc...
chrome/browser/ui/views/location_bar/location_bar_view.h:58: class ImageView;
On 2015/11/06 23:59:23, msw wrote:
> nit: remove

Done.

https://codereview.chromium.org/1428423002/diff/1/chrome/renderer/searchbox/s...
File chrome/renderer/searchbox/searchbox.h (right):

https://codereview.chromium.org/1428423002/diff/1/chrome/renderer/searchbox/s...
chrome/renderer/searchbox/searchbox.h:167: void OnToggleVoiceSearch();
On 2015/11/06 23:59:23, msw wrote:
> Remove this and the base decl?

Done.

https://codereview.chromium.org/1428423002/diff/1/chrome/renderer/searchbox/s...
File chrome/renderer/searchbox/searchbox_extension.cc (right):

https://codereview.chromium.org/1428423002/diff/1/chrome/renderer/searchbox/s...
chrome/renderer/searchbox/searchbox_extension.cc:627: void
SearchBoxExtension::DispatchToggleVoiceSearch(
On 2015/11/06 23:59:23, msw wrote:
> Remove this and kDispatchToggleVoiceSearchScript?

Done.

[Evan Stade, 2015-11-07 00:43:09]

The CQ bit was checked by estade@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-11-07 00:45:04]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1428423002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1428423002/20001

[samarth, 2015-11-07 02:45:16]

lgtm for search

Confirmed again that this is not used on the server-side for Google.

Thanks for the cleanup!
Samarth

[Evan Stade, 2015-11-07 03:12:35]

The CQ bit was checked by estade@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-11-07 03:12:59]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1428423002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1428423002/60001

[commit-bot: I haz the power, 2015-11-07 04:25:35]

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

[commit-bot: I haz the power, 2015-11-07 04:25:38]

Dry run: This issue passed the CQ dry run.

[msw, 2015-11-09 18:51:12]

lgtm (regarding the cl desc: i was mostly confused by all the remaining code i
found that you've now removed, maybe just mention that this shouldn't affect the
ntp's fake omnibox mic icon for voice input thingy)

[Evan Stade, 2015-11-09 18:52:27]

Description was changed from

==========
Remove setVoiceSearchSupported part of EmbeddedSearch SearchBox API.

also remove the mic search view from the location bar.

BUG=522168
==========

to

==========
Remove setVoiceSearchSupported part of EmbeddedSearch SearchBox API.

also remove the mic search view from the location bar. The NTP "omnibox" still
has voice search but that's unrelated.

BUG=522168
==========

[Evan Stade, 2015-11-09 18:52:42]

Description was changed from

==========
Remove setVoiceSearchSupported part of EmbeddedSearch SearchBox API.

also remove the mic search view from the location bar. The NTP "omnibox" still
has voice search but that's unrelated.

BUG=522168
==========

to

==========
Remove setVoiceSearchSupported part of EmbeddedSearch SearchBox API.

also remove the mic search view from the location bar. The NTP "omnibox" still
has voice search but that's unrelated.

BUG=522168
TBR=oshima@chromium.org
==========

[Evan Stade, 2015-11-09 18:53:10]

> regarding the cl desc: [...]

done

TBR'ing oshima for asset deletion.

[Evan Stade, 2015-11-09 18:53:15]

The CQ bit was checked by estade@chromium.org

[Evan Stade, 2015-11-09 18:53:15]

The patchset sent to the CQ was uploaded after l-g-t-m from samarth@chromium.org
Link to the patchset: https://codereview.chromium.org/1428423002/#ps60001
(title: "mac fix")

[commit-bot: I haz the power, 2015-11-09 18:54:46]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1428423002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1428423002/60001

[commit-bot: I haz the power, 2015-11-09 20:38:38]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-11-09 20:40:00]

Patchset 4 (id:??) landed as
https://crrev.com/e3168e1578dc4bbc928d86976a33db284ede2164
Cr-Commit-Position: refs/heads/master@{#358641}

[robliao, 2015-11-09 21:18:16]

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

The reason for reverting is: ..\..\chrome\app\chrome_dll.rc(118) : error RC2104
: undefined keyword or key name: IDC_TOGGLE_SPEECH_INPUT.