Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-10-21 17:25:36 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1524480 - Set NS_FRAME_MAY_BE_TRANSFORMED bit when we have nsChangeHint_UpdateTransformLayer; r=hiro

Browse Source 
Typically we set the NS_FRAME_MAY_BE_TRANSFORMED bit on a frame when one of the
following situations arises:

a. We update the keyframes of a KeyframeEffect to include transforms or we
   create a new KeyframeEffect that animates transforms (in
   KeyframeEffect::SetKeyframes), or
b. We retarget a KeyframeEffect with transforms at a new element (in
   KeyframeEffect::SetTarget), or
c. We create an nsFrame with transform animations applied to it (in
   nsFrame::Init), or
d. We get a nsChangeHint_AddOrRemoveTransform hint in
   RestyleManager::ProcessRestyledFrames and decide to update the frame bit on
   the frame and its continuations accordingly.

However, there are some situations where we can have a transform animation on
a frame where none of the above are triggered.

For example, if the style frame is not unavailable (e.g.  a display:none
element) when the KeyframeEffect is initialized we will fail to the frame bit in
(a) and if we never retarget the effect we will never set reach (b).

Furthermore, if we have an animation that is "not relevant" (e.g.  idle) and
hence not registered with the EffectSet when the frame is initialized we will
fail to set the frame bit in (c).

Finally, if the the animation does not produce a style change that causes the
nsChangeHint_AddOrRemoveTransform bit to be set (e.g. the transform animation
begins at 'none') we will not set the frame bit in (d).

As a result, we can end up failing to set the NS_FRAME_MAY_BE_TRANSFORMED bit
for some content.

The crashtest included in this patch produces such a case and, without the code
changes in this patch, will fail the assertion in ApplyRenderingChangeToTree[1]:

  NS_ASSERTION(!(aChange & nsChangeHint_UpdateTransformLayer) ||
                   aFrame->IsTransformed() ||
                   aFrame->StyleDisplay()->HasTransformStyle(),
               "Unexpected UpdateTransformLayer hint");

That is because although the nsChangeHint_UpdateTransformLayer bit will be set,
aFrame->IsTransformed() will return false because the frame bit has not been
set, and aFrame->StyleDisplay()->HasTransformStyle() will return false because
the animation sets the transform to 'none'.

Not only will this assertion fail, but once we cease flushing style as part of
triggering an animation later in this patch, the reftest
layout/reftests/web-animations/stacking-context-transform-changing-effect.html
will begin to fail. That reftest produces a similar situation to the crashtest
but it currently does not fail because the style flush that happens as part of
creating an animation ensures the style frame is available at the point when the
animation is triggered and hence case (a) from above is hit.

This patch addresses this by detecting the case where we have this change hint
set but not the corresponding frame bit, and setting the frame bit.

[1] https://searchfox.org/mozilla-central/rev/9eb30227b21e0aa40d51d9f9b08bb0b113c5fadb/layout/base/RestyleManager.cpp#1191-1199

Differential Revision: https://phabricator.services.mozilla.com/D18911

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Brian Birtles 2019-02-15 06:35:55 +00:00
parent 4bf4b6e949
commit 78671d57b6
3 changed files with 56 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

37
dom/animation/test/crashtests/1524480-1.html Normal file
View File

@ -0,0 +1,37 @@
<!doctype html>
<html class="reftest-wait">
<meta charset=utf-8>
<style>
div {
display: none;
width: 100px;
height: 100px;
background: blue;
}
</style>
<div id=div></div>
<script>
async function test() {
const animation = div.animate({ transform: ['none', 'none'] }, 1000);
animation.cancel();
await waitForFrame();
div.style.display = 'block';
await waitForFrame();
await waitForFrame();
animation.play();
await animation.finished;
document.documentElement.className = "";
}
function waitForFrame() {
return new Promise(resolve => requestAnimationFrame(resolve));
}
test();
</script>
</html>

1
dom/animation/test/crashtests/crashtests.list
View File

@ -42,3 +42,4 @@ pref(dom.animations-api.core.enabled,true) pref(dom.animations-api.implicit-keyf
pref(dom.animations-api.core.enabled,true) pref(dom.animations-api.timelines.enabled,true) pref(dom.animations-api.implicit-keyframes.enabled,true) load 1411318-1.html
pref(dom.animations-api.implicit-keyframes.enabled,true) load 1468294-1.html
pref(dom.animations-api.implicit-keyframes.enabled,true) load 1467277-1.html
load 1524480-1.html

18
layout/base/RestyleManager.cpp
View File

@ -1615,6 +1615,24 @@ void RestyleManager::ProcessRestyledFrames(nsStyleChangeList& aChangeList) {
hint &= ~nsChangeHint_UpdatePostTransformOverflow;
}
if ((hint & nsChangeHint_UpdateTransformLayer) &&
!(frame->GetStateBits() & NS_FRAME_MAY_BE_TRANSFORMED) &&
frame->HasAnimationOfTransform()) {
// If we have an nsChangeHint_UpdateTransformLayer hint but no
// corresponding frame bit, it's possible we have a transform animation
// with transform style 'none' that was initialized independently from
// this frame and associated after the fact.
//
// In that case we should set the frame bit.
//
// FIXME: Bug 1527210 - Use the primary frame here instead so that
// we handle display: table correctly.
for (nsIFrame* cont = frame; cont;
cont = nsLayoutUtils::GetNextContinuationOrIBSplitSibling(cont)) {
cont->AddStateBits(NS_FRAME_MAY_BE_TRANSFORMED);
}
}
if (hint & nsChangeHint_AddOrRemoveTransform) {
// When dropping a running transform animation we will first add an
// nsChangeHint_UpdateTransformLayer hint as part of the animation-only