Title: [210777] trunk/Source/WebCore
- Revision
- 210777
- Author
- akl...@apple.com
- Date
- 2017-01-15 02:48:04 -0800 (Sun, 15 Jan 2017)
Log Message
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.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (210776 => 210777)
--- trunk/Source/WebCore/ChangeLog 2017-01-15 08:23:31 UTC (rev 210776)
+++ trunk/Source/WebCore/ChangeLog 2017-01-15 10:48:04 UTC (rev 210777)
@@ -1,3 +1,37 @@
+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-15 Myles C. Maxfield <mmaxfi...@apple.com>
[Cocoa] Unify font fallback between macOS and iOS for when the font-family list is exhausted
Modified: trunk/Source/WebCore/dom/Document.cpp (210776 => 210777)
--- trunk/Source/WebCore/dom/Document.cpp 2017-01-15 08:23:31 UTC (rev 210776)
+++ trunk/Source/WebCore/dom/Document.cpp 2017-01-15 10:48:04 UTC (rev 210777)
@@ -2199,8 +2199,11 @@
{
ASSERT(hasLivingRenderTree());
ASSERT(frame());
+ ASSERT(frame()->view());
ASSERT(page());
+ FrameView& frameView = *frame()->view();
+
// Prevent Widget tree changes from committing until the RenderView is dead and gone.
WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
@@ -2211,8 +2214,7 @@
documentWillBecomeInactive();
- if (FrameView* frameView = view())
- frameView->detachCustomScrollbars();
+ frameView.willDestroyRenderTree();
#if ENABLE(FULLSCREEN_API)
if (m_fullScreenRenderer)
@@ -2238,6 +2240,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: trunk/Source/WebCore/page/FrameView.cpp (210776 => 210777)
--- trunk/Source/WebCore/page/FrameView.cpp 2017-01-15 08:23:31 UTC (rev 210776)
+++ trunk/Source/WebCore/page/FrameView.cpp 2017-01-15 10:48:04 UTC (rev 210777)
@@ -644,6 +644,27 @@
updateScrollableAreaSet();
}
+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: trunk/Source/WebCore/page/FrameView.h (210776 => 210777)
--- trunk/Source/WebCore/page/FrameView.h 2017-01-15 08:23:31 UTC (rev 210776)
+++ trunk/Source/WebCore/page/FrameView.h 2017-01-15 10:48:04 UTC (rev 210777)
@@ -589,6 +589,9 @@
void didRestoreFromPageCache();
+ void willDestroyRenderTree();
+ void didDestroyRenderTree();
+
protected:
bool scrollContentsFastPath(const IntSize& scrollDelta, const IntRect& rectToScroll, const IntRect& clipRect) final;
void scrollContentsSlowPath(const IntRect& updateRect) final;
Modified: trunk/Source/WebCore/page/animation/AnimationController.cpp (210776 => 210777)
--- trunk/Source/WebCore/page/animation/AnimationController.cpp 2017-01-15 08:23:31 UTC (rev 210776)
+++ trunk/Source/WebCore/page/animation/AnimationController.cpp 2017-01-15 10:48:04 UTC (rev 210777)
@@ -761,4 +761,9 @@
}
#endif
+bool AnimationController::hasAnimations() const
+{
+ return m_data->hasAnimations();
+}
+
} // namespace WebCore
Modified: trunk/Source/WebCore/page/animation/AnimationController.h (210776 => 210777)
--- trunk/Source/WebCore/page/animation/AnimationController.h 2017-01-15 08:23:31 UTC (rev 210776)
+++ trunk/Source/WebCore/page/animation/AnimationController.h 2017-01-15 10:48:04 UTC (rev 210777)
@@ -91,6 +91,8 @@
void scrollWasUpdated();
#endif
+ bool hasAnimations() const;
+
private:
const std::unique_ptr<AnimationControllerPrivate> m_data;
};
Modified: trunk/Source/WebCore/rendering/RenderElement.cpp (210776 => 210777)
--- trunk/Source/WebCore/rendering/RenderElement.cpp 2017-01-15 08:23:31 UTC (rev 210776)
+++ trunk/Source/WebCore/rendering/RenderElement.cpp 2017-01-15 10:48:04 UTC (rev 210777)
@@ -1087,9 +1087,6 @@
removeLayers(layer);
}
- if (m_style.hasFixedBackgroundImage() && !settings().fixedBackgroundsPaintRelativeToDocument())
- view().frameView().removeSlowRepaintObject(this);
-
if (isOutOfFlowPositioned() && parent()->childrenInline())
parent()->dirtyLinesFromChangedChild(*this);
@@ -1116,6 +1113,9 @@
void RenderElement::willBeDestroyed()
{
+ if (m_style.hasFixedBackgroundImage() && !settings().fixedBackgroundsPaintRelativeToDocument())
+ view().frameView().removeSlowRepaintObject(this);
+
animation().cancelAnimations(*this);
destroyLeftoverChildren();
@@ -1133,6 +1133,7 @@
}
}
#endif
+
clearLayoutRootIfNeeded();
}
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
