[gjs: 5/10] atoms: Allow "symbol atoms" for keys not exposed to JS
- From: Philip Chimento <pchimento src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gjs: 5/10] atoms: Allow "symbol atoms" for keys not exposed to JS
- Date: Thu, 8 Nov 2018 03:23:32 +0000 (UTC)
commit 5c1a24ad2fb1ca5249eb96c6ce75e771160cb472
Author: Philip Chimento <philip chimento gmail com>
Date: Wed Sep 5 14:08:24 2018 -0400
atoms: Allow "symbol atoms" for keys not exposed to JS
There are a few places where we want to have property keys that are
well-known internally, but not available to JS code. We create "symbol
atoms" for these, where you can get a jsid from the GjsAtoms object, but
it refers to a JS::Symbol instead of a string.
Previously we had two instances of this, and both were implemented in a
different way. For "__gjsPrivateNS" we just used a string and hoped that
nobody used that string in client code. For "__GObject__hook_up_vfunc" we
did create a JS::Symbol and had some complicated static member in
ObjectInstance to expose it to imports._gi. Both of these implementations
are simplified with this patch.
gi/object.cpp | 20 ++------------------
gi/object.h | 4 ----
gi/private.cpp | 4 +++-
gjs/atoms.cpp | 16 ++++++++++++++++
gjs/atoms.h | 8 +++++++-
gjs/context.cpp | 2 ++
6 files changed, 30 insertions(+), 24 deletions(-)
---
diff --git a/gi/object.cpp b/gi/object.cpp
index 9b081834..fcc58be4 100644
--- a/gi/object.cpp
+++ b/gi/object.cpp
@@ -72,7 +72,6 @@ static_assert(sizeof(ObjectInstance) <= 88,
std::stack<JS::PersistentRootedObject> ObjectInstance::object_init_list{};
static bool weak_pointer_callback = false;
ObjectInstance *ObjectInstance::wrapped_gobject_list = nullptr;
-JS::PersistentRootedSymbol ObjectInstance::hook_up_vfunc_root;
extern struct JSClass gjs_object_instance_class;
GJS_DEFINE_PRIV_FROM_JS(ObjectBase, gjs_object_instance_class)
@@ -2149,19 +2148,6 @@ gjs_object_define_static_methods(JSContext *context,
return true;
}
-JS::Symbol*
-ObjectInstance::hook_up_vfunc_symbol(JSContext *cx)
-{
- if (!hook_up_vfunc_root.initialized()) {
- JS::RootedString descr(cx,
- JS_NewStringCopyZ(cx, "__GObject__hook_up_vfunc"));
- if (!descr)
- g_error("Out of memory defining internal symbol");
- hook_up_vfunc_root.init(cx, JS::NewSymbol(cx, descr));
- }
- return hook_up_vfunc_root;
-}
-
bool
gjs_define_object_class(JSContext *context,
JS::HandleObject in_object,
@@ -2245,9 +2231,8 @@ gjs_define_object_class(JSContext *context,
/* Hook_up_vfunc can't be included in gjs_object_instance_proto_funcs
* because it's a custom symbol. */
- JS::RootedId hook_up_vfunc(context,
- SYMBOL_TO_JSID(ObjectInstance::hook_up_vfunc_symbol(context)));
- if (!JS_DefineFunctionById(context, prototype, hook_up_vfunc,
+ const GjsAtoms& atoms = GjsContextPrivate::atoms(context);
+ if (!JS_DefineFunctionById(context, prototype, atoms.hook_up_vfunc(),
&ObjectBase::hook_up_vfunc, 3,
GJS_MODULE_PROP_FLAGS))
return false;
@@ -2270,7 +2255,6 @@ gjs_define_object_class(JSContext *context,
if (!gtype_obj)
return false;
- const GjsAtoms& atoms = GjsContextPrivate::atoms(context);
return JS_DefinePropertyById(context, constructor, atoms.gtype(), gtype_obj,
JSPROP_PERMANENT);
}
diff --git a/gi/object.h b/gi/object.h
index 536aea87..cd5d9a58 100644
--- a/gi/object.h
+++ b/gi/object.h
@@ -508,13 +508,9 @@ class ObjectInstance : public ObjectBase {
JS::MutableHandleObject obj);
/* Methods connected to "public" API */
- private:
- static JS::PersistentRootedSymbol hook_up_vfunc_root;
-
public:
GJS_USE
bool typecheck_object(JSContext* cx, GType expected_type, bool throw_error);
- static JS::Symbol* hook_up_vfunc_symbol(JSContext* cx);
/* Notification callbacks */
diff --git a/gi/private.cpp b/gi/private.cpp
index 5c938e7d..5a99dd7c 100644
--- a/gi/private.cpp
+++ b/gi/private.cpp
@@ -26,6 +26,7 @@
#include <unordered_map>
+#include "gjs/context-private.h"
#include "gjs/jsapi-util-args.h"
#include "gjs/jsapi-util.h"
#include "gjs/jsapi-wrapper.h"
@@ -406,7 +407,8 @@ GJS_JSAPI_RETURN_CONVENTION
static bool hook_up_vfunc_symbol_getter(JSContext* cx, unsigned argc,
JS::Value* vp) {
JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
- args.rval().setSymbol(ObjectInstance::hook_up_vfunc_symbol(cx));
+ const GjsAtoms& atoms = GjsContextPrivate::atoms(cx);
+ args.rval().setSymbol(JSID_TO_SYMBOL(atoms.hook_up_vfunc()));
return true;
}
diff --git a/gjs/atoms.cpp b/gjs/atoms.cpp
index 147bddef..799840d3 100644
--- a/gjs/atoms.cpp
+++ b/gjs/atoms.cpp
@@ -35,9 +35,25 @@ GjsAtoms::GjsAtoms(JSContext* cx) {
#undef INITIALIZE_ATOM
}
+/* Requires a current compartment. Unlike the atoms initialized in the
+ * constructor, this can GC, so it needs to be done after the tracing has been
+ * set up. */
+void GjsAtoms::init_symbols(JSContext* cx) {
+ JS::RootedString descr(cx);
+ JS::Symbol* symbol;
+
+#define INITIALIZE_SYMBOL_ATOM(identifier, str) \
+ descr = JS_AtomizeAndPinString(cx, str); \
+ symbol = JS::NewSymbol(cx, descr); \
+ m_##identifier = SYMBOL_TO_JSID(symbol);
+ FOR_EACH_SYMBOL_ATOM(INITIALIZE_SYMBOL_ATOM)
+#undef INITIALIZE_SYMBOL_ATOM
+}
+
void GjsAtoms::trace(JSTracer* trc) {
#define TRACE_ATOM(identifier, str) \
JS::TraceEdge<jsid>(trc, &m_##identifier, "Atom " str);
FOR_EACH_ATOM(TRACE_ATOM)
+ FOR_EACH_SYMBOL_ATOM(TRACE_ATOM)
#undef TRACE_ATOM
}
diff --git a/gjs/atoms.h b/gjs/atoms.h
index 0e648e88..69ef49ba 100644
--- a/gjs/atoms.h
+++ b/gjs/atoms.h
@@ -56,7 +56,6 @@
macro(overrides, "overrides") \
macro(param_spec, "ParamSpec") \
macro(parent_module, "__parentModule__") \
- macro(private_ns_marker, "__gjsPrivateNS") \
macro(program_invocation_name, "programInvocationName") \
macro(prototype, "prototype") \
macro(search_path, "searchPath") \
@@ -68,15 +67,21 @@
macro(window, "window") \
macro(x, "x") \
macro(y, "y")
+
+#define FOR_EACH_SYMBOL_ATOM(macro) \
+ macro(hook_up_vfunc, "__GObject__hook_up_vfunc") \
+ macro(private_ns_marker, "__gjsPrivateNS")
// clang-format on
class GjsAtoms {
#define DECLARE_ATOM_MEMBER(identifier, str) JS::Heap<jsid> m_##identifier;
FOR_EACH_ATOM(DECLARE_ATOM_MEMBER)
+ FOR_EACH_SYMBOL_ATOM(DECLARE_ATOM_MEMBER)
#undef DECLARE_ATOM_MEMBER
public:
explicit GjsAtoms(JSContext* cx);
+ void init_symbols(JSContext* cx);
void trace(JSTracer* trc);
@@ -87,6 +92,7 @@ class GjsAtoms {
return JS::HandleId::fromMarkedLocation(&m_##identifier.get()); \
}
FOR_EACH_ATOM(DECLARE_ATOM_ACCESSOR)
+ FOR_EACH_SYMBOL_ATOM(DECLARE_ATOM_ACCESSOR)
#undef DECLARE_ATOM_ACCESSOR
};
diff --git a/gjs/context.cpp b/gjs/context.cpp
index 8096cd3f..999279c4 100644
--- a/gjs/context.cpp
+++ b/gjs/context.cpp
@@ -462,6 +462,8 @@ GjsContextPrivate::GjsContextPrivate(JSContext* cx, GjsContext* public_context)
m_global = global;
JS_AddExtraGCRootsTracer(m_cx, &GjsContextPrivate::trace, this);
+ m_atoms.init_symbols(m_cx);
+
JS::RootedObject importer(m_cx,
gjs_create_root_importer(m_cx, m_search_path));
if (!importer) {
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]