- Revision
- 183788
- Author
- [email protected]
- Date
- 2015-05-04 20:22:35 -0700 (Mon, 04 May 2015)
Log Message
RenderWidget::setWidgetGeometry() can end up destroying *this*.
https://bugs.webkit.org/show_bug.cgi?id=144601
Reviewed by Andreas Kling.
This is a speculative fix to ensure we don't crash on an invalid *this* renderer
while flattening the current iframe.
Calling RenderWidget::setWidgetGeometry() can result in destroying the current renderer.
While it is not a issue in case of normal layout flow as widget positions are updated at post layout,
frame flattening initiates this action in the middle of layout.
This patch re-introduces refcount model for RenderWidgets so that the renderer is protected during layout
when frame flattening is in use.
* rendering/RenderFrameBase.cpp:
(WebCore::RenderFrameBase::layoutWithFlattening): Let's be paranoid about child view.
* rendering/RenderObject.cpp:
(WebCore::RenderObject::destroy):
* rendering/FrameView.cpp:
(WebCore::FrameView::layout):
* rendering/RenderView.h:
* rendering/RenderWidget.cpp:
(WebCore::RenderWidget::~RenderWidget):
* rendering/RenderWidget.h:
(WebCore::RenderWidget::ref):
(WebCore::RenderWidget::deref):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (183787 => 183788)
--- trunk/Source/WebCore/ChangeLog 2015-05-05 02:40:28 UTC (rev 183787)
+++ trunk/Source/WebCore/ChangeLog 2015-05-05 03:22:35 UTC (rev 183788)
@@ -1,3 +1,31 @@
+2015-05-04 Zalan Bujtas <[email protected]>
+
+ RenderWidget::setWidgetGeometry() can end up destroying *this*.
+ https://bugs.webkit.org/show_bug.cgi?id=144601
+
+ Reviewed by Andreas Kling.
+
+ This is a speculative fix to ensure we don't crash on an invalid *this* renderer
+ while flattening the current iframe.
+ Calling RenderWidget::setWidgetGeometry() can result in destroying the current renderer.
+ While it is not a issue in case of normal layout flow as widget positions are updated at post layout,
+ frame flattening initiates this action in the middle of layout.
+ This patch re-introduces refcount model for RenderWidgets so that the renderer is protected during layout
+ when frame flattening is in use.
+
+ * rendering/RenderFrameBase.cpp:
+ (WebCore::RenderFrameBase::layoutWithFlattening): Let's be paranoid about child view.
+ * rendering/RenderObject.cpp:
+ (WebCore::RenderObject::destroy):
+ * rendering/FrameView.cpp:
+ (WebCore::FrameView::layout):
+ * rendering/RenderView.h:
+ * rendering/RenderWidget.cpp:
+ (WebCore::RenderWidget::~RenderWidget):
+ * rendering/RenderWidget.h:
+ (WebCore::RenderWidget::ref):
+ (WebCore::RenderWidget::deref):
+
2015-05-04 Doug Russell <[email protected]>
AX: setting focus via accessibility object needs to set isSynchronizing in resulting selection intent
Modified: trunk/Source/WebCore/page/FrameView.cpp (183787 => 183788)
--- trunk/Source/WebCore/page/FrameView.cpp 2015-05-05 02:40:28 UTC (rev 183787)
+++ trunk/Source/WebCore/page/FrameView.cpp 2015-05-05 03:22:35 UTC (rev 183788)
@@ -1361,6 +1361,8 @@
if (m_needsFullRepaint)
root->view().repaintRootContents();
+ root->view().releaseProtectedRenderWidgets();
+
ASSERT(!root->needsLayout());
layer->updateLayerPositionsAfterLayout(renderView()->layer(), updateLayerPositionFlags(layer, subtree, m_needsFullRepaint));
Modified: trunk/Source/WebCore/rendering/RenderFrameBase.cpp (183787 => 183788)
--- trunk/Source/WebCore/rendering/RenderFrameBase.cpp 2015-05-05 02:40:28 UTC (rev 183787)
+++ trunk/Source/WebCore/rendering/RenderFrameBase.cpp 2015-05-05 03:22:35 UTC (rev 183788)
@@ -54,13 +54,13 @@
void RenderFrameBase::layoutWithFlattening(bool hasFixedWidth, bool hasFixedHeight)
{
- FrameView* childFrameView = childView();
- RenderView* childRoot = childFrameView ? childFrameView->frame().contentRenderer() : 0;
+ view().protectRenderWidgetUntilLayoutIsDone(*this);
+ RenderView* childRoot = childView() ? childView()->frame().contentRenderer() : 0;
if (!childRoot || !shouldExpandFrame(width(), height(), hasFixedWidth, hasFixedHeight)) {
updateWidgetPosition();
- if (childFrameView)
- childFrameView->layout();
+ if (childView())
+ childView()->layout();
clearNeedsLayout();
return;
}
@@ -83,18 +83,20 @@
setWidth(std::max(width(), childRoot->minPreferredLogicalWidth() + hBorder));
// update again to pass the new width to the child frame
updateWidgetPosition();
- childFrameView->layout();
+ if (childView())
+ childView()->layout();
}
- // expand the frame by setting frame height = content height
- if (isScrollable || !hasFixedHeight || childRoot->isFrameSet())
- setHeight(std::max<LayoutUnit>(height(), childFrameView->contentsHeight() + vBorder));
- if (isScrollable || !hasFixedWidth || childRoot->isFrameSet())
- setWidth(std::max<LayoutUnit>(width(), childFrameView->contentsWidth() + hBorder));
-
+ if (childView()) {
+ // expand the frame by setting frame height = content height
+ if (isScrollable || !hasFixedHeight || childRoot->isFrameSet())
+ setHeight(std::max<LayoutUnit>(height(), childView()->contentsHeight() + vBorder));
+ if (isScrollable || !hasFixedWidth || childRoot->isFrameSet())
+ setWidth(std::max<LayoutUnit>(width(), childView()->contentsWidth() + hBorder));
+ }
updateWidgetPosition();
- ASSERT(!childFrameView->layoutPending());
+ ASSERT(!childView()->layoutPending());
ASSERT(!childRoot->needsLayout());
ASSERT(!childRoot->firstChild() || !childRoot->firstChild()->firstChildSlow() || !childRoot->firstChild()->firstChildSlow()->needsLayout());
Modified: trunk/Source/WebCore/rendering/RenderObject.cpp (183787 => 183788)
--- trunk/Source/WebCore/rendering/RenderObject.cpp 2015-05-05 02:40:28 UTC (rev 183787)
+++ trunk/Source/WebCore/rendering/RenderObject.cpp 2015-05-05 03:22:35 UTC (rev 183788)
@@ -60,6 +60,7 @@
#include "RenderScrollbarPart.h"
#include "RenderTheme.h"
#include "RenderView.h"
+#include "RenderWidget.h"
#include "SVGRenderSupport.h"
#include "Settings.h"
#include "StyleResolver.h"
@@ -2024,6 +2025,10 @@
#endif
willBeDestroyed();
+ if (is<RenderWidget>(*this)) {
+ downcast<RenderWidget>(*this).deref();
+ return;
+ }
delete this;
}
Modified: trunk/Source/WebCore/rendering/RenderView.h (183787 => 183788)
--- trunk/Source/WebCore/rendering/RenderView.h 2015-05-05 02:40:28 UTC (rev 183787)
+++ trunk/Source/WebCore/rendering/RenderView.h 2015-05-05 03:22:35 UTC (rev 183788)
@@ -26,6 +26,7 @@
#include "LayoutState.h"
#include "Region.h"
#include "RenderBlockFlow.h"
+#include "RenderWidget.h"
#include "SelectionSubtreeRoot.h"
#include <memory>
#include <wtf/HashSet.h>
@@ -244,6 +245,9 @@
void scheduleLazyRepaint(RenderBox&);
void unscheduleLazyRepaint(RenderBox&);
+ void protectRenderWidgetUntilLayoutIsDone(RenderWidget& widget) { m_protectedRenderWidgets.append(&widget); }
+ void releaseProtectedRenderWidgets() { m_protectedRenderWidgets.clear(); }
+
protected:
virtual void mapLocalToContainer(const RenderLayerModelObject* repaintContainer, TransformState&, MapCoordinatesFlags, bool* wasFixed) const override;
virtual const RenderObject* pushMappingToContainer(const RenderLayerModelObject* ancestorToStopAt, RenderGeometryMap&) const override;
@@ -362,6 +366,7 @@
bool m_hasFlippedBlockDescendants;
HashSet<RenderElement*> m_renderersWithPausedImageAnimation;
+ Vector<RefPtr<RenderWidget>> m_protectedRenderWidgets;
#if ENABLE(SERVICE_CONTROLS)
SelectionRectGatherer m_selectionRectGatherer;
Modified: trunk/Source/WebCore/rendering/RenderWidget.cpp (183787 => 183788)
--- trunk/Source/WebCore/rendering/RenderWidget.cpp 2015-05-05 02:40:28 UTC (rev 183787)
+++ trunk/Source/WebCore/rendering/RenderWidget.cpp 2015-05-05 03:22:35 UTC (rev 183788)
@@ -106,6 +106,7 @@
RenderWidget::~RenderWidget()
{
+ ASSERT(!m_refCount);
}
// Widgets are always placed on integer boundaries, so rounding the size is actually
Modified: trunk/Source/WebCore/rendering/RenderWidget.h (183787 => 183788)
--- trunk/Source/WebCore/rendering/RenderWidget.h 2015-05-05 02:40:28 UTC (rev 183787)
+++ trunk/Source/WebCore/rendering/RenderWidget.h 2015-05-05 03:22:35 UTC (rev 183788)
@@ -74,6 +74,9 @@
WeakPtr<RenderWidget> createWeakPtr() { return m_weakPtrFactory.createWeakPtr(); }
+ void ref() { ++m_refCount; }
+ void deref();
+
protected:
RenderWidget(HTMLFrameOwnerElement&, Ref<RenderStyle>&&);
@@ -102,8 +105,16 @@
WeakPtrFactory<RenderWidget> m_weakPtrFactory;
RefPtr<Widget> m_widget;
IntRect m_clipRect; // The rectangle needs to remain correct after scrolling, so it is stored in content view coordinates, and not clipped to window.
+ unsigned m_refCount { 1 };
};
+inline void RenderWidget::deref()
+{
+ ASSERT(m_refCount);
+ if (!--m_refCount)
+ delete this;
+}
+
} // namespace WebCore
SPECIALIZE_TYPE_TRAITS_RENDER_OBJECT(RenderWidget, isWidget())
