Delete the old hotwording integration.

Delete the old hotwording integration.

BUG=439143
Committed: https://crrev.com/34d3f96d36fcac03d665f4fff98c5bb3944124ec
Cr-Commit-Position: refs/heads/master@{#322333}

[Anand Mistry (off Chromium), 2015-03-17 00:18:26]

amistry@chromium.org changed reviewers:
+ kcarattini@chromium.org, mgiuca@chromium.org

[kcarattini, 2015-03-17 00:41:13]

https://codereview.chromium.org/992173002/diff/20001/chrome/browser/search/ho...
File chrome/browser/search/hotword_service.cc (right):

https://codereview.chromium.org/992173002/diff/20001/chrome/browser/search/ho...
chrome/browser/search/hotword_service.cc:139:
extension_misc::kHotwordSharedModuleId)) {
If you are changing the meaning of the stat, I think you should change the name
too (maybe 'HotwordSharedModuleAvailability')

https://codereview.chromium.org/992173002/diff/20001/chrome/browser/search/ho...
chrome/browser/search/hotword_service.cc:327: // Disable the old extension so it
doesn't interfere with the new stuff.
Do we need to disable the hotword helper here as well if people have it from a
previous installation?

https://codereview.chromium.org/992173002/diff/20001/chrome/common/extensions...
File chrome/common/extensions/api/_permission_features.json (left):

https://codereview.chromium.org/992173002/diff/20001/chrome/common/extensions...
chrome/common/extensions/api/_permission_features.json:493:
"09FDCB5851B8F3378DB630D06E316076E89C95A6",  // hotword nacl
Doesn't the 'new' hotword extension need to be listed here?

[Anand Mistry (off Chromium), 2015-03-17 03:00:46]

https://codereview.chromium.org/992173002/diff/20001/chrome/browser/search/ho...
File chrome/browser/search/hotword_service.cc (right):

https://codereview.chromium.org/992173002/diff/20001/chrome/browser/search/ho...
chrome/browser/search/hotword_service.cc:139:
extension_misc::kHotwordSharedModuleId)) {
On 2015/03/17 00:41:13, kcarattini wrote:
> If you are changing the meaning of the stat, I think you should change the
name
> too (maybe 'HotwordSharedModuleAvailability')

Hmm. I don't think we should be changing the meaning, so I've opted to delete
this code entirely. Adding metrics for the shared module should be a separate
change. Added TODO to track this.

https://codereview.chromium.org/992173002/diff/20001/chrome/browser/search/ho...
chrome/browser/search/hotword_service.cc:327: // Disable the old extension so it
doesn't interfere with the new stuff.
On 2015/03/17 00:41:13, kcarattini wrote:
> Do we need to disable the hotword helper here as well if people have it from a
> previous installation?

hotword_helper is a component that's being deleted in this change, so it's
unnecessary.

https://codereview.chromium.org/992173002/diff/20001/chrome/common/extensions...
File chrome/common/extensions/api/_permission_features.json (left):

https://codereview.chromium.org/992173002/diff/20001/chrome/common/extensions...
chrome/common/extensions/api/_permission_features.json:493:
"09FDCB5851B8F3378DB630D06E316076E89C95A6",  // hotword nacl
On 2015/03/17 00:41:13, kcarattini wrote:
> Doesn't the 'new' hotword extension need to be listed here?

Done as a separate change since it's orthogonal to this one.

[kcarattini, 2015-03-17 22:55:22]

https://codereview.chromium.org/992173002/diff/20001/chrome/common/extensions...
File chrome/common/extensions/api/_permission_features.json (left):

https://codereview.chromium.org/992173002/diff/20001/chrome/common/extensions...
chrome/common/extensions/api/_permission_features.json:493:
"09FDCB5851B8F3378DB630D06E316076E89C95A6",  // hotword nacl
On 2015/03/17 03:00:46, Anand Mistry wrote:
> On 2015/03/17 00:41:13, kcarattini wrote:
> > Doesn't the 'new' hotword extension need to be listed here?
> 
> Done as a separate change since it's orthogonal to this one.

How was it working if it wasn't listed here in the first place?

[Anand Mistry (off Chromium), 2015-03-18 01:33:41]

On 2015/03/17 22:55:22, kcarattini wrote:
>
https://codereview.chromium.org/992173002/diff/20001/chrome/common/extensions...
> File chrome/common/extensions/api/_permission_features.json (left):
> 
>
https://codereview.chromium.org/992173002/diff/20001/chrome/common/extensions...
> chrome/common/extensions/api/_permission_features.json:493:
> "09FDCB5851B8F3378DB630D06E316076E89C95A6",  // hotword nacl
> On 2015/03/17 03:00:46, Anand Mistry wrote:
> > On 2015/03/17 00:41:13, kcarattini wrote:
> > > Doesn't the 'new' hotword extension need to be listed here?
> > 
> > Done as a separate change since it's orthogonal to this one.
> 
> How was it working if it wasn't listed here in the first place?

My understanding is that because it's a component extension, it automatically
has special privileges.

[kcarattini, 2015-03-18 01:42:52]

lgtm

[Matt Giuca, 2015-03-19 00:46:06]

Reviewed all the files, but mostly in app_list. You will need more reviewers for
the rest.

https://codereview.chromium.org/992173002/diff/60001/chrome/browser/ui/app_li...
File chrome/browser/ui/app_list/start_page_service.cc (right):

https://codereview.chromium.org/992173002/diff/60001/chrome/browser/ui/app_li...
chrome/browser/ui/app_list/start_page_service.cc:381:
"appList.startPage.onAppListHidden");
You deleted this function; I think you have to delete the call too.

