[Reland]: Repaint when bgcolor animation is triggered #27801
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wpt-pr-bot
approved these changes
Feb 26, 2021
chromium-wpt-export-bot
force-pushed
the
chromium-export-cl-2716060
branch
5 times, most recently
from
324a0f6
to
e4bb114
Compare
This CL is the reland of: https://chromium-review.googlesource.com/c/chromium/src/+/2698847 The above CL was reverted because the layout test added was flaky. In this CL, PS#1 is the reverted CL, and we need to make sure that the test is no longer flaky. It turns out that it was the non-virtual version that was flaky, and the reason is that double raf doesn't seem to be enough to make the animation jump into the mid-point. I have tested locally with 500 runs, and the failure rate is ~99%, Oh, that's bad... The reason is that run_web_tests might run with a 1ms raf timer which would be too fast to guarantee the screenshot would be after the step change. The solution is very simple, which is to use triple raf instead of double raf, I have ran both the virtual and non-virtual version of the test, each one for 500 times locally, and there is no failure at all. Bug: 1177320 Change-Id: I95f576994a5d67bf8319b5b1f33e8e6fca6d4bb8 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2716060 Commit-Queue: Xida Chen <xidachen@chromium.org> Reviewed-by: Robert Flack <flackr@chromium.org> Reviewed-by: Kevin Ellis <kevers@chromium.org> Cr-Commit-Position: refs/heads/master@{#859050}
chromium-wpt-export-bot
force-pushed
the
chromium-export-cl-2716060
branch
from
e4bb114
to
4111af2
Compare
|
hello @xidachen, this test is very flaky on Firefox, I think (at least on Firefox) we need to make sure the target element gets styled initially with the before-change style (rgb(0, 200, 0)) before triggering the transition with the after-change style (rgb(200, 0, 0)) to trigger the transition surely. Just waiting a rAF callback before triggering the transition doesn't seem to make sure it. So I think adding |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This CL is the reland of:
https://chromium-review.googlesource.com/c/chromium/src/+/2698847
The above CL was reverted because the layout test added was flaky.
In this CL, PS#1 is the reverted CL, and we need to make sure
that the test is no longer flaky.
It turns out that it was the non-virtual version that was flaky,
and the reason is that double raf doesn't seem to be enough to
make the animation jump into the mid-point. I have tested
locally with 500 runs, and the failure rate is ~99%, Oh, that's
bad...
The reason is that run_web_tests might run with a 1ms raf timer
which would be too fast to guarantee the screenshot would be
after the step change.
The solution is very simple, which is to use triple raf instead
of double raf, I have ran both the virtual and non-virtual version
of the test, each one for 500 times locally, and there is no
failure at all.
Bug: 1177320
Change-Id: I95f576994a5d67bf8319b5b1f33e8e6fca6d4bb8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2716060
Commit-Queue: Xida Chen <xidachen@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Reviewed-by: Kevin Ellis <kevers@chromium.org>
Cr-Commit-Position: refs/heads/master@{#859050}