Summary: | [chromium] Ensure animations get ticked at least once when added. | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | vollick | ||||||||||||||
Component: | WebKit Misc. | Assignee: | vollick | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | cc-bugs, danakj, jamesr, tkent, webkit.review.bot | ||||||||||||||
Priority: | P2 | ||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
Bug Depends on: | |||||||||||||||||
Bug Blocks: | 84033 | ||||||||||||||||
Attachments: |
|
Description
vollick
2012-05-09 13:43:13 PDT
Created attachment 141015 [details]
Patch
Comment on attachment 141015 [details]
Patch
Nice! This looks much cleaner. R=me, but if you don't mind upload a version that applies to ToT and seeing if it EWS's clean before landing.
Created attachment 141056 [details]
Patch
James, I've put this up for review again because something came up during the
merge I need to run by you.
A couple of the unit tests in CCLayerTreeHostTest had been marked flaky, but I
think I have addressed the flakiness. The background ticking was taking 1000
seconds, rather than milliseconds. I've retested these test using
--gtest_repeat=1000 --gtest_break_on_failure and they succeeded so I've removed
the FLAKY_ prefix.
Comment on attachment 141056 [details]
Patch
Oh wow.... yeah seconds and milliseconds are different things, as it turns out. Nice catch!
Comment on attachment 141056 [details] Patch Rejecting attachment 141056 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: form/graphics/chromium/cc/CCThreadProxy.h patching file Source/WebKit/chromium/tests/CCLayerTreeHostTest.cpp Hunk #1 succeeded at 1135 (offset 56 lines). Hunk #2 succeeded at 1154 (offset 56 lines). Hunk #3 succeeded at 1189 (offset 56 lines). Hunk #4 succeeded at 1266 (offset 56 lines). Hunk #5 succeeded at 1292 (offset 56 lines). Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'James Robi..." exit_code: 1 cwd: /mnt/git/webkit-commit-queue/ Full output: http://queues.webkit.org/results/12666015 I mighta totally just broke this applying, while it was in the CQ. Sorry! Created attachment 141067 [details]
Patch
freshening patch
Comment on attachment 141067 [details] Patch Clearing flags on attachment: 141067 Committed r116594: <http://trac.webkit.org/changeset/116594> All reviewed patches have been landed. Closing bug. (In reply to comment #8) > (From update of attachment 141067 [details]) > Clearing flags on attachment: 141067 > > Committed r116594: <http://trac.webkit.org/changeset/116594> I'm afraid this change made a test flaky. http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showExpectations=true&showLargeExpectations=true&tests=fast%2Flayers%2Fscroll-with-transform-composited-layer.html Is it possible? (In reply to comment #10) > > I'm afraid this change made a test flaky. > http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showExpectations=true&showLargeExpectations=true&tests=fast%2Flayers%2Fscroll-with-transform-composited-layer.html > > > Is it possible? Not only scroll-with-transform-composited-layer.html. Some composition-related tests are flaky since around this change. http://test-results.appspot.com/dashboards/flakiness_dashboard.html#showLargeExpectations=true&tests=composit Let me try rolling out the patch to see whether it resolves the flakiness. Go for it unless Ian has any other ideas. Reverted r116594 for reason: r116594 might have made some composition tests flaky. Committed r116714: <http://trac.webkit.org/changeset/116714> I'm gonna guess the answer ends up being we need the timer to only start animations and not produce frames. Created attachment 141323 [details]
Patch
Attempting to fix flake introduced by this test.
This patch used to bail before drawing if we were not visible, but may not be valid and I have removed this condition.
The timer should only be used if we add animation, which is not the case for the flaky compositor tests.
(In reply to comment #14) > Reverted r116594 for reason: > > r116594 might have made some composition tests flaky. > > Committed r116714: <http://trac.webkit.org/changeset/116714> I confirmed that the flakiness was resolved by the rollout. Can you try to reproduce the flake locally first, Ian, and confirm whether this new patch actually resolves it or not? I'd rather not debug flake via the landing + using the real bots if possible. (In reply to comment #18) > Can you try to reproduce the flake locally first, Ian, and confirm whether this new patch actually resolves it or not? I'd rather not debug flake via the landing + using the real bots if possible. Any progress here? If not then it might be worth landing this anyway and seeing what happens. (In reply to comment #19) > (In reply to comment #18) > > Can you try to reproduce the flake locally first, Ian, and confirm whether this new patch actually resolves it or not? I'd rather not debug flake via the landing + using the real bots if possible. > > Any progress here? If not then it might be worth landing this anyway and seeing what happens. No real progress. Tests do not seem flaky for me when I run locally. Running on mac_layout with the extra condition on drawing, I had a flaky compositing-related test that I did not have without. With: http://build.chromium.org/p/tryserver.chromium/builders/mac_layout/builds/357 Without: http://build.chromium.org/p/tryserver.chromium/builders/mac_layout/builds/361 I don't think that this is nearly conclusive proof that the latest patch fixes the flake, but I agree that it might be worth relanding to see. Comment on attachment 141323 [details]
Patch
OK, thanks for checking. Let's give this a try then - please keep an eye on the bots after this lands to see if the flake re-appears.
Comment on attachment 141323 [details] Patch Rejecting attachment 141323 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: (offset -18 lines). Hunk #4 succeeded at 284 (offset -16 lines). Hunk #5 succeeded at 360 (offset -14 lines). patching file Source/WebCore/platform/graphics/chromium/cc/CCSingleThreadProxy.h patching file Source/WebCore/platform/graphics/chromium/cc/CCThreadProxy.h patching file Source/WebKit/chromium/tests/CCLayerTreeHostTest.cpp Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'James Robi..." exit_code: 1 cwd: /mnt/git/webkit-commit-queue/ Full output: http://queues.webkit.org/results/12685864 Created attachment 142023 [details]
Patch
Freshening patch.
Created attachment 142024 [details]
Patch
Freshening patch again. Sorry for the spam.
Comment on attachment 142024 [details] Patch Clearing flags on attachment: 142024 Committed r117171: <http://trac.webkit.org/changeset/117171> All reviewed patches have been landed. Closing bug. |