Supervised Users always need custodian approval for permission increase

chrome/browser/extensions/extension_disabled_ui.cc

Index: chrome/browser/extensions/extension_disabled_ui.cc
diff --git a/chrome/browser/extensions/extension_disabled_ui.cc b/chrome/browser/extensions/extension_disabled_ui.cc
index 885fc5df26e5eace51143b79028c3e9e6d843d87..c8b024d713773eee60ce73b52c928241b2ecebd7 100644
--- a/chrome/browser/extensions/extension_disabled_ui.cc
+++ b/chrome/browser/extensions/extension_disabled_ui.cc
@@ -319,9 +319,7 @@ ExtensionDisabledGlobalError::GetBubbleViewMessages() {
 
 base::string16 ExtensionDisabledGlobalError::GetBubbleViewAcceptButtonLabel() {
   if (extensions::util::IsExtensionSupervised(extension_,
-                                              service_->profile()) &&
-      extensions::util::NeedCustodianApprovalForPermissionIncrease(
-          service_->profile())) {
+                                              service_->profile())) {
     // TODO(treib): Probably use a new string here once we get UX design.
     // For now, just use "OK". crbug.com/461261
     return l10n_util::GetStringUTF16(IDS_OK);
@@ -338,16 +336,9 @@ base::string16 ExtensionDisabledGlobalError::GetBubbleViewAcceptButtonLabel() {
 base::string16 ExtensionDisabledGlobalError::GetBubbleViewCancelButtonLabel() {
   if (extensions::util::IsExtensionSupervised(extension_,
                                               service_->profile())) {
-    if (extensions::util::NeedCustodianApprovalForPermissionIncrease(
-        service_->profile())) {
-      // If the supervised user can't approve the update, then there is no
-      // "cancel" button.
-      return base::string16();
-    } else {
-      // Supervised users can not remove extensions, so use "cancel" here
-      // instead of "uninstall".
-      return l10n_util::GetStringUTF16(IDS_CANCEL);
-    }
+    // The supervised user can't approve the update, and hence there is no
+    // "cancel" button.

[asargent_no_longer_on_chrome, 2016/10/05 23:23:34]

Can you clarify this a little bit? Do you mean that we should literally never
hit this code? If so should we have a CHECK here?

Or is it the act of returning an empty string which causes there to be no cancel
button on the dialog?

[mamir, 2016/10/06 09:09:23]

It's the latter.  We return an empty string which causes the cancel button not
to be shown in the dialog.

+    return base::string16();
   }
   return l10n_util::GetStringUTF16(IDS_EXTENSIONS_UNINSTALL);
 }
@@ -358,9 +349,7 @@ void ExtensionDisabledGlobalError::OnBubbleViewDidClose(Browser* browser) {
 void ExtensionDisabledGlobalError::BubbleViewAcceptButtonPressed(
     Browser* browser) {
   if (extensions::util::IsExtensionSupervised(extension_,
-                                              service_->profile()) &&
-      extensions::util::NeedCustodianApprovalForPermissionIncrease(
-          service_->profile())) {
+                                              service_->profile())) {
     return;
   }
   // Delay extension reenabling so this bubble closes properly.
@@ -372,16 +361,11 @@ void ExtensionDisabledGlobalError::BubbleViewAcceptButtonPressed(
 
 void ExtensionDisabledGlobalError::BubbleViewCancelButtonPressed(
     Browser* browser) {
-  if (extensions::util::IsExtensionSupervised(extension_,
-                                              service_->profile())) {
-    // For custodian-installed extensions, this button should only exist if the
-    // supervised user can approve the update. Otherwise there is only an "OK"
-    // button.
-    DCHECK(!extensions::util::NeedCustodianApprovalForPermissionIncrease(
-        service_->profile()));
-    // Supervised users may never remove custodian-installed extensions.
-    return;
-  }
+  // For custodian-installed extensions, this button should not exist because
+  // there is only an "OK" button.
+  // Supervised users may never remove custodian-installed extensions.
+  DCHECK(!extensions::util::IsExtensionSupervised(extension_,
+                                                  service_->profile()));
 
   uninstall_dialog_.reset(extensions::ExtensionUninstallDialog::Create(
       service_->profile(), browser->window()->GetNativeWindow(), this));