Implement desktop UI for the WebUSB guard permission

This patch provides UI controls for the new WebUSB guard permission in
the page info popup and on the site settings pages.

Screenshots of the new UI are included in the design doc:
https://goo.gl/uMPMZZ

Bug: 771703
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I4c9dc2cc59a589e558ceb346982d493ac66dc674
Reviewed-on: https://chromium-review.googlesource.com/982595Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: Raymes Khoury <raymes@chromium.org>
Reviewed-by: Emily Stark <estark@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Dave Schuyler <dschuyler@chromium.org>
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548866}

chrome/app/settings_strings.grdp

@@ -2566,9 +2566,18 @@
   <message name="IDS_SETTINGS_SITE_SETTINGS_SENSORS" desc="Label for the sensors permission in Site Settings. Sensors are motion and light sensors, specifically accelerometers, gyroscope, magnetometers, and ambient-light sensors">
     Motion or light sensors
   </message>
-  <message name="IDS_SETTINGS_SITE_SETTINGS_USB_DEVICES" desc="Label for the USB devices in site settings.">
+  <message name="IDS_SETTINGS_SITE_SETTINGS_USB_DEVICES" desc="Label for USB devices in site settings.">
     USB devices
   </message>
+  <message name="IDS_SETTINGS_SITE_SETTINGS_USB_DEVICES_ASK" desc="The ask label for USB devices in site settings.">
+    Ask when a site wants to access USB devices
+  </message>
+  <message name="IDS_SETTINGS_SITE_SETTINGS_USB_DEVICES_ASK_RECOMMENDED" desc="The ask label for USB devices in site settings (with the 'recommended' suffix).">
+    Ask when a site wants to access USB devices (recommended)
+  </message>
+  <message name="IDS_SETTINGS_SITE_SETTINGS_USB_DEVICES_BLOCK" desc="The block label for USB devices in site settings.">
+    Do not allow any sites to access USB devices
+  </message>
   <message name="IDS_SETTINGS_SITE_SETTINGS_REMOVE_ZOOM_LEVEL" desc="Title tooltip and accessibility text for the button to remove zoom levels in site settings">
     Remove zoom level
   </message>

chrome/app/settings_strings_grdp/IDS_SETTINGS_SITE_SETTINGS_USB_DEVICES.png.sha1

+68d98edb1bc643b1f47b1f7a2a837b8300b6d111
\ No newline at end of file

chrome/app/settings_strings_grdp/IDS_SETTINGS_SITE_SETTINGS_USB_DEVICES_ASK.png.sha1

+68d98edb1bc643b1f47b1f7a2a837b8300b6d111
\ No newline at end of file

chrome/app/settings_strings_grdp/IDS_SETTINGS_SITE_SETTINGS_USB_DEVICES_ASK_RECOMMENDED.png.sha1

+b2cd1afb8e2c56fc0aebef7b3a30589d1049388c
\ No newline at end of file

chrome/app/settings_strings_grdp/IDS_SETTINGS_SITE_SETTINGS_USB_DEVICES_BLOCK.png.sha1

+e0f9d36f6850e446d429f123fb2919bdd9903ce5
\ No newline at end of file

chrome/browser/content_settings/web_site_settings_uma_util.cc

