Modified: trunk/Source/WebCore/ChangeLog (159217 => 159218)
--- trunk/Source/WebCore/ChangeLog 2013-11-13 20:03:43 UTC (rev 159217)
+++ trunk/Source/WebCore/ChangeLog 2013-11-13 20:03:53 UTC (rev 159218)
@@ -1,3 +1,36 @@
+2013-11-13 Simon Fraser <[email protected]>
+
+ ASSERTION FAILED: m_repaintRect == renderer().clippedOverflowRectForRepaint(renderer().containerForRepaint()) after r135816
+ https://bugs.webkit.org/show_bug.cgi?id=103432
+
+ Reviewed by Dave Hyatt.
+
+ RenderLayer caches repaint rects in m_repaintRect, and on updating layer
+ positions after scrolling, asserts that the cached rect is correct. However,
+ this assertion would sometimes fail if we were scrolling as a result of
+ doing adjustViewSize() in the middle of layout, because we haven't updated
+ layer positions post-layout yet.
+
+ Fix by having the poorly named FrameView::repaintFixedElementsAfterScrolling()
+ skip the layer updating if this FrameView is inside of adjusetViewSize() in
+ layout.
+
+ In order to know if we're inside view size adjusting, add a LayoutPhase
+ member to FrameView, replacing two existing bools that track laying out state.
+
+ Investigative work showed that there are many, many ways to re-enter FrameView::layout(),
+ which makes it hard (but desirable) to more assertions about state changes, but
+ indicated that saving and restoring the state (via TemporaryChange<LayoutPhase>)
+ was a good idea.
+
+ * page/FrameView.cpp:
+ (WebCore::FrameView::FrameView):
+ (WebCore::FrameView::reset):
+ (WebCore::FrameView::updateCompositingLayersAfterStyleChange):
+ (WebCore::FrameView::layout):
+ (WebCore::FrameView::repaintFixedElementsAfterScrolling):
+ * page/FrameView.h:
+
2013-11-13 Myles C. Maxfield <[email protected]>
Delete unused TextPainter function
Modified: trunk/Source/WebCore/page/FrameView.cpp (159217 => 159218)
--- trunk/Source/WebCore/page/FrameView.cpp 2013-11-13 20:03:43 UTC (rev 159217)
+++ trunk/Source/WebCore/page/FrameView.cpp 2013-11-13 20:03:53 UTC (rev 159218)
@@ -173,6 +173,7 @@
, m_canHaveScrollbars(true)
, m_layoutTimer(this, &FrameView::layoutTimerFired)
, m_layoutRoot(0)
+ , m_layoutPhase(OutsideLayout)
, m_inSynchronousPostLayout(false)
, m_postLayoutTasksTimer(this, &FrameView::postLayoutTimerFired)
, m_isTransparent(false)
@@ -259,8 +260,7 @@
m_delayedLayout = false;
m_needsFullRepaint = true;
m_layoutSchedulingEnabled = true;
- m_inLayout = false;
- m_doingPreLayoutStyleUpdate = false;
+ m_layoutPhase = OutsideLayout;
m_inSynchronousPostLayout = false;
m_layoutCount = 0;
m_nestedLayoutCount = 0;
@@ -731,7 +731,7 @@
return;
// If we expect to update compositing after an incipient layout, don't do so here.
- if (m_doingPreLayoutStyleUpdate || layoutPending() || renderView->needsLayout())
+ if (inPreLayoutStyleUpdate() || layoutPending() || renderView->needsLayout())
return;
RenderLayerCompositor& compositor = renderView->compositor();
@@ -1065,9 +1065,13 @@
void FrameView::layout(bool allowSubtree)
{
- if (m_inLayout)
+ if (isInLayout())
return;
+ // Many of the tasks performed during layout can cause this function to be re-entered,
+ // so save the layout phase now and restore it on exit.
+ TemporaryChange<LayoutPhase> layoutPhaseRestorer(m_layoutPhase, InPreLayout);
+
// Protect the view from being deleted during layout (in recalcStyle)
Ref<FrameView> protect(*this);
@@ -1112,11 +1116,12 @@
if (!m_nestedLayoutCount && !m_inSynchronousPostLayout && m_postLayoutTasksTimer.isActive() && !inChildFrameLayoutWithFrameFlattening) {
// This is a new top-level layout. If there are any remaining tasks from the previous
// layout, finish them now.
- m_inSynchronousPostLayout = true;
+ TemporaryChange<bool> inSynchronousPostLayoutChange(m_inSynchronousPostLayout, true);
performPostLayoutTasks();
- m_inSynchronousPostLayout = false;
}
+ m_layoutPhase = InPreLayoutStyleUpdate;
+
// Viewport-dependent media queries may cause us to need completely different style information.
StyleResolver* styleResolver = document.styleResolverIfExists();
if (!styleResolver || styleResolver->affectedByViewportChange()) {
@@ -1132,8 +1137,8 @@
// Always ensure our style info is up-to-date. This can happen in situations where
// the layout beats any sort of style recalc update that needs to occur.
- TemporaryChange<bool> changeDoingPreLayoutStyleUpdate(m_doingPreLayoutStyleUpdate, true);
document.updateStyleIfNeeded();
+ m_layoutPhase = InPreLayout;
subtree = m_layoutRoot;
@@ -1197,6 +1202,7 @@
m_lastViewportSize = fixedLayoutSize();
else
m_lastViewportSize = visibleContentRect(IncludeScrollbars).size();
+
m_lastZoomFactor = root->style().zoom();
// Set the initial vMode to AlwaysOn if we're auto.
@@ -1227,6 +1233,8 @@
rootRenderer->setChildNeedsLayout();
}
}
+
+ m_layoutPhase = InPreLayout;
}
layer = root->enclosingLayer();
@@ -1240,9 +1248,14 @@
}
LayoutStateDisabler layoutStateDisabler(disableLayoutState ? &root->view() : 0);
- m_inLayout = true;
+ ASSERT(m_layoutPhase == InPreLayout);
+ m_layoutPhase = InLayout;
+
beginDeferredRepaints();
forceLayoutParentViewIfNeeded();
+
+ ASSERT(m_layoutPhase == InLayout);
+
root->layout();
#if ENABLE(IOS_TEXT_AUTOSIZING)
float minZoomFontSize = frame().settings().minimumZoomFontSize();
@@ -1259,8 +1272,9 @@
root->layout();
#endif
endDeferredRepaints();
- m_inLayout = false;
+ ASSERT(m_layoutPhase == InLayout);
+
if (subtree)
root->view().popLayoutState(root);
@@ -1269,11 +1283,15 @@
// Close block here to end the scope of changeSchedulingEnabled and layoutStateDisabler.
}
+ m_layoutPhase = InViewSizeAdjust;
+
bool neededFullRepaint = m_needsFullRepaint;
if (!subtree && !toRenderView(*root).printing())
adjustViewSize();
+ m_layoutPhase = InPostLayout;
+
m_needsFullRepaint = neededFullRepaint;
// Now update the positions of all layers.
@@ -1314,9 +1332,8 @@
if (inChildFrameLayoutWithFrameFlattening)
updateWidgetPositions();
else {
- m_inSynchronousPostLayout = true;
+ TemporaryChange<bool> inSynchronousPostLayoutChange(m_inSynchronousPostLayout, true);
performPostLayoutTasks(); // Calls resumeScheduledEvents().
- m_inSynchronousPostLayout = false;
}
}
@@ -1947,6 +1964,10 @@
// FIXME: this function is misnamed; its primary purpose is to update RenderLayer positions.
void FrameView::repaintFixedElementsAfterScrolling()
{
+ // If we're scrolling as a result of updating the view size after layout, we'll update widgets and layer positions soon anyway.
+ if (m_layoutPhase == InViewSizeAdjust)
+ return;
+
// For fixed position elements, update widget positions and compositing layers after scrolling,
// but only if we're not inside of layout.
if (m_nestedLayoutCount <= 1 && hasViewportConstrainedObjects()) {
Modified: trunk/Source/WebCore/page/FrameView.h (159217 => 159218)
--- trunk/Source/WebCore/page/FrameView.h 2013-11-13 20:03:43 UTC (rev 159217)
+++ trunk/Source/WebCore/page/FrameView.h 2013-11-13 20:03:53 UTC (rev 159218)
@@ -109,7 +109,7 @@
void scheduleRelayoutOfSubtree(RenderElement&);
void unscheduleRelayout();
bool layoutPending() const;
- bool isInLayout() const { return m_inLayout; }
+ bool isInLayout() const { return m_layoutPhase == InLayout; }
RenderObject* layoutRoot(bool _onlyDuringLayout_ = false) const;
void clearLayoutRoot() { m_layoutRoot = nullptr; }
@@ -446,6 +446,18 @@
void reset();
void init();
+ enum LayoutPhase {
+ OutsideLayout,
+ InPreLayout,
+ InPreLayoutStyleUpdate,
+ InLayout,
+ InViewSizeAdjust,
+ InPostLayout,
+ };
+ LayoutPhase layoutPhase() const { return m_layoutPhase; }
+
+ bool inPreLayoutStyleUpdate() const { return m_layoutPhase == InPreLayoutStyleUpdate; }
+
virtual bool isFrameView() const OVERRIDE { return true; }
friend class RenderWidget;
@@ -563,10 +575,9 @@
Timer<FrameView> m_layoutTimer;
bool m_delayedLayout;
RenderElement* m_layoutRoot;
-
+
+ LayoutPhase m_layoutPhase;
bool m_layoutSchedulingEnabled;
- bool m_inLayout;
- bool m_doingPreLayoutStyleUpdate;
bool m_inSynchronousPostLayout;
int m_layoutCount;
unsigned m_nestedLayoutCount;
