Views/Permissions: Use shorter strings for site setting drop downs.

chrome/browser/ui/website_settings/permission_menu_model.cc

Index: chrome/browser/ui/website_settings/permission_menu_model.cc
diff --git a/chrome/browser/ui/website_settings/permission_menu_model.cc b/chrome/browser/ui/website_settings/permission_menu_model.cc
index 2828c7794e27a189cfad904b21bdac84be5c0a9e..1a91fcfbbc419b5124d07b5f24df73f88ab16585 100644
--- a/chrome/browser/ui/website_settings/permission_menu_model.cc
+++ b/chrome/browser/ui/website_settings/permission_menu_model.cc
@@ -27,6 +27,7 @@ PermissionMenuModel::PermissionMenuModel(
   DCHECK(!callback_.is_null());
   base::string16 label;
 
+  // Retrieve the string to show for the default setting for this permission.
   ContentSetting effective_default_setting = permission_.default_setting;
 
 #if BUILDFLAG(ENABLE_PLUGINS)
@@ -68,12 +69,13 @@ PermissionMenuModel::PermissionMenuModel(
   // of simply setting a shorter label on the menubutton.
   if (ui::MaterialDesignController::IsSecondaryUiMaterial()) {
     label = WebsiteSettingsUI::PermissionActionToUIString(
-        profile, info.type, info.setting, effective_default_setting,
+        profile, info.type, CONTENT_SETTING_DEFAULT, effective_default_setting,
         info.source);
   }
 
   AddCheckItem(CONTENT_SETTING_DEFAULT, label);
 
+  // Retrieve the string to show for allowing the permission.
   // Notifications does not support CONTENT_SETTING_ALLOW in incognito.
   bool allow_disabled_for_notifications =
       permission_.is_incognito &&
@@ -86,6 +88,11 @@ PermissionMenuModel::PermissionMenuModel(
       (!is_media_permission || content::IsOriginSecure(url))) {
     label = l10n_util::GetStringUTF16(
         IDS_WEBSITE_SETTINGS_MENU_ITEM_ALLOW);
+    if (ui::MaterialDesignController::IsSecondaryUiMaterial()) {
+      label = WebsiteSettingsUI::PermissionActionToUIString(
+          profile, info.type, CONTENT_SETTING_ALLOW, effective_default_setting,
+          info.source);
+    }
     AddCheckItem(CONTENT_SETTING_ALLOW, label);
   }
 
@@ -99,7 +106,13 @@ PermissionMenuModel::PermissionMenuModel(
     AddCheckItem(CONTENT_SETTING_DETECT_IMPORTANT_CONTENT, label);
   }
 
+  // Retrieve the string to show for blocking the permission.

[palmer, 2017/03/07 23:39:46]

Nit: These comments suggest that moving this code into well-named helper
functions might clarify the intent of the code. E.g.

  label = GetBlockingPermissionLabel(profile, info.type, CONTENT_SETTING_BLOCK,
                                     effective_default_setting, info.source);

or such.

I could be wrong; your call. :)

[Patti Lor, 2017/03/08 06:58:41]

Yeah, I did consider this - it's pretty simple for the "blocked" permission
type, but looking at "allowed" and "default" it's a lot more unclear - "allowed"
permissions aren't added to the menu at all based on certain conditions, and
default permissions are a lot more complicated since you also need access to the
HostContentSettingsMapFactory etc, resulting in them all having pretty different
function signatures (and adding them as methods is probably also bad)... so I
decided just to leave it after adding the comments.

Thanks for the suggestion though - just giving some context into why I decided
not to do that in case you were interested -- let me know if you still think
it's worth doing.

   label = l10n_util::GetStringUTF16(IDS_WEBSITE_SETTINGS_MENU_ITEM_BLOCK);
+  if (ui::MaterialDesignController::IsSecondaryUiMaterial()) {
+    label = WebsiteSettingsUI::PermissionActionToUIString(
+        profile, info.type, CONTENT_SETTING_BLOCK, effective_default_setting,
+        info.source);
+  }
   AddCheckItem(CONTENT_SETTING_BLOCK, label);
 }