Disable SettingsEnforcement when the machine is connected to a domain.

Disable SettingsEnforcement when the machine is connected to a domain.

This add ~0.18ms to startup on my machine; note that it's already called 4 times before me (the first call costing ~0.38m followed by 3 X 0.18ms).

See my analysis on http://codereview.chromium.org/140553005

It's not clear whether this can get worse in some scenarios, but I'm already calling this later than the current caller so...

I'm also keeping the result as a static to avoid calling this multiple times (e.g. when doing unloaded profile initialization we don't want to call this multiple times in a loop in a short time span).

And we have timing stats for Settings.FilterOnLoadTime which will let us know if there are any big outliers coming out of this CL.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=251500

[gab, 2014-02-07 22:11:37]

Erik PTAL.

@cpu: is my reasoning of how this affects startup under different configs not
completely broken?

[cpu_(ooo_6.6-7.5), 2014-02-10 20:10:45]

sounds reasonable.

[gab, 2014-02-11 21:09:07]

Thanks, ping erikwright

[gab, 2014-02-13 14:29:17]

Robert, can you please take a look given Erik's very busy on other CLs?

Thanks,
Gab

https://codereview.chromium.org/156693003/diff/270001/chrome/browser/prefs/pr...
File chrome/browser/prefs/pref_hash_filter.cc (right):

https://codereview.chromium.org/156693003/diff/270001/chrome/browser/prefs/pr...
chrome/browser/prefs/pref_hash_filter.cc:90: do_filtering =
base::win::IsEnrolledToDomain();
Do we want a UMA stat for this?

[robertshield, 2014-02-13 14:31:12]

https://codereview.chromium.org/156693003/diff/270001/chrome/browser/prefs/pr...
File chrome/browser/prefs/pref_hash_filter.cc (right):

https://codereview.chromium.org/156693003/diff/270001/chrome/browser/prefs/pr...
chrome/browser/prefs/pref_hash_filter.cc:90: do_filtering =
base::win::IsEnrolledToDomain();
shouldn't this have a ! in front of it?

Also, yes we want an UMA stat.

[erikwright (departed), 2014-02-13 14:34:04]

Other question:

Is the intent to disable all tracking, or just to disable protection?

If the intent is to disable all tracking, I would put this check in
chrome_pref_service_factory.cc and just not return a PrefHashStoreImpl.

If the intent is to disable just the protection, I would put it there too, but
use it to affect the configuration of the tracked preferences.

[gab, 2014-02-13 15:58:55]

Thanks for your comments.

@erik: I indeed originally intended to do it in chrome_pref_service_factory.cc;
not sure why I didn't go that way after all... but yea this is definitely
better.

We don't want to disable the PrefHashFilter altogether as we still want to keep
tracking/seeding those users.

Cheers!
Gab

[Alexei Svitkine (slow), 2014-02-14 15:52:54]

lgtm for histograms

[gab, 2014-02-14 18:43:31]

On 2014/02/14 15:52:54, Alexei Svitkine wrote:
> lgtm for histograms

Thanks, @robertshield ping :)!

[robertshield, 2014-02-14 20:19:04]

lgtm

[gab, 2014-02-14 21:02:59]

The CQ bit was checked by gab@chromium.org

[commit-bot: I haz the power, 2014-02-14 21:09:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gab@chromium.org/156693003/380001

[commit-bot: I haz the power, 2014-02-15 03:48:25]

Change committed as 251500