[gjs/wip/ptomato/mozjs52: 21/40] js: Set JSPROP_RESOLVING when defining properties

[Philip Chimento <pchimento src gnome org>]

commit ad3cd03c5e98095651de390c9b47975b01db9c50
Author: Philip Chimento <philip chimento gmail com>
Date:   Tue Mar 21 10:30:59 2017 +0000

    js: Set JSPROP_RESOLVING when defining properties
    
    JS_DefineProperty() can now call into a class's resolve hook, so when
    pre-defining properties on classes with resolve hooks we have to include
    JSPROP_RESOLVING in the flags so that the resolve hook is not called.

 gi/repo.cpp                 |    4 ++--
 gjs/importer.cpp            |   28 +++++++++++++---------------
 gjs/jsapi-dynamic-class.cpp |   26 ++++++++++++++++++++++++--
 3 files changed, 39 insertions(+), 19 deletions(-)
---
diff --git a/gi/repo.cpp b/gi/repo.cpp
index 3727e23..5c10be9 100644
--- a/gi/repo.cpp
+++ b/gi/repo.cpp
@@ -272,7 +272,7 @@ repo_new(JSContext *context)
 
     JS::RootedObject versions(context, JS_NewPlainObject(context));
     gjs_object_define_property(context, repo, GJS_STRING_GI_VERSIONS,
-                               versions, JSPROP_PERMANENT);
+                               versions, JSPROP_PERMANENT | JSPROP_RESOLVING);
 
     /* GLib/GObject/Gio are fixed at 2.0, since we depend on them
      * internally.
@@ -291,7 +291,7 @@ repo_new(JSContext *context)
     JS::RootedObject private_ns(context, JS_NewPlainObject(context));
     gjs_object_define_property(context, repo,
                                GJS_STRING_PRIVATE_NS_MARKER, private_ns,
-                               JSPROP_PERMANENT);
+                               JSPROP_PERMANENT | JSPROP_RESOLVING);
 
     return repo;
 }
diff --git a/gjs/importer.cpp b/gjs/importer.cpp
index dd77cd1..a7f1a2b 100644
--- a/gjs/importer.cpp
+++ b/gjs/importer.cpp
@@ -100,6 +100,12 @@ define_meta_properties(JSContext       *context,
 {
     bool parent_is_module;
 
+    /* For these meta-properties, don't set ENUMERATE since we wouldn't want to
+     * copy these symbols to any other object for example. RESOLVING is used to
+     * make sure we don't try to invoke a "resolve" operation, since this
+     * function may be called from inside one. */
+    unsigned attrs = JSPROP_READONLY | JSPROP_PERMANENT | JSPROP_RESOLVING;
+
     /* We define both __moduleName__ and __parentModule__ to null
      * on the root importer
      */
@@ -112,16 +118,12 @@ define_meta_properties(JSContext       *context,
 
     if (full_path != NULL) {
         JS::RootedString file(context, JS_NewStringCopyZ(context, full_path));
-        if (!JS_DefineProperty(context, module_obj, "__file__", file,
-                               /* don't set ENUMERATE since we wouldn't want to copy
-                                * this symbol to any other object for example.
-                                */
-                               JSPROP_READONLY | JSPROP_PERMANENT))
+        if (!JS_DefineProperty(context, module_obj, "__file__", file, attrs))
             return false;
     }
 
-    /* Null is used instead of undefined to make sure we don't try to invoke
-     * a "resolve" operation. */
+    /* Null is used instead of undefined for backwards compatibility with code
+     * that explicitly checks for null. */
     JS::RootedValue module_name_val(context, JS::NullValue());
     JS::RootedValue parent_module_val(context, JS::NullValue());
     JS::RootedValue module_path(context, JS::NullValue());
@@ -148,20 +150,16 @@ define_meta_properties(JSContext       *context,
         module_path.setString(JS_NewStringCopyZ(context, module_path_buf));
     }
 
-    /* don't set ENUMERATE since we wouldn't want to copy these symbols to any
-     * other object for example. */
     if (!JS_DefineProperty(context, module_obj,
-                           "__moduleName__", module_name_val,
-                           JSPROP_READONLY | JSPROP_PERMANENT))
+                           "__moduleName__", module_name_val, attrs))
         return false;
 
     if (!JS_DefineProperty(context, module_obj,
-                           "__parentModule__", parent_module_val,
-                           JSPROP_READONLY | JSPROP_PERMANENT))
+                           "__parentModule__", parent_module_val, attrs))
         return false;
 
     if (!JS_DefineProperty(context, module_obj, "__modulePath__", module_path,
-                           JSPROP_READONLY | JSPROP_PERMANENT))
+                           attrs))
         return false;
 
     return true;
@@ -1023,7 +1021,7 @@ gjs_create_importer(JSContext       *context,
     if (!gjs_define_string_array(context, importer,
                                  "searchPath", -1, (const char **)search_path,
                                  /* settable (no READONLY) but not deleteable (PERMANENT) */
-                                 JSPROP_PERMANENT))
+                                 JSPROP_PERMANENT | JSPROP_RESOLVING))
         g_error("no memory to define importer search path prop");
 
     g_strfreev(search_path);
diff --git a/gjs/jsapi-dynamic-class.cpp b/gjs/jsapi-dynamic-class.cpp
index 7d45339..36e830d 100644
--- a/gjs/jsapi-dynamic-class.cpp
+++ b/gjs/jsapi-dynamic-class.cpp
@@ -90,6 +90,16 @@ gjs_init_class_dynamic(JSContext              *context,
     if (!prototype)
         goto out;
 
+    /* Bypass resolve hooks when defining the initial properties */
+    if (clasp->resolve) {
+        JSPropertySpec *ps_iter;
+        JSFunctionSpec *fs_iter;
+        for (ps_iter = proto_ps; ps_iter && ps_iter->name; ps_iter++)
+            ps_iter->flags |= JSPROP_RESOLVING;
+        for (fs_iter = proto_fs; fs_iter && fs_iter->name; fs_iter++)
+            fs_iter->flags |= JSPROP_RESOLVING;
+    }
+
     if (proto_ps && !JS_DefineProperties(context, prototype, proto_ps))
         goto out;
     if (proto_fs && !JS_DefineFunctions(context, prototype, proto_fs))
@@ -108,8 +118,20 @@ gjs_init_class_dynamic(JSContext              *context,
     if (static_fs && !JS_DefineFunctions(context, constructor, static_fs))
         goto out;
 
-    if (!JS_LinkConstructorAndPrototype(context, constructor, prototype))
-        goto out;
+    if (!clasp->resolve) {
+        if (!JS_LinkConstructorAndPrototype(context, constructor, prototype))
+            goto out;
+    } else {
+        /* Have to fake it with JSPROP_RESOLVING, otherwise it will trigger
+         * the resolve hook */
+        if (!JS_DefineProperty(context, constructor, "prototype", prototype,
+                               JSPROP_PERMANENT | JSPROP_READONLY | JSPROP_RESOLVING,
+                               JS_STUBGETTER, JS_STUBSETTER))
+            goto out;
+        if (!JS_DefineProperty(context, prototype, "constructor", constructor,
+                               JSPROP_RESOLVING, JS_STUBGETTER, JS_STUBSETTER))
+            goto out;
+    }
 
     /* The constructor defined by JS_InitClass has no property attributes, but this
        is a more useful default for gjs */