Set enable_hangout_services_extension from branding.

Set enable_hangout_services_extension from branding.

Currently the hangouts services is enabled in Google Chrome branded builds, or
if the flag is set manually.

This change keys only off of the build flag, and sets the default based on the
branding. This makes it simpler to understand and eliminates some preprocessor
complexity.

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

[brettw, 2016-10-11 17:47:32]

brettw@chromium.org changed reviewers:
+ grunell@chromium.org

[brettw, 2016-10-11 17:47:37]

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

[commit-bot: I haz the power, 2016-10-11 17:47:50]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-11 19:48:50]

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

[commit-bot: I haz the power, 2016-10-11 19:48:50]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[brettw, 2016-10-11 23:43:02]

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

[commit-bot: I haz the power, 2016-10-11 23:43:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-12 01:45:12]

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

[commit-bot: I haz the power, 2016-10-12 01:45:13]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Henrik Grunell, 2016-10-12 07:39:54]

Description was changed from

==========
Set enable_hangout_services_extension from branding.

Currently the hangouts services is enabled in Google Chrome branded builds, or
if the flag is set manually.

This change keys only off of the build flag, and sets the default based on the
branding. This makes it simpler to understand and eliminates some preprocessor
complexity.
==========

to

==========
Set enable_hangout_services_extension from branding.

Currently the hangouts services is enabled in Google Chrome branded builds, or
if the flag is set manually.

This change keys only off of the build flag, and sets the default based on the
branding. This makes it simpler to understand and eliminates some preprocessor
complexity.
==========

[Henrik Grunell, 2016-10-12 07:44:10]

Thanks for cleaning this up!

Should
https://cs.chromium.org/chromium/src/chrome/browser/browser_resources.grd?q=e...
also change?

Also, adding kjellander@ to take a look at
https://cs.chromium.org/chromium/src/chrome/browser/browser_resources.grd?q=e...
I wonder if it's even used longer? If yes, will it be affected by this and
should it be updated?

[brettw, 2016-10-12 18:25:30]

brettw@chromium.org changed reviewers:
+ kjellander@chromium.org

[brettw, 2016-10-12 18:25:30]

+kjellander: please see question from grunell above:

[kjellander_chromium, 2016-10-13 02:53:54]

On 2016/10/12 07:44:10, Henrik Grunell wrote:
> Thanks for cleaning this up!
> 
> Should
>
https://cs.chromium.org/chromium/src/chrome/browser/browser_resources.grd?q=e...
> also change?
> 
> Also, adding kjellander@ to take a look at
>
https://cs.chromium.org/chromium/src/chrome/browser/browser_resources.grd?q=e...
> I wonder if it's even used longer? If yes, will it be affected by this and
> should it be updated?

I don't know this area at all, but it was added in
https://codereview.chromium.org/30833003 and the code where it's used is still
present
https://cs.chromium.org/chromium/src/chrome/browser/extensions/component_load...
So I guess it's used? Not sure what should be updated regarding this.

[Henrik Grunell, 2016-10-13 07:02:39]

On 2016/10/13 02:53:54, kjellander_chromium wrote:
> On 2016/10/12 07:44:10, Henrik Grunell wrote:
> > Thanks for cleaning this up!
> > 
> > Should
> >
>
https://cs.chromium.org/chromium/src/chrome/browser/browser_resources.grd?q=e...
> > also change?
> > 
> > Also, adding kjellander@ to take a look at
> >
>
https://cs.chromium.org/chromium/src/chrome/browser/browser_resources.grd?q=e...
> > I wonder if it's even used longer? If yes, will it be affected by this and
> > should it be updated?
> 
> I don't know this area at all, but it was added in
> https://codereview.chromium.org/30833003 and the code where it's used is still
> present
>
https://cs.chromium.org/chromium/src/chrome/browser/extensions/component_load...
> So I guess it's used? Not sure what should be updated regarding this.

Argh, for crying out loud - I pasted the wrong link. :-| Sorry for that. This is
the right one for you kjellander:

https://cs.chromium.org/chromium/src/third_party/webrtc/build/chromium_common...

It's WebRTC code, and I don't know when and how it's used. In particular, at
line 2123 it seems incorrect. Should not care about buildtype.

[kjellander_chromium, 2016-10-13 13:19:59]

On 2016/10/13 07:02:39, Henrik Grunell wrote:
> On 2016/10/13 02:53:54, kjellander_chromium wrote:
> > On 2016/10/12 07:44:10, Henrik Grunell wrote:
> > > Thanks for cleaning this up!
> > > 
> > > Should
> > >
> >
>
https://cs.chromium.org/chromium/src/chrome/browser/browser_resources.grd?q=e...
> > > also change?
> > > 
> > > Also, adding kjellander@ to take a look at
> > >
> >
>
https://cs.chromium.org/chromium/src/chrome/browser/browser_resources.grd?q=e...
> > > I wonder if it's even used longer? If yes, will it be affected by this and
> > > should it be updated?
> > 
> > I don't know this area at all, but it was added in
> > https://codereview.chromium.org/30833003 and the code where it's used is
still
> > present
> >
>
https://cs.chromium.org/chromium/src/chrome/browser/extensions/component_load...
> > So I guess it's used? Not sure what should be updated regarding this.
> 
> Argh, for crying out loud - I pasted the wrong link. :-| Sorry for that. This
is
> the right one for you kjellander:
> 
>
https://cs.chromium.org/chromium/src/third_party/webrtc/build/chromium_common...
> 
> It's WebRTC code, and I don't know when and how it's used. In particular, at
> line 2123 it seems incorrect. Should not care about buildtype.

