[gjs/wip/ptomato/mozjs52: 12/12] WIP - js: Allow access to modules' lexical scope

[Philip Chimento <pchimento src gnome org>]

commit c47add13bef0826c386de18d669e258a85dc3b66
Author: Philip Chimento <philip chimento gmail com>
Date:   Sat Jun 24 22:28:32 2017 -0700

    WIP - js: Allow access to modules' lexical scope
    
    For compatibility with pre-ES6 code, we allow imported modules to access
    members of the lexical scope (i.e. variables defined with 'const' or
    'let') as if they were properties, because that is how SpiderMonkey
    previously implemented 'let' and 'const'.
    
    When such properties are accessed, however, we log a warning that tells
    people to fix their code. Hopefully such uses will become rare and we can
    remove this compatibility workaround.
    
    FIXME: This causes all sorts of unexpected effects with modules, since
    they are executed in a different compartment. I'm not sure this is a
    great idea.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=784196

 gjs/module.cpp                     |  152 ++++++++++++++++++++++++++++++++++-
 installed-tests/js/testImporter.js |   17 ++++
 2 files changed, 164 insertions(+), 5 deletions(-)
---
diff --git a/gjs/module.cpp b/gjs/module.cpp
index 5028d06..f1d0be7 100644
--- a/gjs/module.cpp
+++ b/gjs/module.cpp
@@ -23,6 +23,7 @@
 
 #include <gio/gio.h>
 
+#include "global.h"
 #include "jsapi-private.h"
 #include "jsapi-util.h"
 #include "jsapi-wrapper.h"
