Add Page.executionContextForID
#1009
Conversation
There was a problem hiding this comment.
LGTM, but I wish we had some tests to validate that this really works without deadlocks and data races. I'm saying this because execution contexts and frame sessions have been the most challenging parts of the module to debug, and they used to fail a lot. It'd nice to have nolint for the current linter warnings and then we can lift them in the other console.log PR.
Just added the In regards of tests, I thought about it, but a unit test was providing pretty much no value, and it would be fairly difficult to try to force a data race or a deadlock for frame attach events only in an integration test (as in this PR these new methods are not even called). Isn't this better suited for e2e tests once we also process events and have a better "platform" for them? |
ka3de
force-pushed
the
feat/exectx-for-id
branch
from
9318ba9
to
cf32d1a
Compare
Use a RWMutex as frameSessions could have concurrent accesses from attachFrameSession and also async event processing for the page.
Allows to look for an specific execution context in a given frame session. This is necessary in order to associate frame events (e.g.: consoleAPICalled) with their associated execution context.
Allows to look for an specific execution context in all frames for a given page. This is necessary in order to associate events (e.g.: consoleAPICalled) with their execution context.
This is added temporarily until these methods are used in a related PR (see #1006).
ka3de
force-pushed
the
feat/exectx-for-id
branch
from
cf32d1a
to
783e843
Compare
What?
Adds support to retrieve the associated execution context from within all page frames for a given ID.
Why?
This is required to associate CDP runtime events for a given page, which include the associated execution context ID (e.g.: see onConsoleAPICalled), with the execution context itself among the page frames and their sessions.
Checklist
Related PR(s)/Issue(s)
Related #1006 .