Differentiate new permissions from old in extension permission update bubble

chrome/browser/extensions/extension_disabled_ui.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/extensions/extension_disabled_ui.h"
 
 #include <memory>
 #include <string>
 
 #include "base/bind.h"
@@ skipping 197 matching lines @@
         base::UTF8ToUTF16(extension_->name()));
   } else {
     return l10n_util::GetStringFUTF16(IDS_EXTENSION_DISABLED_ERROR_TITLE,
                                       base::UTF8ToUTF16(extension_->name()));
   }
 }
 
 std::vector<base::string16>
 ExtensionDisabledGlobalError::GetBubbleViewMessages() {
   std::vector<base::string16> messages;
+
+  std::unique_ptr<const extensions::PermissionSet> granted =

[Devlin, 2017/04/19 19:18:33]

no need for extensions:: prefix (this whole file)

[catmullings, 2017/04/20 02:01:59]

Done.

+      service_->extension_prefs()->GetGrantedPermissions(extension_->id());
+
+  const extensions::PermissionSet& active =
+      extension_->permissions_data()->active_permissions();
+
+  std::unique_ptr<const extensions::PermissionSet> requested_permissions =
+      extensions::PermissionSet::CreateDifference(active, *granted);
+
   PermissionMessages permission_warnings =
-      extension_->permissions_data()->GetPermissionMessages();
+      extension_->permissions_data()->GetPermissionMessages(
+          *requested_permissions);
+
   if (is_remote_install_) {
     if (!permission_warnings.empty())
       messages.push_back(
           l10n_util::GetStringUTF16(IDS_EXTENSION_PROMPT_WILL_HAVE_ACCESS_TO));
   } else {
     // TODO(treib): If NeedCustodianApprovalForPermissionIncrease, add an extra
     // message for supervised users. crbug.com/461261
-    messages.push_back(l10n_util::GetStringFUTF16(
+    messages.push_back(l10n_util::GetStringUTF16(
         extension_->is_app() ? IDS_APP_DISABLED_ERROR_LABEL
-                             : IDS_EXTENSION_DISABLED_ERROR_LABEL,
-        base::UTF8ToUTF16(extension_->name())));
-    messages.push_back(l10n_util::GetStringUTF16(
-        IDS_EXTENSION_PROMPT_WILL_NOW_HAVE_ACCESS_TO));
+                             : IDS_EXTENSION_DISABLED_ERROR_LABEL));
   }
   for (const PermissionMessage& msg : permission_warnings) {
     messages.push_back(l10n_util::GetStringFUTF16(IDS_EXTENSION_PERMISSION_LINE,
                                                   msg.message()));
   }
   return messages;
 }
 
 base::string16 ExtensionDisabledGlobalError::GetBubbleViewAcceptButtonLabel() {
   if (extensions::util::IsExtensionSupervised(extension_,
                                               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);
   }
   if (is_remote_install_) {
     return l10n_util::GetStringUTF16(
         extension_->is_app()
             ? IDS_EXTENSION_PROMPT_REMOTE_INSTALL_BUTTON_APP
             : IDS_EXTENSION_PROMPT_REMOTE_INSTALL_BUTTON_EXTENSION);
   }
-  return l10n_util::GetStringUTF16(IDS_EXTENSION_PROMPT_RE_ENABLE_BUTTON);
+  return l10n_util::GetStringUTF16(
+      IDS_EXTENSION_PROMPT_PERMISSIONS_ACCEPT_BUTTON);
 }
 
 base::string16 ExtensionDisabledGlobalError::GetBubbleViewCancelButtonLabel() {
   if (extensions::util::IsExtensionSupervised(extension_,
                                               service_->profile())) {
     // The supervised user can't approve the update, and hence there is no
     // "cancel" button. Return an empty string such that the "cancel" button
     // is not shown in the dialog.
     return base::string16();
   }
-  return l10n_util::GetStringUTF16(IDS_EXTENSIONS_UNINSTALL);

[Devlin, 2017/04/19 19:18:33]

I'm a little worried to re-open this to bikeshedding, but what about actually
keeping the uninstall string instead of remove?  I remember you brought up the
good point that "remove" made it unclear if it were the extension or the
permissions; "uninstall" might address that, while still being short?  Maybe
worth chatting with Shimi about quickly?

I'd be fine either way; just thought I'd mention it.

[catmullings, 2017/04/20 02:01:59]

Sure, will look into it.

+  return l10n_util::GetStringUTF16(IDS_EXTENSION_PROMPT_UNINSTALL_BUTTON);
 }
 
 void ExtensionDisabledGlobalError::OnBubbleViewDidClose(Browser* browser) {
   // If the user takes an action, |user_response_| is set in
   // BubbleView[Cancel|Accept]Pressed(). Otherwise, the IGNORE value set in the
   // constructor is correct.
   UMA_HISTOGRAM_ENUMERATION("Extensions.DisabledUIUserResponseRemoteInstall2",
                             user_response_,
                             EXTENSION_DISABLED_UI_BUCKET_BOUNDARY);
   UMA_HISTOGRAM_ENUMERATION("Extensions.DisabledUIUserResponse2",
@@ skipping 125 matching lines @@
       ->LoadImageAsync(extension,
                        image,
                        size,
                        base::Bind(&AddExtensionDisabledErrorWithIcon,
                                   service->AsWeakPtr(),
                                   extension->id(),
                                   is_remote_install));
 }
 
 }  // namespace extensions