Attachment #41190 for bug #26770

View Details Raw Unified Return to Bug 26770



2009-10-14  Simon Fraser  <simon.fraser@apple.com>

        Reviewed by NOBODY (OOPS!).

        Transitions fail to run sometimes
        https://bugs.webkit.org/show_bug.cgi?id=26770
        
        Add a test for starting an accelerated animation on an element with a visibility:hidden ancestor,
        to ensure that it does not block subsequent transitions.

        * compositing/animation/animated-composited-inside-hidden-expected.txt: Added.
        * compositing/animation/animated-composited-inside-hidden.html: Added.

2009-10-13  Simon Fraser  <simon.fraser@apple.com>

        Reviewed by Dan Bernstein.

Line 0 a/LayoutTests/compositing/animation/animated-composited-inside-hidden-expected.txt_sec1
- a/LayoutTests/compositing/animation/animated-composited-inside-hidden-expected.txt +4 lines
	1	Tests that starting an animation on an element inside a visibility:hidden element does not block later transitions.
	2
	3	PASS - "left" property for "transition-tester" element at 0.5s saw something close to: 10
	4



<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"
   "http://www.w3.org/TR/html4/loose.dtd">

<html lang="en">
<head>
  <title>Visibility nested</title>
  <style type="text/css" media="screen">
    .box {
      position: relative;
      width: 100px;
      height: 100px;
      border: 1px solid black;
    }
    
    .middle {
      visibility: hidden;
    }

    .inner {
      width: 80px;
      height: 80px;
      -webkit-animation: spin 5s linear infinite;
    }

    @-webkit-keyframes spin {
      0% { -webkit-transform: rotate(0); }
      100% { -webkit-transform: rotate(360deg); }
    }
    
    #transition-tester {
      position: relative;
      left: 0;
      background-color: blue;
      -webkit-transition: left 1s linear;
    }
    
    #container.animating #transition-tester {
      left: 20px;
    }
    
  </style>
  <script src="../../transitions/transition-test-helpers.js" type="text/javascript" charset="utf-8"></script>
  <script type="text/javascript" charset="utf-8">
    const expectedValues = [
      // [time, element-id, property, expected-value, tolerance]
      [0.5, 'transition-tester', 'left', 10, 2],
    ];

    function setupTest()
    {
      document.getElementById('container').className = 'animating';
    }

    runTransitionTest(expectedValues, setupTest, true /* use pause API */);
  </script>
</head>
<body>

<p>Tests that starting an animation on an element inside a visibility:hidden element does not block later transitions.</p>
  <div id="container" class="container">
    <div id="transition-tester" class="box"></div>

    <div class="middle box">
      <div class="inner box"></div>
    </div>
  </div>

<div id="result"></div>

</body>
</html>



2009-10-14  Simon Fraser  <simon.fraser@apple.com>

        Reviewed by NOBODY (OOPS!).

        Transitions fail to run sometimes
        https://bugs.webkit.org/show_bug.cgi?id=26770

        Fix an issue where we could attempt to start accelerated animations or transitions on
        GraphicsLayer that were not rooted (because of visibility:hidden), which would leave
        the AnimationController's m_waitingForResponse flag in a state that killed subsequent
        software transitions.

        * page/animation/AnimationController.cpp:
        (WebCore::AnimationControllerPrivate::AnimationControllerPrivate):
        (WebCore::AnimationControllerPrivate::endAnimationUpdate):
        (WebCore::AnimationControllerPrivate::receivedStartTimeResponse):
        (WebCore::AnimationControllerPrivate::addToStartTimeResponseWaitList):
        (WebCore::AnimationControllerPrivate::startTimeResponse):
        * page/animation/AnimationControllerPrivate.h:
        Make some methods non-inline for ease of debugging (these are not hot methods).
        Rename m_waitingForAResponse to m_waitingForResponse.
        
        * platform/graphics/GraphicsLayer.h:
        * platform/graphics/GraphicsLayer.cpp:
        (WebCore::GraphicsLayer::hasAncestor):
        New method to report whether the receiver has the given layer as an ancestor. Used for checking
        whether a layer is rooted.

        * rendering/RenderLayerBacking.cpp:
        (WebCore::RenderLayerBacking::startAnimation):
        (WebCore::RenderLayerBacking::startTransition):
        Don't try to start accelerated animations or transitions on non-rooted GraphicsLayers.

