Disable "Ok Google" hotwording in open source builds by default.

Disable "Ok Google" hotwording in open source builds by default.

The compile-time flag "enable_hotwording" is now tied to
branding=Chrome (false by default unless making a Google Chrome build).

Note: Chromium will no longer download/install the Hotword Shared
Module, and will automatically remove the Hotword Shared Module on
startup if it was previously installed. To keep this functionality, add
"enable_hotwording=1" to GYP_DEFINES.

BUG=500922
Committed: https://crrev.com/0366a5184a70b3eefb5fcef2c2e13721669f00d8
Cr-Commit-Position: refs/heads/master@{#335874}

[Matt Giuca, 2015-06-24 00:44:25]

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

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

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

[Matt Giuca, 2015-06-24 00:46:20]

The intention is to keep official builds the same as before, but disable this
flag at compile-time for the default open source build.

[commit-bot: I haz the power, 2015-06-24 00:46:22]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1200413003/1

[Anand Mistry (off Chromium), 2015-06-24 01:10:33]

https://codereview.chromium.org/1200413003/diff/1/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/1200413003/diff/1/build/common.gypi#newcode839
build/common.gypi:839: 'enable_hotwording%': 1,
You want branding=="Chrome", not buildtype. buildtype controls compiler
optimisations.

https://codereview.chromium.org/1200413003/diff/1/chrome/browser/BUILD.gn
File chrome/browser/BUILD.gn (right):

https://codereview.chromium.org/1200413003/diff/1/chrome/browser/BUILD.gn#new...
chrome/browser/BUILD.gn:26: enable_hotwording = is_official_build
s/is_official_build/is_chrome_branded/

[Matt Giuca, 2015-06-24 01:41:55]

https://codereview.chromium.org/1200413003/diff/1/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/1200413003/diff/1/build/common.gypi#newcode839
build/common.gypi:839: 'enable_hotwording%': 1,
On 2015/06/24 01:10:33, Anand Mistry wrote:
> You want branding=="Chrome", not buildtype. buildtype controls compiler
> optimisations.

Done.

https://codereview.chromium.org/1200413003/diff/1/chrome/browser/BUILD.gn
File chrome/browser/BUILD.gn (right):

https://codereview.chromium.org/1200413003/diff/1/chrome/browser/BUILD.gn#new...
chrome/browser/BUILD.gn:26: enable_hotwording = is_official_build
On 2015/06/24 01:10:33, Anand Mistry wrote:
> s/is_official_build/is_chrome_branded/

Done.

[Matt Giuca, 2015-06-24 01:42:00]

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

[commit-bot: I haz the power, 2015-06-24 01:42:13]

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

[Nico, 2015-06-24 01:46:30]

https://codereview.chromium.org/1200413003/diff/20001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/1200413003/diff/20001/build/common.gypi#newco...
build/common.gypi:841: ['branding=="Chrome"', {
Isn't buildtype=Official a better check?

[Matt Giuca, 2015-06-24 01:59:47]

https://codereview.chromium.org/1200413003/diff/20001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/1200413003/diff/20001/build/common.gypi#newco...
build/common.gypi:841: ['branding=="Chrome"', {
On 2015/06/24 01:46:30, Nico wrote:
> Isn't buildtype=Official a better check?

See Anand's previous comment. (I initially had buildtype==Official.)

From looking at other examples (e.g. bundling Flash), it looks like
branding==Chrome is the correct check for proprietary stuff. buildtype==Official
is about optimizations; branding controls both the iconography/titling, and the
inclusion of proprietary components.

[Nico, 2015-06-24 02:26:32]

On 2015/06/24 01:59:47, Matt Giuca wrote:
> https://codereview.chromium.org/1200413003/diff/20001/build/common.gypi
> File build/common.gypi (right):
> 
>
https://codereview.chromium.org/1200413003/diff/20001/build/common.gypi#newco...
> build/common.gypi:841: ['branding=="Chrome"', {
> On 2015/06/24 01:46:30, Nico wrote:
> > Isn't buildtype=Official a better check?
> 
> See Anand's previous comment. (I initially had buildtype==Official.)
> 
> From looking at other examples (e.g. bundling Flash), it looks like
> branding==Chrome is the correct check for proprietary stuff.
buildtype==Official
> is about optimizations; branding controls both the iconography/titling, and
the
> inclusion of proprietary components.

The way I look at it is that branding controls the branding and/or things that
require a src-internal checkout (since that's needed for the chrome branding
resource files anyways), and buildtype controls compiler flags that might be
interesting for benchmarking and that don't require src-internal. Downloading
the hotword thingy doesn't require src-internal, but it has performance
implications (the download, and having hotwording active on the NTP). So
buildtype seems more appropriate to me.

[commit-bot: I haz the power, 2015-06-24 03:14:42]

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

[commit-bot: I haz the power, 2015-06-24 03:14:43]

Dry run: This issue passed the CQ dry run.

[Matt Giuca, 2015-06-24 03:18:21]

> The way I look at it is that branding controls the branding and/or things that
> require a src-internal checkout (since that's needed for the chrome branding
> resource files anyways), and buildtype controls compiler flags that might be
> interesting for benchmarking and that don't require src-internal. Downloading
> the hotword thingy doesn't require src-internal, but it has performance
> implications (the download, and having hotwording active on the NTP). So
> buildtype seems more appropriate to me.

I don't have a strong opinion about this, but I think the idea is that if
buildtype==Official is just about performance, then theoretically* a downstream
should be able to build a Chromium with buildtype==Official but not
branding==Chrome, and get the performance benefits of buildtype==Official.

The other thing that I think is important here is that we *want* proprietary
material to be associated with the Chrome brand, not Chromium. If our stated
goal is "Chrome can have proprietary material, Chromium cannot," then
branding==Chrome is semantically the correct flag to gate this by.

*Maybe it's not possible right now because people are misusing
buildtype==Official, but we should be making things better not worse.

[Nico, 2015-06-24 03:22:23]

On 2015/06/24 03:18:21, Matt Giuca wrote:
> > The way I look at it is that branding controls the branding and/or things
that
> > require a src-internal checkout (since that's needed for the chrome branding
> > resource files anyways), and buildtype controls compiler flags that might be
> > interesting for benchmarking and that don't require src-internal.
Downloading
> > the hotword thingy doesn't require src-internal, but it has performance
> > implications (the download, and having hotwording active on the NTP). So
> > buildtype seems more appropriate to me.
> 
> I don't have a strong opinion about this, but I think the idea is that if
> buildtype==Official is just about performance, then theoretically* a
downstream
> should be able to build a Chromium with buildtype==Official but not
> branding==Chrome, and get the performance benefits of buildtype==Official.
> 
> The other thing that I think is important here is that we *want* proprietary
> material to be associated with the Chrome brand, not Chromium. If our stated
> goal is "Chrome can have proprietary material, Chromium cannot," then
> branding==Chrome is semantically the correct flag to gate this by.
> 
> *Maybe it's not possible right now because people are misusing
> buildtype==Official, but we should be making things better not worse.

I too don't have a strong opinion (I've stated my weak opinion above). So lgtm
either way I suppose.

[Matt Giuca, 2015-06-24 05:36:40]

The CQ bit was checked by mgiuca@chromium.org

[commit-bot: I haz the power, 2015-06-24 05:36:56]

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

[commit-bot: I haz the power, 2015-06-24 05:41:46]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-06-24 05:42:34]

Patchset 2 (id:??) landed as
https://crrev.com/0366a5184a70b3eefb5fcef2c2e13721669f00d8
Cr-Commit-Position: refs/heads/master@{#335874}