https://codereview.chromium.org/992173002/diff/60001/chrome/common/extensions...
File chrome/common/extensions/extension_constants.h (right):

https://codereview.chromium.org/992173002/diff/60001/chrome/common/extensions...
chrome/common/extensions/extension_constants.h:83: extern const char
kHotwordNewExtensionId[];
In a follow-up CL, would you be able to rename this to kHotwordExtensionId?

(Don't do it here, too confusing.)

[Anand Mistry (off Chromium), 2015-03-19 05:09:15]

amistry@chromium.org changed reviewers:
+ benwells@chromium.org, thestig@chromium.org, xhwang@chromium.org

[Anand Mistry (off Chromium), 2015-03-19 05:09:16]

xhwang@chromium.org: Please review changes in chrome/browser/media

benwells@chromium.org: Please review extensions stuff

thestig@chromium.org: For OWNERS.

https://codereview.chromium.org/992173002/diff/60001/chrome/browser/ui/app_li...
File chrome/browser/ui/app_list/start_page_service.cc (right):

https://codereview.chromium.org/992173002/diff/60001/chrome/browser/ui/app_li...
chrome/browser/ui/app_list/start_page_service.cc:381:
"appList.startPage.onAppListHidden");
On 2015/03/19 00:46:05, Matt Giuca (OOO til Mar 23) wrote:
> You deleted this function; I think you have to delete the call too.

Done.

https://codereview.chromium.org/992173002/diff/60001/chrome/common/extensions...
File chrome/common/extensions/extension_constants.h (right):

https://codereview.chromium.org/992173002/diff/60001/chrome/common/extensions...
chrome/common/extensions/extension_constants.h:83: extern const char
kHotwordNewExtensionId[];
On 2015/03/19 00:46:05, Matt Giuca (OOO til Mar 23) wrote:
> In a follow-up CL, would you be able to rename this to kHotwordExtensionId?

Yes, will do in a follow-up.

