Permissions: Show the reason for permission decisions made on the user's behalf.

chrome/browser/ui/cocoa/page_info/website_settings_bubble_controller_unittest.mm

Index: chrome/browser/ui/cocoa/page_info/website_settings_bubble_controller_unittest.mm
diff --git a/chrome/browser/ui/cocoa/page_info/website_settings_bubble_controller_unittest.mm b/chrome/browser/ui/cocoa/page_info/website_settings_bubble_controller_unittest.mm
index 45dbb1dffa3bb3ec89abbcb248cad31745e943b1..fc511fb559fd705254b6a658af8a89498ad044f2 100644
--- a/chrome/browser/ui/cocoa/page_info/website_settings_bubble_controller_unittest.mm
+++ b/chrome/browser/ui/cocoa/page_info/website_settings_bubble_controller_unittest.mm
@@ -63,39 +63,32 @@ enum PermissionMenuIndices {
 };
 
 const ContentSettingsType kTestPermissionTypes[] = {
-  CONTENT_SETTINGS_TYPE_IMAGES,
-  CONTENT_SETTINGS_TYPE_JAVASCRIPT,
-  CONTENT_SETTINGS_TYPE_PLUGINS,
-  CONTENT_SETTINGS_TYPE_POPUPS,
-  CONTENT_SETTINGS_TYPE_GEOLOCATION,
-  CONTENT_SETTINGS_TYPE_NOTIFICATIONS,
-  CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC
-};
+    CONTENT_SETTINGS_TYPE_IMAGES,
+    CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA,
+    CONTENT_SETTINGS_TYPE_JAVASCRIPT,
+    CONTENT_SETTINGS_TYPE_PLUGINS,
+    CONTENT_SETTINGS_TYPE_POPUPS,
+    CONTENT_SETTINGS_TYPE_GEOLOCATION,
+    CONTENT_SETTINGS_TYPE_NOTIFICATIONS,
+    CONTENT_SETTINGS_TYPE_MEDIASTREAM_MIC};
 
 const ContentSetting kTestSettings[] = {
-  CONTENT_SETTING_DEFAULT,
-  CONTENT_SETTING_DEFAULT,
-  CONTENT_SETTING_ALLOW,
-  CONTENT_SETTING_BLOCK,
-  CONTENT_SETTING_ALLOW,
-  CONTENT_SETTING_BLOCK,
-  CONTENT_SETTING_BLOCK
-};
+    CONTENT_SETTING_DEFAULT, CONTENT_SETTING_DEFAULT, CONTENT_SETTING_DEFAULT,
+    CONTENT_SETTING_ALLOW,   CONTENT_SETTING_BLOCK,   CONTENT_SETTING_ALLOW,
+    CONTENT_SETTING_BLOCK,   CONTENT_SETTING_BLOCK};
 
 const ContentSetting kTestDefaultSettings[] = {
-  CONTENT_SETTING_BLOCK,
-  CONTENT_SETTING_ASK
-};
+    CONTENT_SETTING_ALLOW, CONTENT_SETTING_BLOCK, CONTENT_SETTING_ASK};
 
 const content_settings::SettingSource kTestSettingSources[] = {
-  content_settings::SETTING_SOURCE_USER,
-  content_settings::SETTING_SOURCE_USER,
-  content_settings::SETTING_SOURCE_USER,
-  content_settings::SETTING_SOURCE_USER,
-  content_settings::SETTING_SOURCE_POLICY,
-  content_settings::SETTING_SOURCE_POLICY,
-  content_settings::SETTING_SOURCE_EXTENSION
-};
+    content_settings::SETTING_SOURCE_USER,
+    content_settings::SETTING_SOURCE_USER,
+    content_settings::SETTING_SOURCE_USER,
+    content_settings::SETTING_SOURCE_USER,
+    content_settings::SETTING_SOURCE_USER,
+    content_settings::SETTING_SOURCE_POLICY,
+    content_settings::SETTING_SOURCE_POLICY,
+    content_settings::SETTING_SOURCE_EXTENSION};
 
 class WebsiteSettingsBubbleControllerTest : public CocoaTest {
  public:
@@ -181,7 +174,7 @@ class WebsiteSettingsBubbleControllerTest : public CocoaTest {
     // Create a list of 5 different permissions, corresponding to all the
     // possible settings:
     // - [allow, block, ask] by default
-    // - [block, allow] * [by user, by policy, by extension]
+    // - [block, allow] by user
     PermissionInfoList permission_info_list;
     WebsiteSettingsUI::PermissionInfo info;
     for (size_t i = 0; i < arraysize(kTestPermissionTypes); ++i) {
@@ -198,6 +191,16 @@ class WebsiteSettingsBubbleControllerTest : public CocoaTest {
                                std::move(chosen_object_info_list));
   }
 
+  int NumSettingsNotSetByUser() const {
+    int num_non_user_settings = 0;
+    for (size_t i = 0; i < arraysize(kTestSettingSources); ++i) {
+      num_non_user_settings +=
+          (kTestSettingSources[i] != content_settings::SETTING_SOURCE_USER) ? 1
+                                                                            : 0;
+    }
+    return num_non_user_settings;
+  }
+
   content::TestBrowserThreadBundle thread_bundle_;
   TestingProfile profile_;
   content::TestWebContentsFactory web_contents_factory_;
@@ -254,34 +257,37 @@ TEST_F(WebsiteSettingsBubbleControllerTest, SetPermissionInfo) {
 
   // There should be three subviews per permission.
   NSArray* subviews = [[controller_ permissionsView] subviews];
-  EXPECT_EQ(arraysize(kTestPermissionTypes) * 3 , [subviews count]);
+  EXPECT_EQ(arraysize(kTestPermissionTypes) * 3,
+            [subviews count] - NumSettingsNotSetByUser());
 
-  // Ensure that there is a distinct label for each permission.
-  NSMutableSet* labels = [NSMutableSet set];
+  // Ensure that there is a label for each permission.
+  NSMutableArray* permission_labels = [NSMutableArray array];
   for (NSView* view in subviews) {
     if ([view isKindOfClass:[NSTextField class]])
-      [labels addObject:[static_cast<NSTextField*>(view) stringValue]];
+      [permission_labels
+          addObject:[static_cast<NSTextField*>(view) stringValue]];
   }
-  EXPECT_EQ(arraysize(kTestPermissionTypes), [labels count]);
+  EXPECT_EQ(arraysize(kTestPermissionTypes),
+            [permission_labels count] - NumSettingsNotSetByUser());
 
   // Ensure that the button labels are distinct, and look for the correct
   // number of disabled buttons.
   int disabled_count = 0;
-  [labels removeAllObjects];
+  NSMutableSet* button_labels = [NSMutableSet set];
   for (NSView* view in subviews) {
     if ([view isKindOfClass:[NSPopUpButton class]]) {
       NSPopUpButton* button = static_cast<NSPopUpButton*>(view);
-      [labels addObject:[[button selectedCell] title]];
+      [button_labels addObject:[[button selectedCell] title]];
 
       if (![button isEnabled])
         ++disabled_count;
     }
   }
-  EXPECT_EQ(arraysize(kTestPermissionTypes), [labels count]);
+  EXPECT_EQ(5UL, [button_labels count]);
 
-  // 3 of the buttons should be disabled -- the ones that have a setting source
-  // of SETTING_SOURCE_POLICY or SETTING_SOURCE_EXTENSION.
-  EXPECT_EQ(3, disabled_count);
+  // All non-user set settings should have disabled buttons (i.e., the ones with
+  // a setting source of SETTING_SOURCE_POLICY or SETTING_SOURCE_EXTENSION).
+  EXPECT_EQ(NumSettingsNotSetByUser(), disabled_count);
 }
 

[dominickn, 2017/03/26 23:46:24]

Should you add a test for the embargoed permissions here to ensure they have
enabled buttons?

[Patti Lor, 2017/03/27 05:52:40]

I think it's possible, but since we can't tell if something is embargoed from
PermissionInfo alone, I would have to set up a way to manually set the embargo
status from this test, which might be a bit of work. Do you think it's worth
looking into / it makes sense in this file?

 TEST_F(WebsiteSettingsBubbleControllerTest, WindowWidth) {