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

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

 // Copyright 2015 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.
 
 /**
  * Enumeration mapping all possible controlled-by values for exceptions to
  * icons.
  * @enum {string}
  */
 var iconControlledBy = {
@@ skipping 158 matching lines @@
   configureWidget_: function() {
     // The observer for All Sites fires before the attached/ready event, so
     // initialize this here.
     if (this.browserProxy_ === undefined) {
       this.browserProxy_ =
           settings.SiteSettingsPrefsBrowserProxyImpl.getInstance();
     }
 
     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;
+    }
   },
 
   /**
    * Returns which icon, if any, should represent the fact that this exception
    * is controlled.
    * @param {!SiteException} item The item from the list we're computing the
    *    icon for.
    * @return {string} The icon to show (or blank, if none).
    */
   computeIconControlledBy_: function(item) {
     if (this.allSites)
       return '';
     return iconControlledBy[item.source] || '';
   },
 
   /**
    * @param {string} source Where the setting came from.
    * @return {boolean}
    * @private
    */
   shouldShowMenu_: function(source) {
     return !(this.isExceptionControlled_(source) || this.allSites);
   },
 
   /**
-   * 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);
 
     dialog.addEventListener('close', function() {
       dialog.remove();
     });
   },
 
   /**
@@ skipping 17 matching lines @@
    * Process the exception list returned from the native layer.
    * @param {!Array<!Array<SiteException>>} data List of sites (exceptions) to
    *     process.
    * @private
    */
   processExceptions_: function(data) {
     var sites = [];
     for (var i = 0; i < data.length; ++i)
       sites = this.appendSiteList_(sites, data[i]);
     this.sites = this.toSiteArray_(sites);
-    this.updateCategoryVisibility_();
   },
 
   /**
    * Retrieves a list of all known sites (any category/setting).
    * @return {!Promise}
    * @private
    */
   getAllSitesList_: function() {
     var promiseList = [];
     for (var type in settings.ContentSettingsTypes) {
@@ skipping 81 matching lines @@
       }
 
       results.push(siteException);
       lastOrigin = siteException.originForDisplay;
       lastEmbeddingOrigin = siteException.embeddingOriginForDisplay;
     }
     return results;
   },
 
   /**
-   * Setup the values to use for the action menu.
+   * Set up the values to use for the action menu.
    * @private
    */
   setUpActionMenu_: function() {
     this.showAllowAction_ =
         this.categorySubtype != settings.PermissionValues.ALLOW;
     this.showBlockAction_ =
         this.categorySubtype != settings.PermissionValues.BLOCK;
     this.showSessionOnlyAction_ =
         this.categorySubtype != settings.PermissionValues.SESSION_ONLY &&
         this.category == settings.ContentSettingsTypes.COOKIES;
@@ skipping 82 matching lines @@
       return loadTimeData.getStringF('embeddedIncognitoSite',
           item.embeddingOriginForDisplay);
     }
 
     if (item.incognito)
       return loadTimeData.getString('incognitoSite');
     return item.embeddingOriginForDisplay;
   },
 
   /**
-   * 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
    */
   onShowActionMenuTap_: function(e) {
     this.actionMenuSite_ = e.model.item;
     /** @type {!CrActionMenuElement} */ (
         this.$$('dialog[is=cr-action-menu]')).showAt(
             /** @type {!Element} */ (
                 Polymer.dom(/** @type {!Event} */ (e)).localTarget));
   },
 
   /** @private */
   closeActionMenu_: function() {
     this.actionMenuSite_ = null;
     /** @type {!CrActionMenuElement} */ (
         this.$$('dialog[is=cr-action-menu]')).close();
   },
 });