Make it possible to disable dbus on linux

Make it possible to disable dbus on linux

Note that this only applies to "core" functionality.  Some configurations
(eg chromeos, or use_gconf=1) will pull in dbus independently.

BUG=63981
TEST=Make a linux content_shell build with use_dbus=0 use_gconf=0, then check that "ldd content_shell | grep dbus" doesn't show any matches.

[Mostyn Bramley-Moore, 2013-03-03 17:15:12]

Here's another patch which is relevant to embedded linux systems...

[joth, 2013-03-03 19:43:55]

The gyp rules for this could get pretty unwieldy as it expands from a 1D list,
to a grid of OS vs runtime-environment. I don't have any immediate suggestions
on tackling that though.

https://codereview.chromium.org/12385071/diff/12/content/browser/geolocation/...
File content/browser/geolocation/empty_device_data_provider.cc (right):

https://codereview.chromium.org/12385071/diff/12/content/browser/geolocation/...
content/browser/geolocation/empty_device_data_provider.cc:9: // Only define for
platforms that lack a real wifi data provider.
this comment is orphaned without the #if below it. reword to something like
"This definition is only compiled on platforms that lack a real wifi data
provider"

https://codereview.chromium.org/12385071/diff/12/content/content_browser.gypi
File content/content_browser.gypi (right):

https://codereview.chromium.org/12385071/diff/12/content/content_browser.gypi...
content/content_browser.gypi:1089: 'browser/power_save_blocker_empty.cc',
convention is to list all sources up top of file (so put this around line 590)
and then down here only define rules to _remove_ unneeded things from
configurations

https://codereview.chromium.org/12385071/diff/12/content/content_browser.gypi...
content/content_browser.gypi:1096: ['OS!="win" and OS!="mac" and (OS!="linux" or
use_dbus==1)', {
this seems backward. don't we want to remove the null provider in all the cases
where we have a real one? i.e.
OS=="win" or OS==mac or (OS=="linux" and use_dbus==1)

[Mostyn Bramley-Moore, 2013-03-03 21:19:25]

On 2013/03/03 19:43:55, joth wrote:
> The gyp rules for this could get pretty unwieldy as it expands from a 1D list,
> to a grid of OS vs runtime-environment. I don't have any immediate suggestions
> on tackling that though.

I have some ideas for major refactoring of all the chromium gyp files that would
simplify this, but I guess you would be more interested in a local fix for this
file?

[Mostyn Bramley-Moore, 2013-03-03 21:19:35]

https://codereview.chromium.org/12385071/diff/12/content/browser/geolocation/...
File content/browser/geolocation/empty_device_data_provider.cc (right):

https://codereview.chromium.org/12385071/diff/12/content/browser/geolocation/...
content/browser/geolocation/empty_device_data_provider.cc:9: // Only define for
platforms that lack a real wifi data provider.
Done.

https://codereview.chromium.org/12385071/diff/12/content/content_browser.gypi
File content/content_browser.gypi (right):

https://codereview.chromium.org/12385071/diff/12/content/content_browser.gypi...
content/content_browser.gypi:1089: 'browser/power_save_blocker_empty.cc',
Fixed in the latest patchset.

https://codereview.chromium.org/12385071/diff/12/content/content_browser.gypi...
content/content_browser.gypi:1096: ['OS!="win" and OS!="mac" and (OS!="linux" or
use_dbus==1)', {
Whoops- right you are.  Fixed in the latest patchset.

[Nico, 2013-03-03 23:00:44]

What's the motivation for this?

[Mostyn Bramley-Moore, 2013-03-03 23:07:23]

On 2013/03/03 23:00:44, Nico wrote:
> What's the motivation for this?

I'm working on builds for embedded linux systems, where dbus is not available.

[Nico, 2013-03-03 23:14:21]

Will this build configuration be covered by any bots?

https://codereview.chromium.org/12385071/diff/13/content/browser/power_save_b...
File content/browser/power_save_blocker_empty.cc (left):

https://codereview.chromium.org/12385071/diff/13/content/browser/power_save_b...
content/browser/power_save_blocker_empty.cc:21: // This may be called on any
thread.
Are you sure this won't get implemented on android?

https://codereview.chromium.org/12385071/diff/13/content/browser/power_save_b...
File content/browser/power_save_blocker_empty.cc (right):

https://codereview.chromium.org/12385071/diff/13/content/browser/power_save_b...
content/browser/power_save_blocker_empty.cc:1: // Copyright (c) 2012-2013 The
Chromium Authors. All rights reserved.
we don't do date ranges in copyrights

[Mostyn Bramley-Moore, 2013-03-03 23:36:54]

On 2013/03/03 23:14:21, Nico wrote:
> Will this build configuration be covered by any bots?

Not currently in the official chromium build bots, though I would love for that
to change.  Is there a process through which I (as a non-googler) can request
this?

