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

10 files changed, 260 insertions, 19 deletions

diff --git a/Makefile.am b/Makefile.am
index 6ae6c7ea..66c82d56 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 60a03da7..477928f0 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 00000000..363a4186
--- /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 00000000..fc275063
--- /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 1e562d37..35ddfc4a 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 2b74eb1d..d4be4050 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 9583a795..67a7c359 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 cc414854..62c02d2b 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 38901dbe..f787cbd6 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 dc8228b3..7adf466b 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