Replace toggles with buttons in chrome://extensions for child users

When a child user has an extension installed but lacks sufficient
custodian approval, the extension is force-disabled on the
chrome://extensions page. The toggles for enabling the extension are
force-disabled as well. This CL replaces the force-disabled toggles
with buttons that open the parental approval dialog directly on the
chrome://extensions page, so that the child user can obtain custodian
approval to enable the extension.

Bug: 1030958
Change-Id: I284221d37ba2ce51fd2afe178bbf1eaf69f2e1f6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1956227
Commit-Queue: Toby Huang <tobyhuang@chromium.org>
Reviewed-by: Michael Giuffrida <michaelpg@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#729633}

chrome/app/extensions_strings.grdp

@@ -19,6 +19,9 @@
   <message name="IDS_EXTENSIONS_CORRUPTED_EXTENSION" desc="The warning for the user that an extension may have been tampered with on disk.">
     This extension may have been corrupted.
   </message>
+  <message name="IDS_EXTENSIONS_ENABLE_BUTTON" desc="The text displayed on the enable button when the extension is disabled pending custodian approval on the chrome://extensions page">
+    Enable
+  </message>
   <message name="IDS_EXTENSIONS_ENABLE_ERROR_COLLECTION" desc="The checkbox for enabling error collection for an extension.">
     Collect errors
   </message>

chrome/app/extensions_strings_grdp/IDS_EXTENSIONS_ENABLE_BUTTON.png.sha1

+f2a24389c19c6a880a5f1e7266d47e6e70b874b0
\ No newline at end of file

chrome/browser/resources/extensions/detail_view.html

@@ -161,7 +161,7 @@
             icon-class="[[getIndicatorIcon_(data.controlledInfo.type)]]"
             icon-aria-label="[[data.controlledInfo.type]]">
         </cr-tooltip-icon>
-        <template is="dom-if" if="[[isTerminated_(data.state)]]">
+        <template is="dom-if" if="[[showReloadButton_(data.state)]]">
           <cr-button id="terminated-reload-button" class="action-button"
               on-click="onReloadTap_">
             $i18n{itemReload}
@@ -174,10 +174,15 @@
                 '$i18nPolymer{extensionEnabled}')]]"
             aria-describedby="name"
             checked="[[isEnabled_(data.state)]]"
-            on-change="onEnableChange_"
+            on-change="onEnableToggleChange_"
             disabled="[[!isEnableToggleEnabled_(data.*)]]"
-            hidden$="[[isTerminated_(data.state)]]">
+            hidden$="[[!showEnableToggle_(data.*)]]">
         </cr-toggle>
+        <cr-button id="enableButton"
+            on-click="onEnableButtonClick_"
+            hidden$="[[!showEnableButton_(data.*)]]">
+          $i18n{enableButton}
+        </cr-button>
       </div>
     </div>
     <div id="warnings" hidden$="[[!hasWarnings_(data.*)]]">
@@ -189,7 +194,7 @@
             [[item]]
           </template>
         </span>
-        <template is="dom-if" if="[[!isTerminated_(data.state)]]">
+        <template is="dom-if" if="[[!showReloadButton_(data.state)]]">
           <cr-button id="warnings-reload-button" class="action-button"
               on-click="onReloadTap_">
             $i18n{itemReload}
@@ -208,7 +213,7 @@
       </div>
       <div class="section continuation warning control-line"
           id="corrupted-warning"
-          hidden$="[[!data.disableReasons.corruptInstall]]">
+          hidden$="[[!showRepairButton_(data.disableReasons.corruptInstall)]]">
         <iron-icon class="warning-icon" icon="cr:warning"></iron-icon>
         <span>$i18n{itemCorruptInstall}</span>
         <cr-button id="repair-button" class="action-button"

chrome/browser/resources/extensions/detail_view.js

@@ -31,7 +31,7 @@ import {afterNextRender, html, Polymer} from 'chrome://resources/polymer/v3_0/po
 
 import {ItemDelegate} from './item.js';
 import {ItemBehavior} from './item_behavior.js';