@@ -44,12 +45,25 @@ class GjsModule {
 
     /* Private data accessors */
 
+    enum Slot {
+        LEXICAL_ENV,
+        NSLOTS
+    };
+
     static inline GjsModule *
     priv(JSObject *module)
     {
         return static_cast<GjsModule *>(JS_GetPrivate(module));
     }
 
+    static inline JSObject *
+    lexical_env(JSObject *module)
+    {
+        JS::Value v_lexical = JS_GetReservedSlot(module, Slot::LEXICAL_ENV);
+        g_assert(v_lexical.isObjectOrNull());
+        return v_lexical.toObjectOrNull();
+    }
+
     /* Creates a JS module object. Use instead of the class's constructor */
     static JSObject *
     create(JSContext  *cx,
@@ -57,6 +71,7 @@ class GjsModule {
     {
         JSObject *module = JS_NewObject(cx, &GjsModule::klass);
         JS_SetPrivate(module, new GjsModule(name));
+        JS_SetReservedSlot(module, Slot::LEXICAL_ENV, JS::NullValue());
         return module;
     }
 
@@ -77,6 +92,38 @@ class GjsModule {
         return true;
     }
 
+    static bool
+    is_global_property(jsid prop_name)
+    {
+        if (!JSID_IS_STRING(prop_name))
+            return false;
+        JSFlatString *str = JSID_TO_FLAT_STRING(prop_name);
+        return (JS_FlatStringEqualsAscii(str, "log") ||
+            JS_FlatStringEqualsAscii(str, "logError") ||
+            JS_FlatStringEqualsAscii(str, "print") ||
+            JS_FlatStringEqualsAscii(str, "printerr") ||
+            JS_FlatStringEqualsAscii(str, "imports"));
+    }
+
+    bool
+    obtain_lexical_scope(JSContext       *cx,
+                         JS::HandleObject module,
+                         JS::HandleObject global)
+    {
+        gjs_debug(GJS_DEBUG_IMPORTER, "Importing lexical environment of module "
+                  "%s for compatibility", m_name);
+
+        JS::RootedObject lexical(cx, JS_GlobalLexicalEnvironment(global));
+
+        JSAutoCompartment ac(cx, module);
+
+        if (!JS_WrapObject(cx, &lexical))
+            return false;
+
+        JS_SetReservedSlot(module, Slot::LEXICAL_ENV, JS::ObjectValue(*lexical));
+        return true;
+    }
+
     /* Carries out the actual execution of the module code */
     bool
     evaluate_import(JSContext       *cx,
@@ -86,19 +133,66 @@ class GjsModule {
                     const char      *filename,
                     int              line_number)
     {
+        JS::RootedObject outer_global(cx, gjs_get_import_global(cx));
+        JS::RootedObject global(cx, gjs_create_global_object(cx));
+        if (!global)
+            return false;
+
+        JS::AutoIdVector props(cx);
+        if (!js::GetPropertyKeys(cx, outer_global,
+                                 JSITER_OWNONLY | JSITER_SYMBOLS,
+                                 &props))
+            return false;
+
+        JSAutoCompartment ac(cx, global);
+
+        if (!gjs_define_global_properties(cx, global))
+            return false;
+
+        /* Copy any properties from the global object over to the module-
+         * specific global object, that client code might have defined
+         * before the import. */
+        {
+            JSAutoCompartment outer_compartment(cx, outer_global);
+            for (size_t ix = 0; ix < props.length(); ++ix) {
+                if (is_global_property(props[ix]))
+                    continue;
+                if (!JS_CopyPropertyFrom(cx, props[ix], global, outer_global))
+                    return false;
+            }
+        }
+
+        /* Wrap the outer global as the inner global's "window" object,
+         * instead of pointing it to the inner global itself; this allows
+         * the module to define properties on the actual global object */
+        JS::RootedId window_name(cx, gjs_intern_string_to_id(cx, "window"));
+        JS::RootedObject wrapped_outer_global(cx, outer_global);
+        if (!JS_WrapObject(cx, &wrapped_outer_global))
+            return false;
+        if (!JS_DefinePropertyById(cx, global, window_name, wrapped_outer_global,
+                                   JSPROP_READONLY | JSPROP_PERMANENT |
+                                   JSPROP_REDEFINE_NONCONFIGURABLE))
+            return false;
+
+        JS::RootedObject wrapped_module(cx, module);
+        if (!JS_WrapObject(cx, &wrapped_module))
+            return false;
+
         JS::CompileOptions options(cx);
         options.setUTF8(true)
                .setFileAndLine(filename, line_number)
                .setSourceIsLazy(true);
 
         JS::RootedScript compiled_script(cx);
-        if (!JS::Compile(cx, options, script, script_len, &compiled_script))
+        if (!JS::CompileForNonSyntacticScope(cx, options, script, script_len,
+                                             &compiled_script))
             return false;
 
         JS::AutoObjectVector scope_chain(cx);
-        scope_chain.append(module);
+        scope_chain.append(wrapped_module);
         JS::RootedValue ignored_retval(cx);
-        if (!JS_ExecuteScript(cx, scope_chain, compiled_script, &ignored_retval))
+        if (!JS_ExecuteScript(cx, scope_chain, compiled_script, &ignored_retval) ||
+            !obtain_lexical_scope(cx, module, global))
             return false;
 
         gjs_schedule_gc_if_needed(cx);
@@ -138,6 +232,53 @@ class GjsModule {
 
     /* JSClass operations */
 
+    bool
+    resolve_impl(JSContext       *cx,
+                 JS::HandleObject module,
+                 JS::HandleId     id,
+                 bool            *resolved)
+    {
+        JS::RootedObject lexical(cx, lexical_env(module));
+        if (!lexical) {
+            *resolved = false;
+            return true;  /* nothing imported yet */
+        }
+
+        if (!JS_HasPropertyById(cx, lexical, id, resolved))
+            return false;
+        if (!*resolved)
+            return true;
+
+        /* The property is present in the lexical environment. This should not
+         * be supported according to ES6. For compatibility with earlier GJS,
+         * we treat it as if it were a real property, but warn about it. */
+
+        GjsAutoJSChar prop_name(cx);
+        if (!gjs_get_string_id(cx, id, &prop_name))
+            return false;
+
+        g_warning("Some code accessed the property '%s' on the module '%s'. "
+                  "That property was defined with 'let' or 'const' inside the "
+                  "module. This was previously supported, but is not correct "
+                  "according to the ES6 standard. Any symbols to be exported "
+                  "from a module must be defined with 'var'. The property "
+                  "access will work as previously for the time being, but "
+                  "please fix your code anyway.", prop_name.get(), m_name);
+
+        JS::Rooted<JS::PropertyDescriptor> desc(cx);
+        return JS_GetPropertyDescriptorById(cx, lexical, id, &desc) &&
+            JS_DefinePropertyById(cx, module, id, desc);
+    }
+
+    static bool
+    resolve(JSContext       *cx,
+            JS::HandleObject module,
+            JS::HandleId     id,
+            bool            *resolved)
+    {
+        return priv(module)->resolve_impl(cx, module, id, resolved);
+    }
+
     static void
     finalize(JSFreeOp *op,
              JSObject *module)
@@ -151,14 +292,15 @@ class GjsModule {
         nullptr,  /* getProperty */
         nullptr,  /* setProperty */
         nullptr,  /* enumerate */
-        nullptr,  /* resolve */
+        &GjsModule::resolve,
         nullptr,  /* mayResolve */
         &GjsModule::finalize,
     };
 
     static constexpr JSClass klass = {
         "GjsModule",
-        JSCLASS_HAS_PRIVATE | JSCLASS_BACKGROUND_FINALIZE,
+        JSCLASS_HAS_PRIVATE | JSCLASS_BACKGROUND_FINALIZE |
+        JSCLASS_HAS_RESERVED_SLOTS(Slot::NSLOTS),
         &GjsModule::class_ops,
     };
 
diff --git a/installed-tests/js/testImporter.js b/installed-tests/js/testImporter.js
index 35879b1..e216561 100644
--- a/installed-tests/js/testImporter.js
+++ b/installed-tests/js/testImporter.js
@@ -164,6 +164,23 @@ describe('Importer', function () {
             LexicalScope = imports.lexicalScope;
         });
 
+        it('will log a compatibility warning when accessed', function () {
+            const GLib = imports.gi.GLib;
+            GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_WARNING,
+                "Some code accessed the property 'b' on the module " +
+                "'lexicalScope'.*");
+            GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_WARNING,
+                "Some code accessed the property 'c' on the module " +
+                "'lexicalScope'.*");
+
+            void LexicalScope.b;
+            void LexicalScope.c;
+
+            // g_test_assert_expected_messages() is a macro, not introspectable
+            GLib.test_assert_expected_messages_internal('Gjs',
+                'testImporter.js', 179, '');
+        });
+
         it('can be accessed', function () {
             expect(LexicalScope.a).toEqual(1);
             expect(LexicalScope.b).toEqual(2);