> 
> (Don't do it here, too confusing.)

[xhwang, 2015-03-19 05:16:17]

xhwang@chromium.org changed reviewers:
+ miu@chromium.org, tommi@chromium.org
- xhwang@chromium.org

[xhwang, 2015-03-19 05:17:45]

Replace myself with the correct owners:

miu: chrome/browser/media/media_stream_capture_indicator.cc
tommi: chrome/browser/media/media_capture_devices_dispatcher.cc

[Matt Giuca, 2015-03-19 06:09:12]

lgtm

[benwells, 2015-03-20 02:47:06]

https://codereview.chromium.org/992173002/diff/80001/chrome/common/extensions...
File chrome/common/extensions/extension_constants.h (right):

https://codereview.chromium.org/992173002/diff/80001/chrome/common/extensions...
chrome/common/extensions/extension_constants.h:80: extern const char
kHotwordOldExtensionId[];
Is this deliberately left in? Is it needed?

[Anand Mistry (off Chromium), 2015-03-20 03:21:51]

https://codereview.chromium.org/992173002/diff/80001/chrome/common/extensions...
File chrome/common/extensions/extension_constants.h (right):

https://codereview.chromium.org/992173002/diff/80001/chrome/common/extensions...
chrome/common/extensions/extension_constants.h:80: extern const char
kHotwordOldExtensionId[];
On 2015/03/20 02:47:06, benwells wrote:
> Is this deliberately left in? Is it needed?

It's still used by HotwordService, but doesn't need to be here any more. Moved
it to hotword_service.cc

[benwells, 2015-03-20 04:06:33]

lgtm

[miu, 2015-03-20 04:43:51]

media_stream_capture_indicator lgtm % a concern:

https://codereview.chromium.org/992173002/diff/100001/chrome/browser/media/me...
File chrome/browser/media/media_stream_capture_indicator.cc (right):

https://codereview.chromium.org/992173002/diff/100001/chrome/browser/media/me...
chrome/browser/media/media_stream_capture_indicator.cc:60:
extension_misc::kHotwordNewExtensionId,
Out of curiosity, why is/should the hotword extension be exempt from the tab
media indicator?  My knee-jerk reaction is that there's a user privacy concern
here w.r.t. not informing the user they are being recorded.  Feel free to
discuss w/ me in a separate e-mail thread.

[Anand Mistry (off Chromium), 2015-03-23 23:18:46]

tommi, thestig: Pingu! (http://en.wikipedia.org/wiki/Pingu)

[Lei Zhang, 2015-03-23 23:41:18]

non-{app_list, extensions, media} chrome/ bits lgtm

https://codereview.chromium.org/992173002/diff/100001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/app_list/start_page_handler.h (right):

https://codereview.chromium.org/992173002/diff/100001/chrome/browser/ui/webui...
chrome/browser/ui/webui/app_list/start_page_handler.h:18: class
ExtensionRegistry;
no longer needed

[Anand Mistry (off Chromium), 2015-03-24 06:15:36]

https://codereview.chromium.org/992173002/diff/100001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/app_list/start_page_handler.h (right):

https://codereview.chromium.org/992173002/diff/100001/chrome/browser/ui/webui...
chrome/browser/ui/webui/app_list/start_page_handler.h:18: class
ExtensionRegistry;
On 2015/03/23 23:41:18, Lei Zhang wrote:
> no longer needed

Done.

[Anand Mistry (off Chromium), 2015-03-26 05:26:09]

The CQ bit was checked by amistry@chromium.org

[Anand Mistry (off Chromium), 2015-03-26 05:26:10]

The patchset sent to the CQ was uploaded after l-g-t-m from
kcarattini@chromium.org, mgiuca@chromium.org, benwells@chromium.org,
thestig@chromium.org, miu@chromium.org
Link to the patchset: https://codereview.chromium.org/992173002/#ps140001
(title: "Rebase.")

[commit-bot: I haz the power, 2015-03-26 05:26:17]

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

[commit-bot: I haz the power, 2015-03-26 06:11:32]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2015-03-26 06:12:27]

Patchset 8 (id:??) landed as
https://crrev.com/34d3f96d36fcac03d665f4fff98c5bb3944124ec
Cr-Commit-Position: refs/heads/master@{#322333}