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

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

Index: chrome/browser/resources/settings/site_settings/site_list.js
diff --git a/chrome/browser/resources/settings/site_settings/site_list.js b/chrome/browser/resources/settings/site_settings/site_list.js
index c42f17a8867cedf6f15dbb3068ce1fafcddcb8ba..e835e2ff652637b51af5617475ffe4363e764575 100644
--- a/chrome/browser/resources/settings/site_settings/site_list.js
+++ b/chrome/browser/resources/settings/site_settings/site_list.js
@@ -176,6 +176,12 @@ Polymer({
 
     this.setUpActionMenu_();
     this.populateList_();
+
+    // The Session permissions are only for cookies.
+    if (this.categorySubtype == settings.PermissionValues.SESSION_ONLY) {
+      this.$.category.hidden =

[dpapad, 2016/11/07 21:59:35]

Could this be done with a data binding that depends on |categorySubtype| and
|category| instead?

[dschuyler, 2016/11/07 23:54:04]

There is a binding style handler setup on line 130. Though that's now
observing more bindings than necessary, I'll trim that down.

+          this.category != settings.ContentSettingsTypes.COOKIES;
+    }
   },
 
   /**
@@ -201,21 +207,13 @@ Polymer({
   },
 
   /**
-   * Makes sure the visibility is correct for this widget.
-   * @private
-   */
-  updateCategoryVisibility_: function() {
-    this.$.category.hidden =
-        !this.showSiteList_(this.sites, this.categoryEnabled);
-  },
-
-  /**
    * A handler for the Add Site button.
    * @private
    */
   onAddSiteTap_: function() {
     var dialog = document.createElement('add-site-dialog');

[dpapad, 2016/11/07 21:59:35]

Nit (optional): This function could be simplified if the dialog was declared in
the HTML. We have several such examples, that go as follows:

onTap_: function() {
  this.dialogModel_ = contentSetting; // The model feeding the dialog.
  this.showDialog_ = true; // this is bound do a dom-if with restamp.
}

// This listener is registered via HTML on-close="..."
onDialogClosed_: function() {
  this.showDialog_ = false;
  this.dialogModel_ = null;
}

[dschuyler, 2016/11/07 23:54:04]

Going by advice I got while visiting the Polymer folks: there's less
overhead to create an element than adding template dom-if. I think
we should work out whether there is an impact with creating nodes vs.
dom-if and convert them all one way or the other so that they are 
consistent. Creating an element in JS also has a small advantage that
the closure compiler can type check the properties (which doesn't 
currently happen afaik in html).

[dpapad, 2016/11/08 00:12:32]

SG.

I would be positively surprised if the compiler's Polymer pass understands
createElement() return type without casting it to anything. As it stands now, I
just think it is using its lenient policy when a type is not available.

Regarding converting all <dialog> declarations to either programmatic or
declarative, I think it might be too disruptive. Declarative has a lot of
advantages (data binding, listener registration), so it would be far from
trivial to accommodate all cases with only programmatic DOM manipulation.

     dialog.category = this.category;
+    dialog.contentSetting = this.categorySubtype;
     this.shadowRoot.appendChild(dialog);
 
     dialog.open(this.categorySubtype);
@@ -253,7 +251,6 @@ Polymer({
     for (var i = 0; i < data.length; ++i)
       sites = this.appendSiteList_(sites, data[i]);
     this.sites = this.toSiteArray_(sites);
-    this.updateCategoryVisibility_();
   },
 
   /**
@@ -355,7 +352,7 @@ Polymer({
   },
 
   /**
-   * Setup the values to use for the action menu.
+   * Set up the values to use for the action menu.
    * @private
    */
   setUpActionMenu_: function() {
@@ -458,42 +455,6 @@ Polymer({
   },
 
   /**
-   * Returns true if this widget is showing the Allow list.
-   * @private
-   */
-  isAllowList_: function() {
-    return this.categorySubtype == settings.PermissionValues.ALLOW;
-  },
-
-  /**
-   * Returns true if this widget is showing the Session Only list.
-   * @private
-   */
-  isSessionOnlyList_: function() {
-    return this.categorySubtype == settings.PermissionValues.SESSION_ONLY;
-  },
-
-  /**
-   * Returns whether to show the site list.
-   * @param {Array} siteList The list of all sites to display for this category
-   *     subtype.
-   * @param {boolean} toggleState The state of the global toggle for this
-   *     category.
-   * @private
-   */
-  showSiteList_: function(siteList, toggleState) {
-    // The Block list is only shown when the category is set to Allow since it
-    // is redundant to also list all the sites that are blocked.
-    if (this.isAllowList_())
-      return true;
-
-    if (this.isSessionOnlyList_())
-      return siteList.length > 0;
-
-    return toggleState;
-  },
-
-  /**
    * @param {!{model: !{item: !SiteException}}} e
    * @private
    */