[Edbrowse-dev] compartments

[Edbrowse-dev] compartments

[Karl Dahlke]

Sorry to bother you; don't know if this should move off-line.

> a runtime is created which holds everything else

Ok.

> A runtime can contain multiple contexts,

Ok.

> Each context contains multiple compartments;

Ok.

> In edbrowse each window has its own js context.

Ok.

> Each page within that window has its own global object.

But there is only one page within the window.
An edbrowse window has one html file, one context,
and One global object via JS_NewGlobalObject().
Each context has one global object, and thus one compartment.
So the context is given as the first argument to a js function,
and from there there should be just one compartment and one global object.
I don't see how js could ever get the wrong compartment,
there is only one to choose from.

> What this means when switching pages is that the context needs to be switched

Switch to another edbrowse page and you get another js context,
with its one and only one global object.

I don't doubt you at all when you say the compartment calls
are necessary, I'm just trying to figure out why they are necessary.
If they are, maybe could be confined to jSyncup(),
which starts the javascript thread, and j new context to build
the global for the first time.

Karl Dahlke

Re: [Edbrowse-dev] compartments

[Adam Thompson]

On Tue, Feb 04, 2014 at 02:17:19PM -0500, Karl Dahlke wrote:
> > In edbrowse each window has its own js context.
> 
> Ok.
> 
> > Each page within that window has its own global object.
> 
> But there is only one page within the window.
> An edbrowse window has one html file, one context,
> and One global object via JS_NewGlobalObject().
> Each context has one global object, and thus one compartment.
> So the context is given as the first argument to a js function,
> and from there there should be just one compartment and one global object.
> I don't see how js could ever get the wrong compartment,
> there is only one to choose from.

In theory, but the compartment field is, by default, NULL.
The context needs to explicitly be told to use the compartment of the global object.

> > What this means when switching pages is that the context needs to be switched
> 
> Switch to another edbrowse page and you get another js context,
> with its one and only one global object.
> 
> I don't doubt you at all when you say the compartment calls
> are necessary, I'm just trying to figure out why they are necessary.
> If they are, maybe could be confined to jSyncup(),
> which starts the javascript thread, and j new context to build
> the global for the first time.

When I say a page within a window I'm referring to the ability of edbrowse to go back pages.
This seems to be where the trouble happens with compartments,
and to fix this I just used the autocompartment construct.
How are js contexts handled in this case?

Cheers,
Adam.

[Edbrowse-dev] compartments

[Karl Dahlke]

> When I say a page within a window I'm referring
> to the ability of edbrowse to go back pages.

cw = cw->prev

That's a little over simplified, but that's the idea,
go back to the previous edbrowse window, a completely separate window.
Really not different from switching buffers.
The previous window has its own context, its own global object,
its own html page, etc.

>In theory, but the compartment field is, by default, NULL.

Ok.

I'm not going to mess with it now, I'm working on the representation of lines,
but I may want to remove all those calls and just put one in jSyncup(),
and see if that works.
That is the function that is always called when you enter the world of javascript.
Click a button, submit a form, change a field with onchange code,
any of those things, calls jSyncup().
This takes any changes you made to the form in html
and updates the corresponding js variables,
so you see it would have to be called first before running any js.
If it also set the compartment we would be good, I think.
And we don't really have to be RAII to do this,
just set the comparment to what it should be at that point,
though RAII doesn't hurt.
There might be a simpler function call.
I'll experiment when I'm not in the middle of other changes.

Karl Dahlke

Re: [Edbrowse-dev] compartments

[Adam Thompson]

On Tue, Feb 04, 2014 at 05:03:41PM -0500, Karl Dahlke wrote:
> > When I say a page within a window I'm referring
> > to the ability of edbrowse to go back pages.
> 
> cw = cw->prev
> 
> That's a little over simplified, but that's the idea,
> go back to the previous edbrowse window, a completely separate window.
> Really not different from switching buffers.
> The previous window has its own context, its own global object,
> its own html page, etc.

Ok. I'm not arguing this is a nice solution,
but when I tried to work around it, something wasn't working correctly.
I'm not entirely sure why but this was the only way I could get it to behave.

