Modified: trunk/Source/WebCore/ChangeLog (188297 => 188298)
--- trunk/Source/WebCore/ChangeLog 2015-08-11 21:42:05 UTC (rev 188297)
+++ trunk/Source/WebCore/ChangeLog 2015-08-11 21:50:32 UTC (rev 188298)
@@ -1,3 +1,18 @@
+2015-08-11 Zalan Bujtas <[email protected]>
+
+ Invalid FrameView::m_viewportRenderer after layout is finished.
+ https://bugs.webkit.org/show_bug.cgi?id=147848
+ rdar://problem/22205197
+
+ Reviewed by Simon Fraser.
+
+ We cache the current viewport renderer (FrameView::m_viewportRenderer) right before layout.
+ It gets dereferenced later when layout is finished to update the overflow status.
+ If the viewport renderer gets destroyed during layout, we end up with a dangling pointer.
+ This patch replaces the pointer caching with type caching (none, body, document).
+
+ Unable to construct a test case.
+
2015-08-11 Brent Fulgham <[email protected]>
[Win] Switch Windows build to Visual Studio 2015
Modified: trunk/Source/WebCore/page/FrameView.cpp (188297 => 188298)
--- trunk/Source/WebCore/page/FrameView.cpp 2015-08-11 21:42:05 UTC (rev 188297)
+++ trunk/Source/WebCore/page/FrameView.cpp 2015-08-11 21:50:32 UTC (rev 188298)
@@ -172,7 +172,6 @@
, m_baseBackgroundColor(Color::white)
, m_mediaType("screen")
, m_overflowStatusDirty(true)
- , m_viewportRenderer(nullptr)
, m_wasScrolledByUser(false)
, m_inProgrammaticScroll(false)
, m_safeToPropagateScrollToParent(true)
@@ -621,18 +620,19 @@
IntSize FrameView::contentsSizeRespectingOverflow() const
{
RenderView* renderView = this->renderView();
- if (!renderView || !m_viewportRenderer || !is<RenderBox>(m_viewportRenderer) || !frame().isMainFrame())
+ auto* viewportRenderer = this->viewportRenderer();
+ if (!renderView || !is<RenderBox>(viewportRenderer) || !frame().isMainFrame())
return contentsSize();
ASSERT(frame().view() == this);
FloatRect contentRect = renderView->unscaledDocumentRect();
- RenderBox& viewportRendererBox = downcast<RenderBox>(*m_viewportRenderer);
+ RenderBox& viewportRendererBox = downcast<RenderBox>(*viewportRenderer);
- if (m_viewportRenderer->style().overflowX() == OHIDDEN)
+ if (viewportRendererBox.style().overflowX() == OHIDDEN)
contentRect.setWidth(std::min<float>(contentRect.width(), viewportRendererBox.frameRect().width()));
- if (m_viewportRenderer->style().overflowY() == OHIDDEN)
+ if (viewportRendererBox.style().overflowY() == OHIDDEN)
contentRect.setHeight(std::min<float>(contentRect.height(), viewportRendererBox.frameRect().height()));
if (renderView->hasTransform())
@@ -641,7 +641,7 @@
return IntSize(contentRect.size());
}
-void FrameView::applyOverflowToViewport(RenderElement* renderer, ScrollbarMode& hMode, ScrollbarMode& vMode)
+void FrameView::applyOverflowToViewport(const RenderElement& renderer, ScrollbarMode& hMode, ScrollbarMode& vMode)
{
// Handle the overflow:hidden/scroll case for the body/html elements. WinIE treats
// overflow:hidden and overflow:scroll on <body> as applying to the document's
@@ -654,13 +654,13 @@
bool overrideHidden = frame().isMainFrame() && ((frame().frameScaleFactor() > 1) || headerHeight() || footerHeight());
- EOverflow overflowX = renderer->style().overflowX();
- EOverflow overflowY = renderer->style().overflowY();
+ EOverflow overflowX = renderer.style().overflowX();
+ EOverflow overflowY = renderer.style().overflowY();
- if (is<RenderSVGRoot>(*renderer)) {
+ if (is<RenderSVGRoot>(renderer)) {
// FIXME: evaluate if we can allow overflow for these cases too.
// Overflow is always hidden when stand-alone SVG documents are embedded.
- if (downcast<RenderSVGRoot>(*renderer).isEmbeddedThroughFrameContainingSVGDocument()) {
+ if (downcast<RenderSVGRoot>(renderer).isEmbeddedThroughFrameContainingSVGDocument()) {
overflowX = OHIDDEN;
overflowY = OHIDDEN;
}
@@ -701,8 +701,6 @@
// Don't set it at all. Values of OPAGEDX and OPAGEDY are handled by applyPaginationToViewPort().
;
}
-
- m_viewportRenderer = renderer;
}
void FrameView::applyPaginationToViewport()
@@ -733,7 +731,7 @@
void FrameView::calculateScrollbarModesForLayout(ScrollbarMode& hMode, ScrollbarMode& vMode, ScrollbarModesCalculationStrategy strategy)
{
- m_viewportRenderer = nullptr;
+ m_viewportRendererType = ViewportRendererType::None;
const HTMLFrameOwnerElement* owner = frame().ownerElement();
if (owner && (owner->scrollingMode() == ScrollbarAlwaysOff)) {
@@ -750,24 +748,47 @@
vMode = ScrollbarAlwaysOff;
}
- if (!m_layoutRoot) {
- Document* document = frame().document();
- auto documentElement = document->documentElement();
- RenderElement* rootRenderer = documentElement ? documentElement->renderer() : nullptr;
- auto* body = document->bodyOrFrameset();
- if (body && body->renderer()) {
- if (is<HTMLFrameSetElement>(*body) && !frameFlatteningEnabled()) {
- vMode = ScrollbarAlwaysOff;
- hMode = ScrollbarAlwaysOff;
- } else if (is<HTMLBodyElement>(*body)) {
- // It's sufficient to just check the X overflow,
- // since it's illegal to have visible in only one direction.
- RenderElement* o = rootRenderer->style().overflowX() == OVISIBLE && is<HTMLHtmlElement>(*document->documentElement()) ? body->renderer() : rootRenderer;
- applyOverflowToViewport(o, hMode, vMode);
+ if (m_layoutRoot)
+ return;
+
+ auto* document = frame().document();
+ if (!document)
+ return;
+
+ auto documentElement = document->documentElement();
+ if (!documentElement)
+ return;
+
+ auto* bodyOrFrameset = document->bodyOrFrameset();
+ auto* rootRenderer = documentElement->renderer();
+ if (!bodyOrFrameset || !bodyOrFrameset->renderer()) {
+ if (rootRenderer) {
+ applyOverflowToViewport(*rootRenderer, hMode, vMode);
+ m_viewportRendererType = ViewportRendererType::Document;
+ }
+ return;
+ }
+
+ if (is<HTMLFrameSetElement>(*bodyOrFrameset) && !frameFlatteningEnabled()) {
+ vMode = ScrollbarAlwaysOff;
+ hMode = ScrollbarAlwaysOff;
+ return;
+ }
+
+ if (is<HTMLBodyElement>(*bodyOrFrameset) && rootRenderer) {
+ // It's sufficient to just check the X overflow,
+ // since it's illegal to have visible in only one direction.
+ if (rootRenderer->style().overflowX() == OVISIBLE && is<HTMLHtmlElement>(documentElement)) {
+ auto* bodyRenderer = bodyOrFrameset->renderer();
+ if (bodyRenderer) {
+ applyOverflowToViewport(*bodyRenderer, hMode, vMode);
+ m_viewportRendererType = ViewportRendererType::Body;
}
- } else if (rootRenderer)
- applyOverflowToViewport(rootRenderer, hMode, vMode);
- }
+ } else {
+ applyOverflowToViewport(*rootRenderer, hMode, vMode);
+ m_viewportRendererType = ViewportRendererType::Document;
+ }
+ }
}
void FrameView::willRecalcStyle()
@@ -3263,9 +3284,37 @@
setNeedsLayout();
}
+RenderElement* FrameView::viewportRenderer() const
+{
+ if (m_viewportRendererType == ViewportRendererType::None)
+ return nullptr;
+
+ auto* document = frame().document();
+ if (!document)
+ return nullptr;
+
+ if (m_viewportRendererType == ViewportRendererType::Document) {
+ auto* documentElement = document->documentElement();
+ if (!documentElement)
+ return nullptr;
+ return documentElement->renderer();
+ }
+
+ if (m_viewportRendererType == ViewportRendererType::Body) {
+ auto* body = document->body();
+ if (!body)
+ return nullptr;
+ return body->renderer();
+ }
+
+ ASSERT_NOT_REACHED();
+ return nullptr;
+}
+
void FrameView::updateOverflowStatus(bool horizontalOverflow, bool verticalOverflow)
{
- if (!m_viewportRenderer)
+ auto* viewportRenderer = this->viewportRenderer();
+ if (!viewportRenderer)
return;
if (m_overflowStatusDirty) {
@@ -3284,7 +3333,7 @@
RefPtr<OverflowEvent> overflowEvent = OverflowEvent::create(horizontalOverflowChanged, horizontalOverflow,
verticalOverflowChanged, verticalOverflow);
- overflowEvent->setTarget(m_viewportRenderer->element());
+ overflowEvent->setTarget(viewportRenderer->element());
frame().document()->enqueueOverflowEvent(overflowEvent.release());
}
Modified: trunk/Source/WebCore/page/FrameView.h (188297 => 188298)
--- trunk/Source/WebCore/page/FrameView.h 2015-08-11 21:42:05 UTC (rev 188297)
+++ trunk/Source/WebCore/page/FrameView.h 2015-08-11 21:50:32 UTC (rev 188298)
@@ -594,7 +594,7 @@
virtual void scrollPositionChangedViaPlatformWidgetImpl(const IntPoint& oldPosition, const IntPoint& newPosition) override;
- void applyOverflowToViewport(RenderElement*, ScrollbarMode& hMode, ScrollbarMode& vMode);
+ void applyOverflowToViewport(const RenderElement&, ScrollbarMode& hMode, ScrollbarMode& vMode);
void applyPaginationToViewport();
void updateOverflowStatus(bool horizontalOverflow, bool verticalOverflow);
@@ -675,6 +675,8 @@
void removeFromAXObjectCache();
void notifyWidgets(WidgetNotification);
+ RenderElement* viewportRenderer() const;
+
HashSet<Widget*> m_widgetsInRenderTree;
static double sCurrentPaintTimeStamp; // used for detecting decoded resource thrash in the cache
@@ -718,8 +720,9 @@
bool m_overflowStatusDirty;
bool m_horizontalOverflow;
- bool m_verticalOverflow;
- RenderElement* m_viewportRenderer;
+ bool m_verticalOverflow;
+ enum class ViewportRendererType { None, Document, Body };
+ ViewportRendererType m_viewportRendererType { ViewportRendererType::None };
Pagination m_pagination;
