Fix for #13307. I've raised it as a draft initially as there's a potential edge case that this may need to account for in #maybeReassignOOPIFRequestClient - namely, if we end up with different sessions for a document request on the main frame (top-level page). Is this something you're aware can happen? If so, what do you think would be the best way of making this info available for this check in NetworkManager, i.e. does the CDPSession that just received a Network.loadingFinished or Network.loadingFailed event belong to the 'main' target or not?

what do you mean by different sessions for a document request? The only thing that can happen is that the network manager gets a new client on pre-rendering activation but that is not associated with a network request so it should not have an effect on ongoing network activity (except that the client before the activation will get disconnected and its network data would not be available anymore). I think the solution in this PR for updating the client to the client that received the loading finished/failed events makes sense.

Apologies I should have given a clearer example. With the OOPIF scenario, this new check works because the HTTPRequest instance for its navigation request is initialised with the client of the main target where its initial CDP events are fired, but then it’s Network.loadingFinished or Network.loadingFailed are fired on the OOPIF target, so this check can compare the clients in the #onLoadingFinished and #onLoadingFailed handlers and reassign clients if needed.

My question was is there a possible scenario where we’d receive a navigation request for the main target where its initial CDP events are fired on the main target client (and so it’s HTTPRequest instance is initialised with this client), but then it’s Network.loadingFinished or Network.loadingFailed events are fired on a different client, leading to this new check being true for the main target and its session being reassigned (e.g. if Puppeteer swaps out frame clients for the main target some reason)? If that’s not a possibility then all good :)

I can see there’s some failing tests in CI, i’ll try and take a look at those this evening.

Got it, thanks for the explanation, so I do not think the scenario you describe can currently happen.

The failing tests seem to be for the WebDriver BiDi implementation that do not support getting the response bodies yet. Let's mark the test as FAIL in test/TestExpectations.json for "webDriverBiDi" on all platforms.

you can run npm run format to fix all formatting/lint issues that could be automatically fixed (it includes also the formatting for the expectation files)

Ahh ok thanks, have applied those changes now.