https://codereview.chromium.org/12385071/diff/13/content/browser/power_save_b...
File content/browser/power_save_blocker_empty.cc (left):

https://codereview.chromium.org/12385071/diff/13/content/browser/power_save_b...
content/browser/power_save_blocker_empty.cc:21: // This may be called on any
thread.
On 2013/03/03 23:14:22, Nico wrote:
> Are you sure this won't get implemented on android?

Android has its own copy in:
content/browser/power_save_blocker_android.cc

This file is a copy of power_save_blocker_android.cc but without the TODO note,
since this feature is not likely to be implemented on embedded linux targets.  I
thought it was safer to make an explicit "empty" copy than simply re-using the
android file (which looks like it might get an implementation one day).

https://codereview.chromium.org/12385071/diff/13/content/browser/power_save_b...
File content/browser/power_save_blocker_empty.cc (right):

https://codereview.chromium.org/12385071/diff/13/content/browser/power_save_b...
content/browser/power_save_blocker_empty.cc:1: // Copyright (c) 2012-2013 The
Chromium Authors. All rights reserved.
OK- should I leave it as 2012, or bump it to 2013?

[stuartmorgan, 2013-03-04 07:30:35]

Why am I on this review? If you're going to put 6 (!) people on a review,
explain in the review message what parts of the CL you want each person to look
at. Having 6 people review the whole CL isn't a good use of anyone's time.

[Mostyn Bramley-Moore, 2013-03-04 07:43:08]

On 2013/03/04 07:30:35, stuartmorgan wrote:
> Why am I on this review? If you're going to put 6 (!) people on a review,
> explain in the review message what parts of the CL you want each person to
look
> at. Having 6 people review the whole CL isn't a good use of anyone's time.

Sorry about that- I am new to the chromium code review system and still learning
the policies and processes.

I CC'ed you because you were the last person to touch some of the dbus lines I
modified in chrome/chrome_tests_unit.gypi - should I put a summary of why I
added each reviewer in the description field of this issue, or in a reply
message (like here)?

[Nico, 2013-03-04 08:55:00]

How hard is it for you to maintain this bit out of tree?

https://codereview.chromium.org/12385071/diff/13/content/browser/power_save_b...
File content/browser/power_save_blocker_empty.cc (right):

https://codereview.chromium.org/12385071/diff/13/content/browser/power_save_b...
content/browser/power_save_blocker_empty.cc:1: // Copyright (c) 2012-2013 The
Chromium Authors. All rights reserved.
On 2013/03/03 23:36:55, Mostyn Bramley-Moore wrote:
> OK- should I leave it as 2012, or bump it to 2013?

2013

[Mostyn Bramley-Moore, 2013-03-04 09:37:04]

On 2013/03/04 08:55:00, Nico wrote:
> How hard is it for you to maintain this bit out of tree?

Which bit- power_save_blocker_empty.cc ?  Or this patchset in general?

[Mostyn Bramley-Moore, 2013-03-04 09:37:13]

https://codereview.chromium.org/12385071/diff/13/content/browser/power_save_b...
File content/browser/power_save_blocker_empty.cc (right):

https://codereview.chromium.org/12385071/diff/13/content/browser/power_save_b...
content/browser/power_save_blocker_empty.cc:1: // Copyright (c) 2012-2013 The
Chromium Authors. All rights reserved.
On 2013/03/04 08:55:00, Nico wrote:
> On 2013/03/03 23:36:55, Mostyn Bramley-Moore wrote:
> > OK- should I leave it as 2012, or bump it to 2013?
> 
> 2013

Done.

[Nico, 2013-03-04 09:43:50]

On Mon, Mar 4, 2013 at 10:37 AM,  <mostynb@opera.com> wrote:
> On 2013/03/04 08:55:00, Nico wrote:
>>
>> How hard is it for you to maintain this bit out of tree?
>
>
> Which bit- power_save_blocker_empty.cc ?  Or this patchset in general?

This patch set in general. It makes our gyp files slightly more
confusing. (I believe our stance is that we generally accept patches
if they make the code better structured or don't add overhead to other
devs.)

What's your game plan in general? Do you want to upstream all the
opera tweaks you have? Just the obvious ones? Are you going to have
bots that test your configuration? Since you asked above: You'll
probably have to have your own build waterfall for your builds (?).

More general: Was there some thread somewhere (chromium-dev) where
this was discussed that I missed? I'm guessing you'll be answering
these questions over and over again on all your reviews, so it might
make sense to discuss this in general on the list instead of in every
patch :-)

[Mostyn Bramley-Moore, 2013-03-04 10:35:48]

