After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 691109 - Remove support for multiple contexts within a runtime
Remove support for multiple contexts within a runtime
Status: RESOLVED FIXED
Product: gjs
Classification: Bindings
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gjs-maint
gjs-maint
Depends on: 691108
Blocks:
 
 
Reported: 2013-01-04 05:01 UTC by Jasper St. Pierre (not reading bugmail)
Modified: 2013-01-07 18:36 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
context: Don't allow users to pass external runtimes to GjsContext (5.75 KB, patch)
2013-01-04 05:01 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
Remove support for multiple contexts within a runtime (15.26 KB, patch)
2013-01-04 05:01 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
context: Remove RuntimeData and gjs_runtime_init/gjs_runtime_destroy (4.80 KB, patch)
2013-01-04 05:01 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review

Description Jasper St. Pierre (not reading bugmail) 2013-01-04 05:01:17 UTC
This has always been a very hacky, weird situation, and I've thought
about just sort of scrapping the weird stack situation we have here.

After some eavesdropping on #jsapi talking about merging JSContext
and JSRuntime, I felt validated, and wrote this patch set.
Comment 1 Jasper St. Pierre (not reading bugmail) 2013-01-04 05:01:19 UTC
Created attachment 232701 [details] [review]
context: Don't allow users to pass external runtimes to GjsContext

Upstream wants to merge JSContext and JSRuntime, because the distinction
has been a constant source and confusion and pain as they rearrange the
runtime. As a first step towards making this shift on the gjs side, we
need to discourage external consumers of gjs from making the distinction.

Remove support for the "runtime" property which allows consumers of gjs
to specify their own JS runtime.
Comment 2 Jasper St. Pierre (not reading bugmail) 2013-01-04 05:01:21 UTC
Created attachment 232702 [details] [review]
Remove support for multiple contexts within a runtime

Remove code and support code to allow for multiple contexts, and
simply store one context. In a thread-safe world, we'd put each
context in thread-local storage, but gjs is not thread-safe, nor
do we expect it will be in the future.
Comment 3 Jasper St. Pierre (not reading bugmail) 2013-01-04 05:01:23 UTC
Created attachment 232703 [details] [review]
context: Remove RuntimeData and gjs_runtime_init/gjs_runtime_destroy

Now that RuntimeData only contains one pointer, we can just swap
out the allocation with a direct reference to the pointer instead.
The leftover initialization functions, not needing to do memory management,
become one-liners and we can simply squash them where used.
Comment 4 Jasper St. Pierre (not reading bugmail) 2013-01-04 05:02:15 UTC
Depends on bug #691108 only for some reindenting changes that confuse diff/patch. Can easily rebase without it, if wanted.
Comment 5 Colin Walters 2013-01-04 17:04:32 UTC
(In reply to comment #0)
> This has always been a very hacky, weird situation, and I've thought
> about just sort of scrapping the weird stack situation we have here.
> 
> After some eavesdropping on #jsapi talking about merging JSContext
> and JSRuntime, I felt validated, and wrote this patch set.

I think the main reason the JSContext/JSRuntime distinction is basically obsolete is because they have compartments now, which maps *much* better to the web browser model (domains).

We don't have anything analogous to that situation in GNOME apps because we use multiple operating system processes, so for us runtime == context == compartment.
Comment 6 Colin Walters 2013-01-07 18:03:08 UTC
Review of attachment 232701 [details] [review]:

Looks correct to me.
Comment 7 Colin Walters 2013-01-07 18:03:08 UTC
Review of attachment 232701 [details] [review]:

Looks correct to me.
Comment 8 Colin Walters 2013-01-07 18:04:58 UTC
Review of attachment 232702 [details] [review]:

Looks great.
Comment 9 Colin Walters 2013-01-07 18:05:42 UTC
Review of attachment 232703 [details] [review]:

Also looks great.
Comment 10 Jasper St. Pierre (not reading bugmail) 2013-01-07 18:36:18 UTC
Attachment 232701 [details] pushed as d9f8e52 - context: Don't allow users to pass external runtimes to GjsContext
Attachment 232702 [details] pushed as 244f186 - Remove support for multiple contexts within a runtime
Attachment 232703 [details] pushed as baf096b - context: Remove RuntimeData and gjs_runtime_init/gjs_runtime_destroy