Skip to content

Generate#2

Merged
francisbouvier merged 6 commits into
mainfrom
generate
May 23, 2023
Merged

Generate#2
francisbouvier merged 6 commits into
mainfrom
generate

Conversation

@francisbouvier
Copy link
Copy Markdown
Member

No description provided.

Signed-off-by: Francis Bouvier <francis.bouvier@gmail.com>
Signed-off-by: Francis Bouvier <francis.bouvier@gmail.com>
Signed-off-by: Francis Bouvier <francis.bouvier@gmail.com>
Signed-off-by: Francis Bouvier <francis.bouvier@gmail.com>
Signed-off-by: Francis Bouvier <francis.bouvier@gmail.com>
Signed-off-by: Francis Bouvier <francis.bouvier@gmail.com>
@francisbouvier francisbouvier merged commit 8f2d591 into main May 23, 2023
@francisbouvier francisbouvier deleted the generate branch May 23, 2023 15:06
karlseguin added a commit to karlseguin/browser that referenced this pull request Feb 8, 2025
1 - Use getOrPut to avoid making 2 map lookups where possible.

2 - Use an arena allocator for Values, which makes memory management simpler.

3 - Because of lightpanda-io#2, we no longer need to allocate key or values which don't need
    to be unescaped. The downside is that the input string has to outlive the
    query.Values (but I think this is currently always the case)

4 - Optimize unescape logic & allocations

5 - Improve test coverage
karlseguin added a commit to karlseguin/browser that referenced this pull request Feb 10, 2025
1 - Use getOrPut to avoid making 2 map lookups where possible.

2 - Use an arena allocator for Values, which makes memory management simpler.

3 - Because of lightpanda-io#2, we no longer need to allocate key or values which don't need
    to be unescaped. The downside is that the input string has to outlive the
    query.Values (but I think this is currently always the case)

4 - Optimize unescape logic & allocations

5 - Improve test coverage
karlseguin added a commit that referenced this pull request Jan 16, 2026
When we create a js.Context, we create the underlying v8.Context and store it
for the duration of the page lifetime. This works because we have a global
HandleScope - the v8.Context (which is really a v8::Local<v8::Context>) is that
to the global HandleScope, effectively making it a global.

If we want to remove our global HandleScope, then we can no longer pin the
v8.Context in our js.Context. Our js.Context now only holds a v8.Global of the
v8.Context (v8::Global<v8::Context).

This PR introduces a new type, js.Local, which takes over a lot of the
functionality previously found in either js.Caller or js.Context. The simplest
way to think about it is:

1 - For v8 -> zig calls, we create a js.Caller (as always)
2 - For zig -> v8 calls, we go through the js.Context (as always)
3 - The shared functionality, which works on a v8.Context, now belongs to js.Local

For #1 (v8 -> zig), creating a js.Local for a js.Caller is really simple and
centralized. v8 largely gives us everything we need from the
FunctionCallbackInfo or PropertyCallbackInfo.  For #2, it's messier, because we
can only create a local v8::Context if we have a HandleScope, which we may or
may not.

Unfortunately, in many cases, what to do becomes the responsibility of the caller
and much of the code has to become aware of this local-ness. What does it means
for our code? The impact is on WebAPIs that store .Global. Because the global
can't do anything. You always need to convert that .Global to a local
(e.g. js.Function.Global -> js.Function).

If you're 100% sure the WebAPI is only being invoked by a v8 callback, you can
use `page.js.local.?.toLocal(some_global).call(...)` to get the local value.

If you're 100% sure the WebAPI is only being invoked by Zig, you need to create
 `js.Local.Scope` to get access to a local:

```zig
var ls: js.Local.Scope = undefined;
page.js.localScope(&ls);
defer ls.deinit();
ls.toLocal(some_global).call(...)
// can also access `&ls.local` for APIs that require a *const js.Local
```
For functions that can be invoked by either V8 or Zig, you should generally push
the responsibility to the caller by accepting a `local: *const js.Local`. If the
caller is a v8 callback, it can pass `page.js.local.?`. If the caller is a Zig
callback, it can create a `Local.Scope`.

