Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(45)

Issue 1156213003: Cancel animations before calling ContainerNode::attach in Element::attach. (Closed)

Created:
4 years, 11 months ago by esprehn
Modified:
4 years, 11 months ago
CC:
blink-reviews, blink-reviews-dom_chromium.org, dglazkov+blink, eae+blinkwatch, rwlbuis, sof
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

Cancel animations before calling ContainerNode::attach in Element::attach. We need to cancel the animation first since the cancel() call can then end up calling setNeedsStyleRecalc on ourself which would leave a dirty bit set. By doing it before ContainerNode::attach the dirty bit will be cleared. A future patch will add an ASSERT that we never leave any dirty bits. BUG=492730 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=196370

Patch Set 1 #

Patch Set 2 : with asserts. #

Patch Set 3 : No asserts yet. #

Patch Set 4 : Add a test. #

Patch Set 5 : Add a test. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+68 lines, -7 lines) Patch
A LayoutTests/animations/display-none-cancel-computedstyle.html View 1 2 3 4 1 chunk +50 lines, -0 lines 0 comments Download
A LayoutTests/animations/display-none-cancel-computedstyle-expected.txt View 1 2 3 1 chunk +11 lines, -0 lines 0 comments Download
M Source/core/dom/Element.cpp View 2 2 chunks +7 lines, -7 lines 0 comments Download

Messages

Total messages: 20 (8 generated)
esprehn
4 years, 11 months ago (2015-06-02 04:52:17 UTC) #2
esprehn
4 years, 11 months ago (2015-06-02 16:35:40 UTC) #4
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1156213003/40001
4 years, 11 months ago (2015-06-02 16:35:59 UTC) #6
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: linux_blink_rel on tryserver.blink (JOB_FAILED, http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/64476)
4 years, 11 months ago (2015-06-02 18:17:48 UTC) #8
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1156213003/60001
4 years, 11 months ago (2015-06-02 22:04:46 UTC) #10
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1156213003/80001
4 years, 11 months ago (2015-06-02 22:07:21 UTC) #12
esprehn
Okay now there's a test, plz review? :)
4 years, 11 months ago (2015-06-02 22:08:18 UTC) #13
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
4 years, 11 months ago (2015-06-02 23:38:18 UTC) #15
esprehn
ping. :)
4 years, 11 months ago (2015-06-03 02:55:27 UTC) #16
dstockwell
lgtm
4 years, 11 months ago (2015-06-03 04:57:52 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1156213003/80001
4 years, 11 months ago (2015-06-03 05:03:32 UTC) #19
commit-bot: I haz the power
4 years, 11 months ago (2015-06-03 05:07:50 UTC) #20
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=196370

Powered by Google App Engine
This is Rietveld 408576698