[gjs] js: Workaround for function with custom prototype



commit 72c0298c5f9df9036ed67fd504db84cbc028daaa
Author: Philip Chimento <philip endlessm com>
Date:   Tue Nov 22 13:37:12 2016 -0500

    js: Workaround for function with custom prototype
    
    It's not possible in JS to directly create a function object with a
    custom prototype. We previously got around this by directly altering the
    prototype by setting the __proto__ property, but SpiderMonkey now
    conspicuously warns that this will make your code slow.
    
    It would be possible to do this with ES6 Proxy objects, although
    SpiderMonkey 31 doesn't support the particular getPrototypeOf() proxy
    trap that we would need in order to implement this correctly --- at
    least not in JS. Therefore we implement the proxy in C++.
    
    We add a debug topic for proxies and a memory counter.
    
    All in all, the proxy is probably still slower than a function object
    with a real prototype would be, but hopefully faster than direct
    alteration of the prototype. At the very least we can avoid printing a
    big warning every time our class framework is used.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=751252

 Makefile.am                     |    2 +
 gjs/context.cpp                 |    3 +
 gjs/jsapi-constructor-proxy.cpp |  191 +++++++++++++++++++++++++++++++++++++++
 gjs/jsapi-constructor-proxy.h   |   37 ++++++++
 gjs/mem.cpp                     |    4 +-
 gjs/mem.h                       |    1 +
 modules/lang.js                 |   19 ++--
 modules/overrides/GObject.js    |   18 ++--
 util/log.cpp                    |    3 +
 util/log.h                      |    1 +
 10 files changed, 260 insertions(+), 19 deletions(-)
---
diff --git a/Makefile.am b/Makefile.am
index 6ae6c7e..66c82d5 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -83,6 +83,8 @@ libgjs_la_SOURCES =           \
        gjs/gi.cpp              \
        gjs/coverage-internal.h \
        gjs/coverage.cpp \
+       gjs/jsapi-constructor-proxy.cpp \
+       gjs/jsapi-constructor-proxy.h   \
        gjs/jsapi-private.cpp   \
        gjs/jsapi-private.h             \
        gjs/jsapi-util.cpp      \
diff --git a/gjs/context.cpp b/gjs/context.cpp
index 60a03da..477928f 100644
--- a/gjs/context.cpp
+++ b/gjs/context.cpp
@@ -27,6 +27,7 @@
 
 #include "context-private.h"
 #include "importer.h"
+#include "jsapi-constructor-proxy.h"
 #include "jsapi-private.h"
 #include "jsapi-util.h"
 #include "jsapi-wrapper.h"
@@ -450,6 +451,8 @@ gjs_context_constructed(GObject *object)
     js_context->global.set(global);
     JS_AddExtraGCRootsTracer(js_context->runtime, gjs_context_tracer, js_context);
 
