Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor CDP #422

Merged
merged 18 commits into from
Feb 20, 2025
+1,387 −2,360
Merged

Refactor CDP #422

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
c0c0694
Make TCP server websocket-aware
karlseguin Feb 6, 2025
733c6b4
remove websocket.zig dependency from build
karlseguin Feb 6, 2025
72eaab6
zig fmt
karlseguin Feb 6, 2025
f41c1cb
"fix" test compilation
karlseguin Feb 7, 2025
4d9cc55
Increase fuzz count. Add test for [too] large HTTP requests
karlseguin Feb 7, 2025
bdb7044
Make websocket client reader stateful
karlseguin Feb 7, 2025
14fe4f6
support continuation frames
karlseguin Feb 7, 2025
6ab64d1
Refactor CDP
karlseguin Feb 10, 2025
d282055
Merge branch 'main' into cdp_struct
karlseguin Feb 12, 2025
1846d0b
drats, zig fmt again
karlseguin Feb 12, 2025
a43a6a2
Merge branch 'main' into cdp_struct
karlseguin Feb 13, 2025
b1c3de6
zig fmt
karlseguin Feb 13, 2025
b60a91f
fix memory leak
karlseguin Feb 17, 2025
c4eeef2
On CDP process error, let client decide how to close
karlseguin Feb 17, 2025
18080ce
fix test
karlseguin Feb 17, 2025
f508288
Fix segfault when multiple inflight Send completions fail
karlseguin Feb 17, 2025
521a740
Merge branch 'main' into cdp_struct
karlseguin Feb 20, 2025
bc45608
zig fmt
karlseguin Feb 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
195 changes: 106 additions & 89 deletions src/browser/browser.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
const std = @import("std");
const builtin = @import("builtin");

const Allocator = std.mem.Allocator;

const Types = @import("root").Types;

