[gjs/wip/ptomato/mozjs31prep] js: Remove the concept of a "context global"



commit a92321f0483de043fc48de52ebeda6f8b78b261c
Author: Philip Chimento <philip endlessm com>
Date:   Tue Oct 25 17:49:18 2016 -0700

    js: Remove the concept of a "context global"
    
    The idea of a global object associated with a JSContext used to be a
    thing, but was already deprecated in mozjs24. We continued to use it by
    peeking into a private API, but that is going away in mozjs31 as well.
    
    The object that we were really trying to get, when calling that private
    API, was actually the same object as returned by gjs_get_import_global():
    the object created in gjs_init_context_standard() as the GjsContext's
    global object. Therefore we can simply remove gjs_get_global_object(),
    the wrapper for the aforementioned private API, and call
    gjs_get_import_global() instead.
    
    The one problem with this was that the object was not referenced by the
    GjsContext anymore by the time the GjsContext's dispose function ran, and
    it was needed in order to dispose other stuff, so this would crash. It
    previously worked because the object still existed as the context global,
    you just couldn't reach it from the GjsContext. Now when we tried to
    reach it at dispose time, we just got a null pointer.
    
    This was due to my misunderstanding, in a previous commit, how a JS::Heap
    was supposed to work. Now instead of rooting the global object in
    GjsContext, we trace it, which allows it to remain alive until GjsContext
    is completely gone and then eventually be garbage-collected by JS.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=742249

 gjs/context.cpp             |   20 +++++++++++---------
 gjs/coverage.cpp            |    4 ++--
 gjs/jsapi-private.cpp       |    5 -----
 gjs/jsapi-util-error.cpp    |    2 +-
 gjs/jsapi-util.cpp          |    5 ++---
 gjs/jsapi-util.h            |    1 -
 test/gjs-test-call-args.cpp |    6 +++---
 test/gjs-test-utils.cpp     |    2 +-
 8 files changed, 20 insertions(+), 25 deletions(-)
---
diff --git a/gjs/context.cpp b/gjs/context.cpp
index 1d2d1c0..1f72f1e 100644
--- a/gjs/context.cpp
+++ b/gjs/context.cpp
@@ -325,16 +325,19 @@ gjs_context_class_init(GjsContextClass *klass)
 }
 
 static void
+gjs_context_tracer(JSTracer *trc, void *data)
+{
+    GjsContext *gjs_context = reinterpret_cast<GjsContext *>(data);
+    JS_CallHeapObjectTracer(trc, &gjs_context->global, "GJS global object");
+}
+
+static void
 gjs_context_dispose(GObject *object)
 {
     GjsContext *js_context;
 
     js_context = GJS_CONTEXT(object);
 
-    if (js_context->global != NULL) {
-        js_context->global = NULL;
-    }
-
     if (js_context->context != NULL) {
 
         gjs_debug(GJS_DEBUG_CONTEXT,
@@ -342,9 +345,6 @@ gjs_context_dispose(GObject *object)
 
         JS_BeginRequest(js_context->context);
 
-        JS_RemoveObjectRoot(js_context->context, js_context->global.unsafeGet());
-        js_context->global.set(NULL);
-
         /* Do a full GC here before tearing down, since once we do
          * that we may not have the JS_GetPrivate() to access the
          * context
@@ -365,6 +365,9 @@ gjs_context_dispose(GObject *object)
             js_context->auto_gc_id = 0;
         }
 
+        JS_RemoveExtraGCRootsTracer(js_context->runtime, gjs_context_tracer,
+                                    js_context);
+
         /* Tear down JS */
         JS_DestroyContext(js_context->context);
         js_context->context = NULL;
@@ -448,8 +451,7 @@ gjs_context_constructed(GObject *object)
         g_error("Failed to define properties on the global object");
 
     js_context->global.set(global);
-    JS_AddNamedObjectRoot(js_context->context, js_context->global.unsafeGet(),
-                          "global object");
+    JS_AddExtraGCRootsTracer(js_context->runtime, gjs_context_tracer, js_context);
 
     /* We create the global-to-runtime root importer with the
      * passed-in search path. If someone else already created
diff --git a/gjs/coverage.cpp b/gjs/coverage.cpp
index a9fda12..9c1c666 100644
--- a/gjs/coverage.cpp
+++ b/gjs/coverage.cpp
@@ -1569,7 +1569,7 @@ bootstrap_coverage(GjsCoverage *coverage)
     JSContext *context = (JSContext *) gjs_context_get_native_context(priv->context);
     JSAutoRequest ar(context);
 
-    JSObject *debuggee = gjs_get_global_object(context);
+    JSObject *debuggee = gjs_get_import_global(context);
     JS::CompartmentOptions options;
     options.setVersion(JSVERSION_LATEST);
     JS::RootedObject debugger_compartment(JS_GetRuntime(context),
@@ -1703,7 +1703,7 @@ gjs_coverage_constructed(GObject *object)
     JS_SetOptions(context, options_flags);
 
     if (!bootstrap_coverage(coverage)) {
-        JSAutoCompartment compartment(context, gjs_get_global_object(context));
+        JSAutoCompartment compartment(context, gjs_get_import_global(context));
         gjs_log_exception(context);
     }
 }
diff --git a/gjs/jsapi-private.cpp b/gjs/jsapi-private.cpp
index 632d1da..e032e4d 100644
--- a/gjs/jsapi-private.cpp
+++ b/gjs/jsapi-private.cpp
@@ -75,8 +75,3 @@ gjs_error_reporter(JSContext     *context,
 
     g_log(G_LOG_DOMAIN, level, "JS %s: [%s %d]: %s", warning, report->filename, report->lineno, message);
 }
-
-JSObject *
-gjs_get_global_object(JSContext *cx){
-    return js::GetDefaultGlobalForContext(cx);
-}
diff --git a/gjs/jsapi-util-error.cpp b/gjs/jsapi-util-error.cpp
index 9b24b48..3724667 100644
--- a/gjs/jsapi-util-error.cpp
+++ b/gjs/jsapi-util-error.cpp
@@ -55,7 +55,7 @@ gjs_throw_valist(JSContext       *context,
 
     s = g_strdup_vprintf(format, args);
 
-    JSAutoCompartment compartment(context, gjs_get_global_object(context));
+    JSAutoCompartment compartment(context, gjs_get_import_global(context));
 
     JS_BeginRequest(context);
 
diff --git a/gjs/jsapi-util.cpp b/gjs/jsapi-util.cpp
index 6213842..37a635b 100644
--- a/gjs/jsapi-util.cpp
+++ b/gjs/jsapi-util.cpp
@@ -94,7 +94,6 @@ gjs_init_context_standard (JSContext              *context,
     if (global == NULL)
         return false;
 
-    /* Set the context's global */
     JSAutoCompartment ac(context, global);
 
     if (!JS_InitStandardClasses(context, global))
@@ -115,7 +114,7 @@ gjs_set_global_slot (JSContext     *context,
                      JS::Value      value)
 {
     JSObject *global;
-    global = gjs_get_global_object(context);
+    global = gjs_get_import_global(context);
     JS_SetReservedSlot(global, JSCLASS_GLOBAL_SLOT_COUNT + slot, value);
 }
 
@@ -124,7 +123,7 @@ gjs_get_global_slot (JSContext     *context,
                      GjsGlobalSlot  slot)
 {
     JSObject *global;
-    global = gjs_get_global_object(context);
+    global = gjs_get_import_global(context);
     return JS_GetReservedSlot(global, JSCLASS_GLOBAL_SLOT_COUNT + slot);
 }
 
diff --git a/gjs/jsapi-util.h b/gjs/jsapi-util.h
index 9f39be2..78681bb 100644
--- a/gjs/jsapi-util.h
+++ b/gjs/jsapi-util.h
@@ -386,7 +386,6 @@ bool gjs_call_function_value(JSContext             *context,
 void        gjs_error_reporter               (JSContext       *context,
                                               const char      *message,
                                               JSErrorReport   *report);
-JSObject*   gjs_get_global_object            (JSContext *cx);
 
 JSBool      gjs_string_to_utf8               (JSContext       *context,
                                               const JS::Value  string_val,
diff --git a/test/gjs-test-call-args.cpp b/test/gjs-test-call-args.cpp
index 0bfa0e3..544c3bc 100644
--- a/test/gjs-test-call-args.cpp
+++ b/test/gjs-test-call-args.cpp
@@ -248,7 +248,7 @@ setup(GjsUnitTestFixture *fx,
 {
     gjs_unit_test_fixture_setup(fx, unused);
 
-    JS::RootedObject global(fx->cx, gjs_get_global_object(fx->cx));
+    JS::RootedObject global(fx->cx, gjs_get_import_global(fx->cx));
     bool success = JS_DefineFunctions(fx->cx, global, native_test_funcs);
     g_assert_true(success);
 }
@@ -262,7 +262,7 @@ run_code(GjsUnitTestFixture *fx,
     JS::CompileOptions options(fx->cx, JSVERSION_UNKNOWN);
     options.setFileAndLine("unit test", 1);
 
-    JS::RootedObject global(fx->cx, gjs_get_global_object(fx->cx));
+    JS::RootedObject global(fx->cx, gjs_get_import_global(fx->cx));
     bool ok = JS::Evaluate(fx->cx, global, options, script, strlen(script), NULL);
     JS_ReportPendingException(fx->cx);
 
@@ -279,7 +279,7 @@ run_code_expect_exception(GjsUnitTestFixture *fx,
     JS::CompileOptions options(fx->cx, JSVERSION_UNKNOWN);
     options.setFileAndLine("unit test", 1);
 
-    JS::RootedObject global(fx->cx, gjs_get_global_object(fx->cx));
+    JS::RootedObject global(fx->cx, gjs_get_import_global(fx->cx));
     bool ok = JS::Evaluate(fx->cx, global, options, script, strlen(script), NULL);
     g_assert_false(ok);
     g_assert_true(JS_IsExceptionPending(fx->cx));
diff --git a/test/gjs-test-utils.cpp b/test/gjs-test-utils.cpp
index 1e2ba27..7ed2afd 100644
--- a/test/gjs-test-utils.cpp
+++ b/test/gjs-test-utils.cpp
@@ -62,7 +62,7 @@ gjs_unit_test_fixture_setup(GjsUnitTestFixture *fx,
 
     JS_BeginRequest(fx->cx);
 
-    JS::RootedObject global(fx->cx, gjs_get_global_object(fx->cx));
+    JS::RootedObject global(fx->cx, gjs_get_import_global(fx->cx));
     fx->compartment = JS_EnterCompartment(fx->cx, global);
 }
 


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