Closed
Bug 1087536
Opened 9 years ago
Closed 9 years ago
don't rerun selector matching for main thread animation/transition ticks
Categories
(Core :: CSS Parsing and Computation, defect)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: dbaron, Assigned: dbaron)
References
Details
Attachments
(3 files)
3.74 KB,
patch
|
birtles
:
review+
|
Details | Diff | Splinter Review |
1.30 KB,
patch
|
birtles
:
review+
|
Details | Diff | Splinter Review |
2.68 KB,
patch
|
birtles
:
review+
|
Details | Diff | Splinter Review |
This is like bug 977991 for the style attribute, but instead for ticks of animations and transitions. In bug 996796 I added a fast way to do restyles for only the transition or animation styles, but at the time only used it for the miniflush replacement. I intended to use it more. It turns out I need to use it more prior to completing bug 960465.
Assignee | ||
Comment 1•9 years ago
|
||
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=5ecd2a91e3b8 https://tbpl.mozilla.org/?tree=Try&rev=5ecd2a91e3b8
Assignee | ||
Comment 2•9 years ago
|
||
There's clearly something wrong with my current patches; at least some of the above test failures were due to the patches here, as were problems with the Firefox OS lockscreen I was seeing on my phone.
Assignee | ||
Comment 3•9 years ago
|
||
I found the problem; it resulted in patch 1 and 2 of the resulting patch series. https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=ee7750b4a481 https://tbpl.mozilla.org/?tree=Try&rev=ee7750b4a481
Assignee | ||
Comment 4•9 years ago
|
||
Without this patch, patch 3 will cause bugs where we'll never remove the cover rule we create during the process of starting a transition. This won't actually be problematic during the transition, since the transition will overwrite it, but once the transition completes, the cover rule will still be around, and we'll be stuck with the pre-transition value instead of the post-transition value. It's possible it also fixes existing bugs prior to the patch series in this bug.
Attachment #8523380 -
Flags: review?(birtles)
Assignee | ||
Comment 5•9 years ago
|
||
I confirmed that this assertion fires (along with the other failures) when running layout/style/test/test_transitions_events.html with patch 3 but not patch 1.
Attachment #8523381 -
Flags: review?(birtles)
Assignee | ||
Comment 6•9 years ago
|
||
This depends on bug 1086937 patch 1 because it requires that ResolveStyleWithReplacement support eRestyle_ChangeAnimationPhase on ::before and ::after pseudo-elements. It also depends on patch 1 of this bug for the reasons described in patch 1's commit message. This is needed for bug 960465 so that we can use these hints to detect whether pending restyles include restyles other than those for animations. In other words, patches for bug 960465 (or perhaps a dependent bug that lands before it) will require that all animation restyles use an animation-specific nsRestyleHint. It is also, on its own, a performance improvement for animations and transitions, since we will stop rerunning selector matching on the animating element during the progress of the animations or transitions. Once we remove eRestyle_ChangeAnimationPhase the performance improvement will even become slightly better. Note that the eRestyle_ChangeAnimationPhase is needed in some cases because we use PostRestyleForAnimation in the non-animation-restyle phase when we have a style rule that we need to add during the animation restyle phase. (It's not needed during the progress of the animation, though. But hopefully both eRestyle_ChangeAnimationPhase will go away soon, after bug 960465. And hopefully the way we tick animations will also change to look more like the animation-only restyle, but without the main-thread-suppressed (throttled) animations.)
Attachment #8523382 -
Flags: review?(birtles)
Updated•9 years ago
|
Attachment #8523380 -
Flags: review?(birtles) → review+
Updated•9 years ago
|
Attachment #8523381 -
Flags: review?(birtles) → review+
Updated•9 years ago
|
Attachment #8523382 -
Flags: review?(birtles) → review+
Assignee | ||
Comment 7•9 years ago
|
||
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/65bab07a4f2f remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/c7ad03b147fc remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/7b93ce497136
Comment 8•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/65bab07a4f2f https://hg.mozilla.org/mozilla-central/rev/c7ad03b147fc https://hg.mozilla.org/mozilla-central/rev/7b93ce497136
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in
before you can comment on or make changes to this bug.
Description
•