WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
86013
[chromium] Ensure animations get ticked at least once when added.
https://bugs.webkit.org/show_bug.cgi?id=86013
Summary
[chromium] Ensure animations get ticked at least once when added.
vollick
Reported
2012-05-09 13:43:13 PDT
Without the threaded compositor, it is possible to add an animation and never have it committed or ticked (DRT for example). This patch ensures that these animations do get started.
Attachments
Patch
(16.30 KB, patch)
2012-05-09 14:33 PDT
,
vollick
no flags
Details
Formatted Diff
Diff
Patch
(16.63 KB, patch)
2012-05-09 17:08 PDT
,
vollick
no flags
Details
Formatted Diff
Diff
Patch
(17.20 KB, patch)
2012-05-09 18:13 PDT
,
vollick
no flags
Details
Formatted Diff
Diff
Patch
(17.14 KB, patch)
2012-05-10 21:45 PDT
,
vollick
no flags
Details
Formatted Diff
Diff
Patch
(16.35 KB, patch)
2012-05-15 12:15 PDT
,
vollick
no flags
Details
Formatted Diff
Diff
Patch
(16.34 KB, patch)
2012-05-15 12:16 PDT
,
vollick
no flags
Details
Formatted Diff
Diff
Show Obsolete
(5)
View All
Add attachment
proposed patch, testcase, etc.
vollick
Comment 1
2012-05-09 14:33:37 PDT
Created
attachment 141015
[details]
Patch
James Robinson
Comment 2
2012-05-09 14:51:39 PDT
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.
vollick
Comment 3
2012-05-09 17:08:45 PDT
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.
James Robinson
Comment 4
2012-05-09 17:11:23 PDT
Comment on
attachment 141056
[details]
Patch Oh wow.... yeah seconds and milliseconds are different things, as it turns out. Nice catch!
WebKit Review Bot
Comment 5
2012-05-09 17:41:58 PDT
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
Dana Jansens
Comment 6
2012-05-09 17:44:03 PDT
I mighta totally just broke this applying, while it was in the CQ. Sorry!
vollick
Comment 7
2012-05-09 18:13:39 PDT
Created
attachment 141067
[details]
Patch freshening patch
WebKit Review Bot
Comment 8
2012-05-09 18:52:10 PDT
Comment on
attachment 141067
[details]
Patch Clearing flags on attachment: 141067 Committed
r116594
: <
http://trac.webkit.org/changeset/116594
>
WebKit Review Bot
Comment 9
2012-05-09 18:52:15 PDT
All reviewed patches have been landed. Closing bug.
Kent Tamura
Comment 10
2012-05-09 21:23:51 PDT
(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?
Kent Tamura
Comment 11
2012-05-10 04:33:41 PDT
(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
Kent Tamura
Comment 12
2012-05-10 18:16:34 PDT
Let me try rolling out the patch to see whether it resolves the flakiness.
James Robinson
Comment 13
2012-05-10 18:18:10 PDT
Go for it unless Ian has any other ideas.
Kent Tamura
Comment 14
2012-05-10 18:19:28 PDT
Reverted
r116594
for reason:
r116594
might have made some composition tests flaky. Committed
r116714
: <
http://trac.webkit.org/changeset/116714
>
James Robinson
Comment 15
2012-05-10 18:21:10 PDT
I'm gonna guess the answer ends up being we need the timer to only start animations and not produce frames.
vollick
Comment 16
2012-05-10 21:45:45 PDT
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.
Kent Tamura
Comment 17
2012-05-11 07:11:40 PDT
(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.
James Robinson
Comment 18
2012-05-11 10:30:34 PDT
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.
James Robinson
Comment 19
2012-05-14 10:15:45 PDT
(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.
vollick
Comment 20
2012-05-15 11:21:45 PDT
(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.
James Robinson
Comment 21
2012-05-15 11:31:06 PDT
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.
WebKit Review Bot
Comment 22
2012-05-15 11:43:57 PDT
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
vollick
Comment 23
2012-05-15 12:15:21 PDT
Created
attachment 142023
[details]
Patch Freshening patch.
vollick
Comment 24
2012-05-15 12:16:55 PDT
Created
attachment 142024
[details]
Patch Freshening patch again. Sorry for the spam.
WebKit Review Bot
Comment 25
2012-05-15 15:50:16 PDT
Comment on
attachment 142024
[details]
Patch Clearing flags on attachment: 142024 Committed
r117171
: <
http://trac.webkit.org/changeset/117171
>
WebKit Review Bot
Comment 26
2012-05-15 15:50:21 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug