- Revision
- 222098
- Author
- an...@apple.com
- Date
- 2017-09-15 10:44:49 -0700 (Fri, 15 Sep 2017)
Log Message
AnimationBase should ref the element
https://bugs.webkit.org/show_bug.cgi?id=176993
Reviewed by Simon Fraser.
We now longer have renderer pointer. Element can be reffed for safety.
This doesn't create reference cycle as the element pointer is cleared when render tree is
torn down. This happens at the latest when the element is removed from the tree.
* page/animation/AnimationBase.cpp:
(WebCore::AnimationBase::~AnimationBase):
(WebCore::AnimationBase::clear):
* page/animation/AnimationBase.h:
(WebCore::AnimationBase::~AnimationBase): Deleted.
(WebCore::AnimationBase::clear): Deleted.
* page/animation/ImplicitAnimation.cpp:
(WebCore::ImplicitAnimation::pauseAnimation):
(WebCore::ImplicitAnimation::sendTransitionEvent):
(WebCore::ImplicitAnimation::reset):
* page/animation/KeyframeAnimation.cpp:
(WebCore::KeyframeAnimation::pauseAnimation):
(WebCore::KeyframeAnimation::endAnimation):
(WebCore::KeyframeAnimation::sendAnimationEvent):
(WebCore::KeyframeAnimation::resolveKeyframeStyles):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (222097 => 222098)
--- trunk/Source/WebCore/ChangeLog 2017-09-15 17:37:18 UTC (rev 222097)
+++ trunk/Source/WebCore/ChangeLog 2017-09-15 17:44:49 UTC (rev 222098)
@@ -1,3 +1,31 @@
+2017-09-15 Antti Koivisto <an...@apple.com>
+
+ AnimationBase should ref the element
+ https://bugs.webkit.org/show_bug.cgi?id=176993
+
+ Reviewed by Simon Fraser.
+
+ We now longer have renderer pointer. Element can be reffed for safety.
+
+ This doesn't create reference cycle as the element pointer is cleared when render tree is
+ torn down. This happens at the latest when the element is removed from the tree.
+
+ * page/animation/AnimationBase.cpp:
+ (WebCore::AnimationBase::~AnimationBase):
+ (WebCore::AnimationBase::clear):
+ * page/animation/AnimationBase.h:
+ (WebCore::AnimationBase::~AnimationBase): Deleted.
+ (WebCore::AnimationBase::clear): Deleted.
+ * page/animation/ImplicitAnimation.cpp:
+ (WebCore::ImplicitAnimation::pauseAnimation):
+ (WebCore::ImplicitAnimation::sendTransitionEvent):
+ (WebCore::ImplicitAnimation::reset):
+ * page/animation/KeyframeAnimation.cpp:
+ (WebCore::KeyframeAnimation::pauseAnimation):
+ (WebCore::KeyframeAnimation::endAnimation):
+ (WebCore::KeyframeAnimation::sendAnimationEvent):
+ (WebCore::KeyframeAnimation::resolveKeyframeStyles):
+
2017-09-15 Brent Fulgham <bfulg...@apple.com>
Make DocumentLoader a FrameDestructionObserver
Modified: trunk/Source/WebCore/page/animation/AnimationBase.cpp (222097 => 222098)
--- trunk/Source/WebCore/page/animation/AnimationBase.cpp 2017-09-15 17:37:18 UTC (rev 222097)
+++ trunk/Source/WebCore/page/animation/AnimationBase.cpp 2017-09-15 17:44:49 UTC (rev 222098)
@@ -86,6 +86,10 @@
m_totalDuration = m_animation->duration() * m_animation->iterationCount();
}
+AnimationBase::~AnimationBase()
+{
+}
+
RenderElement* AnimationBase::renderer() const
{
return m_element ? m_element->renderer() : nullptr;
@@ -99,6 +103,13 @@
return downcast<RenderBoxModelObject>(renderer);
}
+void AnimationBase::clear()
+{
+ endAnimation();
+ m_element = nullptr;
+ m_compositeAnimation = nullptr;
+}
+
void AnimationBase::setNeedsStyleRecalc(Element* element)
{
if (!element || element->document().renderTreeBeingDestroyed())
Modified: trunk/Source/WebCore/page/animation/AnimationBase.h (222097 => 222098)
--- trunk/Source/WebCore/page/animation/AnimationBase.h 2017-09-15 17:37:18 UTC (rev 222097)
+++ trunk/Source/WebCore/page/animation/AnimationBase.h 2017-09-15 17:44:49 UTC (rev 222098)
@@ -49,16 +49,12 @@
WTF_MAKE_FAST_ALLOCATED;
public:
AnimationBase(const Animation& transition, Element&, CompositeAnimation&);
- virtual ~AnimationBase() { }
+ virtual ~AnimationBase();
+ Element* element() const { return m_element.get(); }
RenderElement* renderer() const;
RenderBoxModelObject* compositedRenderer() const;
- void clear()
- {
- endAnimation();
- m_element = nullptr;
- m_compositeAnimation = nullptr;
- }
+ void clear();
double duration() const;
@@ -239,7 +235,10 @@
bool computeTransformedExtentViaTransformList(const FloatRect& rendererBox, const RenderStyle&, LayoutRect& bounds) const;
bool computeTransformedExtentViaMatrix(const FloatRect& rendererBox, const RenderStyle&, LayoutRect& bounds) const;
- Element* m_element;
+private:
+ RefPtr<Element> m_element;
+
+protected:
CompositeAnimation* m_compositeAnimation; // Ideally this would be a reference, but it has to be cleared if an animation is destroyed inside an event callback.
Ref<Animation> m_animation;
Modified: trunk/Source/WebCore/page/animation/ImplicitAnimation.cpp (222097 => 222098)
--- trunk/Source/WebCore/page/animation/ImplicitAnimation.cpp 2017-09-15 17:37:18 UTC (rev 222097)
+++ trunk/Source/WebCore/page/animation/ImplicitAnimation.cpp 2017-09-15 17:44:49 UTC (rev 222098)
@@ -58,7 +58,7 @@
bool ImplicitAnimation::shouldSendEventForListener(Document::ListenerType inListenerType) const
{
- return m_element->document().hasListenerType(inListenerType);
+ return element()->document().hasListenerType(inListenerType);
}
bool ImplicitAnimation::animate(CompositeAnimation& compositeAnimation, const RenderStyle& targetStyle, std::unique_ptr<RenderStyle>& animatedStyle, bool& didBlendStyle)
@@ -144,7 +144,7 @@
renderer->transitionPaused(timeOffset, m_animatingProperty);
// Restore the original (unanimated) style
if (!paused())
- setNeedsStyleRecalc(m_element);
+ setNeedsStyleRecalc(element());
}
void ImplicitAnimation::endAnimation()
@@ -176,7 +176,7 @@
String propertyName = getPropertyNameString(m_animatingProperty);
// Dispatch the event
- auto element = makeRefPtr(m_element);
+ auto element = makeRefPtr(this->element());
ASSERT(!element || element->document().pageCacheState() == Document::NotInPageCache);
if (!element)
@@ -202,8 +202,8 @@
m_toStyle = RenderStyle::clonePtr(to);
- if (m_element)
- Style::loadPendingResources(*m_toStyle, m_element->document(), m_element);
+ if (element())
+ Style::loadPendingResources(*m_toStyle, element()->document(), element());
// Restart the transition.
if (m_fromStyle && m_toStyle && !compositeAnimation.isSuspended())
Modified: trunk/Source/WebCore/page/animation/KeyframeAnimation.cpp (222097 => 222098)
--- trunk/Source/WebCore/page/animation/KeyframeAnimation.cpp 2017-09-15 17:37:18 UTC (rev 222097)
+++ trunk/Source/WebCore/page/animation/KeyframeAnimation.cpp 2017-09-15 17:44:49 UTC (rev 222098)
@@ -284,7 +284,7 @@
void KeyframeAnimation::pauseAnimation(double timeOffset)
{
- if (!m_element)
+ if (!element())
return;
if (auto* renderer = compositedRenderer())
@@ -292,12 +292,12 @@
// Restore the original (unanimated) style
if (!paused())
- setNeedsStyleRecalc(m_element);
+ setNeedsStyleRecalc(element());
}
void KeyframeAnimation::endAnimation()
{
- if (!m_element)
+ if (!element())
return;
if (auto* renderer = compositedRenderer())
@@ -305,12 +305,12 @@
// Restore the original (unanimated) style
if (!paused())
- setNeedsStyleRecalc(m_element);
+ setNeedsStyleRecalc(element());
}
bool KeyframeAnimation::shouldSendEventForListener(Document::ListenerType listenerType) const
{
- return m_element->document().hasListenerType(listenerType);
+ return element()->document().hasListenerType(listenerType);
}
void KeyframeAnimation::onAnimationStart(double elapsedTime)
@@ -349,7 +349,7 @@
if (shouldSendEventForListener(listenerType)) {
// Dispatch the event
- auto element = makeRefPtr(m_element);
+ auto element = makeRefPtr(this->element());
ASSERT(!element || element->document().pageCacheState() == Document::NotInPageCache);
if (!element)
@@ -389,16 +389,16 @@
void KeyframeAnimation::resolveKeyframeStyles()
{
- if (!m_element)
+ if (!element())
return;
- if (auto* styleScope = Style::Scope::forOrdinal(*m_element, m_animation->nameStyleScopeOrdinal()))
- styleScope->resolver().keyframeStylesForAnimation(*m_element, m_unanimatedStyle.get(), m_keyframes);
+ if (auto* styleScope = Style::Scope::forOrdinal(*element(), m_animation->nameStyleScopeOrdinal()))
+ styleScope->resolver().keyframeStylesForAnimation(*element(), m_unanimatedStyle.get(), m_keyframes);
// Ensure resource loads for all the frames.
for (auto& keyframe : m_keyframes.keyframes()) {
if (auto* style = const_cast<RenderStyle*>(keyframe.style()))
- Style::loadPendingResources(*style, m_element->document(), m_element);
+ Style::loadPendingResources(*style, element()->document(), element());
}
}
