Ensure that privates are private.

extensions/renderer/resources/utils.js

Index: extensions/renderer/resources/utils.js
diff --git a/extensions/renderer/resources/utils.js b/extensions/renderer/resources/utils.js
index 66094b9098335859a4ee148232355c996ab9de77..3e0f3d421aa5c7aaa898bc3c1476b00ea87b244d 100644
--- a/extensions/renderer/resources/utils.js
+++ b/extensions/renderer/resources/utils.js
@@ -2,7 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-var createClassWrapper = requireNative('utils').createClassWrapper;
 var nativeDeepCopy = requireNative('utils').deepCopy;
 var schemaRegistry = requireNative('schema_registry');
 var CHECK = requireNative('logging').CHECK;
@@ -65,15 +64,21 @@ function loadTypeSchema(typeName, defaultSchema) {
 }
 
 /**
- * Takes a private class implementation |cls| and exposes a subset of its
- * methods |functions| and properties |properties| and |readonly| in a public
- * wrapper class that it returns. Within bindings code, you can access the
- * implementation from an instance of the wrapper class using
+ * Takes a private class implementation |PrivateClass| and exposes a subset of

[Devlin, 2016/04/22 17:08:21]

nit: since this is just an argument, it should be publicClass (and privateClass)

[robwu, 2016/04/22 17:37:40]

Done.

+ * its methods |functions| and properties |properties| and |readonly| to a
+ * public wrapper class that should be passed in. Within bindings code, you can
+ * access the implementation from an instance of the wrapper class using
  * privates(instance).impl, and from the implementation class you can access
  * the wrapper using this.wrapper (or implInstance.wrapper if you have another
  * instance of the implementation class).
- * @param {string} name The name of the exposed wrapper class.
- * @param {Object} cls The class implementation.
+ *
+ * PublicClass should initialize the class by calling:

[Devlin, 2016/04/22 17:08:21]

Let's make this blatantly clear:

* |publicClass| should be a constructor that calls constructPrivate() like so:
* function PublicClass() {
*   privates(PublicClass).constructPrivate(this, arguments);
* }

[robwu, 2016/04/22 17:37:40]

Done.

+ *
+ *    privates(PublicClass).constructPrivate(this, arguments);
+ *
+ * @param {function} PublicClass The publicly exposed wrapper class. This must
+ *   be a named function, and the name appears in stack traces.
+ * @param {Object} PrivateClass The class implementation.
  * @param {{superclass: ?Function,
  *          functions: ?Array<string>,
  *          properties: ?Array<string>,
@@ -83,14 +88,50 @@ function loadTypeSchema(typeName, defaultSchema) {
  *     class; |functions| represents the names of functions which should be
  *     delegated to the implementation; |properties| are gettable/settable
  *     properties and |readonly| are read-only properties.
+ *     |exposed| MUST either specify all properties, or not inherit from an
+ *     unsafe prototype. If this pre-condition is failed, the expose function
+ *     cannot guarantee that private properties stay hidden.
  */
-function expose(name, cls, exposed) {
-  var publicClass = createClassWrapper(name, cls, exposed.superclass);
+function expose(PublicClass, PrivateClass, exposed) {
+  DCHECK(!(PrivateClass instanceof $Object.self));
+  DCHECK(!(exposed instanceof $Object.self));
+
+  // This should be called by PublicClass.
+  privates(PublicClass).constructPrivate = function(self, args) {
+    if (!(self instanceof PublicClass)) {
+      throw new Error('Please use "new ' + PublicClass.name + '"');
+    }
+    // The "instanceof PublicClass" check can easily be spoofed, so we check
+    // whether the private impl is already set before continuing.
+    var privateSelf = privates(self);
+    if ('impl' in privateSelf) {
+      throw new Error('Object ' + PublicClass.name + ' is already constructed');
+    }
+    var privateObj = $Object.create(PrivateClass.prototype);
+    $Function.apply(PrivateClass, privateObj, args);
+    privateObj.wrapper = self;
+    privateSelf.impl = privateObj;
+  };
+
+  function getPrivateImpl(self) {
+    var impl = privates(self).impl;
+    if (!(impl instanceof PrivateClass)) {
+      // Either the object is not constructed, or the property descriptor is
+      // used on a target that is not an instance of PublicClass.
+      throw new Error('impl is not an instance of ' + PrivateClass.name);
+    }
+    return impl;
+  }
+
+  var PublicClassPrototype = {
+    // The final prototype will be assigned at the end of this method.
+    __proto__: null,
+  };
 
   if ('functions' in exposed) {
     $Array.forEach(exposed.functions, function(func) {
-      publicClass.prototype[func] = function() {
-        var impl = privates(this).impl;
+      PublicClassPrototype[func] = function() {
+        var impl = getPrivateImpl(this);
         return $Function.apply(impl[func], impl, arguments);
       };
     });
@@ -98,13 +139,14 @@ function expose(name, cls, exposed) {
 
   if ('properties' in exposed) {
     $Array.forEach(exposed.properties, function(prop) {
-      $Object.defineProperty(publicClass.prototype, prop, {
+      $Object.defineProperty(PublicClassPrototype, prop, {
+        __proto__: null,
         enumerable: true,
         get: function() {
-          return privates(this).impl[prop];
+          return getPrivateImpl(this)[prop];
         },
         set: function(value) {
-          var impl = privates(this).impl;
+          var impl = getPrivateImpl(this);
           delete impl[prop];
           impl[prop] = value;
         }
@@ -114,16 +156,22 @@ function expose(name, cls, exposed) {
 
   if ('readonly' in exposed) {
     $Array.forEach(exposed.readonly, function(readonly) {
-      $Object.defineProperty(publicClass.prototype, readonly, {
+      $Object.defineProperty(PublicClassPrototype, readonly, {
+        __proto__: null,
         enumerable: true,
         get: function() {
-          return privates(this).impl[readonly];
+          return getPrivateImpl(this)[readonly];
         },
       });
     });
   }
 
-  return publicClass;
+  // The prototype properties have been installed. Now we can safely assign an
+  // unsafe prototype and export the class to the public.
+  PublicClassPrototype.__proto__ = exposed.superclass.prototype || Object;

[Devlin, 2016/04/22 17:08:21]

$Object.self?

[robwu, 2016/04/22 17:37:40]

Done.

+  PublicClass.prototype = PublicClassPrototype;
+
+  return PublicClass;
 }
 
 /**