Diff
Modified: branches/safari-603-branch/Source/WebCore/ChangeLog (211189 => 211190)
--- branches/safari-603-branch/Source/WebCore/ChangeLog 2017-01-26 01:41:07 UTC (rev 211189)
+++ branches/safari-603-branch/Source/WebCore/ChangeLog 2017-01-26 01:41:11 UTC (rev 211190)
@@ -1,5 +1,43 @@
2017-01-25 Matthew Hanson <matthew_han...@apple.com>
+ Merge 210777. rdar://problem/30186526
+
+ 2017-01-15 Andreas Kling <akl...@apple.com>
+
+ FrameView shouldn't keep dangling pointers into dead render trees.
+ <https://webkit.org/b/167011>
+
+ Reviewed by Antti Koivisto.
+
+ Added some pretty paranoid assertions to FrameView that verify all of its raw pointers
+ into the render tree are gone after the render tree has been destroyed.
+ They immediately caught two bugs, also fixed in this patch.
+
+ * page/FrameView.h:
+ * page/FrameView.cpp:
+ (WebCore::FrameView::willDestroyRenderTree):
+ (WebCore::FrameView::didDestroyRenderTree): Added these two callbacks for before/after
+ Document tears down its render tree. The former clears the layout root, and detaches
+ custom scrollbars. The latter contains a bunch of sanity assertions that pointers into
+ the now-destroyed render tree are gone.
+
+ * dom/Document.cpp:
+ (WebCore::Document::destroyRenderTree): Notify FrameView before/after teardown.
+
+ * page/animation/AnimationController.h:
+ * page/animation/AnimationController.cpp:
+ (WebCore::AnimationController::hasAnimations): Added a helper to check if there are
+ any composite animations around, as these contain raw pointers to renderers.
+
+ * rendering/RenderElement.cpp:
+ (WebCore::RenderElement::willBeRemovedFromTree):
+ (WebCore::RenderElement::willBeDestroyed): Moved slow repaint object unregistration
+ from willBeRemovedFromTree() to willBeDestroyed(). The willBeRemovedFromTree() callback
+ is skipped as an optimization during full tree teardown, but willBeDestroyed() always
+ gets called. This fixes a bug where we'd fail to remove dangling pointers.
+
+2017-01-25 Matthew Hanson <matthew_han...@apple.com>
+
Merge r211126. rdar://problem/30174873
2017-01-24 Simon Fraser <simon.fra...@apple.com>
Modified: branches/safari-603-branch/Source/WebCore/dom/Document.cpp (211189 => 211190)
--- branches/safari-603-branch/Source/WebCore/dom/Document.cpp 2017-01-26 01:41:07 UTC (rev 211189)
+++ branches/safari-603-branch/Source/WebCore/dom/Document.cpp 2017-01-26 01:41:11 UTC (rev 211190)
@@ -2219,7 +2219,10 @@
{
ASSERT(hasLivingRenderTree());
ASSERT(m_pageCacheState != InPageCache);
+ ASSERT(frame()->view());
+ FrameView& frameView = *frame()->view();
+
SetForScope<bool> change(m_renderTreeBeingDestroyed, true);
if (this == &topDocument())
@@ -2227,8 +2230,7 @@
documentWillBecomeInactive();
- if (FrameView* frameView = view())
- frameView->detachCustomScrollbars();
+ frameView.willDestroyRenderTree();
#if ENABLE(FULLSCREEN_API)
if (m_fullScreenRenderer)
@@ -2254,6 +2256,8 @@
// Do this before the arena is cleared, which is needed to deref the RenderStyle on TextAutoSizingKey.
m_textAutoSizedNodes.clear();
#endif
+
+ frameView.didDestroyRenderTree();
}
void Document::prepareForDestruction()
Modified: branches/safari-603-branch/Source/WebCore/page/FrameView.cpp (211189 => 211190)
--- branches/safari-603-branch/Source/WebCore/page/FrameView.cpp 2017-01-26 01:41:07 UTC (rev 211189)
+++ branches/safari-603-branch/Source/WebCore/page/FrameView.cpp 2017-01-26 01:41:11 UTC (rev 211190)
@@ -636,6 +636,27 @@
return ScrollView::createScrollbar(orientation);
}
+void FrameView::willDestroyRenderTree()
+{
+ detachCustomScrollbars();
+ m_layoutRoot = nullptr;
+}
+
+void FrameView::didDestroyRenderTree()
+{
+ ASSERT(!m_layoutRoot);
+ ASSERT(m_widgetsInRenderTree.isEmpty());
+
+ // If the render tree is destroyed below FrameView::updateEmbeddedObjects(), there will still be a null sentinel in the set.
+ // Everything else should have removed itself as the tree was felled.
+ ASSERT(!m_embeddedObjectsToUpdate || m_embeddedObjectsToUpdate->isEmpty() || (m_embeddedObjectsToUpdate->size() == 1 && m_embeddedObjectsToUpdate->first() == nullptr));
+
+ ASSERT(!m_viewportConstrainedObjects || m_viewportConstrainedObjects->isEmpty());
+ ASSERT(!m_slowRepaintObjects || m_slowRepaintObjects->isEmpty());
+
+ ASSERT(!frame().animation().hasAnimations());
+}
+
void FrameView::setContentsSize(const IntSize& size)
{
if (size == contentsSize())
Modified: branches/safari-603-branch/Source/WebCore/page/FrameView.h (211189 => 211190)
--- branches/safari-603-branch/Source/WebCore/page/FrameView.h 2017-01-26 01:41:07 UTC (rev 211189)
+++ branches/safari-603-branch/Source/WebCore/page/FrameView.h 2017-01-26 01:41:11 UTC (rev 211190)
@@ -587,6 +587,9 @@
bool shouldPlaceBlockDirectionScrollbarOnLeft() const final;
+ void willDestroyRenderTree();
+ void didDestroyRenderTree();
+
protected:
bool scrollContentsFastPath(const IntSize& scrollDelta, const IntRect& rectToScroll, const IntRect& clipRect) override;
void scrollContentsSlowPath(const IntRect& updateRect) override;
Modified: branches/safari-603-branch/Source/WebCore/page/animation/AnimationController.cpp (211189 => 211190)
--- branches/safari-603-branch/Source/WebCore/page/animation/AnimationController.cpp 2017-01-26 01:41:07 UTC (rev 211189)
+++ branches/safari-603-branch/Source/WebCore/page/animation/AnimationController.cpp 2017-01-26 01:41:11 UTC (rev 211190)
@@ -760,4 +760,9 @@
}
#endif
+bool AnimationController::hasAnimations() const
+{
+ return m_data->hasAnimations();
+}
+
} // namespace WebCore
Modified: branches/safari-603-branch/Source/WebCore/page/animation/AnimationController.h (211189 => 211190)
--- branches/safari-603-branch/Source/WebCore/page/animation/AnimationController.h 2017-01-26 01:41:07 UTC (rev 211189)
+++ branches/safari-603-branch/Source/WebCore/page/animation/AnimationController.h 2017-01-26 01:41:11 UTC (rev 211190)
@@ -91,6 +91,8 @@
void scrollWasUpdated();
#endif
+ bool hasAnimations() const;
+
private:
const std::unique_ptr<AnimationControllerPrivate> m_data;
};
Modified: branches/safari-603-branch/Source/WebCore/rendering/RenderElement.cpp (211189 => 211190)
--- branches/safari-603-branch/Source/WebCore/rendering/RenderElement.cpp 2017-01-26 01:41:07 UTC (rev 211189)
+++ branches/safari-603-branch/Source/WebCore/rendering/RenderElement.cpp 2017-01-26 01:41:11 UTC (rev 211190)
@@ -1083,9 +1083,6 @@
removeLayers(layer);
}
- if (m_style.hasFixedBackgroundImage() && !frame().settings().fixedBackgroundsPaintRelativeToDocument())
- view().frameView().removeSlowRepaintObject(this);
-
if (isOutOfFlowPositioned() && parent()->childrenInline())
parent()->dirtyLinesFromChangedChild(*this);
@@ -1112,6 +1109,9 @@
void RenderElement::willBeDestroyed()
{
+ if (m_style.hasFixedBackgroundImage() && !frame().settings().fixedBackgroundsPaintRelativeToDocument())
+ view().frameView().removeSlowRepaintObject(this);
+
animation().cancelAnimations(*this);
destroyLeftoverChildren();
@@ -1129,6 +1129,7 @@
}
}
#endif
+
clearLayoutRootIfNeeded();
}