2009-10-13  Drew Wilson  <atwilson@atwilson-macpro.local>

        Reviewed by David Levin.



    , m_lastStyleAvailableWaiter(0)
    , m_responseWaiters(0)
    , m_lastResponseWaiter(0)
    , m_waitingForResponse(false)
{
}


    return m_beginAnimationUpdateTime;
}

void AnimationControllerPrivate::endAnimationUpdate()
{
    styleAvailable();
    if (!m_waitingForResponse)
        startTimeResponse(beginAnimationUpdateTime());
}

void AnimationControllerPrivate::receivedStartTimeResponse(double time)
{
    m_waitingForResponse = false;
    startTimeResponse(time);
}

PassRefPtr<RenderStyle> AnimationControllerPrivate::getAnimatedStyleForRenderer(RenderObject* renderer)
{
    if (!renderer)

    ASSERT(!animation->next());
    
    if (willGetResponse)
        m_waitingForResponse = true;
    
    if (m_responseWaiters)
        m_lastResponseWaiter->setNext(animation);

    }
}

void AnimationControllerPrivate::startTimeResponse(double time)
{
    // Go through list of waiters and send them on their way
    for (AnimationBase* animation = m_responseWaiters; animation; ) {
        AnimationBase* nextAnimation = animation->next();
        animation->setNext(0);
        animation->onAnimationStartResponse(time);
        animation = nextAnimation;
    }
    




    double beginAnimationUpdateTime();
    void setBeginAnimationUpdateTime(double t) { m_beginAnimationUpdateTime = t; }
    void endAnimationUpdate();
    void receivedStartTimeResponse(double);
        styleAvailable();
        if (!m_waitingForAResponse)
            startTimeResponse(beginAnimationUpdateTime());
    }
    
    void receivedStartTimeResponse(double t)
    {
        m_waitingForAResponse = false;
        startTimeResponse(t);
    }
    
    void addToStyleAvailableWaitList(AnimationBase*);
    void removeFromStyleAvailableWaitList(AnimationBase*);    

    
    AnimationBase* m_responseWaiters;
    AnimationBase* m_lastResponseWaiter;
    bool m_waitingForResponse;
};

} // namespace WebCore



    removeFromParent();
}

bool GraphicsLayer::hasAncestor(GraphicsLayer* ancestor) const
{
    for (GraphicsLayer* curr = parent(); curr; curr = curr->parent()) {
        if (curr == ancestor)
            return true;
    }
    
    return false;
}

void GraphicsLayer::addChild(GraphicsLayer* childLayer)
{
    ASSERT(childLayer != this);

Lines 172-177 public: a/WebCore/platform/graphics/GraphicsLayer.h_sec1
- a/WebCore/platform/graphics/GraphicsLayer.h +3 lines
172	GraphicsLayer* parent() const { return m_parent; };	172	GraphicsLayer* parent() const { return m_parent; };
173	void setParent(GraphicsLayer* layer) { m_parent = layer; } // Internal use only.	173	void setParent(GraphicsLayer* layer) { m_parent = layer; } // Internal use only.
174		174
		175	// Returns true if the layer has the given layer as an ancestor (excluding self).
		176	bool hasAncestor(GraphicsLayer*) const;
		177
175	const Vector<GraphicsLayer*>& children() const { return m_children; }	178	const Vector<GraphicsLayer*>& children() const { return m_children; }
176		179
177	// Add child layers. If the child is already parented, it will be removed from its old parent.	180	// Add child layers. If the child is already parented, it will be removed from its old parent.

Lines 1034-1039 bool RenderLayerBacking::startAnimation(double beginTime, const Animation* anim, a/WebCore/rendering/RenderLayerBacking.cpp_sec1
- a/WebCore/rendering/RenderLayerBacking.cpp +8 lines
1034	if (!hasOpacity && !hasTransform)	1034	if (!hasOpacity && !hasTransform)
1035	return false;	1035	return false;
1036		1036
		1037	// Don't start animations if we're not rooted, because we won't get the callback that the animation started.
		1038	if (!m_graphicsLayer->hasAncestor(compositor()->rootPlatformLayer()))
		1039	return false;
		1040
1037	KeyframeValueList transformVector(AnimatedPropertyWebkitTransform);	1041	KeyframeValueList transformVector(AnimatedPropertyWebkitTransform);
1038	KeyframeValueList opacityVector(AnimatedPropertyOpacity);	1042	KeyframeValueList opacityVector(AnimatedPropertyOpacity);
1039		1043
Lines 1075-1080 bool RenderLayerBacking::startTransition(double beginTime, int property, const R a/WebCore/rendering/RenderLayerBacking.cpp_sec2
1075	bool didAnimate = false;	1079	bool didAnimate = false;
1076	ASSERT(property != cAnimateAll);	1080	ASSERT(property != cAnimateAll);
1077		1081
		1082	// Don't start animations if we're not rooted, because we won't get the callback that the animation started.
		1083	if (!m_graphicsLayer->hasAncestor(compositor()->rootPlatformLayer()))
		1084	return false;
		1085
1078	if (property == (int)CSSPropertyOpacity) {	1086	if (property == (int)CSSPropertyOpacity) {
1079	const Animation* opacityAnim = toStyle->transitionForProperty(CSSPropertyOpacity);	1087	const Animation* opacityAnim = toStyle->transitionForProperty(CSSPropertyOpacity);
1080	if (opacityAnim && !opacityAnim->isEmptyOrZeroDuration()) {	1088	if (opacityAnim && !opacityAnim->isEmptyOrZeroDuration()) {

Return to Bug 26770

Lines 55-61 AnimationControllerPrivate::AnimationControllerPrivate(Frame* frame) a/WebCore/page/animation/AnimationController.cpp_sec1
- a/WebCore/page/animation/AnimationController.cpp -4 / +17 lines
55	, m_lastStyleAvailableWaiter(0)	55	, m_lastStyleAvailableWaiter(0)
56	, m_responseWaiters(0)	56	, m_responseWaiters(0)
57	, m_lastResponseWaiter(0)	57	, m_lastResponseWaiter(0)
58	, m_waitingForAResponse(false)	58	, m_waitingForResponse(false)
59	{	59	{
60	}	60	}
61		61
Lines 279-284 double AnimationControllerPrivate::beginAnimationUpdateTime() a/WebCore/page/animation/AnimationController.cpp_sec2
279	return m_beginAnimationUpdateTime;	279	return m_beginAnimationUpdateTime;
280	}	280	}
281		281
		282	void AnimationControllerPrivate::endAnimationUpdate()
		283	{
		284	styleAvailable();
		285	if (!m_waitingForResponse)
		286	startTimeResponse(beginAnimationUpdateTime());
		287	}
		288
		289	void AnimationControllerPrivate::receivedStartTimeResponse(double time)
		290	{
		291	m_waitingForResponse = false;
		292	startTimeResponse(time);
		293	}
		294
282	PassRefPtr<RenderStyle> AnimationControllerPrivate::getAnimatedStyleForRenderer(RenderObject* renderer)	295	PassRefPtr<RenderStyle> AnimationControllerPrivate::getAnimatedStyleForRenderer(RenderObject* renderer)
283	{	296	{
284	if (!renderer)	297	if (!renderer)
Lines 378-384 void AnimationControllerPrivate::addToStartTimeResponseWaitList(AnimationBase* a a/WebCore/page/animation/AnimationController.cpp_sec3
378	ASSERT(!animation->next());	391	ASSERT(!animation->next());
379		392
380	if (willGetResponse)	393	if (willGetResponse)
381	m_waitingForAResponse = true;	394	m_waitingForResponse = true;
382		395
383	if (m_responseWaiters)	396	if (m_responseWaiters)
384	m_lastResponseWaiter->setNext(animation);	397	m_lastResponseWaiter->setNext(animation);
Lines 408-420 void AnimationControllerPrivate::removeFromStartTimeResponseWaitList(AnimationBa a/WebCore/page/animation/AnimationController.cpp_sec4
408	}	421	}
409	}	422	}
410		423
411	void AnimationControllerPrivate::startTimeResponse(double t)	424	void AnimationControllerPrivate::startTimeResponse(double time)
412	{	425	{
413	// Go through list of waiters and send them on their way	426	// Go through list of waiters and send them on their way
414	for (AnimationBase* animation = m_responseWaiters; animation; ) {	427	for (AnimationBase* animation = m_responseWaiters; animation; ) {
415	AnimationBase* nextAnimation = animation->next();	428	AnimationBase* nextAnimation = animation->next();
416	animation->setNext(0);	429	animation->setNext(0);
417	animation->onAnimationStartResponse(t);	430	animation->onAnimationStartResponse(time);
418	animation = nextAnimation;	431	animation = nextAnimation;
419	}	432	}
420		433

Lines 1-3 a/LayoutTests/ChangeLog_sec1
- a/LayoutTests/ChangeLog +13 lines
		1	2009-10-14 Simon Fraser <simon.fraser@apple.com>
		2
		3	Reviewed by NOBODY (OOPS!).
		4
		5	Transitions fail to run sometimes
		6	https://bugs.webkit.org/show_bug.cgi?id=26770
		7
		8	Add a test for starting an accelerated animation on an element with a visibility:hidden ancestor,
		9	to ensure that it does not block subsequent transitions.
		10
		11	* compositing/animation/animated-composited-inside-hidden-expected.txt: Added.
		12	* compositing/animation/animated-composited-inside-hidden.html: Added.
		13
1	2009-10-13 Simon Fraser <simon.fraser@apple.com>	14	2009-10-13 Simon Fraser <simon.fraser@apple.com>
2		15
3	Reviewed by Dan Bernstein.	16	Reviewed by Dan Bernstein.

Line 0 a/LayoutTests/compositing/animation/animated-composited-inside-hidden.html_sec1
- a/LayoutTests/compositing/animation/animated-composited-inside-hidden.html +71 lines
		1	<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"
		2	"http://www.w3.org/TR/html4/loose.dtd">
		3
		4	<html lang="en">
		5	<head>
		6	<title>Visibility nested</title>
		7	<style type="text/css" media="screen">
		8	.box {
		9	position: relative;
		10	width: 100px;
		11	height: 100px;
		12	border: 1px solid black;
		13	}
		14
		15	.middle {
		16	visibility: hidden;
		17	}
		18
		19	.inner {
		20	width: 80px;
		21	height: 80px;
		22	-webkit-animation: spin 5s linear infinite;
		23	}
		24
		25	@-webkit-keyframes spin {
		26	0% { -webkit-transform: rotate(0); }
		27	100% { -webkit-transform: rotate(360deg); }
		28	}
		29
		30	#transition-tester {
		31	position: relative;
		32	left: 0;
		33	background-color: blue;
		34	-webkit-transition: left 1s linear;
		35	}
		36
		37	#container.animating #transition-tester {
		38	left: 20px;
		39	}
		40
		41	</style>
		42	<script src="../../transitions/transition-test-helpers.js" type="text/javascript" charset="utf-8"></script>
		43	<script type="text/javascript" charset="utf-8">
		44	const expectedValues = [
		45	// [time, element-id, property, expected-value, tolerance]
		46	[0.5, 'transition-tester', 'left', 10, 2],
		47	];
		48
		49	function setupTest()
		50	{
		51	document.getElementById('container').className = 'animating';
		52	}
		53
		54	runTransitionTest(expectedValues, setupTest, true /* use pause API */);
		55	</script>
		56	</head>
		57	<body>
		58
		59	<p>Tests that starting an animation on an element inside a visibility:hidden element does not block later transitions.</p>
		60	<div id="container" class="container">
		61	<div id="transition-tester" class="box"></div>
		62
		63	<div class="middle box">
		64	<div class="inner box"></div>
65	</div>
66	</div>
67
68	<div id="result"></div>
69
70	</body>
71	</html>

Lines 1-3 a/WebCore/ChangeLog_sec1
- a/WebCore/ChangeLog +33 lines
		1	2009-10-14 Simon Fraser <simon.fraser@apple.com>
		2
		3	Reviewed by NOBODY (OOPS!).
		4
		5	Transitions fail to run sometimes
		6	https://bugs.webkit.org/show_bug.cgi?id=26770
		7
		8	Fix an issue where we could attempt to start accelerated animations or transitions on
		9	GraphicsLayer that were not rooted (because of visibility:hidden), which would leave
		10	the AnimationController's m_waitingForResponse flag in a state that killed subsequent
		11	software transitions.
		12
		13	* page/animation/AnimationController.cpp:
		14	(WebCore::AnimationControllerPrivate::AnimationControllerPrivate):
		15	(WebCore::AnimationControllerPrivate::endAnimationUpdate):
		16	(WebCore::AnimationControllerPrivate::receivedStartTimeResponse):
		17	(WebCore::AnimationControllerPrivate::addToStartTimeResponseWaitList):
		18	(WebCore::AnimationControllerPrivate::startTimeResponse):
		19	* page/animation/AnimationControllerPrivate.h:
		20	Make some methods non-inline for ease of debugging (these are not hot methods).
		21	Rename m_waitingForAResponse to m_waitingForResponse.
		22
		23	* platform/graphics/GraphicsLayer.h:
		24	* platform/graphics/GraphicsLayer.cpp:
		25	(WebCore::GraphicsLayer::hasAncestor):
		26	New method to report whether the receiver has the given layer as an ancestor. Used for checking
		27	whether a layer is rooted.
		28
		29	* rendering/RenderLayerBacking.cpp:
		30	(WebCore::RenderLayerBacking::startAnimation):
		31	(WebCore::RenderLayerBacking::startTransition):
		32	Don't try to start accelerated animations or transitions on non-rooted GraphicsLayers.
		33
1	2009-10-13 Drew Wilson <atwilson@atwilson-macpro.local>	34	2009-10-13 Drew Wilson <atwilson@atwilson-macpro.local>
2		35
3	Reviewed by David Levin.	36	Reviewed by David Levin.

Lines 80-97 public: a/WebCore/page/animation/AnimationControllerPrivate.h_sec1
- a/WebCore/page/animation/AnimationControllerPrivate.h -13 / +3 lines
80		80
81	double beginAnimationUpdateTime();	81	double beginAnimationUpdateTime();
82	void setBeginAnimationUpdateTime(double t) { m_beginAnimationUpdateTime = t; }	82	void setBeginAnimationUpdateTime(double t) { m_beginAnimationUpdateTime = t; }
83	void endAnimationUpdate()	83	void endAnimationUpdate();
84	{	84	void receivedStartTimeResponse(double);
85	styleAvailable();
86	if (!m_waitingForAResponse)
87	startTimeResponse(beginAnimationUpdateTime());
88	}
89
90	void receivedStartTimeResponse(double t)
91	{
92	m_waitingForAResponse = false;
93	startTimeResponse(t);
94	}
95		85
96	void addToStyleAvailableWaitList(AnimationBase*);	86	void addToStyleAvailableWaitList(AnimationBase*);
97	void removeFromStyleAvailableWaitList(AnimationBase*);	87	void removeFromStyleAvailableWaitList(AnimationBase*);
Lines 127-133 private: a/WebCore/page/animation/AnimationControllerPrivate.h_sec2
127		117
128	AnimationBase* m_responseWaiters;	118	AnimationBase* m_responseWaiters;
129	AnimationBase* m_lastResponseWaiter;	119	AnimationBase* m_lastResponseWaiter;
130	bool m_waitingForAResponse;	120	bool m_waitingForResponse;
131	};	121	};
132		122
133	} // namespace WebCore	123	} // namespace WebCore

Lines 86-91 GraphicsLayer::~GraphicsLayer() a/WebCore/platform/graphics/GraphicsLayer.cpp_sec1
- a/WebCore/platform/graphics/GraphicsLayer.cpp +10 lines
86	removeFromParent();	86	removeFromParent();
87	}	87	}
88		88
		89	bool GraphicsLayer::hasAncestor(GraphicsLayer* ancestor) const
		90	{
		91	for (GraphicsLayer* curr = parent(); curr; curr = curr->parent()) {
		92	if (curr == ancestor)
		93	return true;
		94	}
		95
		96	return false;
		97	}
		98
89	void GraphicsLayer::addChild(GraphicsLayer* childLayer)	99	void GraphicsLayer::addChild(GraphicsLayer* childLayer)
90	{	100	{
91	ASSERT(childLayer != this);	101	ASSERT(childLayer != this);