Skip to content

Codify the requestAnimationFrame delay in test-wait.js as the test-completion timing - #241

Open
gsnedders wants to merge 1 commit into
web-platform-tests:mainfrom
gsnedders:test-wait-double-rAF
Open

Codify the requestAnimationFrame delay in test-wait.js as the test-completion timing#241
gsnedders wants to merge 1 commit into
web-platform-tests:mainfrom
gsnedders:test-wait-double-rAF

Conversation

@gsnedders

Copy link
Copy Markdown
Member

@jcscottiii

Copy link
Copy Markdown
Contributor

I talked to Xianzhu about this. They said:

I agree with https://issues.chromium.org/issues/41311503#comment3 that the WebDriver should be able to reliably capture an image reflecting the document's latest status, without the client needing to do anything.

I don't remember many details, but crrev.com/c/4505400 now looks like a temporary solution to make the two web test runners do the same thing. It was probably a workaround for the issue where some ref tests had [double-]rAF etc., which required waiting, but the tests didn't declare reftest-wait, causing flaky premature finishes (e.g. crbug.com/40899923, crbug.com/ crrev.com/c/40901018). Now I think we should find and fix the tests instead of working around them in the test runner.

@gsnedders

Copy link
Copy Markdown
Member Author

I agree with https://issues.chromium.org/issues/41311503#comment3 that the WebDriver should be able to reliably capture an image reflecting the document's latest status, without the client needing to do anything.

But there's not really any good way to define this — nothing really specifies when a frame gets displayed to the user — and that makes it hard to update the WebDriver spec to require this.

The reality is any executor implementation using WebDriver is going to have a notably longer delay than one within the browser process, and I'd expect having a longer delay than is necessary would make it more likely that it has settled fully.

The challenge with removing it is we basically have to run every reftest numerous times to validate that we aren't adding new instability into the suite, noting that that could easily depend on hardware performance. I'm not complete opposed to trying — but there is definitely risk here.

Per discussion with @jgraham in the infra meeting earlier in the week, there's also concerns about changing the internal Gecko implementation (which just waits for MozAfterPaint).

There's a few bits of experimentation we can do:

  1. We could try removing the double rAF from wptrunner, and run full runs of Chrome and Safari with it, and see how many tests it changes the results of.
  2. We could try adding the double rAF to WebKit, and see how many tests it changes the results of.
  3. We could try removing the double rAF from content_shell, and see how many tests it changes the results of.
  4. We could make the Firefox internal reftest implementation wait for MozAfterPaint or the second rAF, whichever comes later, and see how many tests it changes the results of.

None of these are foolproof, and anything that moves the timing forward (1, 3) has a risk of exposing more flakiness we don't yet know about.

@gsnedders

Copy link
Copy Markdown
Member Author
  • We could try adding the double rAF to WebKit, and see how many tests it changes the results of.

WebKit/WebKit#71591

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.

2 participants