-import {computeInspectableViewLabel, getItemSource, getItemSourceString, isControlled, isEnabled, userCanChangeEnablement} from './item_util.js';
+import {computeInspectableViewLabel, EnableControl, getEnableControl, getItemSource, getItemSourceString, isControlled, isEnabled, userCanChangeEnablement} from './item_util.js';
 import {navigation, Page} from './navigation_helper.js';
 
 Polymer({
@@ -152,16 +152,6 @@ Polymer({
     return userCanChangeEnablement(this.data);
   },
 
-  /**
-   * Returns true if the extension is in the terminated state.
-   * @return {boolean}
-   * @private
-   */
-  isTerminated_: function() {
-    return this.data.state ===
-        chrome.developerPrivate.ExtensionState.TERMINATED;
-  },
-
   /**
    * @return {boolean}
    * @private
@@ -238,10 +228,15 @@ Polymer({
   },
 
   /** @private */
-  onEnableChange_: function() {
+  onEnableToggleChange_: function() {
     this.delegate.setItemEnabled(this.data.id, this.$.enableToggle.checked);
   },
 
+  /** @private */
+  onEnableButtonClick_: function() {
+    this.delegate.setItemEnabled(this.data.id, true);
+  },
+
   /**
    * @param {!{model: !{item: !chrome.developerPrivate.ExtensionView}}} e
    * @private
@@ -382,4 +377,44 @@ Polymer({
     return this.hasRuntimeHostPermissions_() &&
         !this.data.permissions.runtimeHostPermissions.hasAllHosts;
   },
+
+  /**
+   * Returns true if the reload button should be shown.
+   * @return {boolean}
+   * @private
+   */
+  showReloadButton_: function() {
+    return getEnableControl(this.data) === EnableControl.RELOAD;
+  },
+
+  /**
+   * Returns true if the repair button should be shown.
+   * @return {boolean}
+   * @private
+   */
+  showRepairButton_: function() {
+    return getEnableControl(this.data) === EnableControl.REPAIR;
+  },
+
+  /**
+   * Returns true if the enable toggle should be shown.
+   * @return {boolean}
+   * @private
+   */
+  showEnableToggle_: function() {
+    const enableControl = getEnableControl(this.data);
+    // We still show the toggle even if we also show the repair button in the
+    // detail view, because the repair button appears just beneath it.
+    return enableControl === EnableControl.ENABLE_TOGGLE ||
+        enableControl === EnableControl.REPAIR;
+  },
+
+  /**
+   * Returns true if the enable button should be shown.
+   * @return {boolean}
+   * @private
+   */
+  showEnableButton_: function() {
+    return getEnableControl(this.data) === EnableControl.ENABLE_BUTTON;
+  },
 });

chrome/browser/resources/extensions/item.html

@@ -299,13 +299,14 @@
           aria-label="$i18n{itemReload}" aria-describedby="a11yAssociation"
           on-click="onReloadTap_"></cr-icon-button>
     </template>
-    <template is="dom-if" if="[[data.disableReasons.corruptInstall]]">
+    <template is="dom-if"
+        if="[[showRepairButton_(data.disableReasons.corruptInstall)]]">
       <cr-button id="repair-button" class="action-button"
           aria-describedby="a11yAssociation" on-click="onRepairTap_">
         $i18n{itemRepair}
       </cr-button>
     </template>
-    <template is="dom-if" if="[[isTerminated_(data.state)]]">
+    <template is="dom-if" if="[[showReloadButton_(data.state)]]">
       <cr-button id="terminated-reload-button" on-click="onReloadTap_"
           aria-describedby="a11yAssociation" class="action-button">
         $i18n{itemReload}
@@ -317,9 +318,16 @@
             '$i18nPolymer{appEnabled}',
             '$i18nPolymer{extensionEnabled}')]]"
         aria-describedby="a11yAssociation"
-        checked="[[isEnabled_(data.state)]]" on-change="onEnableChange_"
+        checked="[[isEnabled_(data.state)]]"
+        on-change="onEnableToggleChange_"
         disabled="[[!isEnableToggleEnabled_(data.*)]]"
         hidden$="[[!showEnableToggle_(data.*)]]">
     </cr-toggle>
+    <cr-button id="enableButton"
+        on-click="onEnableButtonClick_"
+        aria-describedby="a11yAssociation"
+        hidden$="[[!showEnableButton_(data.*)]]">
+      $i18n{enableButton}
+    </cr-button>
   </div>
 </div>

chrome/browser/resources/extensions/item.js

@@ -26,7 +26,7 @@ import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
 import {flush, html, Polymer} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
 
 import {ItemBehavior} from './item_behavior.js';
