Add Closure compiling instructions for site_list.

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 613008a45b174e9debbf7b4933dd693a7e82014e..0b0df22e37362dcd37a45f26fc18b7bf865fc932 100644
--- a/chrome/browser/resources/settings/site_settings/site_list.js
+++ b/chrome/browser/resources/settings/site_settings/site_list.js
@@ -143,7 +143,9 @@ Polymer({
         this.computeCategoryExceptionsPrefName(this.category));
     var sites = pref.value;
     for (var origin in sites) {
-      if (sites[origin].setting == this.categorySubtype) {
+      /** @type {{setting:Number}}*/

[Dan Beam, 2015/11/09 19:28:55]

this @type doesn't make sense here, but this might

/** @type {{setting: number}} */(sites[origin]).setting

[Finnur, 2015/11/10 11:58:50]

It is modeled after other code in the tree and did take care of the closure
error, but your version also works (if I add enclosing parentheses) and is
clearer -- so I switched.

[Dan Beam, 2015/11/11 07:01:39]

i think it's just confusing the compiler into thinking this is an object with a
key but that could also have more keys (which it's just not braking about, but
does in stricter modes)

+      var setting = sites[origin].setting;

[Finnur, 2015/11/09 15:56:09]

Is this (and the change in onOriginTap_) the right way to address Closure errors
such as:

  ERROR - Property setting never defined on ?

Note: This change of mine is flagged as a JavaScript style guide violation: 

  line 146: Don't use wrapper types (i.e. new String() or @type {String})
      /** @type {{setting:Number}}*/
                          ^^^^^^

But I don't quite understand what that means and even though it points to the
JavaScript style guide, I can't find the reference for this...

Is this a false-positive?

[Dan Beam, 2015/11/09 19:28:55]

Number -> number

[Finnur, 2015/11/10 11:58:50]

Bah. That's unfortunate (Polymer wanting |Number|, but Closure wanting
|number|). Anyway, that takes care of both problems (Closure and style guide
issue).

[Dan Beam, 2015/11/11 07:01:39]

Polymer({
  properties: {
    doUseWrappers: {
      type: Number,  // but not as jsdoc annotations denoting their @type
      // if this was typed with jsdoc, it'd be @type {number}
    },
  },
});

+      if (setting == this.categorySubtype) {
         var tokens = origin.split(',');
         newList.push({url: tokens[0]});
       }
@@ -169,6 +171,7 @@ Polymer({
    * @private
    */
   onOriginTap_: function(event) {
+    /** @type {{model:Object}}*/

[Dan Beam, 2015/11/09 19:28:55]

i don't understand what this is supposed to do?  are you trying to describe
|event|?

[Finnur, 2015/11/10 00:39:43]

Um... I think so. Well, the error (as I recall, I'm not at my desk since it is
past midnight here :)) was that it complained that |item| on |?| was not
defined. But if I define model it stops complaining. I couldn't find much
documentation on these errors and how to handle them, so I modeled this approach
after other js code in the tree. Do you recommend a different approach?

[Dan Beam, 2015/11/10 05:58:58]

have you actually been able to run the compiler?

[Finnur, 2015/11/10 11:58:50]

Yes, couldn't be doing this without it -- got the Closure compiler working after
switching to my Linux box (due to unrelated issue crbug.com/544889).


And now that I'm at my desk testing this I can properly describe what I'm trying
to do.

  this.selectedOrigin = event.model.item.url;

I'm obviously trying to get the origin that was selected from the list. With
only the assignment above (and no Closure hint) I get this:

  ERROR - Property model never defined on event

So, I'm trying to describe |model| -- or |event| if that describes |model| as
well.

Note: If I add this line above it, that error goes away and the Closure compiler
is happy (no further complaints):

  /** @type {{model:Object}}*/

As I said, this is modelled after other code in the tree, so at least there is
precedent.

However, if I instead use the same approach as you suggested above:

  var model = /** @type {{model: Object}} */((event).model);
  this.selectedOrigin = model.item.url;

... then the error becomes:

  ERROR - Property item never defined on model

... and that error remains even if I define it like so:

  var model = /** @type {{model: Object}} */((event).model);
  var item = /** @type {{item: Object}} */((model).item);

I also suspect that if I manage to define |item|, then |url| will need to be
described.
That was what I tried first. I searched through a sizable chunk of js code in
the tree, looking for "function(event" (without the double quotes) and could not
find any file that described an event in a way that worked with the above code.
A lot of functions didn't bother defining |event| in a generic way, some just
define it as Event, some as ChromeKeyboardEvent) but the end result for me was
all the same. It still complained about |model|.

     this.selectedOrigin = event.model.item.url;
   },
 
@@ -182,7 +185,7 @@ Polymer({
 
   /**
    * Returns the appropriate header value for display.
-   * @param {array<string>} siteList The list of all sites to display for this
+   * @param {Array<string>} siteList The list of all sites to display for this
    *     category subtype.
    * @param {boolean} toggleState The state of the global toggle for this
    *     category.
@@ -213,7 +216,7 @@ Polymer({
 
   /**
    * Returns whether to show the site list.
-   * @param {array} siteList The list of all sites to display for this category
+   * @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.