Added prefs ui for enabling/disabling background mode on windows.

Added prefs ui for enabling/disabling background mode on windows.
BUG=53173
TEST=none yet (will add tests when BackgroundModeManager handles prefs notifications)

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=57225

[John Gregg, 2010-08-24 16:40:19]

LGTM, just a couple minor comments.

http://codereview.chromium.org/3200007/diff/1/3
File chrome/app/generated_resources.grd (right):

http://codereview.chromium.org/3200007/diff/1/3#newcode5592
chrome/app/generated_resources.grd:5592: Allow installed apps to run in the
background and launch on system start
this may not be a question for you, but noticing we use both "applications" and
"apps" in these strings.  should we be consistent?

http://codereview.chromium.org/3200007/diff/1/7
File chrome/browser/views/options/advanced_contents_view.cc (right):

http://codereview.chromium.org/3200007/diff/1/7#newcode1293
chrome/browser/views/options/advanced_contents_view.cc:1293: // OptionsPageView
overrides:
I found this comment confusing given the class definition and had to go check
that AdvancedSection inherits from OptionsPageView.  I don't know if it's worth
clarifying, your call...

[edit: never mind, i see this is consistent in the file]

http://codereview.chromium.org/3200007/diff/1/7#newcode1327
chrome/browser/views/options/advanced_contents_view.cc:1327: if (source ==
learn_more_link_) {
you DCHECK above, probably should here too, it's the only link in the view...

http://codereview.chromium.org/3200007/diff/1/7#newcode1364
chrome/browser/views/options/advanced_contents_view.cc:1364:
enable_background_mode_.GetValue());
i guess this BooleanPrefMember is always kept up to date?  or GetValue() returns
the current?  in that case ok.

add { } though, since i think this counts as a multi-line if even if though it's
one statement.

[Andrew T Wilson (Slow), 2010-08-24 19:47:10]

http://codereview.chromium.org/3200007/diff/1/3
File chrome/app/generated_resources.grd (right):

http://codereview.chromium.org/3200007/diff/1/3#newcode5592
chrome/app/generated_resources.grd:5592: Allow installed apps to run in the
background and launch on system start
On 2010/08/24 16:40:19, John Gregg wrote:
> this may not be a question for you, but noticing we use both "applications"
and
> "apps" in these strings.  should we be consistent?

Great point. I clarified with the apps team, and now use "web apps" everywhere
(instead of "apps" or "applications").

http://codereview.chromium.org/3200007/diff/1/7
File chrome/browser/views/options/advanced_contents_view.cc (right):

http://codereview.chromium.org/3200007/diff/1/7#newcode1327
chrome/browser/views/options/advanced_contents_view.cc:1327: if (source ==
learn_more_link_) {
On 2010/08/24 16:40:19, John Gregg wrote:
> you DCHECK above, probably should here too, it's the only link in the view...

Done.

http://codereview.chromium.org/3200007/diff/1/7#newcode1364
chrome/browser/views/options/advanced_contents_view.cc:1364:
enable_background_mode_.GetValue());
On 2010/08/24 16:40:19, John Gregg wrote:
> i guess this BooleanPrefMember is always kept up to date?  or GetValue()
returns
> the current?  in that case ok.
> 
> add { } though, since i think this counts as a multi-line if even if though
it's
> one statement.

Done. Actually had this edit locally, but I guess I forgot to upload it.