Make extensions developer mode adhere to policy

chrome/browser/resources/extensions/extensions.js

Index: chrome/browser/resources/extensions/extensions.js
diff --git a/chrome/browser/resources/extensions/extensions.js b/chrome/browser/resources/extensions/extensions.js
index fbf7b3cb2a4dcc18f7b7bbd05364d9aa12e2ac26..8007582950117f79d1e9f9fa682420b378ed501b 100644
--- a/chrome/browser/resources/extensions/extensions.js
+++ b/chrome/browser/resources/extensions/extensions.js
@@ -196,6 +196,8 @@ cr.define('extensions', function() {
 
       /** @const */
       var supervised = profileInfo.isSupervised;
+      var developerModeDisabledByPolicy =
+          profileInfo.isDeveloperModeDisabledByPolicy;
 
       var pageDiv = $('extension-settings');
       pageDiv.classList.toggle('profile-is-supervised', supervised);
@@ -203,7 +205,13 @@ cr.define('extensions', function() {
 
       var devControlsCheckbox = $('toggle-dev-on');
       devControlsCheckbox.checked = profileInfo.inDeveloperMode;
-      devControlsCheckbox.disabled = supervised;
+      devControlsCheckbox.disabled =
+          supervised || developerModeDisabledByPolicy;
+
+      // This is necessary e.g. if developer mode is now disabled by policy
+      // but extension developer tools were visible

[Devlin, 2016/11/28 17:34:01]

nitty-nit: comments should end with punctuation (i.e., add a '.').

[pmarko, 2016/11/29 12:40:22]

Done.

+      this.updateDevControlsVisibility_(true);

[Devlin, 2016/11/28 17:34:01]

nit: We probably don't need to animate here - usually that's only done in
response to some sort of user action.

[pmarko, 2016/11/29 12:40:22]

Good point - passing false. Done.

+      this.setDevToggleControlledIndicator_(developerModeDisabledByPolicy);
 
       $('load-unpacked').disabled = !profileInfo.canLoadUnpacked;
       var extensionList = $('extension-settings-list');
@@ -219,6 +227,32 @@ cr.define('extensions', function() {
     },
 
     /**
+     * Shows or hides the 'controlled by policy' indicator on the dev-toggle
+     * checkbox.
+     * @param {boolean} developerModeDisabledByPolicy true if the indicator
+     *     should be showing.
+     * @private
+     */
+    setDevToggleControlledIndicator_: function(developerModeDisabledByPolicy) {
+      var devToggleControlledIndicator = document.querySelector(
+          '#dev-toggle .controlled-setting-indicator');
+
+      if (!(devToggleControlledIndicator instanceof cr.ui.ControlledIndicator))
+        cr.ui.ControlledIndicator.decorate(devToggleControlledIndicator);
+
+      if (developerModeDisabledByPolicy) {
+        var controlledBy = 'policy';
+        devToggleControlledIndicator.setAttribute('controlled-by',

[Devlin, 2016/11/28 17:34:01]

nit: prefer wrapping so that either all arguments are on the same line:
devToggleControlledIndicator.setAttribute(
    'controlled-by', controlledBy);
or so that they align:
devToggleControlledIndicator.setAttribute('controlled-by',
                                          controlledBy);

(Goes for all places)

[pmarko, 2016/11/29 12:40:22]

Done.

+            controlledBy);
+        devToggleControlledIndicator.setAttribute('text' + controlledBy,
+            loadTimeData.getString('controlledSettingPolicy'));
+      }
+      else {

[Devlin, 2016/11/28 17:34:01]

else on same line as closing }

[pmarko, 2016/11/29 12:40:22]

Done.

+        devToggleControlledIndicator.removeAttribute('controlled-by');

[Devlin, 2016/11/28 17:34:01]

does removing the attribute fully hide the element?  It seems like it would be
easier to either use .hidden or to just create/remove the element fully (as we
do in extension_list.js).

[pmarko, 2016/11/29 12:40:22]

Removing it actually fully hides it due to controlled-indicator.css which
states:
.controlled-setting-indicator:not([controlled-by]) {
  display: none;
}
As far as I understand it, this is also the mechanism controlled_setting.js uses
for showing the indicator (including deciding which icon to display)/hiding it
(in detail, a listener on the Preference's controlledBy attrbiute transfers its
value to the controlledBy of the indicator object). That mechanism is used on
the main (non-extensions) settings page, that's why I did it that way.

Do you think it would be better
- if we constantly set controlled-by=policy and did visibility control using
.hidden
- if we removed/recreated the element on the fily
- if we did it using the controlled-by attribute and added a comment that
removing the attribute fully hides the element?

[Devlin, 2016/11/30 19:19:50]

I like option 3 - adding a comment that this hides the element from the DOM. 
That should be sufficient.

[pmarko, 2016/12/02 00:20:07]

Done.

+      }
+    },
+
+    /**
      * Shows the loading spinner and hides elements that shouldn't be visible
      * while loading.
      * @param {boolean} isLoading