@@ -21,6 +21,9 @@ void WebSiteSettingsUmaUtil::LogPermissionChange(ContentSettingsType type,
   } else if (setting == ContentSetting::CONTENT_SETTING_BLOCK) {
     UMA_HISTOGRAM_EXACT_LINEAR("WebsiteSettings.Menu.PermissionChanged.Blocked",
                                histogram_value, num_values);
+  } else if (setting == ContentSetting::CONTENT_SETTING_ASK) {
+    UMA_HISTOGRAM_EXACT_LINEAR("WebsiteSettings.Menu.PermissionChanged.Ask",
+                               histogram_value, num_values);
   } else if (setting == ContentSetting::CONTENT_SETTING_DEFAULT) {
     UMA_HISTOGRAM_EXACT_LINEAR("WebsiteSettings.Menu.PermissionChanged.Reset",
                                histogram_value, num_values);

chrome/browser/resources/settings/privacy_page/privacy_page.html

@@ -481,6 +481,12 @@
       </template>
       <template is="dom-if" route-path="/content/usbDevices" no-search>
         <settings-subpage page-title="$i18n{siteSettingsUsbDevices}">
+          <category-default-setting
+              toggle-off-label="$i18n{siteSettingsUsbDevicesBlock}"
+              toggle-on-label=
+                  "$i18n{siteSettingsUsbDevicesAskRecommended}"
+              category="{{ContentSettingsTypes.USB_DEVICES}}">
+          </category-default-setting>
           <usb-devices></usb-devices>
         </settings-subpage>
       </template>

chrome/browser/resources/settings/site_settings/all_sites.js

@@ -51,7 +51,6 @@ Polymer({
         continue;
       // </if>
       if (type == settings.ContentSettingsTypes.PROTOCOL_HANDLERS ||
-          type == settings.ContentSettingsTypes.USB_DEVICES ||
           type == settings.ContentSettingsTypes.ZOOM_LEVELS) {
         // Some categories store their data in a custom way.
         continue;

chrome/browser/resources/settings/site_settings/category_default_setting.js

@@ -116,6 +116,7 @@ Polymer({
       case settings.ContentSettingsTypes.NOTIFICATIONS:
       case settings.ContentSettingsTypes.UNSANDBOXED_PLUGINS:
       case settings.ContentSettingsTypes.MIDI_DEVICES:
+      case settings.ContentSettingsTypes.USB_DEVICES:
         // "Ask" vs "Blocked".
         this.browserProxy.setDefaultValueForContentType(
             this.category,

chrome/browser/resources/settings/site_settings/constants.js

@@ -28,7 +28,7 @@ settings.ContentSettingsTypes = {
   AUTOMATIC_DOWNLOADS: 'multiple-automatic-downloads',
   BACKGROUND_SYNC: 'background-sync',
   MIDI_DEVICES: 'midi-sysex',
-  USB_DEVICES: 'usb-chooser-data',
+  USB_DEVICES: 'usb-devices',
   ZOOM_LEVELS: 'zoom-levels',
   PROTECTED_CONTENT: 'protected-content',
   ADS: 'ads',

chrome/browser/resources/settings/site_settings/site_details.html

@@ -139,6 +139,10 @@
           category="{{ContentSettingsTypes.MIDI_DEVICES}}" icon="settings:midi"
           id="midiDevices" label="$i18n{siteSettingsMidiDevices}">
       </site-details-permission>
+      <site-details-permission
+          category="{{ContentSettingsTypes.USB_DEVICES}}" icon="settings:usb"
+          id="usbDevices" label="$i18n{siteSettingsUsbDevices}">
+      </site-details-permission>
       <site-details-permission
           category="{{ContentSettingsTypes.UNSANDBOXED_PLUGINS}}"
           icon="cr:extension" id="unsandboxedPlugins"

chrome/browser/resources/settings/site_settings/site_details_permission.html

@@ -56,14 +56,16 @@
                   '$i18nPolymer{siteSettingsActionAllowDefault}',
                   '$i18nPolymer{siteSettingsActionBlockDefault}')]]
             </option>
-            <option id="allow" value$="[[ContentSetting.ALLOW]]">
+            <option id="allow" value$="[[ContentSetting.ALLOW]]"
+                hidden$="[[!showAllowedSetting_(category)]]">
               $i18n{siteSettingsActionAllow}
             </option>
             <option id="block" value$="[[ContentSetting.BLOCK]]">
               $i18n{siteSettingsActionBlock}
             </option>
             <option id="ask" value$="[[ContentSetting.ASK]]"
-                hidden$="[[!isNonDefaultAsk_(site.setting, site.source)]]">
+                hidden$="[[!showAskSetting_(category, site.setting,
+                                            site.source)]]">
               $i18n{siteSettingsActionAsk}
             </option>
           </select>

chrome/browser/resources/settings/site_settings/site_details_permission.js

@@ -171,6 +171,32 @@ Polymer({
         source == settings.SiteSettingSource.INSECURE_ORIGIN);
   },
 
+  /**
+   * Returns true if the 'allow' option should be shown.
+   * @param {!settings.ContentSettingsTypes} category The permission type.
+   * @return {boolean}
+   * @private
+   */
+  showAllowedSetting_: function(category) {
+    return category != settings.ContentSettingsTypes.USB_DEVICES;
+  },
+
+  /**
+   * Returns true if the 'ask' option should be shown.
+   * @param {!settings.ContentSettingsTypes} category The permission type.
+   * @param {!settings.ContentSetting} setting The setting of the permission.
+   * @param {!settings.SiteSettingSource} source The source of the permission.
+   * @return {boolean}
+   * @private
+   */
+  showAskSetting_: function(category, setting, source) {
+    // For chooser-based permissions 'ask' takes the place of 'allow'.
+    if (category == settings.ContentSettingsTypes.USB_DEVICES)
+      return true;
+
+    return this.isNonDefaultAsk_(setting, source);
+  },
+
   /**
    * Returns true if the permission is set to a non-default 'ask'. Currently,
    * this only gets called when |this.site| is updated.

chrome/browser/resources/settings/site_settings_page/site_settings_page.html

@@ -365,14 +365,23 @@
         <button aria-label="$i18n{siteSettingsZoomLevels}"></button>
       </paper-icon-button-light>
     </div>
-    <div id="usb-devices" class="settings-box"
+    <div id="usb-devices" class="settings-box two-line"
         category$="[[ContentSettingsTypes.USB_DEVICES]]"
         data-route="SITE_SETTINGS_USB_DEVICES" on-click="onTapNavigate_"
         actionable>
       <iron-icon icon="settings:usb"></iron-icon>
-      <div class="middle">$i18n{siteSettingsUsbDevices}</div>
+      <div class="middle">
+        $i18n{siteSettingsUsbDevices}
+        <div class="secondary" id="usbDevicesSecondary">
+          [[defaultSettingLabel_(
+              default_.usbDevices,
+              '$i18nPolymer{siteSettingsUsbDevicesAsk}',
+              '$i18nPolymer{siteSettingsUsbDevicesBlock}')]]
+        </div>
+      </div>
       <paper-icon-button-light class="subpage-arrow">
-        <button aria-label="$i18n{siteSettingsUsbDevices}"></button>
+        <button aria-label="$i18n{siteSettingsUsbDevices}"
+            area-describedby="usbDevicesSecondary"></button>
       </paper-icon-button-light>
     </div>
     <div id="pdf-documents" class="settings-box"

chrome/browser/resources/settings/site_settings_page/site_settings_page.js

@@ -149,9 +149,8 @@ Polymer({
     const keys = Object.keys(settings.ContentSettingsTypes);
     for (let i = 0; i < keys.length; ++i) {
       const key = settings.ContentSettingsTypes[keys[i]];
-      // Default labels are not applicable to USB and ZOOM.
-      if (key == settings.ContentSettingsTypes.USB_DEVICES ||
-          key == settings.ContentSettingsTypes.ZOOM_LEVELS)
+      // Default labels are not applicable to ZOOM.
+      if (key == settings.ContentSettingsTypes.ZOOM_LEVELS)
         continue;
       // Protocol handlers are not available (and will DCHECK) in guest mode.
       if (this.isGuest_ &&

chrome/browser/ui/page_info/page_info.cc

@@ -118,6 +118,7 @@ ContentSettingsType kPermissionType[] = {
     CONTENT_SETTINGS_TYPE_AUTOPLAY,
     CONTENT_SETTINGS_TYPE_MIDI_SYSEX,
     CONTENT_SETTINGS_TYPE_CLIPBOARD_READ,
+    CONTENT_SETTINGS_TYPE_USB_GUARD,
 };
 
 // Checks whether this permission is currently the factory default, as set by
@@ -835,6 +836,12 @@ void PageInfo::PresentSitePermissions() {
   for (const ChooserUIInfo& ui_info : kChooserUIInfo) {
     ChooserContextBase* context = ui_info.get_context(profile_);
     const GURL origin = site_url_.GetOrigin();
+
+    // Hide individual object permissions because when the chooser is blocked
+    // previously granted device permissions are also ignored.
+    if (!context->CanRequestObjectPermission(origin, origin))
+      continue;
+
     auto chosen_objects = context->GetGrantedObjects(origin, origin);
     for (std::unique_ptr<base::DictionaryValue>& object : chosen_objects) {
       chosen_object_info_list.push_back(

chrome/browser/ui/page_info/page_info_ui.cc

@@ -129,6 +129,7 @@ const PermissionsUIInfo kPermissionsUIInfo[] = {
     {CONTENT_SETTINGS_TYPE_SOUND, IDS_PAGE_INFO_TYPE_SOUND},
     {CONTENT_SETTINGS_TYPE_CLIPBOARD_READ, IDS_PAGE_INFO_TYPE_CLIPBOARD},
     {CONTENT_SETTINGS_TYPE_SENSORS, IDS_PAGE_INFO_TYPE_SENSORS},
+    {CONTENT_SETTINGS_TYPE_USB_GUARD, IDS_PAGE_INFO_TYPE_USB},
 };
 
 std::unique_ptr<PageInfoUI::SecurityDescription> CreateSecurityDescription(
@@ -475,6 +476,9 @@ const gfx::ImageSkia PageInfoUI::GetPermissionIcon(const PermissionInfo& info,
     case CONTENT_SETTINGS_TYPE_SENSORS:
       icon = &kSensorsIcon;
       break;
+    case CONTENT_SETTINGS_TYPE_USB_GUARD:
+      icon = &vector_icons::kUsbIcon;
+      break;
     default:
       // All other |ContentSettingsType|s do not have icons on desktop or are
       // not shown in the Page Info bubble.

chrome/browser/ui/page_info/permission_menu_model.cc

@@ -73,16 +73,7 @@ PermissionMenuModel::PermissionMenuModel(Profile* profile,
   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 &&
-      permission_.type == CONTENT_SETTINGS_TYPE_NOTIFICATIONS;
-  // Media only supports CONTENT_SETTTING_ALLOW for secure origins.
-  bool is_media_permission =
-      permission_.type == CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC ||
-      permission_.type == CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA;
-  if (!allow_disabled_for_notifications &&
-      (!is_media_permission || content::IsOriginSecure(url))) {
+  if (ShouldShowAllow(url)) {
     label = l10n_util::GetStringUTF16(IDS_PAGE_INFO_MENU_ITEM_ALLOW);
     if (ui::MaterialDesignController::IsSecondaryUiMaterial()) {
       label = PageInfoUI::PermissionActionToUIString(
@@ -113,6 +104,18 @@ PermissionMenuModel::PermissionMenuModel(Profile* profile,
         info.source);
   }
   AddCheckItem(CONTENT_SETTING_BLOCK, label);
+
+  // Retrieve the string to show for allowing the user to be asked about the
+  // permission.
+  if (ShouldShowAsk(url)) {
+    label = l10n_util::GetStringUTF16(IDS_PAGE_INFO_MENU_ITEM_ASK);
+    if (ui::MaterialDesignController::IsSecondaryUiMaterial()) {
+      label = PageInfoUI::PermissionActionToUIString(
+          profile, info.type, CONTENT_SETTING_ASK, effective_default_setting,
+          info.source);
+    }
+    AddCheckItem(CONTENT_SETTING_ASK, label);
+  }
 }
 
 PermissionMenuModel::~PermissionMenuModel() {}
@@ -136,3 +139,28 @@ void PermissionMenuModel::ExecuteCommand(int command_id, int event_flags) {
   permission_.setting = static_cast<ContentSetting>(command_id);
   callback_.Run(permission_);
 }
+
+bool PermissionMenuModel::ShouldShowAllow(const GURL& url) {
+  // Notifications does not support CONTENT_SETTING_ALLOW in incognito.
+  if (permission_.is_incognito &&
+      permission_.type == CONTENT_SETTINGS_TYPE_NOTIFICATIONS) {
+    return false;
+  }
+
+  // Media only supports CONTENT_SETTING_ALLOW for secure origins.
+  if ((permission_.type == CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC ||
+       permission_.type == CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA) &&
+      !content::IsOriginSecure(url)) {
+    return false;
+  }
+
+  // Chooser permissions do not support CONTENT_SETTING_ALLOW.
+  if (permission_.type == CONTENT_SETTINGS_TYPE_USB_GUARD)
+    return false;
+
+  return true;
+}
+
+bool PermissionMenuModel::ShouldShowAsk(const GURL& url) {
+  return permission_.type == CONTENT_SETTINGS_TYPE_USB_GUARD;
+}

chrome/browser/ui/page_info/permission_menu_model.h

@@ -34,6 +34,9 @@ class PermissionMenuModel : public ui::SimpleMenuModel,
   void ExecuteCommand(int command_id, int event_flags) override;
 
  private:
+  bool ShouldShowAllow(const GURL& url);
+  bool ShouldShowAsk(const GURL& url);
+
   HostContentSettingsMap* host_content_settings_map_;
 
   // The permission info represented by the menu model.

chrome/browser/ui/page_info/permission_menu_model_unittest.cc

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/page_info/permission_menu_model.h"
+#include "base/strings/utf_string_conversions.h"
 #include "chrome/test/base/testing_profile.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -42,6 +43,7 @@ TEST_F(PermissionMenuModelTest, TestDefault) {
   permission.default_setting = CONTENT_SETTING_ALLOW;
   permission.source = content_settings::SETTING_SOURCE_USER;
   permission.is_incognito = false;
+
   PermissionMenuModel model(profile(), GURL("http://www.google.com"),
                             permission, callback.callback());
   EXPECT_EQ(3, model.GetItemCount());
@@ -82,3 +84,17 @@ TEST_F(PermissionMenuModelTest, TestIncognitoNotifications) {
                                       permission, callback.callback());
   EXPECT_EQ(2, incognito_model.GetItemCount());
 }
+
+TEST_F(PermissionMenuModelTest, TestUsbGuard) {
+  TestCallback callback;
+  PageInfoUI::PermissionInfo permission;
+  permission.type = CONTENT_SETTINGS_TYPE_USB_GUARD;
+  permission.setting = CONTENT_SETTING_ASK;
+  permission.default_setting = CONTENT_SETTING_ASK;
+  permission.source = content_settings::SETTING_SOURCE_USER;
+  permission.is_incognito = false;
+
+  PermissionMenuModel model(profile(), GURL("http://www.google.com"),
+                            permission, callback.callback());
+  EXPECT_EQ(3, model.GetItemCount());
+}

chrome/browser/ui/views/page_info/page_info_bubble_view_unittest.cc

@@ -118,7 +118,7 @@ class PageInfoBubbleViewTestApi {
     views::View* view = GetPermissionSelectorAt(selector_index)->button();
     DCHECK_EQ(views::Combobox::kViewClassName, view->GetClassName());
     views::Combobox* combobox = static_cast<views::Combobox*>(view);
-    combobox->SetSelectedIndex(menu_index);
+    combobox->SetSelectedRow(menu_index);
   }
 
   // Simulates updating the number of cookies.
@@ -314,6 +314,10 @@ TEST_F(PageInfoBubbleViewTest, SetPermissionInfo) {
 
   // However, since the setting is now default, recreating the dialog with those
   // settings should omit the permission from the UI.
+  //
+  // TODO(https://crbug.com/829576): Reconcile the comment above with the fact
+  // that |num_expected_children| is not, at this point, 0 and therefore the
+  // permission is not being omitted from the UI.
   api_->SetPermissionInfo(list);
   EXPECT_EQ(num_expected_children, api_->permissions_view()->child_count());
 }
@@ -358,6 +362,74 @@ TEST_F(PageInfoBubbleViewTest, SetPermissionInfoWithUsbDevice) {
   EXPECT_FALSE(store->HasDevicePermission(origin, origin, device));
 }
 
+TEST_F(PageInfoBubbleViewTest, SetPermissionInfoForUsbGuard) {
+  // This test exercises PermissionSelectorRow in a way that it is not used in
+  // practice. In practice, every setting in PermissionSelectorRow starts off
+  // "set", so there is always one option checked in the resulting MenuModel.
+  // This test creates settings that are left at their defaults, leading to zero
+  // checked options, and checks that the text on the MenuButtons is right. On
+  // Mac, the behavior matches, but only when SecondaryUiMd is enabled.
+  const bool is_md = base::FeatureList::IsEnabled(features::kSecondaryUiMd);
+#if defined(OS_MACOSX)
+  if (!is_md)
+    return;
+#endif
+
+  PermissionInfoList list(1);
+  list.back().type = CONTENT_SETTINGS_TYPE_USB_GUARD;
+  list.back().source = content_settings::SETTING_SOURCE_USER;
+  list.back().is_incognito = false;
+  list.back().setting = CONTENT_SETTING_ASK;
+
+  // Initially, no permissions are shown because they are all set to default.
+  int num_expected_children = 0;
+  EXPECT_EQ(num_expected_children, api_->permissions_view()->child_count());
+
+  // Verify calling SetPermissionInfo() directly updates the UI.
+  num_expected_children += kViewsPerPermissionRow * list.size();
+  list.back().setting = CONTENT_SETTING_BLOCK;
+  api_->SetPermissionInfo(list);
+  EXPECT_EQ(base::ASCIIToUTF16("Block"), api_->GetPermissionButtonTextAt(0));
+
+  // Simulate a user selection via the UI. Note this will also cover logic in
+  // PageInfo to update the pref.
+  if (is_md) {
+    // Under MD, the changed setting is always read from the combobox selected
+    // index when changed in the UI. PermissionSelectorRow::PermissionChanged()
+    // ignores the argument, except to detect whether it is the default.
+    api_->SimulateUserSelectingComboboxItemAt(0, 2);
+  } else {
+    list.back().setting = CONTENT_SETTING_ASK;
+    api_->GetPermissionSelectorAt(0)->PermissionChanged(list.back());
+  }
+  EXPECT_EQ(num_expected_children, api_->permissions_view()->child_count());
+  EXPECT_EQ(base::ASCIIToUTF16("Ask"), api_->GetPermissionButtonTextAt(0));
+
+  // Setting to the default via the UI should keep the button around.
+  if (is_md) {
+    api_->SimulateUserSelectingComboboxItemAt(0, 0);
+    // Under MD, PermissionMenuModel gets strings using PageInfoUI::
+    // PermissionActionToUIString(..) rather than directly from the
+    // ResourceBundle.
+    EXPECT_EQ(base::ASCIIToUTF16("Ask (default)"),
+              api_->GetPermissionButtonTextAt(0));
+  } else {
+    list.back().setting = CONTENT_SETTING_ASK;
+    api_->GetPermissionSelectorAt(0)->PermissionChanged(list.back());
+    EXPECT_EQ(base::ASCIIToUTF16("Ask"), api_->GetPermissionButtonTextAt(0));
+  }
+  EXPECT_EQ(num_expected_children, api_->permissions_view()->child_count());
+
+  // However, since the setting is now default, recreating the dialog with those
+  // settings should omit the permission from the UI.
+  //
+  // TODO(https://crbug.com/829576): Reconcile the comment above with the fact
+  // that |num_expected_children| is not, at this point, 0 and therefore the
+  // permission is not being omitted from the UI.
+  api_->SetPermissionInfo(list);
+  EXPECT_EQ(num_expected_children, api_->permissions_view()->child_count());
+}
+
 // Test that updating the number of cookies used by the current page doesn't add
 // any extra views to Page Info.
 TEST_F(PageInfoBubbleViewTest, UpdatingSiteDataRetainsLayout) {

chrome/browser/ui/views/page_info/permission_selector_row.cc

@@ -175,13 +175,15 @@ class ComboboxModelAdapter : public ui::ComboboxModel {
 };
 
 void ComboboxModelAdapter::OnPerformAction(int index) {
-  model_->ExecuteCommand(index, 0);
+  int command_id = model_->GetCommandIdAt(index);
+  model_->ExecuteCommand(command_id, 0);
 }
 
 int ComboboxModelAdapter::GetCheckedIndex() {
   int checked_index = -1;
   for (int i = 0; i < model_->GetItemCount(); ++i) {
-    if (model_->IsCommandIdChecked(i)) {
+    int command_id = model_->GetCommandIdAt(i);
+    if (model_->IsCommandIdChecked(command_id)) {
       // This function keeps track of |checked_index| instead of returning early
       // here so that it can DCHECK that there's exactly one selected item,
       // which is not normally guaranteed by MenuModel, but *is* true of

chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc

@@ -2071,6 +2071,11 @@ void AddSiteSettingsStrings(content::WebUIDataSource* html_source,
     {"siteSettingsMidiDevicesBlock",
      IDS_SETTINGS_SITE_SETTINGS_MIDI_DEVICES_BLOCK},
     {"siteSettingsUsbDevices", IDS_SETTINGS_SITE_SETTINGS_USB_DEVICES},
+    {"siteSettingsUsbDevicesAsk", IDS_SETTINGS_SITE_SETTINGS_USB_DEVICES_ASK},
+    {"siteSettingsUsbDevicesAskRecommended",
+     IDS_SETTINGS_SITE_SETTINGS_USB_DEVICES_ASK_RECOMMENDED},
+    {"siteSettingsUsbDevicesBlock",
+     IDS_SETTINGS_SITE_SETTINGS_USB_DEVICES_BLOCK},
     {"siteSettingsRemoveZoomLevel",
      IDS_SETTINGS_SITE_SETTINGS_REMOVE_ZOOM_LEVEL},
     {"siteSettingsZoomLevels", IDS_SETTINGS_SITE_SETTINGS_ZOOM_LEVELS},

chrome/browser/ui/webui/site_settings_helper.cc

@@ -76,6 +76,7 @@ const ContentSettingsTypeNameEntry kContentSettingsTypeGroupNames[] = {
     {CONTENT_SETTINGS_TYPE_CLIPBOARD_READ, "clipboard"},
     {CONTENT_SETTINGS_TYPE_SENSORS, "sensors"},
     {CONTENT_SETTINGS_TYPE_PAYMENT_HANDLER, "payment-handler"},
+    {CONTENT_SETTINGS_TYPE_USB_GUARD, "usb-devices"},
 
     // Add new content settings here if a corresponding Javascript string
     // representation for it is not required. Note some exceptions, such as
@@ -101,7 +102,6 @@ const ContentSettingsTypeNameEntry kContentSettingsTypeGroupNames[] = {
     {CONTENT_SETTINGS_TYPE_ACCESSIBILITY_EVENTS, nullptr},
     {CONTENT_SETTINGS_TYPE_CLIPBOARD_WRITE, nullptr},
     {CONTENT_SETTINGS_TYPE_PLUGINS_DATA, nullptr},
-    {CONTENT_SETTINGS_TYPE_USB_GUARD, nullptr},
 };
 static_assert(arraysize(kContentSettingsTypeGroupNames) ==
                   // ContentSettingsType starts at -1, so add 1 here.

chrome/test/data/webui/settings/site_details_tests.js

@@ -86,6 +86,9 @@ suite('SiteDetails', function() {
       test_util.createContentSettingTypeToValuePair(
           settings.ContentSettingsTypes.PAYMENT_HANDLER,
           [test_util.createRawSiteException('https://foo.com:443')]),
+      test_util.createContentSettingTypeToValuePair(
+          settings.ContentSettingsTypes.USB_DEVICES,
+          [test_util.createRawSiteException('https://foo.com:443')]),
     ]);
 
     browserProxy = new TestSiteSettingsPrefsBrowserProxy();
@@ -106,7 +109,6 @@ suite('SiteDetails', function() {
     const nonSiteDetailsContentSettingsTypes = [
       settings.ContentSettingsTypes.COOKIES,
       settings.ContentSettingsTypes.PROTOCOL_HANDLERS,
-      settings.ContentSettingsTypes.USB_DEVICES,
       settings.ContentSettingsTypes.ZOOM_LEVELS,
     ];
     if (!cr.isChromeOS)

chrome/test/data/webui/settings/test_util.js

@@ -163,6 +163,8 @@ cr.define('test_util', function() {
         settings.ContentSetting.BLOCK;
     defaults[settings.ContentSettingsTypes.SENSORS].setting =
         settings.ContentSetting.ALLOW;
+    defaults[settings.ContentSettingsTypes.USB_DEVICES].setting =
+        settings.ContentSetting.ASK;
     defaultsList.forEach((override) => {
       defaults[override.setting] = override.value;
     });

components/OWNERS

@@ -14,10 +14,8 @@ per-file dom_distiller_strings.grdp=file://components/dom_distiller/OWNERS
 per-file error_page_strings.grdp=file://components/error_page/OWNERS
 per-file ntp_snippets_strings.grdp=file://components/ntp_snippets/OWNERS
 per-file omnibox_strings.grdp=file://components/omnibox/OWNERS
-per-file page_info_strings.grdp=estark@chromium.org
-per-file page_info_strings.grdp=felt@chromium.org
-per-file page_info_strings.grdp=meacer@chromium.org
-per-file page_info_strings.grdp=patricialor@chromium.org
+per-file page_info_strings.grdp=file://chrome/browser/ui/page_info/OWNERS
+per-file page_info_strings_grdp=file://chrome/browser/ui/page_info/OWNERS
 per-file password_manager_strings.grdp=file://components/password_manager/OWNERS
 per-file payments_strings.grdp=file://components/payments/OWNERS
 per-file pdf_strings.grdp=raymes@chromium.org

components/page_info_strings.grdp

@@ -242,6 +242,9 @@
     <message name="IDS_PAGE_INFO_TYPE_SENSORS" desc="The label used for the sensor permission controls in the Page Info popup. Title case format.">
       Motion or Light Sensors
     </message>
+    <message name="IDS_PAGE_INFO_TYPE_USB" desc="The label used for the USB permission controls in the Page Info popup.">
+      USB devices
+    </message>
     <!-- TODO(crbug.com/716303): A few permissions are missing here. -->
 
     <!-- Permission values -->
@@ -290,7 +293,10 @@
     <message name="IDS_PAGE_INFO_MENU_ITEM_BLOCK" desc="The text of the menu item of a permissions menu on the Page Info UI that sets the setting to allow.">
       Always block on this site
     </message>
-    <message name="IDS_PAGE_INFO_MENU_ITEM_DETECT_IMPORTANT_CONTENT" desc="The text of the menu item of a permissions menu on the Page Info UI that sets the setting to detect important content.">
+    <message name="IDS_PAGE_INFO_MENU_ITEM_ASK" desc="The text of the menu item of a permissions menu on the Page Info UI that sets the setting to ask.">
+      Always ask on this site
+    </message>
+     <message name="IDS_PAGE_INFO_MENU_ITEM_DETECT_IMPORTANT_CONTENT" desc="The text of the menu item of a permissions menu on the Page Info UI that sets the setting to detect important content.">
       Always detect important content on this site
     </message>
     <message name="IDS_PAGE_INFO_MENU_ITEM_ADS_BLOCK" desc="The text of the menu item of a permissions menu on the Page Info UI for the ads permission in Block mode">

components/page_info_strings_grdp/IDS_PAGE_INFO_MENU_ITEM_ASK.png.sha1

+614553f1458fbe4e2de4e581bb0850c8831cf165
\ No newline at end of file

components/page_info_strings_grdp/IDS_PAGE_INFO_TYPE_USB.png.sha1

+614553f1458fbe4e2de4e581bb0850c8831cf165
\ No newline at end of file

tools/metrics/histograms/histograms.xml

@@ -104679,6 +104679,15 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
   </summary>
 </histogram>
 
+<histogram name="WebsiteSettings.Menu.PermissionChanged.Ask" enum="ContentType">
+  <owner>miguelg@chromium.org</owner>
+  <owner>finnur@chromium.org</owner>
+  <summary>
+    Count of how often a specific content type (permission) is set to 'Ask'
+    using the content settings menu.
+  </summary>
+</histogram>
+
 <histogram name="WebsiteSettings.Menu.PermissionChanged.Blocked"
     enum="ContentType">
   <owner>miguelg@chromium.org</owner>