On a blocking request that requires authentication, we now handle the two cases
correctly:
1 - if the request is aborted, we don't continue processing (if we did, that
would result in (a) transfer.deinit being called twice and (b) the callbacks
being called twice
2 - if the request is "continue", we queue the transfer to be re-issued, as
opposed to just processing it as-is. We have to queue it because we're
currently inside a process loop and it [probaby] isn't safe to re-enter it.
By using the queue, we wait until the next call to `tick` to re-issue the
request.
First, for macrotasks, it ensures that the correct context is entered.
More important, for microtasks, it protects against use-after-free. This is
something we already did, to some degree, in page.deinit: we would run
microtasks before erasing the page, in case any microtasks referenced the page.
The new approach moves the guard to the context (since we have multiple
contexts) and protects against a microtasks enqueue a new task during shutdown
(something the original never did).
This seems to solve some potential use-after-free issues. By informing the
Inspector that the context is gone, it seems to effectively ensure that no more
messages are sent from the inspector for things related to the context.
1 - Embed Page into Session, avoids having to allocate/deallocate the page
2 - Fix false-positive arena pool leak detection
3 - On form submit, pre-check if navigation will allowed before building the
contents
4 - Make sure QueuedNavigation structure isn't use after page is removed
The WPT tests navigation-api/navigation-methods/navigate-history-push-same-url.html
crashed with a @memcpy arguments alias error.
It seems to be due to the reuse of the previous page.url string.
Forcing to duplicate it fixes the crash.
The BrowserContext currently uses 3 arenas:
1 - Command-specific, which is like the call_arena, but for the processing of a
single CDP command
2 - Notification-specific, which is similar, but for the processing of a single
internal notification event
3 - Arena, which is just the session arena and lives for the duration of the
BrowseContext/Session
This is pretty coarse and can results in significant memory accumulation if a
browser context is re-used for multiple navigations.
This commit introduces 3 changes:
1 - Rather than referencing Session.arena, the BrowerContext.arena is now its
own arena. This doesn't really change anything, but it does help keep things
a bit better separated.
2 - Introduces a page_arena (not to be confused with Page.arena). This arena
exists for the duration of a 1 page, i.e. it's cleared when the
BrowserContext receives the page_created internal notification. The
`captured_responses` now uses this arena, which means captures only exist
for the duration of the current page. This appears to be consistent with
how chrome behaves (In fact, Chrome seems even more aggressive and doesn't
appear to make any guarantees around captured responses). CDP refers to this
lifetime as a "renderer" and has an experimental message, which we don't
support, `Network.configureDurableMessages` to control this.
3 - Isolated Worlds are now more self contained with an arena from the ArenaPool.
There are currently 2 places where the BrowserContext.arena is still used:
1 - the isolated_world list
2 - the custom headers
Although this could be long lived, I believe the above is ok. We should just
really think twice whenever we want to use it for anything else.