As an alternative, it is possible to simply pass the *Page, and check
`if page.js.local == null` and, if so, create a Local.Scope. But this should only
be done for performance reasons. We currently only do this in 1 place, and it's
because the Zig caller doesn't know whether a Local will actually be needed and
it's potentially called on every element creating from the parser.
karlseguin added a commit that referenced this pull request Jan 16, 2026
When we create a js.Context, we create the underlying v8.Context and store it
for the duration of the page lifetime. This works because we have a global
HandleScope - the v8.Context (which is really a v8::Local<v8::Context>) is that
to the global HandleScope, effectively making it a global.

If we want to remove our global HandleScope, then we can no longer pin the
v8.Context in our js.Context. Our js.Context now only holds a v8.Global of the
v8.Context (v8::Global<v8::Context).

This PR introduces a new type, js.Local, which takes over a lot of the
functionality previously found in either js.Caller or js.Context. The simplest
way to think about it is:

1 - For v8 -> zig calls, we create a js.Caller (as always)
2 - For zig -> v8 calls, we go through the js.Context (as always)
3 - The shared functionality, which works on a v8.Context, now belongs to js.Local

For #1 (v8 -> zig), creating a js.Local for a js.Caller is really simple and
centralized. v8 largely gives us everything we need from the
FunctionCallbackInfo or PropertyCallbackInfo.  For #2, it's messier, because we
can only create a local v8::Context if we have a HandleScope, which we may or
may not.

Unfortunately, in many cases, what to do becomes the responsibility of the caller
and much of the code has to become aware of this local-ness. What does it means
for our code? The impact is on WebAPIs that store .Global. Because the global
can't do anything. You always need to convert that .Global to a local
(e.g. js.Function.Global -> js.Function).

If you're 100% sure the WebAPI is only being invoked by a v8 callback, you can
use `page.js.local.?.toLocal(some_global).call(...)` to get the local value.

If you're 100% sure the WebAPI is only being invoked by Zig, you need to create
 `js.Local.Scope` to get access to a local:

```zig
var ls: js.Local.Scope = undefined;
page.js.localScope(&ls);
defer ls.deinit();
ls.toLocal(some_global).call(...)
// can also access `&ls.local` for APIs that require a *const js.Local
```
For functions that can be invoked by either V8 or Zig, you should generally push
the responsibility to the caller by accepting a `local: *const js.Local`. If the
caller is a v8 callback, it can pass `page.js.local.?`. If the caller is a Zig
callback, it can create a `Local.Scope`.

As an alternative, it is possible to simply pass the *Page, and check
`if page.js.local == null` and, if so, create a Local.Scope. But this should only
be done for performance reasons. We currently only do this in 1 place, and it's
because the Zig caller doesn't know whether a Local will actually be needed and
it's potentially called on every element creating from the parser.
@karlseguin karlseguin mentioned this pull request Jan 16, 2026
karlseguin added a commit that referenced this pull request Jan 16, 2026
When we create a js.Context, we create the underlying v8.Context and store it
for the duration of the page lifetime. This works because we have a global
HandleScope - the v8.Context (which is really a v8::Local<v8::Context>) is that
to the global HandleScope, effectively making it a global.

If we want to remove our global HandleScope, then we can no longer pin the
v8.Context in our js.Context. Our js.Context now only holds a v8.Global of the
v8.Context (v8::Global<v8::Context).

This PR introduces a new type, js.Local, which takes over a lot of the
functionality previously found in either js.Caller or js.Context. The simplest
way to think about it is:

1 - For v8 -> zig calls, we create a js.Caller (as always)
2 - For zig -> v8 calls, we go through the js.Context (as always)
3 - The shared functionality, which works on a v8.Context, now belongs to js.Local

For #1 (v8 -> zig), creating a js.Local for a js.Caller is really simple and
centralized. v8 largely gives us everything we need from the
FunctionCallbackInfo or PropertyCallbackInfo.  For #2, it's messier, because we
can only create a local v8::Context if we have a HandleScope, which we may or
may not.

Unfortunately, in many cases, what to do becomes the responsibility of the caller
and much of the code has to become aware of this local-ness. What does it means
for our code? The impact is on WebAPIs that store .Global. Because the global
can't do anything. You always need to convert that .Global to a local
(e.g. js.Function.Global -> js.Function).

If you're 100% sure the WebAPI is only being invoked by a v8 callback, you can
use `page.js.local.?.toLocal(some_global).call(...)` to get the local value.

