Add a lock to the managed user settings page and require authentication for unlocking.

chrome/browser/resources/options/managed_user_settings.js

Index: chrome/browser/resources/options/managed_user_settings.js
diff --git a/chrome/browser/resources/options/managed_user_settings.js b/chrome/browser/resources/options/managed_user_settings.js
index 3300cdb0c3fe3993464176a08e38b32cec7ba940..292c9f8782431e7a293b15fc87b2605552860f29 100644
--- a/chrome/browser/resources/options/managed_user_settings.js
+++ b/chrome/browser/resources/options/managed_user_settings.js
@@ -31,6 +31,8 @@ cr.define('options', function() {
   ManagedUserSettings.prototype = {
     // Inherit from SettingsDialog.
     __proto__: SettingsDialog.prototype,
+    authenticationChecked: false,

[Bernhard Bauer, 2013/02/15 10:05:20]

I think a better name for this would be "isAuthenticated"? Also, would it make
sense to combine these two flags into a tri-state variable (unauthenticated,
checking, authenticated)?

[Adrian Kuegel, 2013/02/20 12:42:13]

Yes, good idea. I changed it like that.

+    authenticationChecking: false,
 
     /**
      * Initialize the page.
@@ -48,21 +50,90 @@ cr.define('options', function() {
         OptionsPage.navigateToPage('setPassphrase');
       };
 
+      $('use-passphrase-checkbox').onclick = function() {
+        $('set-passphrase').disabled = !$('use-passphrase-checkbox').checked;
+      };
+
+      $('unlock-settings').onclick = function() {
+        if (!this.isAuthenticationChecking) {

[Bernhard Bauer, 2013/02/15 10:05:20]

You could invert the condition and early-return.

[Adrian Kuegel, 2013/02/20 12:42:13]

Done.

+          chrome.send('displayPassphraseDialog',
+                      ['ManagedUserSettings.isAuthenticated']);
+          this.authenticationChecking = true;
+        }
+      };
     },
     /** @override */
     handleConfirm: function() {
-      chrome.send('confirmManagedUserSettings');
-      SettingsDialog.prototype.handleConfirm.call(this);
+      if ($('use-passphrase-checkbox').checked) {
+        chrome.send('isPassphraseSet', ['ManagedUserSettings.isPassphraseSet']);
+      } else {
+        chrome.send('confirmManagedUserSettings');
+        chrome.send('resetPassphrase');
+        SettingsDialog.prototype.handleConfirm.call(this);
+      }
+    },
+    updateControls: function(disable) {
+      $('set-passphrase').disabled = disable;

[Bernhard Bauer, 2013/02/15 10:05:20]

Please don't simply set the disabled attribute. There might be other reasons why
a control can be disabled, and so updating the attribute directly might
interfere. Instead, there is a setDisabled() method on pref UI controls (in
pref_ui.js).

[Adrian Kuegel, 2013/02/20 12:42:13]

If I understand that right, I can only use this on pref UI controls. I guess a
pref UI control is one which has a pref="..." in the HTML, so I modified it only
for those controls.

+      $('get-content-packs-button').disabled = disable;
+      $('contentpacks-allow').disabled = disable;
+      $('contentpacks-warn').disabled = disable;
+      $('contentpacks-block').disabled = disable;
+      $('safe-search-checkbox').disabled = disable;
+      $('disable-signin-checkbox').disabled = disable;
+      $('disable-history-deletion-checkbox').disabled = disable;
+      $('use-passphrase-checkbox').disabled = disable;
+      $('unlock-settings').disabled = !disable;
     },
+    didShowPage: function() {
+      chrome.send('isPassphraseSet',

[Bernhard Bauer, 2013/02/15 10:05:20]

Instead of asking the browser asynchronously, could you just store this value on
the Javascript side?

[Adrian Kuegel, 2013/02/20 12:42:13]

Done.

+                  ['ManagedUserSettings.initializeSetPassphraseButton']);
+    },
+    didClosePage: function() {
+      // Reset the authentication of the custodian.
+      this.authenticationChecked = false;
+      chrome.send('endAuthentication');
+    },
+  };
+
+  ManagedUserSettings.initializeSetPassphraseButton = function(hasPassphrase) {
+    $('set-passphrase').disabled = !hasPassphrase;
+    $('use-passphrase-checkbox').checked = hasPassphrase;
+    if (hasPassphrase) {
+      ManagedUserSettings.getInstance().updateControls(true);

[Pam (message me for reviews), 2013/02/15 09:31:18]

This would be clearer if the argument were flipped (enable rather than disable).

[Adrian Kuegel, 2013/02/15 09:56:02]

Done. I also renamed the function to enableControls, which should make it even
more clearer.

+    } else {
+      ManagedUserSettings.getInstance().authenticationChecked = true;
+      $('unlock-settings').disabled = true;
+    }
+  };
+
+  ManagedUserSettings.isAuthenticated = function(success) {

[Bernhard Bauer, 2013/02/15 10:05:20]

I think it would be nicer if you delegate to an instance method on
ManagedUserSettings.

[Adrian Kuegel, 2013/02/20 12:42:13]

Done.

+    var instance = ManagedUserSettings.getInstance();
+    if (success) {
+      instance.authenticationChecked = true;
+      instance.updateControls(false);
+    }
+    instance.authenticationChecking = false;
+  };
+
+  ManagedUserSettings.isPassphraseSet = function(success) {

[Bernhard Bauer, 2013/02/15 10:05:20]

|success| means whether the passphrase is set? Could use a clearer variable
name.

[Adrian Kuegel, 2013/02/20 12:42:13]

Done.

+    if (success) {
+      var instance = ManagedUserSettings.getInstance();
+      SettingsDialog.prototype.handleConfirm.call(instance);
+    } else {
+      OptionsPage.navigateToPage('setPassphrase');
+    }
   };
 
   var ManagedUserSettingsForTesting = {
     getSetPassphraseButton: function() {
       return $('set-passphrase');
+    },
+    getUnlockButton: function() {
+      return $('unlock-settings');
     }
   };
 
-   // Export
+  // Export
   return {
     ManagedUserSettings: ManagedUserSettings,
     ManagedUserSettingsForTesting: ManagedUserSettingsForTesting