[gjs/wip/ptomato/mozjs38: 11/28] context: Root interned strings

[Philip Chimento <pchimento src gnome org>]

commit 054dc0e1d43e37fad34a937f12308bd315f77557
Author: Philip Chimento <philip endlessm com>
Date:   Mon Jan 23 15:48:26 2017 -0800

    context: Root interned strings
    
    We put the collection of interned strings into an array of
    JS::PersistentRootedId to make sure they are not garbage-collected;
    otherwise there was nothing keeping them from being freed.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=776966

 gi/boxed.cpp       |    4 ++--
 gi/repo.cpp        |    4 ++--
 gjs/context.cpp    |   21 ++++++++++++++++-----
 gjs/jsapi-util.cpp |   15 +++++++++------
 gjs/jsapi-util.h   |   15 ++++++++-------
 5 files changed, 37 insertions(+), 22 deletions(-)
---
diff --git a/gi/boxed.cpp b/gi/boxed.cpp
index c6d8ddc..bf6e478 100644
--- a/gi/boxed.cpp
+++ b/gi/boxed.cpp
@@ -335,8 +335,8 @@ boxed_new(JSContext             *context,
     if (priv->gtype == G_TYPE_VARIANT) {
         /* Short-circuit construction for GVariants by calling into the JS packing
            function */
-        JS::RootedId constructor_name(context,
-            gjs_context_get_const_string(context, GJS_STRING_NEW_INTERNAL));
+        JS::HandleId constructor_name =
+            gjs_context_get_const_string(context, GJS_STRING_NEW_INTERNAL);
         return boxed_invoke_constructor(context, obj, constructor_name, args);
     }
 
diff --git a/gi/repo.cpp b/gi/repo.cpp
index cd80cd3..4d65f3d 100644
--- a/gi/repo.cpp
+++ b/gi/repo.cpp
@@ -533,8 +533,8 @@ gjs_define_info(JSContext       *context,
 JSObject*
 gjs_lookup_private_namespace(JSContext *context)
 {
-    JS::RootedId ns_name(context,
-        gjs_context_get_const_string(context, GJS_STRING_PRIVATE_NS_MARKER));
+    JS::HandleId ns_name =
+        gjs_context_get_const_string(context, GJS_STRING_PRIVATE_NS_MARKER);
     return gjs_lookup_namespace_object_by_name(context, ns_name);
 }
 
diff --git a/gjs/context.cpp b/gjs/context.cpp
index 093df64..67da414 100644
--- a/gjs/context.cpp
+++ b/gjs/context.cpp
@@ -23,6 +23,8 @@
 
 #include <config.h>
 
+#include <array>
+
 #include <gio/gio.h>
 
 #include "context-private.h"
@@ -75,7 +77,7 @@ struct _GjsContext {
 
     guint    auto_gc_id;
 
-    jsid const_strings[GJS_STRING_LAST];
+    std::array<JS::PersistentRootedId*, GJS_STRING_LAST> const_strings;
 };
 
 /* Keep this consistent with GjsConstString */
@@ -409,6 +411,9 @@ gjs_context_dispose(GObject *object)
                                     js_context);
         js_context->global = NULL;
 
+        for (auto& root : js_context->const_strings)
+            delete root;
+
         /* Tear down JS */
         JS_DestroyContext(js_context->context);
         js_context->context = NULL;
@@ -468,8 +473,11 @@ gjs_context_constructed(GObject *object)
     if (js_context->context == NULL)
         g_error("Failed to create javascript context");
 
-    for (i = 0; i < GJS_STRING_LAST; i++)
-        js_context->const_strings[i] = gjs_intern_string_to_id(js_context->context, const_strings[i]);
+    for (i = 0; i < GJS_STRING_LAST; i++) {
+        js_context->const_strings[i] =
+            new JS::PersistentRootedId(js_context->context,
+                gjs_intern_string_to_id(js_context->context, const_strings[i]));
+    }
 
     JS_BeginRequest(js_context->context);
 
@@ -842,12 +850,15 @@ gjs_context_make_current (GjsContext *context)
     current_context = context;
 }
 