On 2013/03/04 09:43:50, Nico wrote:
> On Mon, Mar 4, 2013 at 10:37 AM,  <mailto:mostynb@opera.com> wrote:
> > On 2013/03/04 08:55:00, Nico wrote:
> >>
> >> How hard is it for you to maintain this bit out of tree?
> >
> >
> > Which bit- power_save_blocker_empty.cc ?  Or this patchset in general?
> 
> This patch set in general. It makes our gyp files slightly more
> confusing. (I believe our stance is that we generally accept patches
> if they make the code better structured or don't add overhead to other
> devs.)

I think I can refactor this to minimise the added complexity if that helps. 
Some possibilities:
* minimise this patch so that use_dbus only affects the content API (perhaps
rename it to content_api_dbus or some other better name)
* instead of checking for OS==linux before including dbus code, check for
use_dbus==1 (aside: is there a way to prevent use_dbus==1 and OS==win from being
set simultaneously?)
* add a new OS type (eg "linux_embedded")

> What's your game plan in general? Do you want to upstream all the
> opera tweaks you have? Just the obvious ones? Are you going to have
> bots that test your configuration? Since you asked above: You'll
> probably have to have your own build waterfall for your builds (?).

We're starting out by cherry-picking some small patches that we think have a
chance of being accepted.  This is the largest patch we have submitted so far,
and as you can see we still have a lot to learn.

We have our own internal build+testing infrastructure to confirm that our local
patches (including this one) work- is that sufficient or do you need something
affiliated with the official buildbot setup?  

> More general: Was there some thread somewhere (chromium-dev) where
> this was discussed that I missed? I'm guessing you'll be answering
> these questions over and over again on all your reviews, so it might
> make sense to discuss this in general on the list instead of in every
> patch :-)

I will forward this advice to my managers :)

[Nico, 2013-03-04 20:48:47]

On Mon, Mar 4, 2013 at 11:35 AM,  <mostynb@opera.com> wrote:
> On 2013/03/04 09:43:50, Nico wrote:
>
>> On Mon, Mar 4, 2013 at 10:37 AM,  <mailto:mostynb@opera.com> wrote:
>> > On 2013/03/04 08:55:00, Nico wrote:
>> >>
>> >> How hard is it for you to maintain this bit out of tree?
>> >
>> >
>> > Which bit- power_save_blocker_empty.cc ?  Or this patchset in general?
>
>
>> This patch set in general. It makes our gyp files slightly more
>> confusing. (I believe our stance is that we generally accept patches
>> if they make the code better structured or don't add overhead to other
>> devs.)
>
>
> I think I can refactor this to minimise the added complexity if that helps.
> Some possibilities:
> * minimise this patch so that use_dbus only affects the content API (perhaps
> rename it to content_api_dbus or some other better name)
> * instead of checking for OS==linux before including dbus code, check for
> use_dbus==1 (aside: is there a way to prevent use_dbus==1 and OS==win from
> being
> set simultaneously?)
> * add a new OS type (eg "linux_embedded")
>
>
>> What's your game plan in general? Do you want to upstream all the
>> opera tweaks you have? Just the obvious ones? Are you going to have
>> bots that test your configuration? Since you asked above: You'll
>> probably have to have your own build waterfall for your builds (?).
>
>
> We're starting out by cherry-picking some small patches that we think have a
> chance of being accepted.  This is the largest patch we have submitted so
> far,
> and as you can see we still have a lot to learn.
>
> We have our own internal build+testing infrastructure to confirm that our
> local
> patches (including this one) work- is that sufficient or do you need
> something
> affiliated with the official buildbot setup?

It doesn't have to be official, but public might help.

As a concrete example: Since our gyp files are suffering a bit from
"dead through a thousand cuts", I (and others) go through
build/common.gypi every now and then and look for stuff that doesn't
seem to be used. If we find something, we try deleting it, and if
trybots are happy we land it. I could see someone coming up with a
revert of this CL here and landing it with a comment like "remove
unneeded complexity". Nothing will stop that person at the moment.

I think for freebsd gyp build stuff we just say "this is minor, so
we'll let it in the tree, but we don't promise that we don't
accidentally break it, and if we do it's up to you to get it running
again". I would accept this CL under the same premise.

You probably have more CLs like this lined up. Can you give me a
preview of how many more of these CLs / lines of gyp changes you
intend to upstream? How many dependencies do you need to disable?

>
>
>> More general: Was there some thread somewhere (chromium-dev) where
>> this was discussed that I missed? I'm guessing you'll be answering
>> these questions over and over again on all your reviews, so it might
>> make sense to discuss this in general on the list instead of in every
>> patch :-)
>
>
> I will forward this advice to my managers :)
>
> https://codereview.chromium.org/12385071/

[Mostyn Bramley-Moore, 2013-03-04 21:53:57]