If you're 100% sure the WebAPI is only being invoked by Zig, you need to create
 `js.Local.Scope` to get access to a local:

```zig
var ls: js.Local.Scope = undefined;
page.js.localScope(&ls);
defer ls.deinit();
ls.toLocal(some_global).call(...)
// can also access `&ls.local` for APIs that require a *const js.Local
```
For functions that can be invoked by either V8 or Zig, you should generally push
the responsibility to the caller by accepting a `local: *const js.Local`. If the
caller is a v8 callback, it can pass `page.js.local.?`. If the caller is a Zig
callback, it can create a `Local.Scope`.

As an alternative, it is possible to simply pass the *Page, and check
`if page.js.local == null` and, if so, create a Local.Scope. But this should only
be done for performance reasons. We currently only do this in 1 place, and it's
because the Zig caller doesn't know whether a Local will actually be needed and
it's potentially called on every element creating from the parser.
karlseguin added a commit that referenced this pull request Jan 18, 2026
When we create a js.Context, we create the underlying v8.Context and store it
for the duration of the page lifetime. This works because we have a global
HandleScope - the v8.Context (which is really a v8::Local<v8::Context>) is that
to the global HandleScope, effectively making it a global.

If we want to remove our global HandleScope, then we can no longer pin the
v8.Context in our js.Context. Our js.Context now only holds a v8.Global of the
v8.Context (v8::Global<v8::Context).

This PR introduces a new type, js.Local, which takes over a lot of the
functionality previously found in either js.Caller or js.Context. The simplest
way to think about it is:

1 - For v8 -> zig calls, we create a js.Caller (as always)
2 - For zig -> v8 calls, we go through the js.Context (as always)
3 - The shared functionality, which works on a v8.Context, now belongs to js.Local

For #1 (v8 -> zig), creating a js.Local for a js.Caller is really simple and
centralized. v8 largely gives us everything we need from the
FunctionCallbackInfo or PropertyCallbackInfo.  For #2, it's messier, because we
can only create a local v8::Context if we have a HandleScope, which we may or
may not.

Unfortunately, in many cases, what to do becomes the responsibility of the caller
and much of the code has to become aware of this local-ness. What does it means
for our code? The impact is on WebAPIs that store .Global. Because the global
can't do anything. You always need to convert that .Global to a local
(e.g. js.Function.Global -> js.Function).

If you're 100% sure the WebAPI is only being invoked by a v8 callback, you can
use `page.js.local.?.toLocal(some_global).call(...)` to get the local value.

If you're 100% sure the WebAPI is only being invoked by Zig, you need to create
 `js.Local.Scope` to get access to a local:

```zig
var ls: js.Local.Scope = undefined;
page.js.localScope(&ls);
defer ls.deinit();
ls.toLocal(some_global).call(...)
// can also access `&ls.local` for APIs that require a *const js.Local
```
For functions that can be invoked by either V8 or Zig, you should generally push
the responsibility to the caller by accepting a `local: *const js.Local`. If the
caller is a v8 callback, it can pass `page.js.local.?`. If the caller is a Zig
callback, it can create a `Local.Scope`.

As an alternative, it is possible to simply pass the *Page, and check
`if page.js.local == null` and, if so, create a Local.Scope. But this should only
be done for performance reasons. We currently only do this in 1 place, and it's
because the Zig caller doesn't know whether a Local will actually be needed and
it's potentially called on every element creating from the parser.
karlseguin added a commit that referenced this pull request Mar 18, 2026
js.Origin was added to allow frames on the same origin to share our zig<->js
maps / identity. It assumes that scripts on different origins will never be
allowed (by v8) to access the same zig instances.

If two different origins DID access the same zig instance, we'd have a few
different problems. First, while the mapping would exist in Origin1's
identity_map, when the zig instance was returned to a script in Origin2, it
would not be found in Origin2's identity_map, and thus create a new v8::Object.
Thus we'd end up with 2 v8::Objects for the same Zig instance. This is
potentially not the end of the world, but not great either as any zig-native
data _would_ be shared (it's the same instance after all), but js-native data
wouldn't.

The real problem this introduces though is with Finalizers. A weak reference
that falls out of scope in Origin1 will get cleaned up, even though it's still
referenced from Origin2.

Now, under normal circumstances, this isn't an issue; v8 _does_ ensure that
cross-origin access isn't allowed (because we set a SecurityToken on the
v8::Context). But it seems like the v8::Inspector isn't bound by these
restrictions and can happily access and share objects across origin.

