Skip to content

Commit ac10d5b

Browse files
committed
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.
1 parent 2522e7f commit ac10d5b

File tree

3 files changed

+33
-45
lines changed

3 files changed

+33
-45
lines changed

src/cdp/domains/fetch.zig

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -179,8 +179,10 @@ fn arePatternsSupported(patterns: []RequestPattern) bool {
179179
}
180180

181181
pub fn requestIntercept(arena: Allocator, bc: anytype, intercept: *const Notification.RequestIntercept) !void {
182-
// unreachable because we _have_ to have a page.
183-
const session_id = bc.session_id orelse unreachable;
182+
// detachTarget could be called, in which case, we still have a page doing
183+
// things, but no session.
184+
const session_id = bc.session_id orelse return;
185+
184186
const target_id = bc.target_id orelse unreachable;
185187

186188
// We keep it around to wait for modifications to the request.
@@ -380,8 +382,10 @@ fn failRequest(cmd: anytype) !void {
380382
}
381383

382384
pub fn requestAuthRequired(arena: Allocator, bc: anytype, intercept: *const Notification.RequestAuthRequired) !void {
383-
// unreachable because we _have_ to have a page.
384-
const session_id = bc.session_id orelse unreachable;
385+
// detachTarget could be called, in which case, we still have a page doing
386+
// things, but no session.
387+
const session_id = bc.session_id orelse return;
388+
385389
const target_id = bc.target_id orelse unreachable;
386390

387391
// We keep it around to wait for modifications to the request.

src/cdp/domains/network.zig

Lines changed: 12 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -230,14 +230,10 @@ pub fn httpRequestFail(arena: Allocator, bc: anytype, msg: *const Notification.R
230230
}
231231

232232
pub fn httpRequestStart(arena: Allocator, bc: anytype, msg: *const Notification.RequestStart) !void {
233-
// Isn't possible to do a network request within a Browser (which our
234-
// notification is tied to), without a page.
235-
std.debug.assert(bc.session.page != null);
236-
237-
var cdp = bc.cdp;
233+
// detachTarget could be called, in which case, we still have a page doing
234+
// things, but no session.
235+
const session_id = bc.session_id orelse return;
238236

239-
// all unreachable because we _have_ to have a page.
240-
const session_id = bc.session_id orelse unreachable;
241237
const target_id = bc.target_id orelse unreachable;
242238
const page = bc.session.currentPage() orelse unreachable;
243239

@@ -248,22 +244,17 @@ pub fn httpRequestStart(arena: Allocator, bc: anytype, msg: *const Notification.
248244

249245
const transfer = msg.transfer;
250246
// We're missing a bunch of fields, but, for now, this seems like enough
251-
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 });
247+
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 });
252248
}
253249