Oh, that. Well first of all that's GYP so it doesn't affect Chrome in any way.
We only added that as a copy of Chromium's common.gypi before it was deleted. It
was done in order to keep our GYP generation alive just a little longer. Luckily
we can soon remove it entirely.

[Henrik Grunell, 2016-10-13 13:22:56]

On 2016/10/13 13:19:59, kjellander_chromium wrote:
> On 2016/10/13 07:02:39, Henrik Grunell wrote:
> > On 2016/10/13 02:53:54, kjellander_chromium wrote:
> > > On 2016/10/12 07:44:10, Henrik Grunell wrote:
> > > > Thanks for cleaning this up!
> > > > 
> > > > Should
> > > >
> > >
> >
>
https://cs.chromium.org/chromium/src/chrome/browser/browser_resources.grd?q=e...
> > > > also change?
> > > > 
> > > > Also, adding kjellander@ to take a look at
> > > >
> > >
> >
>
https://cs.chromium.org/chromium/src/chrome/browser/browser_resources.grd?q=e...
> > > > I wonder if it's even used longer? If yes, will it be affected by this
and
> > > > should it be updated?
> > > 
> > > I don't know this area at all, but it was added in
> > > https://codereview.chromium.org/30833003 and the code where it's used is
> still
> > > present
> > >
> >
>
https://cs.chromium.org/chromium/src/chrome/browser/extensions/component_load...
> > > So I guess it's used? Not sure what should be updated regarding this.
> > 
> > Argh, for crying out loud - I pasted the wrong link. :-| Sorry for that.
This
> is
> > the right one for you kjellander:
> > 
> >
>
https://cs.chromium.org/chromium/src/third_party/webrtc/build/chromium_common...
> > 
> > It's WebRTC code, and I don't know when and how it's used. In particular, at
> > line 2123 it seems incorrect. Should not care about buildtype.
> 
> Oh, that. Well first of all that's GYP so it doesn't affect Chrome in any way.
> We only added that as a copy of Chromium's common.gypi before it was deleted.
It
> was done in order to keep our GYP generation alive just a little longer.
Luckily
> we can soon remove it entirely.

OK, thanks. Then no worries for that file.

Brett, still one outstanding question for you: should this also change?

https://cs.chromium.org/chromium/src/chrome/browser/browser_resources.grd?q=e...

[brettw, 2016-10-14 17:30:33]

Fix

[brettw, 2016-10-14 17:30:33]

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

[commit-bot: I haz the power, 2016-10-14 17:30:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[brettw, 2016-10-14 17:30:55]

I changed that file and updated the grit defines to actually use it (it wasn't
hooked up before!).

[commit-bot: I haz the power, 2016-10-14 18:30:23]

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

[commit-bot: I haz the power, 2016-10-14 18:30:24]

Dry run: This issue passed the CQ dry run.

[kjellander_chromium, 2016-10-17 11:49:10]

On 2016/10/14 17:30:55, brettw (ping on IM after 24h) wrote:
> I changed that file and updated the grit defines to actually use it (it wasn't
> hooked up before!).

Ah, but it was at least working before when since _google_chrome was true when
it was supposed to be turned on.

lgtm

[Henrik Grunell, 2016-10-17 12:24:22]

Thanks Brett.

I've tested and verified the CL manually. With each flag at a time, with both,
and without any of them.

lgtm

[brettw, 2016-10-17 17:39:25]

The CQ bit was checked by brettw@chromium.org

[commit-bot: I haz the power, 2016-10-17 17:39:43]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-17 18:38:53]

Description was changed from

==========
Set enable_hangout_services_extension from branding.

Currently the hangouts services is enabled in Google Chrome branded builds, or
if the flag is set manually.

This change keys only off of the build flag, and sets the default based on the
branding. This makes it simpler to understand and eliminates some preprocessor
complexity.
==========

to

==========
Set enable_hangout_services_extension from branding.

Currently the hangouts services is enabled in Google Chrome branded builds, or
if the flag is set manually.

This change keys only off of the build flag, and sets the default based on the
branding. This makes it simpler to understand and eliminates some preprocessor
complexity.
==========

[commit-bot: I haz the power, 2016-10-17 18:38:55]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-10-17 18:41:38]

Description was changed from

==========
Set enable_hangout_services_extension from branding.

Currently the hangouts services is enabled in Google Chrome branded builds, or
if the flag is set manually.

This change keys only off of the build flag, and sets the default based on the
branding. This makes it simpler to understand and eliminates some preprocessor
complexity.
==========

to

==========
Set enable_hangout_services_extension from branding.

Currently the hangouts services is enabled in Google Chrome branded builds, or
if the flag is set manually.

This change keys only off of the build flag, and sets the default based on the
branding. This makes it simpler to understand and eliminates some preprocessor
complexity.

Committed: https://crrev.com/bbc0f8ce8dac443605ee71a0d88d57e73aa07e75
Cr-Commit-Position: refs/heads/master@{#425730}
==========

[commit-bot: I haz the power, 2016-10-17 18:41:39]

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