The simplest solution I can come up with is to move the mapping from the Origin
to the Session. This does mean that objects might live longer than they have to.
When all references to an origin go out of scope, we can do some cleanup. Not
so when the Session owns this data. But really, how often are iframes on
different origins being created and deleted within the lifetime of a page?

When Origins were first introduces, the Session got burdened with having to
manage multiple lifecycles:
1 - The page-surviving data (e.g. history)
2 - The root page lifecycle (e.g. page_arena, queuedNavigation)
3 - The origin lookup

This commit doesn't change that, but it makes the session responsible for
_a lot_ more of the root page lifecycle (#2 above).

I lied. js.Origin still exists, but it's a shell of its former self. It only
exists to store the SecurityToken name that is re-used for every context with
the same origin.

The v8 namespace leaks into Session.

MutationObserver and IntersectionObserver are now back to using weak/strong refs
which was one of the failing cases before this change.
@karlseguin karlseguin mentioned this pull request Mar 18, 2026
karlseguin added a commit that referenced this pull request Mar 19, 2026
js.Origin was added to allow frames on the same origin to share our zig<->js
maps / identity. It assumes that scripts on different origins will never be
allowed (by v8) to access the same zig instances.

If two different origins DID access the same zig instance, we'd have a few
different problems. First, while the mapping would exist in Origin1's
identity_map, when the zig instance was returned to a script in Origin2, it
would not be found in Origin2's identity_map, and thus create a new v8::Object.
Thus we'd end up with 2 v8::Objects for the same Zig instance. This is
potentially not the end of the world, but not great either as any zig-native
data _would_ be shared (it's the same instance after all), but js-native data
wouldn't.

The real problem this introduces though is with Finalizers. A weak reference
that falls out of scope in Origin1 will get cleaned up, even though it's still
referenced from Origin2.

Now, under normal circumstances, this isn't an issue; v8 _does_ ensure that
cross-origin access isn't allowed (because we set a SecurityToken on the
v8::Context). But it seems like the v8::Inspector isn't bound by these
restrictions and can happily access and share objects across origin.

The simplest solution I can come up with is to move the mapping from the Origin
to the Session. This does mean that objects might live longer than they have to.
When all references to an origin go out of scope, we can do some cleanup. Not
so when the Session owns this data. But really, how often are iframes on
different origins being created and deleted within the lifetime of a page?

When Origins were first introduces, the Session got burdened with having to
manage multiple lifecycles:
1 - The page-surviving data (e.g. history)
2 - The root page lifecycle (e.g. page_arena, queuedNavigation)
3 - The origin lookup

This commit doesn't change that, but it makes the session responsible for
_a lot_ more of the root page lifecycle (#2 above).

I lied. js.Origin still exists, but it's a shell of its former self. It only
exists to store the SecurityToken name that is re-used for every context with
the same origin.

The v8 namespace leaks into Session.

MutationObserver and IntersectionObserver are now back to using weak/strong refs
which was one of the failing cases before this change.
@karlseguin karlseguin mentioned this pull request Apr 24, 2026
gh-actions-shared Bot pushed a commit to xf-checkout/ai-panda-browser that referenced this pull request Apr 29, 2026
Adds limited support for window.open. This leverages the new page container and
behaves similarly to an iframe. There are many things not implemented, but the
most significant are:

1- target=window_name or target=_blank don't work (but this could be added
   pretty easily I think)

2- Windows (which are is just a Frame) are shutdown when the Page is shutdown.
   They would need to be owned by the Session (rather than the Page), but I'm
   not really confident in that right now.

3- No CDP testing. There are maybe CDP-specific messages we need to emit (or
   maybe messages that we shouldn't emit).

As-is, this should help with common cases where:
1. window.open is called but doesn't matter (won't give a JS error)
2. window.open is short-lived, or, more specifically, lives only for the
   duration of the page that opened it (e.g. a login popup).
3. WPT tests! (because most of these fit in lightpanda-io#2).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant