Skip to content

Conversation

JoelEinbinder
Copy link
Contributor

@yury-s yury-s changed the title fix(chromium): wait for existing patches when connecting fix(chromium): wait for existing pages when connecting May 11, 2021
// Target.setAutoAttach has a bug where it does not wait for new Targets being attached.
// However making a dummy call afterwards fixes this.
// This can be removed after https://chromium-review.googlesource.com/c/chromium/src/+/2885888 lands in stable.
await session.send('Target.getTargetInfo');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We used to have code with setDiscoverTargets which did this, maybe it's easier to just revert that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code with setDiscoverTargets was way more complicated had bugs in it related to connecting twice to service workers.

@JoelEinbinder
Copy link
Contributor Author

Looks like this is causing an electron test to fail. Looking into it

@JoelEinbinder JoelEinbinder force-pushed the fix_attach_to_target branch from bf6141e to 9273474 Compare May 13, 2021 16:20
@JoelEinbinder
Copy link
Contributor Author

Electron had a race where it was not expecting pages to appear before the browser was finished launching. I fixed it, and a test that was relying on that behavior. This could affect user code. I don't think we have a problem, because we used firstWindow everywhere in the docs. @pavelfeldman what do you think?

@JoelEinbinder JoelEinbinder merged commit 3bded35 into microsoft:master May 13, 2021
aslushnikov pushed a commit to aslushnikov/playwright that referenced this pull request May 17, 2021
aslushnikov added a commit that referenced this pull request May 17, 2021
…6619)

This cherry-picks two PRs:
- PR #6502 SHA 2ea465b
- PR #6511 SHA 3bded35

Co-authored-by: Joel Einbinder <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

can't get browser's context pages after connect_over_cdp
2 participants