+    gjs_define_constructor_proxy_factory(js_context->context);
+
     /* We create the global-to-runtime root importer with the
      * passed-in search path. If someone else already created
      * the root importer, this is a no-op.
diff --git a/gjs/jsapi-constructor-proxy.cpp b/gjs/jsapi-constructor-proxy.cpp
new file mode 100644
index 0000000..363a418
--- /dev/null
+++ b/gjs/jsapi-constructor-proxy.cpp
@@ -0,0 +1,191 @@
+/* -*- mode: C; c-basic-offset: 4; indent-tabs-mode: nil; -*- */
+/*
+ * Copyright (c) 2016 Endless Mobile, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authored by: Philip Chimento <philip endlessm com>
+ */
+
+#include "jsapi-constructor-proxy.h"
+#include "jsapi-util.h"
+#include "jsapi-wrapper.h"
+#include "mem.h"
+#include "util/log.h"
+
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Winvalid-offsetof"
+#pragma GCC diagnostic ignored "-Wmismatched-tags"
+#pragma GCC diagnostic ignored "-Winconsistent-missing-override"
+#include "jsproxy.h"
+#pragma GCC diagnostic pop
+
+/* This code exposes a __private_GjsConstructorProxy function to JS, which is
+ * approximately equivalent to
+ *
+ * function __private_GjsConstructorProxy(constructor, prototype) {
+ *     let my_prototype = prototype;
+ *     return new Proxy(constructor, {
+ *         getPrototypeOf: function (target) { return my_prototype; },
+ *     });
+ * }
+ *
+ * but with a C++-only flag that routes all property accesses through the
+ * getPrototypeOf() trap, which may or may not be turned on in JS proxies,
+ * I'm not sure.
+ *
+ * COMPAT: SpiderMonkey doesn't support the getPrototypeOf() trap in JS
+ * proxies yet. That has yet to be released, in the upcoming SpiderMonkey 52.
+ * When that is available, then this whole file can be discontinued.
+ *
+ * That is the reason for the existence of this C++ file, but the reason why it
+ * is needed at all is because of Lang.Class and GObject.Class. We must give
+ * class objects (e.g. "const MyClass = new Lang.Class({...})") a custom
+ * prototype, so that "MyClass instanceof Lang.Class" will be true, and MyClass
+ * will have methods from Class.
+ *
+ * Usually you would give an object a custom prototype using Object.create(),
+ * but that's not possible for function or constructor objects, and MyClass of
+ * course must be a constructor. Previously we solved this with
+ * Object.setPrototypeOf(), but that has performance effects on any code that
+ * uses objects whose prototypes have been altered [1], and SpiderMonkey started
+ * printing conspicuous warnings about it.
+ *
+ * [1] https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/setPrototypeOf
+ */
+
+static const char constructor_proxy_create_name[] = "__private_GjsConstructorProxy";
+/* This char's address is an arbitrary identifier for use in debugging */
+static const char constructor_proxy_family = 'p';
+
+enum {
+    SLOT_PROTO,
+};
+
+/* This class is the C++ equivalent of a proxy handler object. In JS, that is
+ * the second argument passed to the "new Proxy(target, handler)" constructor.
+ */
+class GjsConstructorHandler : public js::DirectProxyHandler {
+    static inline JSObject *
+    proto(JS::HandleObject proxy)
+    {
+        return &js::GetProxyExtra(proxy, SLOT_PROTO).toObject();
+    }
+
+public:
+    GjsConstructorHandler() : js::DirectProxyHandler(&constructor_proxy_family)
+    {
+        setHasPrototype(true);
+    }
+
+    bool
+    getPrototypeOf(JSContext              *cx,
+                   JS::HandleObject        proxy,
+                   JS::MutableHandleObject proto_p)
+    override
+    {
+        proto_p.set(proto(proxy));
+        return true;
+    }
+
+    /* This is called when the associated proxy object is finalized, not the
+     * handler itself */
+    void
+    finalize(JSFreeOp *fop,
+             JSObject *proxy)
+    override
+    {
+        GJS_DEC_COUNTER(constructor_proxy);
+        gjs_debug_lifecycle(GJS_DEBUG_PROXY,
+                            "constructor proxy %p destroyed", proxy);
+    }
+
+    static GjsConstructorHandler&
+    singleton(void)
+    {
+        static GjsConstructorHandler the_singleton;
+        return the_singleton;
+    }
+};
+
+/* Visible to JS as __private_GjsConstructorProxy(constructor, prototype) */
+static bool
+create_gjs_constructor_proxy(JSContext *cx,
+                             unsigned   argc,
+                             JS::Value *vp)
+{
+    JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
+
+    if (args.length() < 2) {
+        gjs_throw(cx, "Expected 2 arguments to %s, got %d",
+                  constructor_proxy_create_name, args.length());
+        return false;
+    }
+
+    if (!args[0].isObject() || !JS_ObjectIsFunction(cx, &args[0].toObject())) {
+        /* COMPAT: Use JS::IsConstructor() in mozjs38 */
+        gjs_throw(cx, "First argument must be a constructor");
+        return false;
+    }
+    if (!args[1].isObject()) {
+        gjs_throw(cx, "Second argument must be a prototype object");
+        return false;
+    }
+
+    js::ProxyOptions options;
+    /* "true" makes the proxy callable, otherwise the "call" and "construct"
+     * traps are ignored */
+    options.selectDefaultClass(true);
+
+    JS::RootedObject proxy(cx,
+        js::NewProxyObject(cx, &GjsConstructorHandler::singleton(), args[0],
+                           &args[1].toObject(), nullptr, options));
+    /* We stick this extra object into one of the proxy object's "extra slots",
+     * even though it is private data of the proxy handler. This is because
+     * proxy handlers cannot have trace callbacks. The proxy object does have a
+     * built-in trace callback which traces the "extra slots", so this object
+     * will be kept alive. This also means the handler has no private state at
+     * all, so it can be a singleton. */
+    js::SetProxyExtra(proxy, SLOT_PROTO, args[1]);
+
+    args.rval().setObject(*proxy);
+
+    GJS_INC_COUNTER(constructor_proxy);
+    gjs_debug_lifecycle(GJS_DEBUG_PROXY,
+                        "created constructor proxy %p", proxy.get());
+    return true;
+}
+
+bool
+gjs_define_constructor_proxy_factory(JSContext *cx)
+{
+    bool found;
+    JS::RootedObject global(cx, gjs_get_import_global(cx));
+
+    if (!JS_HasProperty(cx, global, constructor_proxy_create_name, &found))
+        return false;
+    if (found)
+        return true;
+    if (!JS_DefineFunction(cx, global, constructor_proxy_create_name,
+        create_gjs_constructor_proxy, 2, JSPROP_READONLY | JSPROP_PERMANENT))
+        return false;
+
+    gjs_debug(GJS_DEBUG_PROXY, "Initialized constructor proxy factory");
+    return true;
+}
diff --git a/gjs/jsapi-constructor-proxy.h b/gjs/jsapi-constructor-proxy.h
new file mode 100644
index 0000000..fc27506
--- /dev/null
+++ b/gjs/jsapi-constructor-proxy.h
@@ -0,0 +1,37 @@
+/* -*- mode: C; c-basic-offset: 4; indent-tabs-mode: nil; -*- */
+/*
+ * Copyright (c) 2016 Endless Mobile, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#ifndef GJS_JSAPI_CONSTRUCTOR_PROXY_H
+#define GJS_JSAPI_CONSTRUCTOR_PROXY_H
+
+#include <glib.h>
+
+#include "jsapi-wrapper.h"
+
+G_BEGIN_DECLS
+
+bool gjs_define_constructor_proxy_factory(JSContext *cx);
+
+G_END_DECLS
+
+#endif /* GJS_JSAPI_CONSTRUCTOR_PROXY_H */
diff --git a/gjs/mem.cpp b/gjs/mem.cpp
index 1e562d3..35ddfc4 100644
--- a/gjs/mem.cpp
+++ b/gjs/mem.cpp
@@ -48,6 +48,7 @@ GJS_DEFINE_COUNTER(repo)
 GJS_DEFINE_COUNTER(resultset)
 GJS_DEFINE_COUNTER(weakhash)
 GJS_DEFINE_COUNTER(interface)
