Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(video): do not stall video in popups #5941

Merged
merged 1 commit into from Mar 24, 2021
Merged

fix(video): do not stall video in popups #5941

merged 1 commit into from Mar 24, 2021

Conversation

dgozman
Copy link
Contributor

@dgozman dgozman commented Mar 24, 2021

There are two problems, exposed by existing tests:

  • We do not send Page.startScreeencast before Runtime.runIfWaitingForDebugger
    because we launch video recorder in between. This stalls when the page is busy
    immediately after resuming, e.g. with alert().
    Fixed by starting video recorder in advance.

  • We wait for the first frame that may not come - for example, context-wide interception
    is blocking essential resource and first frame.
    Fixed by only waiting for the first frame before reporting the video, not the page.

There are two problems, exposed by existing tests:

- We do not send Page.startScreeencast before Runtime.runIfWaitingForDebugger
  because we launch video recorder in between. This stalls when the page is busy
  immediately after resuming, e.g. with alert().
  Fixed by starting video recorder in advance.

- We wait for the first frame that may not come - for example, context-wide interception
  is blocking essential resource and first frame.
  Fixed by only waiting for the first frame before reporting the video, not the page.
@JoelEinbinder JoelEinbinder changed the title fix(viedo): do not stall video in popups fix(video): do not stall video in popups Mar 24, 2021
src/server/chromium/crPage.ts Show resolved Hide resolved
@dgozman dgozman merged commit 8e97607 into microsoft:master Mar 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants