Move printing.* prefs from local state to profile

Move printing.* prefs from local state to profile

This CL moves the printing.* prefs to the profile where they arguably
should have resided in the first place. Unit tests are simplified by
this change as the prefs can now be checked without verifying that local
state exists. There is no user-visible change as the prefs are not
exposed via the UI.

BUG=137814
TEST=all unit_tests pass, all printing browser_tests pass


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

[bartfab (slow), 2012-07-18 09:58:46]

Hi everyone

Could you please review this CL? It touches a lot of files but actually
implements a rather straightforward change.

[Mattias Nissler (ping if slow), 2012-07-18 10:11:53]

Nice change. LGTM from the policy and prefs side, but please get approval from
people who know printing better than me.

https://chromiumcodereview.appspot.com/10808006/diff/1/chrome/browser/printin...
File chrome/browser/printing/print_preview_unit_test_base.cc (right):

https://chromiumcodereview.appspot.com/10808006/diff/1/chrome/browser/printin...
chrome/browser/printing/print_preview_unit_test_base.cc:19:
profile()->GetPrefs()->SetBoolean(prefs::kPrintPreviewDisabled, false);
Not sure whether it's still a good idea to keep this base class now that it does
provide almost no value :)

[willchan no longer on Chromium, 2012-07-18 13:54:40]

chrome/browser/profiles/ LGTM

[bartfab (slow), 2012-07-18 14:01:24]

https://chromiumcodereview.appspot.com/10808006/diff/1/chrome/browser/printin...
File chrome/browser/printing/print_preview_unit_test_base.cc (right):

https://chromiumcodereview.appspot.com/10808006/diff/1/chrome/browser/printin...
chrome/browser/printing/print_preview_unit_test_base.cc:19:
profile()->GetPrefs()->SetBoolean(prefs::kPrintPreviewDisabled, false);
I retired the class. Done.

[sky, 2012-07-18 15:29:30]

https://chromiumcodereview.appspot.com/10808006/diff/6001/chrome/chrome_tests...
File chrome/chrome_tests.gypi (left):

https://chromiumcodereview.appspot.com/10808006/diff/6001/chrome/chrome_tests...
chrome/chrome_tests.gypi:1479:
'browser/printing/print_preview_unit_test_base.cc',
It doesn't look like you removed these files and I don't see them listed in any
other gyp files. How come you removed them here?

[Albert Bodenhamer, 2012-07-18 17:00:10]

lgtm

Thanks for taking this on.

https://chromiumcodereview.appspot.com/10808006/diff/6001/chrome/browser/prof...
File chrome/browser/profiles/profile_impl.cc (right):

https://chromiumcodereview.appspot.com/10808006/diff/6001/chrome/browser/prof...
chrome/browser/profiles/profile_impl.cc:225:
prefs->RegisterBooleanPref(prefs::kPrintingEnabled,
PrintingEnabled is driven by device policy.  Does it make sense as a user pref?

[bartfab (slow), 2012-07-18 17:18:29]

https://chromiumcodereview.appspot.com/10808006/diff/6001/chrome/browser/prof...
File chrome/browser/profiles/profile_impl.cc (right):

https://chromiumcodereview.appspot.com/10808006/diff/6001/chrome/browser/prof...
chrome/browser/profiles/profile_impl.cc:225:
prefs->RegisterBooleanPref(prefs::kPrintingEnabled,
The pref exists as a vehicle for policy: Policy sets pref, pref influences
Chrome. The pref is not exposed to the user but it must exist in the background.
It did exist already but was registered per-user. Now it is registered
per-profile. This makes no practical difference right now but if we want to, we
can actually give users finer grained control in the far future: We could enable
and disable printing on a per-profile basis.

https://chromiumcodereview.appspot.com/10808006/diff/6001/chrome/chrome_tests...
File chrome/chrome_tests.gypi (left):

https://chromiumcodereview.appspot.com/10808006/diff/6001/chrome/chrome_tests...
chrome/chrome_tests.gypi:1479:
'browser/printing/print_preview_unit_test_base.cc',
This seems to be an issue with the code review tool: The files are shown as
modified and empty. In reality, they were completely deleted.

[Albert Bodenhamer, 2012-07-18 17:23:20]

still lgtm

https://chromiumcodereview.appspot.com/10808006/diff/6001/chrome/browser/prof...
File chrome/browser/profiles/profile_impl.cc (right):

https://chromiumcodereview.appspot.com/10808006/diff/6001/chrome/browser/prof...
chrome/browser/profiles/profile_impl.cc:225:
prefs->RegisterBooleanPref(prefs::kPrintingEnabled,
Sounds good.
On 2012/07/18 17:18:30, bartfab wrote:
> The pref exists as a vehicle for policy: Policy sets pref, pref influences
> Chrome. The pref is not exposed to the user but it must exist in the
background.
> It did exist already but was registered per-user. Now it is registered
> per-profile. This makes no practical difference right now but if we want to,
we
> can actually give users finer grained control in the far future: We could
enable
> and disable printing on a per-profile basis.

[sky, 2012-07-18 20:51:26]

LGTM - just make sure the files actually get deleted.

[bartfab (slow), 2012-07-19 18:14:48]

Hi Brett

Friendly review ping. I have four owners now, one more to go.

[brettw, 2012-07-23 17:42:00]

What part to do need me to review? When I see a CL with a bunch of reviewers and
there's no part that's obviously mine, I usually ignore it.

[bartfab (slow), 2012-07-24 09:53:26]

Brett:

The specific file you own is:

chrome/browser/tab_contents/render_view_context_menu.cc

Plus you are listed as a reviewer for src/chrome and there are a number of files
without more specific owners:

chrome/browser/browser_process_impl.cc
chrome/browser/chrome_content_browser_client.cc
chrome/chrome_tests.gypi
chrome/common/pref_names.cc
chrome/common/pref_names.h

[brettw, 2012-07-25 23:31:19]

above-mentioned files LGTM

[commit-bot: I haz the power, 2012-07-26 08:04:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bartfab@chromium.org/10808006/13027

[commit-bot: I haz the power, 2012-07-26 11:52:11]

Change committed as 148531