[MD settings] show blocked sites even when category is blocked

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

Index: chrome/test/data/webui/settings/site_list_tests.js
diff --git a/chrome/test/data/webui/settings/site_list_tests.js b/chrome/test/data/webui/settings/site_list_tests.js
index 3fe144759b5fb24fb6de0868d419a63b2d745d13..46c1b0d80414d459a4bebb6dbe766e8ba2c972ae 100644
--- a/chrome/test/data/webui/settings/site_list_tests.js
+++ b/chrome/test/data/webui/settings/site_list_tests.js
@@ -364,11 +364,11 @@ cr.define('site_list', function() {
 
       /**
        * Configures the test element for a particular category.
-       * @param {settings.ContentSettingsTypes} category The category to setup.
+       * @param {settings.ContentSettingsTypes} category The category to set up.
        * @param {settings.PermissionValues} subtype Type of list to use.
        * @param {Array<dictionary>} prefs The prefs to use.
        */
-      function setupCategory(category, subtype, prefs) {
+      function setUpCategory(category, subtype, prefs) {
         browserProxy.setPrefs(prefs);
         if (category == settings.ALL_SITES) {
           testElement.categorySubtype = settings.INVALID_CATEGORY_SUBTYPE;
@@ -387,7 +387,7 @@ cr.define('site_list', function() {
       }
 
       test('getExceptionList API used', function() {
-        setupCategory(settings.ContentSettingsTypes.GEOLOCATION,
+        setUpCategory(settings.ContentSettingsTypes.GEOLOCATION,
             settings.PermissionValues.ALLOW, prefsEmpty);
         return browserProxy.whenCalled('getExceptionList').then(
             function(contentType) {
@@ -397,7 +397,7 @@ cr.define('site_list', function() {
       });
 
       test('Empty list', function() {
-        setupCategory(settings.ContentSettingsTypes.GEOLOCATION,
+        setUpCategory(settings.ContentSettingsTypes.GEOLOCATION,
             settings.PermissionValues.ALLOW, prefsEmpty);
         return browserProxy.whenCalled('getExceptionList').then(
             function(contentType) {
@@ -410,16 +410,11 @@ cr.define('site_list', function() {
                   settings.PermissionValues.ALLOW, testElement.categorySubtype);
 
               assertFalse(testElement.$.category.hidden);
-              browserProxy.resetResolver('getExceptionList');
-              testElement.categoryEnabled = false;
-              return browserProxy.whenCalled('getExceptionList');
-            }).then(function(contentType) {
-              assertFalse(testElement.$.category.hidden);
             });
       });
 
       test('initial ALLOW state is correct', function() {
-        setupCategory(settings.ContentSettingsTypes.GEOLOCATION,
+        setUpCategory(settings.ContentSettingsTypes.GEOLOCATION,
             settings.PermissionValues.ALLOW, prefs);
         return browserProxy.whenCalled('getExceptionList').then(
             function(contentType) {
@@ -435,19 +430,12 @@ cr.define('site_list', function() {
               openActionMenu(0);
               assertMenu(['Block', 'Remove'], testElement);
 
-              // Site list should show, no matter what category default is set
-              // to.
-              assertFalse(testElement.$.category.hidden);

[dpapad, 2016/11/10 22:22:45]

Does it still makes sense to assert() for both categoryEnabled true/false
values? If so, could we only remove the whenCalled() calls but preserve the
assertions that the still is showing for both true/false?

[dschuyler, 2016/11/10 22:34:38]

I think it made sense to have the prior testing when those two unrelated
things were linked together. It's something that would be easy to break
because the linking wouldn't be obvious to a newcomer. 

When there is no connection between them I think it's less helpful to 
test the (non-) relation. Creating a new relation between those pieces is
as unlikely as creating a relation between any other boolean and showing
the category.

iiuc the question is about doing something like this:

assertFalse(testElement.$.category.hidden);
testElement.categoryEnabled = false;
Polymer.dom.flush();
assertFalse(testElement.$.category.hidden);

I can add that. If that's desired, please let me know. My opinion is 
that it's not needed (doesn't hurt, just not needed).

-              browserProxy.resetResolver('getExceptionList');
-              testElement.categoryEnabled = false;
-              return browserProxy.whenCalled('getExceptionList');
-            }).then(function(contentType) {
               assertFalse(testElement.$.category.hidden);
             });
       });
 
       test('initial BLOCK state is correct', function() {
-        setupCategory(settings.ContentSettingsTypes.GEOLOCATION,
+        setUpCategory(settings.ContentSettingsTypes.GEOLOCATION,
             settings.PermissionValues.BLOCK, prefs);
         return browserProxy.whenCalled('getExceptionList').then(
             function(contentType) {
@@ -464,18 +452,12 @@ cr.define('site_list', function() {
               openActionMenu(0);
               assertMenu(['Allow', 'Remove'], testElement);
 
-              // Site list should only show when category default is enabled.
               assertFalse(testElement.$.category.hidden);
-              browserProxy.resetResolver('getExceptionList');
-              testElement.categoryEnabled = false;
-              return browserProxy.whenCalled('getExceptionList');
-            }).then(function(contentType) {
-              assertTrue(testElement.$.category.hidden);
             });
       });
 
       test('initial SESSION ONLY state is correct', function() {
-        setupCategory(settings.ContentSettingsTypes.COOKIES,
+        setUpCategory(settings.ContentSettingsTypes.COOKIES,
             settings.PermissionValues.SESSION_ONLY, prefsSessionOnly);
         return browserProxy.whenCalled('getExceptionList').then(
             function(contentType) {
@@ -492,19 +474,12 @@ cr.define('site_list', function() {
               openActionMenu(0);
               assertMenu(['Allow', 'Block', 'Remove'], testElement);
 
-              // Site list should show, no matter what category default is set
-              // to.
-              assertFalse(testElement.$.category.hidden);
-              browserProxy.resetResolver('getExceptionList');
-              testElement.categoryEnabled = false;
-              return browserProxy.whenCalled('getExceptionList');
-            }).then(function(contentType) {
               assertFalse(testElement.$.category.hidden);
             });
       });
 
       test('initial INCOGNITO BLOCK state is correct', function() {
-        setupCategory(settings.ContentSettingsTypes.COOKIES,
+        setUpCategory(settings.ContentSettingsTypes.COOKIES,
             settings.PermissionValues.BLOCK, prefsIncognito);
         return browserProxy.whenCalled('getExceptionList').then(
             function(contentType) {
@@ -536,7 +511,7 @@ cr.define('site_list', function() {
       });
 
       test('initial INCOGNITO ALLOW state is correct', function() {
-        setupCategory(settings.ContentSettingsTypes.COOKIES,
+        setUpCategory(settings.ContentSettingsTypes.COOKIES,
             settings.PermissionValues.ALLOW, prefsIncognito);
         return browserProxy.whenCalled('getExceptionList').then(
             function(contentType) {
@@ -573,7 +548,7 @@ cr.define('site_list', function() {
       });
 
       test('list items shown and clickable when data is present', function() {
-        setupCategory(settings.ContentSettingsTypes.GEOLOCATION,
+        setUpCategory(settings.ContentSettingsTypes.GEOLOCATION,
             settings.PermissionValues.ALLOW, prefs);
         return browserProxy.whenCalled('getExceptionList').then(
             function(contentType) {
@@ -602,7 +577,7 @@ cr.define('site_list', function() {
 
       test('Block list open when Allow list is empty', function() {
         // Prefs: One item in Block list, nothing in Allow list.
-        setupCategory(settings.ContentSettingsTypes.GEOLOCATION,
+        setUpCategory(settings.ContentSettingsTypes.GEOLOCATION,
             settings.PermissionValues.BLOCK, prefsOneDisabled);
         return browserProxy.whenCalled('getExceptionList').then(
             function(contentType) {
@@ -616,7 +591,7 @@ cr.define('site_list', function() {
 
       test('Block list closed when Allow list is not empty', function() {
         // Prefs: Items in both Block and Allow list.
-        setupCategory(settings.ContentSettingsTypes.GEOLOCATION,
+        setUpCategory(settings.ContentSettingsTypes.GEOLOCATION,
             settings.PermissionValues.BLOCK, prefs);
         return browserProxy.whenCalled('getExceptionList').then(
             function(contentType) {
@@ -629,7 +604,7 @@ cr.define('site_list', function() {
 
       test('Allow list is always open (Block list empty)', function() {
         // Prefs: One item in Allow list, nothing in Block list.
-        setupCategory(settings.ContentSettingsTypes.GEOLOCATION,
+        setUpCategory(settings.ContentSettingsTypes.GEOLOCATION,
             settings.PermissionValues.ALLOW, prefsOneEnabled);
         return browserProxy.whenCalled('getExceptionList').then(
             function(contentType) {
@@ -644,7 +619,7 @@ cr.define('site_list', function() {
 
       test('Allow list is always open (Block list non-empty)', function() {
         // Prefs: Items in both Block and Allow list.
-        setupCategory(settings.ContentSettingsTypes.GEOLOCATION,
+        setUpCategory(settings.ContentSettingsTypes.GEOLOCATION,
             settings.PermissionValues.ALLOW, prefs);
         return browserProxy.whenCalled('getExceptionList').then(
             function(contentType) {
@@ -658,7 +633,7 @@ cr.define('site_list', function() {
 
       test('Block list not hidden when empty', function() {
         // Prefs: One item in Allow list, nothing in Block list.
-        setupCategory(settings.ContentSettingsTypes.GEOLOCATION,
+        setUpCategory(settings.ContentSettingsTypes.GEOLOCATION,
             settings.PermissionValues.BLOCK, prefsOneEnabled);
         return browserProxy.whenCalled('getExceptionList').then(
             function(contentType) {
@@ -670,7 +645,7 @@ cr.define('site_list', function() {
 
       test('Allow list not hidden when empty', function() {
         // Prefs: One item in Block list, nothing in Allow list.
-        setupCategory(settings.ContentSettingsTypes.GEOLOCATION,
+        setUpCategory(settings.ContentSettingsTypes.GEOLOCATION,
             settings.PermissionValues.ALLOW, prefsOneDisabled);
         return browserProxy.whenCalled('getExceptionList').then(
             function(contentType) {
@@ -682,7 +657,7 @@ cr.define('site_list', function() {
 
       test('All sites category', function() {
         // Prefs: Multiple and overlapping sites.
-        setupCategory(settings.ALL_SITES, '', prefsVarious);
+        setUpCategory(settings.ALL_SITES, '', prefsVarious);
 
         return browserProxy.whenCalled('getExceptionList').then(
             function(contentType) {
@@ -726,7 +701,7 @@ cr.define('site_list', function() {
 
       test('All sites mixed pattern and origin', function() {
         // Prefs: One site, represented as origin and pattern.
-        setupCategory(settings.ALL_SITES, '', prefsMixedOriginAndPattern);
+        setUpCategory(settings.ALL_SITES, '', prefsMixedOriginAndPattern);
 
         return browserProxy.whenCalled('getExceptionList').then(
             function(contentType) {
@@ -768,7 +743,7 @@ cr.define('site_list', function() {
 
       test('Mixed schemes (present and absent)', function() {
         // Prefs: One item with scheme and one without.
-        setupCategory(settings.ContentSettingsTypes.GEOLOCATION,
+        setUpCategory(settings.ContentSettingsTypes.GEOLOCATION,
             settings.PermissionValues.ALLOW, prefsMixedSchemes);
         return browserProxy.whenCalled('getExceptionList').then(
             function(contentType) {
@@ -778,7 +753,7 @@ cr.define('site_list', function() {
 
       test('Select menu item', function() {
         // Test for error: "Cannot read property 'origin' of undefined".
-        setupCategory(settings.ContentSettingsTypes.GEOLOCATION,
+        setUpCategory(settings.ContentSettingsTypes.GEOLOCATION,
             settings.PermissionValues.ALLOW, prefs);
         return browserProxy.whenCalled('getExceptionList').then(function(
             contentType) {
@@ -792,7 +767,7 @@ cr.define('site_list', function() {
       });
 
       test('Chrome Extension scheme', function() {
-        setupCategory(settings.ContentSettingsTypes.JAVASCRIPT,
+        setUpCategory(settings.ContentSettingsTypes.JAVASCRIPT,
             settings.PermissionValues.BLOCK, prefsChromeExtension);
         return browserProxy.whenCalled('getExceptionList').then(function(
             contentType) {