On 2013/03/04 20:48:47, Nico wrote:
> On Mon, Mar 4, 2013 at 11:35 AM,  <mailto:mostynb@opera.com> wrote:
> > On 2013/03/04 09:43:50, Nico wrote:
> >
> >> On Mon, Mar 4, 2013 at 10:37 AM,  <mailto:mostynb@opera.com> wrote:
> >> > On 2013/03/04 08:55:00, Nico wrote:
> >> >>
> >> >> How hard is it for you to maintain this bit out of tree?
> >> >
> >> >
> >> > Which bit- power_save_blocker_empty.cc ?  Or this patchset in general?
> >
> >
> >> This patch set in general. It makes our gyp files slightly more
> >> confusing. (I believe our stance is that we generally accept patches
> >> if they make the code better structured or don't add overhead to other
> >> devs.)
> >
> >
> > I think I can refactor this to minimise the added complexity if that helps.
> > Some possibilities:
> > * minimise this patch so that use_dbus only affects the content API (perhaps
> > rename it to content_api_dbus or some other better name)
> > * instead of checking for OS==linux before including dbus code, check for
> > use_dbus==1 (aside: is there a way to prevent use_dbus==1 and OS==win from
> > being
> > set simultaneously?)
> > * add a new OS type (eg "linux_embedded")

Do you have any preferences for these options (or any other suggestions)?  

> >> What's your game plan in general? Do you want to upstream all the
> >> opera tweaks you have? Just the obvious ones? Are you going to have
> >> bots that test your configuration? Since you asked above: You'll
> >> probably have to have your own build waterfall for your builds (?).
> >
> >
> > We're starting out by cherry-picking some small patches that we think have a
> > chance of being accepted.  This is the largest patch we have submitted so
> > far,
> > and as you can see we still have a lot to learn.
> >
> > We have our own internal build+testing infrastructure to confirm that our
> > local
> > patches (including this one) work- is that sufficient or do you need
> > something
> > affiliated with the official buildbot setup?
> 
> It doesn't have to be official, but public might help.
> 
> As a concrete example: Since our gyp files are suffering a bit from
> "dead through a thousand cuts", I (and others) go through
> build/common.gypi every now and then and look for stuff that doesn't
> seem to be used. If we find something, we try deleting it, and if
> trybots are happy we land it. I could see someone coming up with a
> revert of this CL here and landing it with a comment like "remove
> unneeded complexity". Nothing will stop that person at the moment.
> 
> I think for freebsd gyp build stuff we just say "this is minor, so
> we'll let it in the tree, but we don't promise that we don't
> accidentally break it, and if we do it's up to you to get it running
> again". I would accept this CL under the same premise.

Personally, I would love to get some automated builders setup with our
configuration(s) that you could use if you wish.  Let's continue this discussion
privately and see what can be arranged.  

But if that doesn't turn out to be possible, then working under the "it's up to
us to keep our config working" premise we would at least reduce the time we need
to spend resolving merge conflicts when taking in new chromium code.  

> You probably have more CLs like this lined up. Can you give me a
> preview of how many more of these CLs / lines of gyp changes you
> intend to upstream? How many dependencies do you need to disable?

It depends somewhat on your preferences for the best way to introduce a new
embedded linux configuration- either a set of library-specific settings, or a
single higher-level "embedded linux" setting that is reused for multiple things.
 If library-specific settings are preferred: one for xdg and maybe another 2-3
libraries that have slipped my mind right now.  And then we will have to look at
the local patches we have left and see what you would find acceptable.  

I have already submitted a use_alsa setting (merged already), and use_udev (in
progress).  

We also have some cleanup work for existing settings, eg use_glib, use_x11,
use_nss (removing implicit assumptions like use_glib==0 means "not unix").

[darin (slow to review), 2013-03-05 06:35:19]

I think it would be helpful to get a view of the bigger picture.  It is easy to
approve these small patches one by one, but there's a risk that that can lead to
a more complicated system.  As Nico says, the current system is already quite
complicated.  It's not clear how many more slight variations are needed.

I think with a little understanding of the bigger picture, we can attempt to
chart a course that perhaps minimizes complexity for everyone.  Thanks!

[jam, 2013-03-05 17:48:21]

On 2013/03/05 06:35:19, darin wrote:
> I think it would be helpful to get a view of the bigger picture.  It is easy
to
> approve these small patches one by one, but there's a risk that that can lead
to
> a more complicated system.  As Nico says, the current system is already quite
> complicated.  It's not clear how many more slight variations are needed.
> 
> I think with a little understanding of the bigger picture, we can attempt to
> chart a course that perhaps minimizes complexity for everyone.  Thanks!

btw https://codereview.chromium.org/12374068/ is related as well, so let's
discuss both together and whatever is decided here can apply there

[Mostyn Bramley-Moore, 2013-11-13 22:59:17]

Obsoleted by https://codereview.chromium.org/59323007/ - closing this.