-import {computeInspectableViewLabel, getItemSource, getItemSourceString, isControlled, isEnabled, SourceType, userCanChangeEnablement} from './item_util.js';
+import {computeInspectableViewLabel, EnableControl, getEnableControl, getItemSource, getItemSourceString, isControlled, isEnabled, SourceType, userCanChangeEnablement} from './item_util.js';
 import {navigation, Page} from './navigation_helper.js';
 
 /** @interface */
@@ -205,10 +205,15 @@ Polymer({
   },
 
   /** @private */
-  onEnableChange_: function() {
+  onEnableToggleChange_: function() {
     this.delegate.setItemEnabled(this.data.id, this.$.enableToggle.checked);
   },
 
+  /** @private */
+  onEnableButtonClick_: function() {
+    this.delegate.setItemEnabled(this.data.id, true);
+  },
+
   /** @private */
   onErrorsTap_: function() {
     if (this.data.installWarnings && this.data.installWarnings.length > 0) {
@@ -294,23 +299,41 @@ Polymer({
     return userCanChangeEnablement(this.data);
   },
 
+  /**
+   * Returns true if the reload button should be shown.
+   * @return {boolean}
+   * @private
+   */
+  showReloadButton_: function() {
+    return getEnableControl(this.data) === EnableControl.RELOAD;
+  },
+
+  /**
+   * Returns true if the repair button should be shown.
+   * @return {boolean}
+   * @private
+   */
+  showRepairButton_: function() {
+    return getEnableControl(this.data) === EnableControl.REPAIR;
+  },
+
+
   /**
    * Returns true if the enable toggle should be shown.
    * @return {boolean}
    * @private
    */
   showEnableToggle_: function() {
-    return !this.isTerminated_() && !this.data.disableReasons.corruptInstall;
+    return getEnableControl(this.data) === EnableControl.ENABLE_TOGGLE;
   },
 
   /**
-   * Returns true if the extension is in the terminated state.
+   * Returns true if the enable button should be shown.
    * @return {boolean}
    * @private
    */
-  isTerminated_: function() {
-    return this.data.state ===
-        chrome.developerPrivate.ExtensionState.TERMINATED;
+  showEnableButton_: function() {
+    return getEnableControl(this.data) === EnableControl.ENABLE_BUTTON;
   },
 
   /**

chrome/browser/resources/extensions/item_util.js

@@ -18,6 +18,14 @@ export const SourceType = {
   UNKNOWN: 'unknown',
 };
 
+/** @enum {string} */
+export const EnableControl = {
+  RELOAD: 'RELOAD',
+  REPAIR: 'REPAIR',
+  ENABLE_BUTTON: 'ENABLE_BUTTON',
+  ENABLE_TOGGLE: 'ENABLE_TOGGLE',
+};
+
 /**
  * Returns true if the extension is enabled, including terminated
  * extensions.
@@ -151,3 +159,31 @@ export function computeInspectableViewLabel(view) {
 
   return label;
 }
+
+/**
+ * Returns true if the extension is in the terminated state.
+ * @param {!chrome.developerPrivate.ExtensionState} state
+ * @return {boolean}
+ * @private
+ */
+function isTerminated_(state) {
+  return state === chrome.developerPrivate.ExtensionState.TERMINATED;
+}
+
+/**
+ * Determines which enable control to display for a given extension.
+ * @param {!chrome.developerPrivate.ExtensionInfo} data
+ * @return {EnableControl}
+ */
+export function getEnableControl(data) {
+  if (isTerminated_(data.state)) {
+    return EnableControl.RELOAD;
+  }
+  if (data.disableReasons.corruptInstall) {
+    return EnableControl.REPAIR;
+  }
+  if (data.disableReasons.custodianApprovalRequired) {
+    return EnableControl.ENABLE_BUTTON;
+  }
+  return EnableControl.ENABLE_TOGGLE;
+}

chrome/browser/ui/webui/extensions/extensions_ui.cc

@@ -168,6 +168,7 @@ content::WebUIDataSource* CreateMdExtensionsSource(Profile* profile,
     {"stopActivityStream", IDS_EXTENSIONS_STOP_ACTIVITY_STREAM},
     {"emptyStreamStarted", IDS_EXTENSIONS_EMPTY_STREAM_STARTED},
     {"emptyStreamStopped", IDS_EXTENSIONS_EMPTY_STREAM_STOPPED},
+    {"enableButton", IDS_EXTENSIONS_ENABLE_BUTTON},
     {"activityArgumentsHeading", IDS_EXTENSIONS_ACTIVITY_ARGUMENTS_HEADING},
     {"webRequestInfoHeading", IDS_EXTENSIONS_WEB_REQUEST_INFO_HEADING},
     {"activityLogMoreActionsLabel",

chrome/test/data/webui/extensions/cr_extensions_browsertest.js

@@ -172,8 +172,8 @@ TEST_F('CrExtensionsItemsTest', 'SourceIndicator', function() {
   this.runMochaTest(extension_item_tests.TestNames.SourceIndicator);
 });
 
-TEST_F('CrExtensionsItemsTest', 'EnableToggle', function() {
-  this.runMochaTest(extension_item_tests.TestNames.EnableToggle);
+TEST_F('CrExtensionsItemsTest', 'EnableToggleAndButton', function() {
+  this.runMochaTest(extension_item_tests.TestNames.EnableToggleAndButton);
 });
 
 TEST_F('CrExtensionsItemsTest', 'RemoveButton', function() {

chrome/test/data/webui/extensions/detail_view_test.js

@@ -64,6 +64,7 @@ suite(extension_detail_view_tests.suiteName, function() {
     expectTrue(testIsVisible('#closeButton'));
     expectTrue(testIsVisible('#icon'));
     expectTrue(testIsVisible('#enableToggle'));
+    expectFalse(testIsVisible('#enableButton'));
     expectFalse(testIsVisible('#extensions-options'));
     expectTrue(
         item.$.description.textContent.indexOf('This is an extension') !== -1);
@@ -169,6 +170,7 @@ suite(extension_detail_view_tests.suiteName, function() {
     expectTrue(testIsVisible('.warning-icon'));
 
     expectTrue(testIsVisible('#enableToggle'));
+    expectFalse(testIsVisible('#enableButton'));
     expectFalse(testIsVisible('#terminated-reload-button'));
 
     // This section tests that the enable toggle is visible but disabled when
@@ -178,19 +180,22 @@ suite(extension_detail_view_tests.suiteName, function() {
     flush();
     expectTrue(testIsVisible('#enableToggle'));
     expectTrue(item.$$('#enableToggle').disabled);
+    expectFalse(testIsVisible('#enableButton'));
     item.set('data.disableReasons.blockedByPolicy', false);
     flush();
 
     item.set('data.disableReasons.custodianApprovalRequired', true);
     flush();
-    expectTrue(testIsVisible('#enableToggle'));
-    expectTrue(item.$$('#enableToggle').disabled);
+    expectFalse(testIsVisible('#enableToggle'));
+    expectTrue(testIsVisible('#enableButton'));
+    expectFalse(item.$$('#enableButton').disabled);
     item.set('data.disableReasons.custodianApprovalRequired', false);
     flush();
 
     item.set('data.state', chrome.developerPrivate.ExtensionState.TERMINATED);
     flush();
     expectFalse(testIsVisible('#enableToggle'));
+    expectFalse(testIsVisible('#enableButton'));
     expectTrue(testIsVisible('#terminated-reload-button'));
 
     // Ensure that the runtime warning reload button is not visible if there
@@ -366,6 +371,9 @@ suite(extension_detail_view_tests.suiteName, function() {
     testWarningVisible('#suspicious-warning', false);
     testWarningVisible('#blacklisted-warning', false);
     testWarningVisible('#update-required-warning', false);
+    const testIsVisible = isVisible.bind(null, item);
+    expectTrue(testIsVisible('#enableToggle'));
+    expectFalse(testIsVisible('#enableButton'));
 
     item.set('data.disableReasons.suspiciousInstall', true);
     flush();

chrome/test/data/webui/extensions/item_test.js

@@ -85,7 +85,8 @@ extension_item_tests.TestNames = {
   FailedReloadFiresLoadError: 'failed reload fires load error',
   Warnings: 'warnings',
   SourceIndicator: 'source indicator',
-  EnableToggle: 'toggle is disabled when necessary',
+  EnableToggleAndButton:
+      'Enable toggle and button are disabled/hidden when necessary',
   RemoveButton: 'remove button hidden when necessary',
   HtmlInName: 'html in extension name',
 };
@@ -149,6 +150,8 @@ suite(extension_item_tests.suiteName, function() {
             'data.state', chrome.developerPrivate.ExtensionState.TERMINATED);
         flush();
         testVisible(item, '#dev-reload-button', false);
+        testVisible(item, '#enableToggle', false);
+        testVisible(item, '#enableButton', false);
       });
 
   /** Tests that the delegate methods are correctly called. */
@@ -159,6 +162,8 @@ suite(extension_item_tests.suiteName, function() {
         item.$['remove-button'], 'deleteItem', [item.data.id]);
     mockDelegate.testClickingCalls(
         item.$['enableToggle'], 'setItemEnabled', [item.data.id, false]);
+    mockDelegate.testClickingCalls(
+        item.$['enableButton'], 'setItemEnabled', [item.data.id, true]);
     mockDelegate.testClickingCalls(
         item.$$('#inspect-views a[is="action-link"]'), 'inspectItemView',
         [item.data.id, item.data.views[0]]);
@@ -184,6 +189,8 @@ suite(extension_item_tests.suiteName, function() {
     flush();
     mockDelegate.testClickingCalls(
         item.$$('#repair-button'), 'repairItem', [item.data.id]);
+    testVisible(item, '#enableToggle', false);
+    testVisible(item, '#enableButton', false);
 
     item.set('data.state', chrome.developerPrivate.ExtensionState.TERMINATED);
     flush();
@@ -320,35 +327,42 @@ suite(extension_item_tests.suiteName, function() {
     expectFalse(isVisible(item, '#source-indicator'));
   });
 
-  test(assert(extension_item_tests.TestNames.EnableToggle), function() {
-    expectFalse(item.$['enableToggle'].disabled);
+  test(
+      assert(extension_item_tests.TestNames.EnableToggleAndButton), function() {
+        expectFalse(item.$['enableToggle'].disabled);
+        expectFalse(item.$['enableButton'].disabled);
 
-    // Test case where user does not have permission.
-    item.set('data.userMayModify', false);
-    flush();
-    expectTrue(item.$['enableToggle'].disabled);
+        // Test case where user does not have permission.
+        item.set('data.userMayModify', false);
+        flush();
+        expectTrue(item.$['enableToggle'].disabled);
+        expectFalse(item.$['enableButton'].disabled);
 
-    // Test case of a blacklisted extension.
-    item.set('data.userMayModify', true);
-    item.set('data.state', 'BLACKLISTED');
-    flush();
-    expectTrue(item.$['enableToggle'].disabled);
+        // Test case of a blacklisted extension.
+        item.set('data.userMayModify', true);
+        item.set('data.state', 'BLACKLISTED');
+        flush();
+        expectTrue(item.$['enableToggle'].disabled);
 
-    // This section tests that the enable toggle is visible but disabled
-    // when disableReasons.blockedByPolicy is true. This test prevents a
-    // regression to crbug/1003014.
-    item.set('data.disableReasons.blockedByPolicy', true);
-    flush();
-    testVisible(item, '#enableToggle', true);
-    expectTrue(item.$['enableToggle'].disabled);
-    item.set('data.disableReasons.blockedByPolicy', false);
-    flush();
+        // This section tests that the enable toggle is visible but disabled
+        // when disableReasons.blockedByPolicy is true. This test prevents a
+        // regression to crbug/1003014.
+        item.set('data.disableReasons.blockedByPolicy', true);
+        flush();
+        testVisible(item, '#enableToggle', true);
+        expectTrue(item.$['enableToggle'].disabled);
+        testVisible(item, '#enableButton', false);
+        item.set('data.disableReasons.blockedByPolicy', false);
+        flush();
 
-    item.set('data.disableReasons.custodianApprovalRequired', true);
-    flush();
-    testVisible(item, '#enableToggle', true);
-    expectTrue(item.$$('#enableToggle').disabled);
-  });
+        item.set('data.disableReasons.custodianApprovalRequired', true);
+        flush();
+        testVisible(item, '#enableToggle', false);
+        testVisible(item, '#enableButton', true);
+        expectFalse(item.$$('#enableButton').disabled);
+        item.set('data.disableReasons.custodianApprovalRequired', false);
+        flush();
+      });
 
   test(assert(extension_item_tests.TestNames.RemoveButton), function() {
     expectFalse(item.$['remove-button'].hidden);