Don't assume that page events means the BrowserContext has a page

CDP currently assumes that if we get a page-related notification (like a
request interception, or page lifecycle event), then we must have a session
and page.

But, Target.detachFromTarget can remove the session from the BrowserContext
while still having the page run (I wonder if we should stop the page at this
point??). So, remove these assumptions and make sure we have a page/session
in the handling of page events.
This commit is contained in:
Karl Seguin
2025-09-05 15:07:30 +08:00
parent 2522e7fe9c
commit ac10d5b2a3
3 changed files with 33 additions and 45 deletions

View File

@@ -179,8 +179,10 @@ fn arePatternsSupported(patterns: []RequestPattern) bool {
}
pub fn requestIntercept(arena: Allocator, bc: anytype, intercept: *const Notification.RequestIntercept) !void {
// unreachable because we _have_ to have a page.
const session_id = bc.session_id orelse unreachable;
// detachTarget could be called, in which case, we still have a page doing
// things, but no session.
const session_id = bc.session_id orelse return;
const target_id = bc.target_id orelse unreachable;
// We keep it around to wait for modifications to the request.
@@ -380,8 +382,10 @@ fn failRequest(cmd: anytype) !void {
}
pub fn requestAuthRequired(arena: Allocator, bc: anytype, intercept: *const Notification.RequestAuthRequired) !void {
// unreachable because we _have_ to have a page.
const session_id = bc.session_id orelse unreachable;
// detachTarget could be called, in which case, we still have a page doing
// things, but no session.
const session_id = bc.session_id orelse return;
const target_id = bc.target_id orelse unreachable;
// We keep it around to wait for modifications to the request.

View File

@@ -230,14 +230,10 @@ pub fn httpRequestFail(arena: Allocator, bc: anytype, msg: *const Notification.R
}
pub fn httpRequestStart(arena: Allocator, bc: anytype, msg: *const Notification.RequestStart) !void {
// Isn't possible to do a network request within a Browser (which our
// notification is tied to), without a page.
std.debug.assert(bc.session.page != null);
// detachTarget could be called, in which case, we still have a page doing
// things, but no session.
const session_id = bc.session_id orelse return;
var cdp = bc.cdp;
// all unreachable because we _have_ to have a page.
const session_id = bc.session_id orelse unreachable;
const target_id = bc.target_id orelse unreachable;
const page = bc.session.currentPage() orelse unreachable;
@@ -248,22 +244,17 @@ pub fn httpRequestStart(arena: Allocator, bc: anytype, msg: *const Notification.
const transfer = msg.transfer;
// We're missing a bunch of fields, but, for now, this seems like enough
try cdp.sendEvent("Network.requestWillBeSent", .{ .requestId = try std.fmt.allocPrint(arena, "REQ-{d}", .{transfer.id}), .frameId = target_id, .loaderId = bc.loader_id, .documentUrl = DocumentUrlWriter.init(&page.url.uri), .request = TransferAsRequestWriter.init(transfer) }, .{ .session_id = session_id });
try bc.cdp.sendEvent("Network.requestWillBeSent", .{ .requestId = try std.fmt.allocPrint(arena, "REQ-{d}", .{transfer.id}), .frameId = target_id, .loaderId = bc.loader_id, .documentUrl = DocumentUrlWriter.init(&page.url.uri), .request = TransferAsRequestWriter.init(transfer) }, .{ .session_id = session_id });
}
pub fn httpResponseHeaderDone(arena: Allocator, bc: anytype, msg: *const Notification.ResponseHeaderDone) !void {
// Isn't possible to do a network request within a Browser (which our
// notification is tied to), without a page.
std.debug.assert(bc.session.page != null);
var cdp = bc.cdp;
// all unreachable because we _have_ to have a page.
const session_id = bc.session_id orelse unreachable;
// detachTarget could be called, in which case, we still have a page doing
// things, but no session.
const session_id = bc.session_id orelse return;
const target_id = bc.target_id orelse unreachable;
// We're missing a bunch of fields, but, for now, this seems like enough
try cdp.sendEvent("Network.responseReceived", .{
try bc.cdp.sendEvent("Network.responseReceived", .{
.requestId = try std.fmt.allocPrint(arena, "REQ-{d}", .{msg.transfer.id}),
.loaderId = bc.loader_id,
.frameId = target_id,
@@ -272,16 +263,11 @@ pub fn httpResponseHeaderDone(arena: Allocator, bc: anytype, msg: *const Notific
}
pub fn httpRequestDone(arena: Allocator, bc: anytype, msg: *const Notification.RequestDone) !void {
// Isn't possible to do a network request within a Browser (which our
// notification is tied to), without a page.
std.debug.assert(bc.session.page != null);
// detachTarget could be called, in which case, we still have a page doing
// things, but no session.
const session_id = bc.session_id orelse return;
var cdp = bc.cdp;
// all unreachable because we _have_ to have a page.
const session_id = bc.session_id orelse unreachable;
try cdp.sendEvent("Network.loadingFinished", .{
try bc.cdp.sendEvent("Network.loadingFinished", .{
.requestId = try std.fmt.allocPrint(arena, "REQ-{d}", .{msg.transfer.id}),
.encodedDataLength = msg.transfer.bytes_received,
}, .{ .session_id = session_id });

View File

@@ -162,19 +162,17 @@ fn navigate(cmd: anytype) !void {
}
pub fn pageNavigate(arena: Allocator, bc: anytype, event: *const Notification.PageNavigate) !void {
// I don't think it's possible that we get these notifications and don't
// have these things setup.
std.debug.assert(bc.session.page != null);
var cdp = bc.cdp;
// detachTarget could be called, in which case, we still have a page doing
// things, but no session.
const session_id = bc.session_id orelse return;
bc.loader_id = bc.cdp.loader_id_gen.next();
const loader_id = bc.loader_id;
const target_id = bc.target_id orelse unreachable;
const session_id = bc.session_id orelse unreachable;
bc.reset();
var cdp = bc.cdp;
const reason_: ?[]const u8 = switch (event.opts.reason) {
.anchor => "anchorClick",
.script => "scriptInitiated",
@@ -292,16 +290,14 @@ pub fn pageCreated(bc: anytype, page: *Page) !void {
}
pub fn pageNavigated(bc: anytype, event: *const Notification.PageNavigated) !void {
// I don't think it's possible that we get these notifications and don't
// have these things setup.
std.debug.assert(bc.session.page != null);
var cdp = bc.cdp;
const timestamp = event.timestamp;
// detachTarget could be called, in which case, we still have a page doing
// things, but no session.
const session_id = bc.session_id orelse return;
const loader_id = bc.loader_id;
const target_id = bc.target_id orelse unreachable;
const session_id = bc.session_id orelse unreachable;
const timestamp = event.timestamp;
var cdp = bc.cdp;
// frameNavigated event
try cdp.sendEvent("Page.frameNavigated", .{
.type = "Navigation",
@@ -370,10 +366,12 @@ pub fn pageNetworkAlmostIdle(bc: anytype, event: *const Notification.PageNetwork
}
fn sendPageLifecycle(bc: anytype, name: []const u8, timestamp: u32) !void {
// detachTarget could be called, in which case, we still have a page doing
// things, but no session.
const session_id = bc.session_id orelse return;
const loader_id = bc.loader_id;
const target_id = bc.target_id orelse unreachable;
const session_id = bc.session_id orelse unreachable;
return bc.cdp.sendEvent("Page.lifecycleEvent", LifecycleEvent{
.name = name,
.frameId = target_id,