> I'm not going to mess with it now, I'm working on the representation of lines,
> but I may want to remove all those calls and just put one in jSyncup(),
> and see if that works.
> That is the function that is always called when you enter the world of javascript.
> Click a button, submit a form, change a field with onchange code,
> any of those things, calls jSyncup().

Are *all* the js functions called from within jsyncup?
I don't think this is the case with creating a new context etc,
so we still need that one. Also, at the time I did the js changes,
html.c was still there so putting it in there wasn't an option.
I've not touched any of the html code,
so I've got no idea how the ac calls have been done there.
At the end of the day, if *all* the js stuff in html.cpp happens within the
jsyncup call (so the ac object is still in scope)
then the compartment will be correct.
This means we can remove *many* of these calls which I'd say is a *good* thing,
but as I said, I was only working in the js files *not* the html code so I had
to set the compartment at the start of each js function which needed it.

> There might be a simpler function call.
There is, but Mozilla have said it's obsolete and they have a fairly good
record of just removing obsolete functions (and not always updating the docs to reflect this).
I can think of a bunch of functions which according to the docs *should* be
there (all be it marked obsolete) but aren't any more.

Now we've got html.cpp I'll have a look at what's been done and what can be cleaned up.

Cheers,
Adam.

[Edbrowse-dev] compartments

[Karl Dahlke]

Yes everything you say makes perfect sense.

I misspoke a little bit yesterday,  or misthought.
There are two paths to javascript,
creating the global object in the first place,
and then activating js later, which always calls jSyncup
to get things ready.
But aha, raii won't work here.
jSyncup calls and sets everything up and returns.
Then all the other js functions are called.
Set the compartment via raii in jSyncup, and when it returns
your compartment is lost.
We can only simplify things if there is a straight,
old fashioned "set this compartment" call,
that does not involve scope or distructors etc.
I don't know if such a function exists.
If yes then we can clean things up a lot.
If no then we can't.

Karl Dahlke

Re: [Edbrowse-dev] compartments

[Adam Thompson]

On Wed, Feb 05, 2014 at 08:55:47AM -0500, Karl Dahlke wrote:
> There are two paths to javascript,
> creating the global object in the first place,
> and then activating js later, which always calls jSyncup
> to get things ready.
> But aha, raii won't work here.
> jSyncup calls and sets everything up and returns.
> Then all the other js functions are called.
> Set the compartment via raii in jSyncup, and when it returns
> your compartment is lost.
This is unfortunate as it'd be a nice simplification.

> We can only simplify things if there is a straight,
> old fashioned "set this compartment" call,
> that does not involve scope or distructors etc.
> I don't know if such a function exists.
> If yes then we can clean things up a lot.
> If no then we can't.

There is but it's being phased out (i.e.
may disappear whenever the mozilla developers get round to deleting it,
with the possibility of such a removal not being documented).
In addition it doesn't check if you've already switched into this compartment
so you could quickly find yourself building up a stack of equal compartment (pointers?
not sure) which just eat memory and can't be freed as they're protected properties
of the js context (I think).
At least I'm assuming that's how things work since you have to leave
compartments in the reverse order to that inwhich you entered them.

I tried to work around the RAII compartment logic,
but the thing exploded when I went back a page for some reason (NULL
compartment property in the context or a different compartment to that of the
global object or something). Anyway the only way to get things working without
a larger rewrite was to just use the recommended (if not particularly nice in
my opinion) RAII way of doing things.

To be honest I'm all for cleaning up these calls,
but I'd rather get all the good things we're doing at the moment stable and
ready for release then work out what kind of rewrite is required to tidy up the
Mozilla update.
Particularly as I think there've been many major bug fixes recently which really should be released relatively soon.
To do this properly we need to have some time when the code is stable
and we can just test for any other major bugs rather than making huge rewrites.
I also need to work out why the debian mozjs24d package causes edbrowse to just
segfault on startup whereas a manually compiled one doesn't (still haven't
heard about the compilation flags).

That being said, my first version of this message caused Edbrowse to segfault
(typing in too longer line I think),
so maybe we need to look into that one as well.

Cheers,
Adam.