-jsid
+/* It's OK to return JS::HandleId here, to avoid an extra root, with the
+ * caveat that you should not use this value after the GjsContext has
+ * been destroyed. */
+JS::HandleId
 gjs_context_get_const_string(JSContext      *context,
                              GjsConstString  name)
 {
     GjsContext *gjs_context = (GjsContext *) JS_GetContextPrivate(context);
-    return gjs_context->const_strings[name];
+    return *gjs_context->const_strings[name];
 }
 
 /**
diff --git a/gjs/jsapi-util.cpp b/gjs/jsapi-util.cpp
index d8ca3ed..4f7204f 100644
--- a/gjs/jsapi-util.cpp
+++ b/gjs/jsapi-util.cpp
@@ -148,8 +148,9 @@ gjs_object_get_property(JSContext             *cx,
                         GjsConstString         property_name,
                         JS::MutableHandleValue value_p)
 {
-    JS::RootedId id(cx, gjs_context_get_const_string(cx, property_name));
-    return JS_GetPropertyById(cx, obj, id, value_p);
+    return JS_GetPropertyById(cx, obj,
+                              gjs_context_get_const_string(cx, property_name),
+                              value_p);
 }
 
 bool
@@ -158,8 +159,9 @@ gjs_object_set_property(JSContext       *cx,
                         GjsConstString   property_name,
                         JS::HandleValue  value)
 {
-    JS::RootedId id(cx, gjs_context_get_const_string(cx, property_name));
-    return JS_SetPropertyById(cx, obj, id, value);
+    return JS_SetPropertyById(cx, obj,
+                              gjs_context_get_const_string(cx, property_name),
+                              value);
 }
 
 bool
@@ -168,8 +170,9 @@ gjs_object_has_property(JSContext       *cx,
                         GjsConstString   property_name,
                         bool            *found)
 {
-    JS::RootedId id(cx, gjs_context_get_const_string(cx, property_name));
-    return JS_HasPropertyById(cx, obj, id, found);
+    return JS_HasPropertyById(cx, obj,
+                              gjs_context_get_const_string(cx, property_name),
+                              found);
 }
 
 bool gjs_object_define_property(JSContext         *cx,
diff --git a/gjs/jsapi-util.h b/gjs/jsapi-util.h
index 1fc33d8..f89060c 100644
--- a/gjs/jsapi-util.h
+++ b/gjs/jsapi-util.h
@@ -455,9 +455,6 @@ typedef enum {
   GJS_STRING_LAST
 } GjsConstString;
 
-jsid              gjs_context_get_const_string  (JSContext       *context,
-                                                 GjsConstString   string);
-
 const char * gjs_strip_unix_shebang(const char *script,
                                     gssize     *script_len,
                                     int        *new_start_line_number);
@@ -490,6 +487,9 @@ bool gjs_object_define_property(JSContext         *cx,
 
 G_END_DECLS
 
+JS::HandleId gjs_context_get_const_string(JSContext     *cx,
+                                          GjsConstString string);
+
 /* Overloaded functions, must be outside G_DECLS. More types are intended to be
  * added as the opportunity arises. */
 
@@ -538,8 +538,9 @@ bool gjs_object_require_property(JSContext        *cx,
                                  GjsConstString    property_name,
                                  T                 value)
 {
-    JS::RootedId id(cx, gjs_context_get_const_string(cx, property_name));
-    return gjs_object_require_property(cx, obj, description, id, value);
+    return gjs_object_require_property(cx, obj, description,
+                                       gjs_context_get_const_string(cx, property_name),
+                                       value);
 }
 
 template<typename T>
@@ -549,9 +550,9 @@ bool gjs_object_require_converted_property(JSContext       *cx,
                                            GjsConstString   property_name,
                                            T                value)
 {
-    JS::RootedId id(cx, gjs_context_get_const_string(cx, property_name));
     return gjs_object_require_converted_property(cx, obj, description,
-                                                 id, value);
+                                                 gjs_context_get_const_string(cx, property_name),
+                                                 value);
 }
 
 #endif  /* __GJS_JSAPI_UTIL_H__ */