+GJS_DEFINE_COUNTER(constructor_proxy)
 
 #define GJS_LIST_COUNTER(name) \
     & gjs_counter_ ## name
@@ -66,7 +67,8 @@ static GjsMemCounter* counters[] = {
     GJS_LIST_COUNTER(repo),
     GJS_LIST_COUNTER(resultset),
     GJS_LIST_COUNTER(weakhash),
-    GJS_LIST_COUNTER(interface)
+    GJS_LIST_COUNTER(interface),
+    GJS_LIST_COUNTER(constructor_proxy),
 };
 
 void
diff --git a/gjs/mem.h b/gjs/mem.h
index 2b74eb1..d4be405 100644
--- a/gjs/mem.h
+++ b/gjs/mem.h
@@ -54,6 +54,7 @@ GJS_DECLARE_COUNTER(repo)
 GJS_DECLARE_COUNTER(resultset)
 GJS_DECLARE_COUNTER(weakhash)
 GJS_DECLARE_COUNTER(interface)
+GJS_DECLARE_COUNTER(constructor_proxy)
 
 #define GJS_INC_COUNTER(name)                \
     do {                                        \
diff --git a/modules/lang.js b/modules/lang.js
index 9583a79..67a7c35 100644
--- a/modules/lang.js
+++ b/modules/lang.js
@@ -201,27 +201,28 @@ Class.prototype._construct = function(params) {
     if (!parent)
         parent = _Base;
 
-    let newClass;
+    let newClassConstructor;
     if (params.Abstract) {
-        newClass = function() {
+        newClassConstructor = function() {
             throw new TypeError('Cannot instantiate abstract class ' + name);
         };
     } else {
-        newClass = function() {
+        newClassConstructor = function() {
             this.__caller__ = null;
 
             return this._construct.apply(this, arguments);
         };
     }
 
-    // Since it's not possible to create a constructor with
-    // a custom [[Prototype]], we have to do this to make
-    // "newClass instanceof Class" work, and so we can inherit
-    // methods/properties of Class.prototype, like wrapFunction.
-    newClass.__proto__ = this.constructor.prototype;
+    // This is our workaround for creating a constructor with a custom
+    // prototype. See jsapi-constructor-proxy.cpp.
+    let newClass = __private_GjsConstructorProxy(newClassConstructor,
+        this.constructor.prototype);
 
     newClass.__super__ = parent;
-    newClass.prototype = Object.create(parent.prototype);
+    // Here we have to set this property on newClassConstructor directly because
+    // otherwise the 'prototype' property on the proxy isn't configurable
+    newClassConstructor.prototype = Object.create(parent.prototype);
     newClass.prototype.constructor = newClass;
 
     newClass._init.apply(newClass, arguments);
diff --git a/modules/overrides/GObject.js b/modules/overrides/GObject.js
index cc41485..62c02d2 100644
--- a/modules/overrides/GObject.js
+++ b/modules/overrides/GObject.js
@@ -141,13 +141,14 @@ const GObjectMeta = new Lang.Class({
         let propertiesArray = _propertiesAsArray(params.Properties);
         delete params.Properties;
 
-        let newClass = Gi.register_type(parent.prototype, gtypename,
+        let newClassConstructor = Gi.register_type(parent.prototype, gtypename,
             gobjectInterfaces, propertiesArray);
 
-        // See Class.prototype._construct in lang.js for the reasoning
-        // behind this direct __proto__ set.
-        newClass.__proto__ = this.constructor.prototype;
+        let newClass = __private_GjsConstructorProxy(newClassConstructor,
+            this.constructor.prototype);
+
         newClass.__super__ = parent;
+        newClass.prototype.constructor = newClass;
 
         newClass._init.apply(newClass, arguments);
 
@@ -205,12 +206,11 @@ GObjectInterface.prototype._construct = function (params) {
     let properties = _propertiesAsArray(params.Properties);
     delete params.Properties;
 
-    let newInterface = Gi.register_interface(gtypename, gobjectInterfaces,
-        properties);
+    let newInterfaceConstructor = Gi.register_interface(gtypename,
+        gobjectInterfaces, properties);
 
-    // See Class.prototype._construct in lang.js for the reasoning
-    // behind this direct __proto__ set.
-    newInterface.__proto__ = this.constructor.prototype;
+    let newInterface = __private_GjsConstructorProxy(newInterfaceConstructor,
+       this.constructor.prototype);
     newInterface.__super__ = GObjectInterface;
     newInterface.prototype.constructor = newInterface;
 
diff --git a/util/log.cpp b/util/log.cpp
index 38901db..f787cbd 100644
--- a/util/log.cpp
+++ b/util/log.cpp
@@ -248,6 +248,9 @@ _Pragma("GCC diagnostic pop")
     case GJS_DEBUG_GERROR:
         prefix = "JS G ERR";
         break;
+    case GJS_DEBUG_PROXY:
+        prefix = "JS CPROXY";
+        break;
     default:
         prefix = "???";
         break;
diff --git a/util/log.h b/util/log.h
index dc8228b..7adf466 100644
--- a/util/log.h
+++ b/util/log.h
@@ -59,6 +59,7 @@ typedef enum {
     GJS_DEBUG_BYTE_ARRAY,
     GJS_DEBUG_GERROR,
     GJS_DEBUG_GFUNDAMENTAL,
+    GJS_DEBUG_PROXY,
 } GjsDebugTopic;
 
 /* These defines are because we have some pretty expensive and


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]