Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2226)

Unified Diff: extensions/renderer/resources/utils.js

Issue 1903303002: Ensure that privates are private. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Fix for failing tests Created 4 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: extensions/renderer/resources/utils.js
diff --git a/extensions/renderer/resources/utils.js b/extensions/renderer/resources/utils.js
index eee1356272ed1fe7d182cb6a6343480f3f07b441..fd70c603ddffe295c6dc31d8efe495ca82c0760b 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
+ * 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 be a constructor that calls constructPrivate() like so:
+ *
+ * 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>,
@@ -84,13 +89,49 @@ function loadTypeSchema(typeName, defaultSchema) {
* delegated to the implementation; |properties| are gettable/settable
* properties and |readonly| are read-only properties.
*/
-function expose(name, cls, exposed) {
- var publicClass = createClassWrapper(name, cls, exposed.superclass);
+function expose(publicClass, privateClass, exposed) {
+ // TODO(robwu): Fix callers and uncomment this assertion.
+ // DCHECK(!(privateClass instanceof $Object.self));
+
+ $Object.setPrototypeOf(exposed, null);
+
+ // 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,
+ constructor: publicClass,
+ };
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,15 +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];
},
});
});
}
+ // The prototype properties have been installed. Now we can safely assign an
+ // unsafe prototype and export the class to the public.
+ var superclass = exposed.superclass || $Object.self;
+ $Object.setPrototypeOf(publicClassPrototype, superclass.prototype);
+ publicClass.prototype = publicClassPrototype;
+
return publicClass;
}
« no previous file with comments | « extensions/renderer/resources/messaging.js ('k') | extensions/renderer/resources/web_request_internal_custom_bindings.js » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698