const parser = @import("netsurf");
Expand Down Expand Up @@ -57,30 +59,44 @@ pub const user_agent = "Lightpanda/1.0";
// A browser contains only one session.
// TODO allow multiple sessions per browser.
pub const Browser = struct {
session: Session = undefined,
agent: []const u8 = user_agent,
loop: *Loop,
session: ?*Session,
allocator: Allocator,
session_pool: SessionPool,

const uri = "about:blank";
const SessionPool = std.heap.MemoryPool(Session);

pub fn init(self: *Browser, alloc: std.mem.Allocator, loop: *Loop, vm: jsruntime.VM) !void {
// We want to ensure the caller initialised a VM, but the browser
// doesn't use it directly...
_ = vm;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's useful to request a vm as parameter here to force the caller to create one before calling.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before, there was one global browser, assigned directly to the server.

Now there's 1 browser per client. Once this is merged, I was planning on making CDP multi-browser (I think when Target.createBrowserContext, we should create a new browser - else if a client creates multiple contexts, they won't get the isolation that they should (i.e. cookies/storage)).

Having this check means storing the VM instance in the Server, in the Client, and eventually in CDP. If this is important, how about a global boolean in zig-js-runtime which is set to true on vm.init and which Env.init checks every time?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK vm.Init https://github.com/lightpanda-io/zig-js-runtime/blob/main/src/engines/v8/v8.zig#L73-L78 calls multiple v8's functions to initiate v8 before any isolate is created (https://github.com/lightpanda-io/zig-v8-fork/blob/fork/src/v8.zig#L78-L91, https://github.com/lightpanda-io/zig-v8-fork/blob/fork/src/v8.zig#L117-L122, https://github.com/lightpanda-io/zig-v8-fork/blob/fork/src/v8.zig#L124-L129)

Now there's 1 browser per client.

Got it 👍

If this is important, how about a global boolean in zig-js-runtime which is set to true on vm.init and which Env.init checks every time?

I guess it's me, I found weird to have to init a vm and never pass it somewhere 😅
But using a boolean is ok too. The problem w/o this check is the error if you don't init the vm isn't very explicit:

$ zig build run


#
# Fatal error in ../../../../v8/src/api/api.cc, line 388
# Check failed: i::GetProcessWideSandbox()->is_initialized().
#
#
#
#FailureMessage Object: 0x7ffe9094fbe0run
└─ run lightpanda failure
error: the following command terminated unexpectedly:
/home/pierre/wrk/browser/.zig-cache/o/b20fc1513f2dcc377631adee7a63ca0f/lightpanda 
Build Summary: 2/4 steps succeeded; 1 failed (disable with --summary none)
run transitive failure
└─ run lightpanda failure
error: the following build command failed with exit code 1:
/home/pierre/wrk/browser/.zig-cache/o/7a9935469ab9b5af96ef2ed0fef6c52a/build /usr/local/zig-0.13.0/zig /home/pierre/wrk/browser /home/pierre/wrk/browser/.zig-cache /home/pierre/.cache/zig --seed 0x3890d754 -Z932405236f7f5d84 run

const uri = "about:blank";

try Session.init(&self.session, alloc, loop, uri);
pub fn init(allocator: Allocator, loop: *Loop) Browser {
return .{
.loop = loop,
.session = null,
.allocator = allocator,
.session_pool = SessionPool.init(allocator),
};
}

pub fn deinit(self: *Browser) void {
self.session.deinit();
self.closeSession();
self.session_pool.deinit();
}

pub fn newSession(
self: *Browser,
alloc: std.mem.Allocator,
loop: *jsruntime.Loop,
) !void {
self.session.deinit();
try Session.init(&self.session, alloc, loop, uri);
pub fn newSession(self: *Browser, ctx: anytype) !*Session {
self.closeSession();

const session = try self.session_pool.create();
try Session.init(session, self.allocator, ctx, self.loop, uri);
self.session = session;
return session;
}

fn closeSession(self: *Browser) void {
if (self.session) |session| {
session.deinit();
self.session_pool.destroy(session);
self.session = null;
}
}

pub fn currentPage(self: *Browser) ?*Page {
Expand All @@ -96,7 +112,7 @@ pub const Browser = struct {
// deinit a page before running another one.
pub const Session = struct {
// allocator used to init the arena.
alloc: std.mem.Allocator,
allocator: Allocator,

// The arena is used only to bound the js env init b/c it leaks memory.
// see https://github.com/lightpanda-io/jsruntime-lib/issues/181
Expand All @@ -109,8 +125,9 @@ pub const Session = struct {

// TODO handle proxy
loader: Loader,
env: Env = undefined,
inspector: ?jsruntime.Inspector = null,

env: Env,
inspector: jsruntime.Inspector,

window: Window,

Expand All @@ -121,20 +138,54 @@ pub const Session = struct {

jstypes: [Types.len]usize = undefined,

fn init(self: *Session, alloc: std.mem.Allocator, loop: *Loop, uri: []const u8) !void {
self.* = Session{
fn init(self: *Session, allocator: Allocator, ctx: anytype, loop: *Loop, uri: []const u8) !void {
self.* = .{
.uri = uri,
.alloc = alloc,
.arena = std.heap.ArenaAllocator.init(alloc),
.env = undefined,
.inspector = undefined,
.allocator = allocator,
.loader = Loader.init(allocator),
.httpClient = .{ .allocator = allocator },
.storageShed = storage.Shed.init(allocator),
.arena = std.heap.ArenaAllocator.init(allocator),
.window = Window.create(null, .{ .agent = user_agent }),
.loader = Loader.init(alloc),
.storageShed = storage.Shed.init(alloc),
.httpClient = undefined,
};

Env.init(&self.env, self.arena.allocator(), loop, null);
self.httpClient = .{ .allocator = alloc };
const arena = self.arena.allocator();

Env.init(&self.env, arena, loop, null);
errdefer self.env.deinit();
try self.env.load(&self.jstypes);

const ContextT = @TypeOf(ctx);
const InspectorContainer = switch (@typeInfo(ContextT)) {
.Struct => ContextT,
.Pointer => |ptr| ptr.child,
.Void => NoopInspector,
else => @compileError("invalid context type"),
};

// const ctx_opaque = @as(*anyopaque, @ptrCast(ctx));
self.inspector = try jsruntime.Inspector.init(
arena,
self.env,
if (@TypeOf(ctx) == void) @constCast(@ptrCast(&{})) else ctx,
InspectorContainer.onInspectorResponse,
InspectorContainer.onInspectorEvent,
);
self.env.setInspector(self.inspector);
}

fn deinit(self: *Session) void {
if (self.page) |*p| {
p.deinit();
}

self.env.deinit();
self.arena.deinit();
self.httpClient.deinit();
self.loader.deinit();
self.storageShed.deinit();
}

fn fetchModule(ctx: *anyopaque, referrer: ?jsruntime.Module, specifier: []const u8) !jsruntime.Module {
Expand All @@ -152,49 +203,21 @@ pub const Session = struct {
return self.env.compileModule(body, specifier);
}

fn deinit(self: *Session) void {
if (self.page) |*p| p.deinit();

if (self.inspector) |inspector| {
inspector.deinit(self.alloc);
}

self.env.deinit();
self.arena.deinit();

self.httpClient.deinit();
self.loader.deinit();
self.storageShed.deinit();
}

pub fn initInspector(
self: *Session,
ctx: anytype,
onResp: jsruntime.InspectorOnResponseFn,
onEvent: jsruntime.InspectorOnEventFn,
) !void {
const ctx_opaque = @as(*anyopaque, @ptrCast(ctx));
self.inspector = try jsruntime.Inspector.init(self.alloc, self.env, ctx_opaque, onResp, onEvent);
self.env.setInspector(self.inspector.?);
}

pub fn callInspector(self: *Session, msg: []const u8) void {
if (self.inspector) |inspector| {
inspector.send(msg, self.env);
} else {
@panic("No Inspector");
}
self.inspector.send(self.env, msg);
}

// NOTE: the caller is not the owner of the returned value,
// the pointer on Page is just returned as a convenience
pub fn createPage(self: *Session) !*Page {
if (self.page != null) return error.SessionPageExists;
const p: Page = undefined;
self.page = p;
Page.init(&self.page.?, self.alloc, self);
self.page = Page.init(self.allocator, self);
return &self.page.?;
}

pub fn currentPage(self: *Session) ?*Page {
return &(self.page orelse return null);
}
};

// Page navigates to an url.
Expand All @@ -203,8 +226,8 @@ pub const Session = struct {
// The page handle all its memory in an arena allocator. The arena is reseted
// when end() is called.
pub const Page = struct {
arena: std.heap.ArenaAllocator,
session: *Session,
arena: std.heap.ArenaAllocator,
doc: ?*parser.Document = null,

// handle url
Expand All @@ -218,17 +241,19 @@ pub const Page = struct {

raw_data: ?[]const u8 = null,

fn init(
self: *Page,
alloc: std.mem.Allocator,
session: *Session,
) void {
self.* = .{
.arena = std.heap.ArenaAllocator.init(alloc),
fn init(allocator: Allocator, session: *Session) Page {
return .{
.session = session,
.arena = std.heap.ArenaAllocator.init(allocator),
};
}

pub fn deinit(self: *Page) void {
self.end();
self.arena.deinit();
self.session.page = null;
}

// start js env.
// - auxData: extra data forwarded to the Inspector
// see Inspector.contextCreated
Expand All @@ -253,18 +278,15 @@ pub const Page = struct {
try polyfill.load(self.arena.allocator(), self.session.env);

// inspector
if (self.session.inspector) |inspector| {
log.debug("inspector context created", .{});
inspector.contextCreated(self.session.env, "", self.origin orelse "://", auxData);
}
log.debug("inspector context created", .{});
self.session.inspector.contextCreated(self.session.env, "", self.origin orelse "://", auxData);
}

// reset js env and mem arena.
pub fn end(self: *Page) void {
self.session.env.stop();
// TODO unload document: https://html.spec.whatwg.org/#unloading-documents

if (self.url) |*u| u.deinit(self.arena.allocator());
self.url = null;
self.location.url = null;
self.session.window.replaceLocation(&self.location) catch |e| {
Expand All @@ -278,14 +300,8 @@ pub const Page = struct {
_ = self.arena.reset(.free_all);
}

pub fn deinit(self: *Page) void {
self.end();
self.arena.deinit();
self.session.page = null;
}

// dump writes the page content into the given file.
pub fn dump(self: *Page, out: std.fs.File) !void {
pub fn dump(self: *const Page, out: std.fs.File) !void {

// if no HTML document pointer available, dump the data content only.
if (self.doc == null) {
Expand Down Expand Up @@ -333,11 +349,9 @@ pub const Page = struct {
}

// own the url
if (self.rawuri) |prev| alloc.free(prev);
self.rawuri = try alloc.dupe(u8, uri);
self.uri = std.Uri.parse(self.rawuri.?) catch try std.Uri.parseAfterScheme("", self.rawuri.?);

if (self.url) |*prev| prev.deinit(alloc);
self.url = try URL.constructor(alloc, self.rawuri.?, null);
self.location.url = &self.url.?;
try self.session.window.replaceLocation(&self.location);
Expand Down Expand Up @@ -435,9 +449,7 @@ pub const Page = struct {
// https://html.spec.whatwg.org/#read-html

// inspector
if (self.session.inspector) |inspector| {
inspector.contextCreated(self.session.env, "", self.origin.?, auxData);
}
self.session.inspector.contextCreated(self.session.env, "", self.origin.?, auxData);

// replace the user context document with the new one.
try self.session.env.setUserContext(.{
Expand Down Expand Up @@ -596,7 +608,7 @@ pub const Page = struct {
};

// the caller owns the returned string
fn fetchData(self: *Page, alloc: std.mem.Allocator, src: []const u8) ![]const u8 {
fn fetchData(self: *Page, alloc: Allocator, src: []const u8) ![]const u8 {
log.debug("starting fetch {s}", .{src});

var buffer: [1024]u8 = undefined;
Expand Down Expand Up @@ -671,7 +683,7 @@ pub const Page = struct {
return .unknown;
}

fn eval(self: Script, alloc: std.mem.Allocator, env: Env, body: []const u8) !void {
fn eval(self: Script, alloc: Allocator, env: Env, body: []const u8) !void {
var try_catch: jsruntime.TryCatch = undefined;
try_catch.init(env);
defer try_catch.deinit();
Expand All @@ -696,3 +708,8 @@ pub const Page = struct {
}
};
};

const NoopInspector = struct {
pub fn onInspectorResponse(_: *anyopaque, _: u32, _: []const u8) void {}
pub fn onInspectorEvent(_: *anyopaque, _: []const u8) void {}
};
Loading