MD Settings: clarify "controlled by" logic

MD Settings: clarify "controlled by" logic

Before, we were using PrefService::Preference::IsExtensionControlled()
to determine if a pref was controlled by an extension. But, as far as I
can tell, that determines if a pref was *registered* by an extension.

What we want to know is whether an extension was installed (either by a
user or by policy) that overrides a function pref value. I re-used the
same code that /options/ used, which is a complicated mixture of enums,
manifest sniffing, and pref filtering. But it works. I've added a small
shim to re-use that from settings, based on pref name. It's not amazing,
but it'll work for now.

What next?

Well, this turns off extension puzzle piece icons and tooltips. This CL
also plumbs more information about whether an extension can be disabled
(a major shortcoming of the old UI, as disable buttons don't work in
some cases).

Next step: show a separate row-like UI.  See crbug.com/614265#c36.

R=stevenjb@chromium.org
BUG=614265
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/455f3cce3da21f8e677100b0c0abd8d35e24876e
Cr-Commit-Position: refs/heads/master@{#432704}

[Dan Beam, 2016-11-05 01:04:38]

Patchset #1 (id:1) has been deleted

[Dan Beam, 2016-11-05 01:12:21]

Patchset #1 (id:20001) has been deleted

[Dan Beam, 2016-11-05 01:12:42]

so, the code works for me locally (at least for proxy)

any sane recommendations on testing?

[stevenjb, 2016-11-09 18:03:30]

https://codereview.chromium.org/2479973002/diff/40001/chrome/common/extension...
File chrome/common/extensions/api/settings_private.idl (right):

https://codereview.chromium.org/2479973002/diff/40001/chrome/common/extension...
chrome/common/extensions/api/settings_private.idl:19: // could rename
"PolicySource" to "ControlledBy" or "EnforcementSource"?
First off, I apologize for half completing this and not documenting it well (by
which I mean, at all).

It's been a while since I looked at this, but my head is at least partly in it
since I've been dealing with similar issues for the proxy configuration (which
is its own special hell).

There are a few approaches we could take here. One that comes to mind is to
rename this to EnforcementSource, rename PolicyEnforcement to EnforcementType
and include { POLICY_ENFORCED, POLICY_RECOMMENDED, EXTENSION_ENFORCED, NONE }.

Let's chat Wed and maybe put together a quick design / brainstorming doc.

[stevenjb, 2016-11-09 20:29:32]

On 2016/11/09 18:03:30, stevenjb wrote:
>
https://codereview.chromium.org/2479973002/diff/40001/chrome/common/extension...
> File chrome/common/extensions/api/settings_private.idl (right):
> 
>
https://codereview.chromium.org/2479973002/diff/40001/chrome/common/extension...
> chrome/common/extensions/api/settings_private.idl:19: // could rename
> "PolicySource" to "ControlledBy" or "EnforcementSource"?
> First off, I apologize for half completing this and not documenting it well
(by
> which I mean, at all).
> 
> It's been a while since I looked at this, but my head is at least partly in it
> since I've been dealing with similar issues for the proxy configuration (which
> is its own special hell).
> 
> There are a few approaches we could take here. One that comes to mind is to
> rename this to EnforcementSource, rename PolicyEnforcement to EnforcementType
> and include { POLICY_ENFORCED, POLICY_RECOMMENDED, EXTENSION_ENFORCED, NONE }.
> 
> Let's chat Wed and maybe put together a quick design / brainstorming doc.

Following up on this, I spoke with Devlin, and there is the case of a supervised
user or a secondary user that may not be able to disable or uninstall an
extension that was not installed by policy.

So, I guess the thing to do is to leave PolicySource as-is (maybe rename it
separately, or maybe just add a comment), and then either abuse the readOnly
boolean with a comment added to the idl or (and I think i favor this) add a new
boolean like extensionCanBeDisabled to PrefObject.

The code to correctly check whether or not an extension can be disabled is here:
https://cs.chromium.org/chromium/src/extensions/browser/management_policy.cc?...

[Dan Beam, 2016-11-12 00:30:56]

Description was changed from

==========
MD Settings: fix extension-controlled prefs logic

Before, we were using PrefService::Preference::IsExtensionControlled()
to determine if a pref was controlled by an extension. But, as far as I
can tell, that determines if a pref was *registered* by an extension.

What we want to know is whether an extension was installed (either by a
user or by policy) that overrides a function pref value. I re-used the
same code that /options/ used, which is a complicated mixture of enums,
manifest sniffing, and pref filtering. But it works. I've added a small
shim to re-use that from settings, based on pref name. It's not amazing,
but it'll work for now.

What next?

Well, this results in some things showing (unactionable) tooltips about
being controlled by an extension. I'd like to turn off and disable icons
and tooltips when a pref is controlled by an extension, and use the data
in prefs to instead show a new row item like in crbug.com/614265#c36.

R=stevenjb@chromium.org
BUG=614265
==========

to

==========
MD Settings: fix extension-controlled prefs logic

Before, we were using PrefService::Preference::IsExtensionControlled()
to determine if a pref was controlled by an extension. But, as far as I
can tell, that determines if a pref was *registered* by an extension.

What we want to know is whether an extension was installed (either by a
user or by policy) that overrides a function pref value. I re-used the
same code that /options/ used, which is a complicated mixture of enums,
manifest sniffing, and pref filtering. But it works. I've added a small
shim to re-use that from settings, based on pref name. It's not amazing,
but it'll work for now.

What next?

Well, this results in some things showing (unactionable) tooltips about
being controlled by an extension. I'd like to turn off and disable icons
and tooltips when a pref is controlled by an extension, and use the data
in prefs to instead show a new row item like in crbug.com/614265#c36.

R=stevenjb@chromium.org
BUG=614265
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Dan Beam, 2016-11-12 00:33:05]

Patchset #2 (id:60001) has been deleted

[Dan Beam, 2016-11-12 00:37:14]

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

[commit-bot: I haz the power, 2016-11-12 00:37:56]

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

[Dan Beam, 2016-11-12 00:40:57]

Description was changed from

==========
MD Settings: fix extension-controlled prefs logic

Before, we were using PrefService::Preference::IsExtensionControlled()
to determine if a pref was controlled by an extension. But, as far as I
can tell, that determines if a pref was *registered* by an extension.

What we want to know is whether an extension was installed (either by a
user or by policy) that overrides a function pref value. I re-used the
same code that /options/ used, which is a complicated mixture of enums,
manifest sniffing, and pref filtering. But it works. I've added a small
shim to re-use that from settings, based on pref name. It's not amazing,
but it'll work for now.

What next?

Well, this results in some things showing (unactionable) tooltips about
being controlled by an extension. I'd like to turn off and disable icons
and tooltips when a pref is controlled by an extension, and use the data
in prefs to instead show a new row item like in crbug.com/614265#c36.

R=stevenjb@chromium.org
BUG=614265
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: clarify "controlled by" logic

Before, we were using PrefService::Preference::IsExtensionControlled()
to determine if a pref was controlled by an extension. But, as far as I
can tell, that determines if a pref was *registered* by an extension.

What we want to know is whether an extension was installed (either by a
user or by policy) that overrides a function pref value. I re-used the
same code that /options/ used, which is a complicated mixture of enums,
manifest sniffing, and pref filtering. But it works. I've added a small
shim to re-use that from settings, based on pref name. It's not amazing,
but it'll work for now.

What next?

Well, this turns off extension puzzle piece icons and tooltips. This CL
also plumbs more information about whether an extension can be disabled
(a major shortcoming of the old UI, as disable buttons don't work in
some cases).

Next step: show a separate row-like UI.  See crbug.com/614265#c36.

R=stevenjb@chromium.org
BUG=614265
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Dan Beam, 2016-11-12 00:45:15]

alright, all aboard the crazy train

[stevenjb, 2016-11-12 01:31:46]

Thanks for digging into this. I clearly made some incorrect assumptions about
how prefs were extension controlled.

https://codereview.chromium.org/2479973002/diff/100001/chrome/browser/extensi...
File chrome/browser/extensions/api/settings_private/prefs_util.cc (right):

https://codereview.chromium.org/2479973002/diff/100001/chrome/browser/extensi...
chrome/browser/extensions/api/settings_private/prefs_util.cc:672: const
settings_private::PrefObject& pref_object) {
Can we add a comment here, similar to what's in the CL description? It was not
at all obvious that GetExtensionControllingPref() does not do what the name
implies. Clearly I should have investigated that more thoroughly, but let's at
least give the next person who comes along some breadcrumbs.

https://codereview.chromium.org/2479973002/diff/100001/ui/webui/resources/cr_...
File ui/webui/resources/cr_elements/policy/cr_policy_indicator_behavior.js
(left):

https://codereview.chromium.org/2479973002/diff/100001/ui/webui/resources/cr_...
ui/webui/resources/cr_elements/policy/cr_policy_indicator_behavior.js:54: break;
I'm using this in the networking proxy code (for now anyway):
https://codereview.chromium.org/2467123003/

Can we leave this in for now? We can fix up that UI later and eliminate this
later.

https://codereview.chromium.org/2479973002/diff/100001/ui/webui/resources/cr_...
File ui/webui/resources/cr_elements/policy/cr_policy_pref_behavior.js (left):

https://codereview.chromium.org/2479973002/diff/100001/ui/webui/resources/cr_...
ui/webui/resources/cr_elements/policy/cr_policy_pref_behavior.js:40: return
CrPolicyIndicatorType.EXTENSION;
Ditto.

[commit-bot: I haz the power, 2016-11-12 01:37:27]

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

[commit-bot: I haz the power, 2016-11-12 01:37:27]

Dry run: This issue passed the CQ dry run.

[Dan Beam, 2016-11-15 05:38:39]

https://codereview.chromium.org/2479973002/diff/100001/chrome/browser/extensi...
File chrome/browser/extensions/api/settings_private/prefs_util.cc (right):

https://codereview.chromium.org/2479973002/diff/100001/chrome/browser/extensi...
chrome/browser/extensions/api/settings_private/prefs_util.cc:672: const
settings_private::PrefObject& pref_object) {
On 2016/11/12 01:31:46, stevenjb wrote:
> Can we add a comment here, similar to what's in the CL description? It was not
> at all obvious that GetExtensionControllingPref() does not do what the name
> implies. Clearly I should have investigated that more thoroughly, but let's at
> least give the next person who comes along some breadcrumbs.

Done.

https://codereview.chromium.org/2479973002/diff/100001/ui/webui/resources/cr_...
File ui/webui/resources/cr_elements/policy/cr_policy_indicator_behavior.js
(left):

https://codereview.chromium.org/2479973002/diff/100001/ui/webui/resources/cr_...
ui/webui/resources/cr_elements/policy/cr_policy_indicator_behavior.js:54: break;
On 2016/11/12 01:31:46, stevenjb wrote:
> I'm using this in the networking proxy code (for now anyway):
> https://codereview.chromium.org/2467123003/
> 
> Can we leave this in for now? We can fix up that UI later and eliminate this
> later.

we talked about this over chat.  you said:

"""
So yeah, let's go ahead and remove it from there and create an element that
matches the mocks.
[snip]
I'll update the per-network proxy settings with that once we have other
examples.
"""

creating the UI is the next step

https://codereview.chromium.org/2479973002/diff/100001/ui/webui/resources/cr_...
File ui/webui/resources/cr_elements/policy/cr_policy_pref_behavior.js (left):

https://codereview.chromium.org/2479973002/diff/100001/ui/webui/resources/cr_...
ui/webui/resources/cr_elements/policy/cr_policy_pref_behavior.js:40: return
CrPolicyIndicatorType.EXTENSION;
On 2016/11/12 01:31:46, stevenjb wrote:
> Ditto.

Datto.

[Dan Beam, 2016-11-15 05:40:12]

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

[commit-bot: I haz the power, 2016-11-15 05:40:28]

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-11-15 06:52:55]

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

[commit-bot: I haz the power, 2016-11-15 06:52:56]

Dry run: This issue passed the CQ dry run.

[Dan Beam, 2016-11-15 19:27:08]

dbeam@chromium.org changed reviewers:
+ asargent@chromium.org

[Dan Beam, 2016-11-15 19:27:08]

