Simplify Site Settings tests.

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.
 
-// Define a global boolean for notifications (only enabled in the test class).
-cr.exportPath('settings_test');
-
-/** @type {boolean} */
-settings_test.siteListNotifyForTest;
-
 /**
  * @fileoverview
  * 'settings-site-list' shows a list of Allowed and Blocked sites for a given
  * category.
  */
 Polymer({
 
   is: 'settings-site-list',
 
   behaviors: [SiteSettingsBehavior, WebUIListenerBehavior],
@@ skipping 10 matching lines @@
     /**
      * The site that was selected by the user in the dropdown list.
      * @type {SiteException}
      */
     selectedSite: {
       type: Object,
       notify: true,
     },
 
     /**
      * Array of sites to display in the widget.

[dpapad, 2016/03/29 21:46:33]

Could you add a type annotation for this? As a reader I can't tell what type do
the contents of this array have.
Something like "@type {!Array<string>}" would be helpful to the reader and also
the compiler.

[Finnur, 2016/03/30 11:22:43]

Done. But this got me thinking about the next var. Should they all have @type,
even the dead-simple ones like allSites?

[michaelpg, 2016/03/31 11:13:30]

Nope, the Polymer pass in Closure automatically annotates Polymer properties! So
allSites has @type {boolean}. This works for all properties, but for Objects and
Arrays it's helpful to specify the exact type of object/array elements.

(the same argument applies to readability)

[Finnur, 2016/03/31 15:05:25]

Good to know. Thanks!

      */
     sites: {
       type: Array,
       value: function() { return []; },
-      notify: true,  // !!settings_test.siteListNotifyForTest,
     },
 
     /**
      * Whether this list is for the All Sites category.
      */
     allSites: {
       type: Boolean,
       value: false,
     },
 
@@ skipping 91 matching lines @@
     // Allowed list is always shown opened by default and All Sites is presented
     // all in one list (nothing closed by default).
     if (this.allSites ||
         this.categorySubtype == settings.PermissionValues.ALLOW) {
       this.$.category.opened = true;
       return;
     }
 
     // Block list should only be shown opened if there is nothing to show in
     // the allowed list.
-    var prefsProxy = settings.SiteSettingsPrefsBrowserProxyImpl.getInstance();
-    prefsProxy.getExceptionList(this.category).then(function(exceptionList) {
-      for (var i = 0; i < exceptionList.length; ++i) {
-        if (exceptionList[i].setting == 'allow')
-          return;
-      }
+    if (this.category != settings.INVALID_CATEGORY_SUBTYPE) {
+      var prefsProxy = settings.SiteSettingsPrefsBrowserProxyImpl.getInstance();

[dpapad, 2016/03/29 21:46:33]

Nit(optional): You can simply name this browserProxy, since there is no other
proxy used in this file.

[Finnur, 2016/03/30 11:22:43]

Done.

+      prefsProxy.getExceptionList(this.category).then(function(exceptionList) {
+        for (var i = 0; i < exceptionList.length; ++i) {

[dpapad, 2016/03/29 21:46:33]

Nit: You can use the Array#some() method instead, as follows.

var allowExists = exceptionList.some(function(exception) {
  return exception.setting == 'allow';
});
if (allowExists) 
  return;

[Finnur, 2016/03/30 11:22:43]

Done.

+          if (exceptionList[i].setting == 'allow')

[dpapad, 2016/03/29 21:46:33]

Isn't there an enum or constant for the literal 'allow'? We should make one if
not.

[Finnur, 2016/03/30 11:22:43]

Done.

+            return;
+        }
+        this.$.category.opened = true;
+      }.bind(this));
+    } else {
       this.$.category.opened = true;
-    }.bind(this));
+    }
   },
 
   /**
    * Makes sure the visibility is correct for this widget (e.g. hidden if the
    * block list is empty).
    * @private
    */
   updateCategoryVisibility_: function() {
     this.$.category.hidden =
         !this.showSiteList_(this.sites, this.categoryEnabled);
@@ skipping 258 matching lines @@
    * Returns the icon to use for a given site.
    * @param {string} url The url of the site to fetch the icon for.
    * @private
    */
   computeSiteIcon_: function(url) {
     // TODO(finnur): For now, we're returning a placeholder image for each site
     // but the actual favicon for each site will need to be returned.
     return 'communication:message';
   },
 });