254250
pub fn httpResponseHeaderDone(arena: Allocator, bc: anytype, msg: *const Notification.ResponseHeaderDone) !void {
255-
// Isn't possible to do a network request within a Browser (which our
256-
// notification is tied to), without a page.
257-
std.debug.assert(bc.session.page != null);
258-
259-
var cdp = bc.cdp;
260-
261-
// all unreachable because we _have_ to have a page.
262-
const session_id = bc.session_id orelse unreachable;
251+
// detachTarget could be called, in which case, we still have a page doing
252+
// things, but no session.
253+
const session_id = bc.session_id orelse return;
263254
const target_id = bc.target_id orelse unreachable;
264255

265256
// We're missing a bunch of fields, but, for now, this seems like enough
266-
try cdp.sendEvent("Network.responseReceived", .{
257+
try bc.cdp.sendEvent("Network.responseReceived", .{
267258
.requestId = try std.fmt.allocPrint(arena, "REQ-{d}", .{msg.transfer.id}),
268259
.loaderId = bc.loader_id,
269260
.frameId = target_id,
@@ -272,16 +263,11 @@ pub fn httpResponseHeaderDone(arena: Allocator, bc: anytype, msg: *const Notific
272263
}
273264

274265
pub fn httpRequestDone(arena: Allocator, bc: anytype, msg: *const Notification.RequestDone) !void {
275-
// Isn't possible to do a network request within a Browser (which our
276-
// notification is tied to), without a page.
277-
std.debug.assert(bc.session.page != null);
278-
279-
var cdp = bc.cdp;
280-
281-
// all unreachable because we _have_ to have a page.
282-
const session_id = bc.session_id orelse unreachable;
266+
// detachTarget could be called, in which case, we still have a page doing
267+
// things, but no session.
268+
const session_id = bc.session_id orelse return;
283269

284-
try cdp.sendEvent("Network.loadingFinished", .{
270+
try bc.cdp.sendEvent("Network.loadingFinished", .{
285271
.requestId = try std.fmt.allocPrint(arena, "REQ-{d}", .{msg.transfer.id}),
286272
.encodedDataLength = msg.transfer.bytes_received,
287273
}, .{ .session_id = session_id });

src/cdp/domains/page.zig

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -162,19 +162,17 @@ fn navigate(cmd: anytype) !void {
162162
}
163163

164164
pub fn pageNavigate(arena: Allocator, bc: anytype, event: *const Notification.PageNavigate) !void {
165-
// I don't think it's possible that we get these notifications and don't
166-
// have these things setup.
167-
std.debug.assert(bc.session.page != null);
168-
169-
var cdp = bc.cdp;
165+
// detachTarget could be called, in which case, we still have a page doing
166+
// things, but no session.
167+
const session_id = bc.session_id orelse return;
170168

171169
bc.loader_id = bc.cdp.loader_id_gen.next();
172170
const loader_id = bc.loader_id;
173171
const target_id = bc.target_id orelse unreachable;
174-
const session_id = bc.session_id orelse unreachable;
175172

176173
bc.reset();
177174

175+
var cdp = bc.cdp;
178176
const reason_: ?[]const u8 = switch (event.opts.reason) {
179177
.anchor => "anchorClick",
180178
.script => "scriptInitiated",
@@ -292,16 +290,14 @@ pub fn pageCreated(bc: anytype, page: *Page) !void {
292290
}
293291

294292
pub fn pageNavigated(bc: anytype, event: *const Notification.PageNavigated) !void {
295-
// I don't think it's possible that we get these notifications and don't
296-
// have these things setup.
297-
std.debug.assert(bc.session.page != null);
298-
299-
var cdp = bc.cdp;
300-
const timestamp = event.timestamp;
293+
// detachTarget could be called, in which case, we still have a page doing
294+
// things, but no session.
295+
const session_id = bc.session_id orelse return;
301296
const loader_id = bc.loader_id;
302297
const target_id = bc.target_id orelse unreachable;
303-
const session_id = bc.session_id orelse unreachable;
298+
const timestamp = event.timestamp;
304299

300+
var cdp = bc.cdp;
305301
// frameNavigated event
306302
try cdp.sendEvent("Page.frameNavigated", .{
307303
.type = "Navigation",
@@ -370,10 +366,12 @@ pub fn pageNetworkAlmostIdle(bc: anytype, event: *const Notification.PageNetwork
370366
}
371367

372368
fn sendPageLifecycle(bc: anytype, name: []const u8, timestamp: u32) !void {
369+
// detachTarget could be called, in which case, we still have a page doing
370+
// things, but no session.
371+
const session_id = bc.session_id orelse return;
372+
373373
const loader_id = bc.loader_id;
374374
const target_id = bc.target_id orelse unreachable;
375-
const session_id = bc.session_id orelse unreachable;
376-
377375
return bc.cdp.sendEvent("Page.lifecycleEvent", LifecycleEvent{
378376
.name = name,
379377
.frameId = target_id,

0 commit comments

Comments
 (0)