+asargent@ for chrome/*/extensions

[stevenjb, 2016-11-15 22:11:19]

lgtm

https://codereview.chromium.org/2479973002/diff/100001/ui/webui/resources/cr_...
File ui/webui/resources/cr_elements/policy/cr_policy_indicator_behavior.js
(left):

https://codereview.chromium.org/2479973002/diff/100001/ui/webui/resources/cr_...
ui/webui/resources/cr_elements/policy/cr_policy_indicator_behavior.js:54: break;
On 2016/11/15 05:38:39, Dan Beam wrote:
> On 2016/11/12 01:31:46, stevenjb wrote:
> > I'm using this in the networking proxy code (for now anyway):
> > https://codereview.chromium.org/2467123003/
> > 
> > Can we leave this in for now? We can fix up that UI later and eliminate this
> > later.
> 
> we talked about this over chat.  you said:
> 
> """
> So yeah, let's go ahead and remove it from there and create an element that
> matches the mocks.
> [snip]
> I'll update the per-network proxy settings with that once we have other
> examples.
> """
> 
> creating the UI is the next step

Acknowledged.

https://codereview.chromium.org/2479973002/diff/100001/ui/webui/resources/cr_...
File ui/webui/resources/cr_elements/policy/cr_policy_pref_behavior.js (left):

https://codereview.chromium.org/2479973002/diff/100001/ui/webui/resources/cr_...
ui/webui/resources/cr_elements/policy/cr_policy_pref_behavior.js:40: return
CrPolicyIndicatorType.EXTENSION;
On 2016/11/15 05:38:39, Dan Beam wrote:
> On 2016/11/12 01:31:46, stevenjb wrote:
> > Ditto.
> 
> Datto.

Acknowledged.

[Dan Beam, 2016-11-16 01:55:00]

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

[commit-bot: I haz the power, 2016-11-16 01:55:21]

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-11-16 04:57:33]

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

[commit-bot: I haz the power, 2016-11-16 04:57:34]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[Dan Beam, 2016-11-16 19:28:13]

ping asargent@

[asargent_no_longer_on_chrome, 2016-11-16 20:58:40]

On 2016/11/16 19:28:13, Dan Beam wrote:
> ping asargent@

Sorry for delay, will get to this soon

[asargent_no_longer_on_chrome, 2016-11-16 21:05:02]

chrome/*/extensions/* lgtm

[Dan Beam, 2016-11-16 21:28:40]

The CQ bit was checked by dbeam@chromium.org

[Dan Beam, 2016-11-16 21:28:40]

The patchset sent to the CQ was uploaded after l-g-t-m from
stevenjb@chromium.org
Link to the patchset: https://codereview.chromium.org/2479973002/#ps140001
(title: "exclude associated prefs")

[commit-bot: I haz the power, 2016-11-16 21:29:46]

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

[Dan Beam, 2016-11-16 22:33:07]

Description was changed from

==========
MD Settings: clarify "controlled by" logic

Before, we were using PrefService::Preference::IsExtensionControlled()
to determine if a pref was controlled by an extension. But, as far as I
can tell, that determines if a pref was *registered* by an extension.

What we want to know is whether an extension was installed (either by a
user or by policy) that overrides a function pref value. I re-used the
same code that /options/ used, which is a complicated mixture of enums,
manifest sniffing, and pref filtering. But it works. I've added a small
shim to re-use that from settings, based on pref name. It's not amazing,
but it'll work for now.

What next?

Well, this turns off extension puzzle piece icons and tooltips. This CL
also plumbs more information about whether an extension can be disabled
(a major shortcoming of the old UI, as disable buttons don't work in
some cases).

Next step: show a separate row-like UI.  See crbug.com/614265#c36.

R=stevenjb@chromium.org
BUG=614265
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: clarify "controlled by" logic

Before, we were using PrefService::Preference::IsExtensionControlled()
to determine if a pref was controlled by an extension. But, as far as I
can tell, that determines if a pref was *registered* by an extension.

What we want to know is whether an extension was installed (either by a
user or by policy) that overrides a function pref value. I re-used the
same code that /options/ used, which is a complicated mixture of enums,
manifest sniffing, and pref filtering. But it works. I've added a small
shim to re-use that from settings, based on pref name. It's not amazing,
but it'll work for now.

What next?

Well, this turns off extension puzzle piece icons and tooltips. This CL
also plumbs more information about whether an extension can be disabled
(a major shortcoming of the old UI, as disable buttons don't work in
some cases).

Next step: show a separate row-like UI.  See crbug.com/614265#c36.

R=stevenjb@chromium.org
BUG=614265
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
NOTRY=true
==========

[Dan Beam, 2016-11-16 22:33:14]

The CQ bit was unchecked by dbeam@chromium.org

[Dan Beam, 2016-11-16 22:33:21]

The CQ bit was checked by dbeam@chromium.org

[commit-bot: I haz the power, 2016-11-16 22:35:08]

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-11-16 22:53:59]

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

[commit-bot: I haz the power, 2016-11-16 22:54:03]

Failed to apply patch for
chrome/browser/extensions/api/settings_private/prefs_util.cc:
While running git apply --index -p1;
  error: patch failed:
chrome/browser/extensions/api/settings_private/prefs_util.cc:19
  error: chrome/browser/extensions/api/settings_private/prefs_util.cc: patch
does not apply

Patch:       chrome/browser/extensions/api/settings_private/prefs_util.cc
Index: chrome/browser/extensions/api/settings_private/prefs_util.cc
diff --git a/chrome/browser/extensions/api/settings_private/prefs_util.cc
b/chrome/browser/extensions/api/settings_private/prefs_util.cc
index
dd73775462784ffb9ffb504d977687d2ebba43c7..ede901aa08a23515744c6ae8fae3c2e7b2bc1e0d
100644
--- a/chrome/browser/extensions/api/settings_private/prefs_util.cc
+++ b/chrome/browser/extensions/api/settings_private/prefs_util.cc
@@ -7,6 +7,8 @@
 #include "build/build_config.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/extensions/chrome_extension_function.h"
+#include "chrome/browser/extensions/settings_api_helpers.h"
+#include "chrome/browser/prefs/session_startup_pref.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/pref_names.h"
 #include "components/autofill/core/common/autofill_pref_names.h"
@@ -19,6 +21,7 @@
 #include "components/proxy_config/proxy_config_pref_names.h"
 #include "components/proxy_config/proxy_config_pref_names.h"
 #include "components/safe_browsing_db/safe_browsing_prefs.h"
+#include "components/search_engines/search_engines_pref_names.h"
 #include "components/spellcheck/browser/pref_names.h"
 #include "components/translate/core/browser/translate_prefs.h"
 #include "components/translate/core/common/translate_pref_names.h"
@@ -26,6 +29,8 @@
 #include "extensions/browser/extension_pref_value_map.h"
 #include "extensions/browser/extension_pref_value_map_factory.h"
 #include "extensions/browser/extension_registry.h"
+#include "extensions/browser/extension_system.h"
+#include "extensions/browser/management_policy.h"
 #include "extensions/common/extension.h"
 
 #if defined(OS_CHROMEOS)
@@ -93,6 +98,10 @@ const PrefsUtil::TypedPrefMap&
PrefsUtil::GetWhitelistedKeys() {
   (*s_whitelist)[::prefs::kUsesSystemTheme] =
       settings_private::PrefType::PREF_TYPE_BOOLEAN;
 #endif
+  (*s_whitelist)[::prefs::kHomePage] =
+      settings_private::PrefType::PREF_TYPE_URL;
+  (*s_whitelist)[::prefs::kHomePageIsNewTabPage] =
+      settings_private::PrefType::PREF_TYPE_BOOLEAN;
   (*s_whitelist)[::prefs::kWebKitDefaultFixedFontSize] =
       settings_private::PrefType::PREF_TYPE_NUMBER;
   (*s_whitelist)[::prefs::kWebKitDefaultFontSize] =
@@ -110,6 +119,12 @@ const PrefsUtil::TypedPrefMap&
PrefsUtil::GetWhitelistedKeys() {
   (*s_whitelist)[::prefs::kDefaultCharset] =
       settings_private::PrefType::PREF_TYPE_STRING;
 
+  // On startup.
+  (*s_whitelist)[::prefs::kRestoreOnStartup] =
+      settings_private::PrefType::PREF_TYPE_NUMBER;
+  (*s_whitelist)[::prefs::kURLsToRestoreOnStartup] =
+      settings_private::PrefType::PREF_TYPE_LIST;
+
   // Downloads settings.
   (*s_whitelist)[::prefs::kDownloadDefaultDirectory] =
       settings_private::PrefType::PREF_TYPE_STRING;
@@ -122,13 +137,9 @@ const PrefsUtil::TypedPrefMap&
PrefsUtil::GetWhitelistedKeys() {
   (*s_whitelist)[::prefs::kLocalDiscoveryNotificationsEnabled] =
       settings_private::PrefType::PREF_TYPE_BOOLEAN;
 
-  // Miscelaneous. TODO(stevenjb): categorize.
+  // Miscellaneous. TODO(stevenjb): categorize.
   (*s_whitelist)[::prefs::kEnableDoNotTrack] =
       settings_private::PrefType::PREF_TYPE_BOOLEAN;
-  (*s_whitelist)[::prefs::kHomePage] =
-      settings_private::PrefType::PREF_TYPE_URL;
-  (*s_whitelist)[::prefs::kHomePageIsNewTabPage] =
-      settings_private::PrefType::PREF_TYPE_BOOLEAN;
   (*s_whitelist)[::prefs::kApplicationLocale] =
       settings_private::PrefType::PREF_TYPE_STRING;
   (*s_whitelist)[::prefs::kNetworkPredictionOptions] =
@@ -143,10 +154,6 @@ const PrefsUtil::TypedPrefMap&
PrefsUtil::GetWhitelistedKeys() {
       settings_private::PrefType::PREF_TYPE_BOOLEAN;
   (*s_whitelist)[::prefs::kSearchSuggestEnabled] =
       settings_private::PrefType::PREF_TYPE_BOOLEAN;
-  (*s_whitelist)[::prefs::kRestoreOnStartup] =
-      settings_private::PrefType::PREF_TYPE_NUMBER;
-  (*s_whitelist)[::prefs::kURLsToRestoreOnStartup] =
-      settings_private::PrefType::PREF_TYPE_LIST;
   (*s_whitelist)[spellcheck::prefs::kSpellCheckDictionaries] =
       settings_private::PrefType::PREF_TYPE_LIST;
   (*s_whitelist)[spellcheck::prefs::kSpellCheckUseSpellingService] =
@@ -381,40 +388,42 @@ std::unique_ptr<settings_private::PrefObject>
PrefsUtil::GetPref(
 
 #if defined(OS_CHROMEOS)
   if (IsPrefPrimaryUserControlled(name)) {
-    pref_object->policy_source =
-        settings_private::PolicySource::POLICY_SOURCE_PRIMARY_USER;
-    pref_object->policy_enforcement =
-        settings_private::PolicyEnforcement::POLICY_ENFORCEMENT_ENFORCED;
-    pref_object->policy_source_name.reset(
+    pref_object->controlled_by =
+        settings_private::ControlledBy::CONTROLLED_BY_PRIMARY_USER;
+    pref_object->enforcement =
+        settings_private::Enforcement::ENFORCEMENT_ENFORCED;
+    pref_object->controlled_by_name.reset(
         new std::string(user_manager::UserManager::Get()
                             ->GetPrimaryUser()
                             ->GetAccountId()
                             .GetUserEmail()));
     return pref_object;
   }
+
   if (IsPrefEnterpriseManaged(name)) {
     // Enterprise managed prefs are treated the same as device policy
restricted
     // prefs in the UI.
-    pref_object->policy_source =
-        settings_private::PolicySource::POLICY_SOURCE_DEVICE_POLICY;
-    pref_object->policy_enforcement =
-        settings_private::PolicyEnforcement::POLICY_ENFORCEMENT_ENFORCED;
+    pref_object->controlled_by =
+        settings_private::ControlledBy::CONTROLLED_BY_DEVICE_POLICY;
+    pref_object->enforcement =
+        settings_private::Enforcement::ENFORCEMENT_ENFORCED;
     return pref_object;
   }
 #endif
 
   if (pref && pref->IsManaged()) {
-    pref_object->policy_source =
-        settings_private::PolicySource::POLICY_SOURCE_USER_POLICY;
-    pref_object->policy_enforcement =
-        settings_private::PolicyEnforcement::POLICY_ENFORCEMENT_ENFORCED;
+    pref_object->controlled_by =
+        settings_private::ControlledBy::CONTROLLED_BY_USER_POLICY;
+    pref_object->enforcement =
+        settings_private::Enforcement::ENFORCEMENT_ENFORCED;
     return pref_object;
   }
+
   if (pref && pref->IsRecommended()) {
-    pref_object->policy_source =
-        settings_private::PolicySource::POLICY_SOURCE_USER_POLICY;
-    pref_object->policy_enforcement =
-        settings_private::PolicyEnforcement::POLICY_ENFORCEMENT_RECOMMENDED;
+    pref_object->controlled_by =
+        settings_private::ControlledBy::CONTROLLED_BY_USER_POLICY;
+    pref_object->enforcement =
+        settings_private::Enforcement::ENFORCEMENT_RECOMMENDED;
     pref_object->recommended_value.reset(
         pref->GetRecommendedValue()->DeepCopy());
     return pref_object;
@@ -426,38 +435,32 @@ std::unique_ptr<settings_private::PrefObject>
PrefsUtil::GetPref(
     // device policy there is no "owner". (In the unlikely case that both
     // situations apply, either badge is potentially relevant, so the order
     // is somewhat arbitrary).
-    pref_object->policy_source =
-        settings_private::PolicySource::POLICY_SOURCE_OWNER;
-    pref_object->policy_enforcement =
-        settings_private::PolicyEnforcement::POLICY_ENFORCEMENT_ENFORCED;
-    pref_object->policy_source_name.reset(new std::string(
+    pref_object->controlled_by =
+        settings_private::ControlledBy::CONTROLLED_BY_OWNER;
+    pref_object->enforcement =
+        settings_private::Enforcement::ENFORCEMENT_ENFORCED;
+    pref_object->controlled_by_name.reset(new std::string(
         user_manager::UserManager::Get()->GetOwnerAccountId().GetUserEmail()));
     return pref_object;
   }
 #endif
 
-  if (pref && pref->IsExtensionControlled()) {
-    std::string extension_id =
-        ExtensionPrefValueMapFactory::GetForBrowserContext(profile_)
-            ->GetExtensionControllingPref(pref->name());
-    const Extension* extension = ExtensionRegistry::Get(profile_)->
-        GetExtensionById(extension_id, ExtensionRegistry::ENABLED);
-    if (extension) {
-      pref_object->policy_source =
-          settings_private::PolicySource::POLICY_SOURCE_EXTENSION;
-      pref_object->policy_enforcement =
-          settings_private::PolicyEnforcement::POLICY_ENFORCEMENT_ENFORCED;
-      pref_object->extension_id.reset(new std::string(extension_id));
-      pref_object->policy_source_name.reset(new
std::string(extension->name()));
-      return pref_object;
-    }
-  }
-  if (pref && (!pref->IsUserModifiable() || IsPrefSupervisorControlled(name)))
{
-    // TODO(stevenjb): Investigate whether either of these should be badged.
-    pref_object->read_only.reset(new bool(true));
+  const Extension* extension = GetExtensionControllingPref(*pref_object);
+  if (extension) {
+    pref_object->controlled_by =
+        settings_private::ControlledBy::CONTROLLED_BY_EXTENSION;
+    pref_object->enforcement =
+        settings_private::Enforcement::ENFORCEMENT_ENFORCED;
+    pref_object->extension_id.reset(new std::string(extension->id()));
+    pref_object->controlled_by_name.reset(new std::string(extension->name()));
+    bool can_be_disabled = !ExtensionSystem::Get(profile_)->management_policy()
+        ->MustRemainEnabled(extension, nullptr);
+    pref_object->extension_can_be_disabled.reset(new bool(can_be_disabled));
     return pref_object;
   }
 
+  // TODO(dbeam): surface !IsUserModifiable or IsPrefSupervisorControlled?
+
   return pref_object;
 }
 
@@ -679,4 +682,19 @@ bool PrefsUtil::IsCrosSetting(const std::string& pref_name)
{
 #endif
 }
 
+const Extension* PrefsUtil::GetExtensionControllingPref(
+    const set…
(message too large)

[Dan Beam, 2016-11-16 23:17:07]

Patchset #6 (id:160001) has been deleted

[Dan Beam, 2016-11-16 23:17:14]

The CQ bit was checked by dbeam@chromium.org

[Dan Beam, 2016-11-16 23:17:14]

The patchset sent to the CQ was uploaded after l-g-t-m from
stevenjb@chromium.org, asargent@chromium.org
Link to the patchset: https://codereview.chromium.org/2479973002/#ps180001
(title: "merge")

[Dan Beam, 2016-11-16 23:17:19]

The CQ bit was unchecked by dbeam@chromium.org

[Dan Beam, 2016-11-16 23:17:27]

Description was changed from

==========
MD Settings: clarify "controlled by" logic

Before, we were using PrefService::Preference::IsExtensionControlled()
to determine if a pref was controlled by an extension. But, as far as I
can tell, that determines if a pref was *registered* by an extension.

What we want to know is whether an extension was installed (either by a
user or by policy) that overrides a function pref value. I re-used the
same code that /options/ used, which is a complicated mixture of enums,
manifest sniffing, and pref filtering. But it works. I've added a small
shim to re-use that from settings, based on pref name. It's not amazing,
but it'll work for now.

What next?

Well, this turns off extension puzzle piece icons and tooltips. This CL
also plumbs more information about whether an extension can be disabled
(a major shortcoming of the old UI, as disable buttons don't work in
some cases).

Next step: show a separate row-like UI.  See crbug.com/614265#c36.

R=stevenjb@chromium.org
BUG=614265
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
NOTRY=true
==========

to

==========
MD Settings: clarify "controlled by" logic

Before, we were using PrefService::Preference::IsExtensionControlled()
to determine if a pref was controlled by an extension. But, as far as I
can tell, that determines if a pref was *registered* by an extension.

What we want to know is whether an extension was installed (either by a
user or by policy) that overrides a function pref value. I re-used the
same code that /options/ used, which is a complicated mixture of enums,
manifest sniffing, and pref filtering. But it works. I've added a small
shim to re-use that from settings, based on pref name. It's not amazing,
but it'll work for now.

What next?

Well, this turns off extension puzzle piece icons and tooltips. This CL
also plumbs more information about whether an extension can be disabled
(a major shortcoming of the old UI, as disable buttons don't work in
some cases).

Next step: show a separate row-like UI.  See crbug.com/614265#c36.

R=stevenjb@chromium.org
BUG=614265
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Dan Beam, 2016-11-16 23:17:32]

The CQ bit was checked by dbeam@chromium.org

[commit-bot: I haz the power, 2016-11-16 23:18:51]

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-11-17 00:35:17]

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

[commit-bot: I haz the power, 2016-11-17 00:35:18]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Dan Beam, 2016-11-17 01:04:08]

The CQ bit was checked by dbeam@chromium.org

[commit-bot: I haz the power, 2016-11-17 01:06:01]

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-11-17 02:09:43]

Description was changed from

==========
MD Settings: clarify "controlled by" logic

Before, we were using PrefService::Preference::IsExtensionControlled()
to determine if a pref was controlled by an extension. But, as far as I
can tell, that determines if a pref was *registered* by an extension.

What we want to know is whether an extension was installed (either by a
user or by policy) that overrides a function pref value. I re-used the
same code that /options/ used, which is a complicated mixture of enums,
manifest sniffing, and pref filtering. But it works. I've added a small
shim to re-use that from settings, based on pref name. It's not amazing,
but it'll work for now.

What next?

Well, this turns off extension puzzle piece icons and tooltips. This CL
also plumbs more information about whether an extension can be disabled
(a major shortcoming of the old UI, as disable buttons don't work in
some cases).

Next step: show a separate row-like UI.  See crbug.com/614265#c36.

R=stevenjb@chromium.org
BUG=614265
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: clarify "controlled by" logic

Before, we were using PrefService::Preference::IsExtensionControlled()
to determine if a pref was controlled by an extension. But, as far as I
can tell, that determines if a pref was *registered* by an extension.

What we want to know is whether an extension was installed (either by a
user or by policy) that overrides a function pref value. I re-used the
same code that /options/ used, which is a complicated mixture of enums,
manifest sniffing, and pref filtering. But it works. I've added a small
shim to re-use that from settings, based on pref name. It's not amazing,
but it'll work for now.

What next?

Well, this turns off extension puzzle piece icons and tooltips. This CL
also plumbs more information about whether an extension can be disabled
(a major shortcoming of the old UI, as disable buttons don't work in
some cases).

Next step: show a separate row-like UI.  See crbug.com/614265#c36.

R=stevenjb@chromium.org
BUG=614265
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[commit-bot: I haz the power, 2016-11-17 02:09:44]

Committed patchset #6 (id:180001)

[commit-bot: I haz the power, 2016-11-17 02:13:19]

Description was changed from

==========
MD Settings: clarify "controlled by" logic

Before, we were using PrefService::Preference::IsExtensionControlled()
to determine if a pref was controlled by an extension. But, as far as I
can tell, that determines if a pref was *registered* by an extension.

What we want to know is whether an extension was installed (either by a
user or by policy) that overrides a function pref value. I re-used the
same code that /options/ used, which is a complicated mixture of enums,
manifest sniffing, and pref filtering. But it works. I've added a small
shim to re-use that from settings, based on pref name. It's not amazing,
but it'll work for now.

What next?

Well, this turns off extension puzzle piece icons and tooltips. This CL
also plumbs more information about whether an extension can be disabled
(a major shortcoming of the old UI, as disable buttons don't work in
some cases).

Next step: show a separate row-like UI.  See crbug.com/614265#c36.

R=stevenjb@chromium.org
BUG=614265
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
MD Settings: clarify "controlled by" logic

Before, we were using PrefService::Preference::IsExtensionControlled()
to determine if a pref was controlled by an extension. But, as far as I
can tell, that determines if a pref was *registered* by an extension.

What we want to know is whether an extension was installed (either by a
user or by policy) that overrides a function pref value. I re-used the
same code that /options/ used, which is a complicated mixture of enums,
manifest sniffing, and pref filtering. But it works. I've added a small
shim to re-use that from settings, based on pref name. It's not amazing,
but it'll work for now.

What next?

Well, this turns off extension puzzle piece icons and tooltips. This CL
also plumbs more information about whether an extension can be disabled
(a major shortcoming of the old UI, as disable buttons don't work in
some cases).

Next step: show a separate row-like UI.  See crbug.com/614265#c36.

R=stevenjb@chromium.org
BUG=614265
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/455f3cce3da21f8e677100b0c0abd8d35e24876e
Cr-Commit-Position: refs/heads/master@{#432704}
==========

[commit-bot: I haz the power, 2016-11-17 02:13:20]

Patchset 6 (id:??) landed as
https://crrev.com/455f3cce3da21f8e677100b0c0abd8d35e24876e
Cr-Commit-Position: refs/heads/master@{#432704}

[Dan Beam, 2017-02-23 01:49:39]

https://codereview.chromium.org/2479973002/diff/180001/chrome/browser/extensi...
File chrome/browser/extensions/api/settings_private/prefs_util.cc (left):

https://codereview.chromium.org/2479973002/diff/180001/chrome/browser/extensi...
chrome/browser/extensions/api/settings_private/prefs_util.cc:440: if (pref &&
pref->IsExtensionControlled()) {
i probably broke stuff with this