Page Info: Hide default permissions with a value of Ask if the default is Ask.

chrome/browser/ui/website_settings/website_settings_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/ui/website_settings/website_settings_ui.h"
 
 #include "base/macros.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/plugins/plugin_utils.h"
 #include "chrome/browser/plugins/plugins_field_trial.h"
@@ skipping 266 matching lines @@
     default:
       NOTREACHED();
       return base::string16();
   }
   int button_text_id = button_text_ids[effective_setting];
   DCHECK_NE(button_text_id, kInvalidResourceID);
   return l10n_util::GetStringUTF16(button_text_id);
 }
 
 // static
+base::string16 WebsiteSettingsUI::GetPermissionButtonString(
+    VisiblePermissions visible_permissions) {

[palmer, 2017/02/24 01:55:02]

Nit: I'd expect |VisiblePermissions| to be some kind of collection of
(hypothetical) |Permissions| objects. Perhaps a better (?) name for the type
would be |PermissionsVisibility|.

[lgarron, 2017/02/24 02:15:26]

I was on the fence, but I need something that conveys description ("this is a
value that conveys that there are N permissions shown") vs. prescription ("this
is a setting that controls how many permissions are show").
PermissionsVisibility sounds like the latter.

Is AmountOfVisiblePermissions too long?

+  switch (visible_permissions) {
+    case VISIBLE_PERMISSIONS_NONE:
+      return l10n_util::GetStringUTF16(IDS_PAGE_INFO_EDIT_PERMISSIONS_LINK);
+    case VISIBLE_PERMISSIONS_SOME_BUT_NOT_ALL:

[palmer, 2017/02/24 01:55:02]

Nit: "Some" is vague. Maybe |PERMISSIONS_VISIBILITY_ONLY_NON_DEFAULT|?

[lgarron, 2017/02/24 02:15:27]

The problem with that is that there can be 0 non-default permissions (in fact,
probably by far the most common case).
Since VisiblePermissions is used to adjust the UI based on how many permissions
are actually shown, we need a good way to express that the number of permissions
shown is, well... "at least 1, but not all".
I'm open to other names that make this clear, though.

+      return l10n_util::GetStringUTF16(IDS_PAGE_INFO_SHOW_ALL_PERMISSIONS_LINK);
+    case VISIBLE_PERMISSIONS_ALL:
+      return l10n_util::GetStringUTF16(IDS_PAGE_INFO_SITE_SETTINGS_LINK);
+    default:
+      NOTREACHED();
+  }
+}
+
+// static
 int WebsiteSettingsUI::GetPermissionIconID(ContentSettingsType type,
                                            ContentSetting setting) {
   bool use_blocked = (setting == CONTENT_SETTING_BLOCK);
   for (const PermissionsUIInfo& info : kPermissionsUIInfo) {
     if (info.type == type)
       return use_blocked ? info.blocked_icon_id : info.allowed_icon_id;
   }
   NOTREACHED();
   return IDR_INFO;
 }
@@ skipping 90 matching lines @@
   }
   return resource_id;
 }
 
 // static
 const gfx::Image& WebsiteSettingsUI::GetConnectionIcon(
     WebsiteSettings::SiteConnectionStatus status) {
   ResourceBundle& rb = ResourceBundle::GetSharedInstance();
   return rb.GetNativeImageNamed(GetConnectionIconID(status));
 }