NTP5: Refactoring appData to use Tile's data implementation.

chrome/browser/resources/ntp_search/most_visited_page.js

Index: chrome/browser/resources/ntp_search/most_visited_page.js
diff --git a/chrome/browser/resources/ntp_search/most_visited_page.js b/chrome/browser/resources/ntp_search/most_visited_page.js
index d5a0b5235ffa196510961db028a44686bea1704e..329a6227ac1316908e549802344b791c2e076521 100644
--- a/chrome/browser/resources/ntp_search/most_visited_page.js
+++ b/chrome/browser/resources/ntp_search/most_visited_page.js
@@ -56,14 +56,18 @@ cr.define('ntp', function() {
      * Update the appearance of this tile according to |data|.
      * @param {Object} data A dictionary of relevant data for the page.
      */
-    setData: function(data) {
+    set data(data) {
+      Object.getOwnPropertyDescriptor(Thumbnail.prototype, 'data').set.apply(
+          this, arguments);
+
       if (this.classList.contains('blacklisted') && data) {
         // Animate appearance of new tile.
         this.classList.add('new-tile-contents');
       }
       this.classList.remove('blacklisted');
-
-      Thumbnail.prototype.setData.apply(this, arguments);
+    },
+    get data() {
+      return this.data_;

[Dan Beam, 2012/12/07 05:05:31]

nit: what does this offer over Thumbnail.data / do you need it?

[pedro (no code reviews), 2012/12/07 21:54:55]

Yes, I do need this. I might be missing something here, but if remove the getter
here and place it in the superclass (Thumbnail or even Tile), it won't work.

[Dan Beam, 2012/12/07 22:31:07]

in JS, "private" members exist on all sub classes as it's actually just a proto
chain reference.

  function base(){}
  base.prototype = {/** @private */ private_: 2};

  function derived(){}
  derived.prototype = {
    __proto__: base.prototype,
    getPublic: function(){ return this.private_; },
  };

  alert(new derived().getPublic()); // alerts 2

and if you really want this it work like classical inheritance, you could just
as easily add an accessor:

  function base(){}
  base.prototype = {
    /** @private */ private_: 2,
    /** @protected */ getProtected: function() { return this.private_; },
  };

  function derived(){}
  derived.prototype = {
    __proto__: base.prototype,
    getPublic: function(){ return this.getProtected(); },
  };

or:

  function base(){}
  base.prototype = {/** @protected */ protected: 2};

  function derived(){}
  derived.prototype = {
    __proto__: base.prototype,
    getPublic: function(){ return this.protected; },
  };

[pedro (no code reviews), 2012/12/10 19:44:43]

I guess I wasn't clear enough in my previous comment. I do know how OO works in
JS, the prototype chain, the lack of real private/protected members, using
closures to emulate private members, etc.

I was talking about how getters/setters behave in the prototype chain. It seems
that if you define a setter on a subclass you must also define a getter,
otherwise if you call the getter from this subclass it will return undefined
(and the expected would be to return the superclass getter).

Check this example:
http://jsfiddle.net/qycMs/

     },
 
     /**