[Chromium] table-section-overflow-clip-crash.html hits an assert
authorojan@chromium.org <ojan@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 7 Feb 2013 00:55:10 +0000 (00:55 +0000)
committerojan@chromium.org <ojan@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 7 Feb 2013 00:55:10 +0000 (00:55 +0000)
https://bugs.webkit.org/show_bug.cgi?id=108594

Reviewed by Levi Weintraub.

Source/WebCore:

When a counter calls setNeedsLayout, it also marks it's containing blocks
as needing layout, so we need to clear the setNeedsLayoutIsForbidden bit on the
containing blocks as well as the counter itself.

Also, use RAII objects for all the places where we clear this bit and make
the setter/getter for it private to RenderObject.

* rendering/RenderCounter.cpp:
(WebCore::RenderCounter::computePreferredLogicalWidths):
* rendering/RenderObject.cpp:
(WebCore::RenderObject::SetLayoutNeededForbiddenScope::SetLayoutNeededForbiddenScope):
(WebCore::RenderObject::markContainingBlocksForLayout):
* rendering/RenderObject.h:
(SetLayoutNeededForbiddenScope):
(RenderObject):
(WebCore::RenderObject::isSetNeedsLayoutForbidden):
(WebCore::RenderObject::setNeedsLayoutIsForbidden):
* rendering/RenderQuote.cpp:
(WebCore::RenderQuote::computePreferredLogicalWidths):
* rendering/RenderTableSection.cpp:
(WebCore::RenderTableSection::calcRowLogicalHeight):
(WebCore::RenderTableSection::layoutRows):
* rendering/mathml/RenderMathMLOperator.cpp:
(WebCore::RenderMathMLOperator::computePreferredLogicalWidths):
* rendering/mathml/RenderMathMLRoot.cpp:
(WebCore::RenderMathMLRoot::computePreferredLogicalWidths):
* rendering/mathml/RenderMathMLRow.cpp:
(WebCore::RenderMathMLRow::computePreferredLogicalWidths):

LayoutTests:

* platform/chromium/TestExpectations:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@142056 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/platform/chromium/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderCounter.cpp
Source/WebCore/rendering/RenderObject.cpp
Source/WebCore/rendering/RenderObject.h
Source/WebCore/rendering/RenderQuote.cpp
Source/WebCore/rendering/RenderTableSection.cpp
Source/WebCore/rendering/mathml/RenderMathMLOperator.cpp
Source/WebCore/rendering/mathml/RenderMathMLRoot.cpp
Source/WebCore/rendering/mathml/RenderMathMLRow.cpp

index f9575ec..e1716e3 100644 (file)
@@ -1,5 +1,14 @@
 2013-02-06  Ojan Vafai  <ojan@chromium.org>
 
+        [Chromium] table-section-overflow-clip-crash.html hits an assert
+        https://bugs.webkit.org/show_bug.cgi?id=108594
+
+        Reviewed by Levi Weintraub.
+
+        * platform/chromium/TestExpectations:
+
+2013-02-06  Ojan Vafai  <ojan@chromium.org>
+
         display:none file upload button crashes
         https://bugs.webkit.org/show_bug.cgi?id=109102
 
index 182c087..7275bc3 100644 (file)
@@ -4344,7 +4344,6 @@ webkit.org/b/108286 platform/chromium/virtual/softwarecompositing/geometry/fixed
 webkit.org/b/108286 platform/chromium/virtual/softwarecompositing/geometry/fixed-position-iframe-composited-page-scale-down.html [ ImageOnlyFailure ]
 
 webkit.org/b/108424 media/video-error-does-not-exist.html [ Failure Crash ]
-webkit.org/b/108594 [ Debug ] tables/table-section-overflow-clip-crash.html [ Crash ]
 
 webkit.org/b/108781 [ Win ] fast/css-grid-layout/grid-preferred-logical-widths.html [ Timeout ]
 webkit.org/b/108789 [ Win Mac ] http/tests/workers/terminate-during-sync-operation.html [ Pass Timeout ]
index c2b8d5f..71c8f29 100644 (file)
@@ -1,5 +1,41 @@
 2013-02-06  Ojan Vafai  <ojan@chromium.org>
 
+        [Chromium] table-section-overflow-clip-crash.html hits an assert
+        https://bugs.webkit.org/show_bug.cgi?id=108594
+
+        Reviewed by Levi Weintraub.
+
+        When a counter calls setNeedsLayout, it also marks it's containing blocks
+        as needing layout, so we need to clear the setNeedsLayoutIsForbidden bit on the
+        containing blocks as well as the counter itself.
+
+        Also, use RAII objects for all the places where we clear this bit and make
+        the setter/getter for it private to RenderObject.
+
+        * rendering/RenderCounter.cpp:
+        (WebCore::RenderCounter::computePreferredLogicalWidths):
+        * rendering/RenderObject.cpp:
+        (WebCore::RenderObject::SetLayoutNeededForbiddenScope::SetLayoutNeededForbiddenScope):
+        (WebCore::RenderObject::markContainingBlocksForLayout):
+        * rendering/RenderObject.h:
+        (SetLayoutNeededForbiddenScope):
+        (RenderObject):
+        (WebCore::RenderObject::isSetNeedsLayoutForbidden):
+        (WebCore::RenderObject::setNeedsLayoutIsForbidden):
+        * rendering/RenderQuote.cpp:
+        (WebCore::RenderQuote::computePreferredLogicalWidths):
+        * rendering/RenderTableSection.cpp:
+        (WebCore::RenderTableSection::calcRowLogicalHeight):
+        (WebCore::RenderTableSection::layoutRows):
+        * rendering/mathml/RenderMathMLOperator.cpp:
+        (WebCore::RenderMathMLOperator::computePreferredLogicalWidths):
+        * rendering/mathml/RenderMathMLRoot.cpp:
+        (WebCore::RenderMathMLRoot::computePreferredLogicalWidths):
+        * rendering/mathml/RenderMathMLRow.cpp:
+        (WebCore::RenderMathMLRow::computePreferredLogicalWidths):
+
+2013-02-06  Ojan Vafai  <ojan@chromium.org>
+
         display:none file upload button crashes
         https://bugs.webkit.org/show_bug.cgi?id=109102
 
index 6c0d4f6..e819d03 100644 (file)
@@ -525,16 +525,11 @@ void RenderCounter::computePreferredLogicalWidths(float lead)
     // the render tree then. When that's done, we also won't need to override
     // computePreferredLogicalWidths at all.
     // https://bugs.webkit.org/show_bug.cgi?id=104829
-    bool oldSetNeedsLayoutIsForbidden = isSetNeedsLayoutForbidden();
-    setNeedsLayoutIsForbidden(false);
+    SetLayoutNeededForbiddenScope layoutForbiddenScope(this, false);
 #endif
 
     setTextInternal(originalText());
 
-#ifndef NDEBUG
-    setNeedsLayoutIsForbidden(oldSetNeedsLayoutIsForbidden);
-#endif
-
     RenderText::computePreferredLogicalWidths(lead);
 }
 
index da6db8e..d31db73 100644 (file)
@@ -94,11 +94,11 @@ using namespace HTMLNames;
 #ifndef NDEBUG
 static void* baseOfRenderObjectBeingDeleted;
 
-RenderObject::SetLayoutNeededForbiddenScope::SetLayoutNeededForbiddenScope(RenderObject* renderObject)
+RenderObject::SetLayoutNeededForbiddenScope::SetLayoutNeededForbiddenScope(RenderObject* renderObject, bool isForbidden)
     : m_renderObject(renderObject)
     , m_preexistingForbidden(m_renderObject->isSetNeedsLayoutForbidden())
 {
-    m_renderObject->setNeedsLayoutIsForbidden(true);
+    m_renderObject->setNeedsLayoutIsForbidden(isForbidden);
 }
 
 RenderObject::SetLayoutNeededForbiddenScope::~SetLayoutNeededForbiddenScope()
@@ -654,6 +654,7 @@ static inline bool objectIsRelayoutBoundary(const RenderObject* object)
 void RenderObject::markContainingBlocksForLayout(bool scheduleRelayout, RenderObject* newRoot)
 {
     ASSERT(!scheduleRelayout || !newRoot);
+    ASSERT(!isSetNeedsLayoutForbidden());
 
     RenderObject* object = container();
     RenderObject* last = this;
@@ -661,6 +662,11 @@ void RenderObject::markContainingBlocksForLayout(bool scheduleRelayout, RenderOb
     bool simplifiedNormalFlowLayout = needsSimplifiedNormalFlowLayout() && !selfNeedsLayout() && !normalChildNeedsLayout();
 
     while (object) {
+#ifndef NDEBUG
+        // FIXME: Remove this once we remove the special cases for counters, quotes and mathml
+        // calling setNeedsLayout during preferred width computation.
+        SetLayoutNeededForbiddenScope layoutForbiddenScope(object, isSetNeedsLayoutForbidden());
+#endif
         // Don't mark the outermost object of an unrooted subtree. That object will be
         // marked when the subtree is added to the document.
         RenderObject* container = object->container();
index c60ffcb..27a7a45 100644 (file)
@@ -226,13 +226,11 @@ public:
 #ifndef NDEBUG
     void setHasAXObject(bool flag) { m_hasAXObject = flag; }
     bool hasAXObject() const { return m_hasAXObject; }
-    bool isSetNeedsLayoutForbidden() const { return m_setNeedsLayoutForbidden; }
-    void setNeedsLayoutIsForbidden(bool flag) { m_setNeedsLayoutForbidden = flag; }
 
     // Helper class forbidding calls to setNeedsLayout() during its lifetime.
     class SetLayoutNeededForbiddenScope {
     public:
-        explicit SetLayoutNeededForbiddenScope(RenderObject*);
+        explicit SetLayoutNeededForbiddenScope(RenderObject*, bool isForbidden = true);
         ~SetLayoutNeededForbiddenScope();
     private:
         RenderObject* m_renderObject;
@@ -272,6 +270,11 @@ protected:
     }
     //////////////////////////////////////////
 private:
+#ifndef NDEBUG
+    bool isSetNeedsLayoutForbidden() const { return m_setNeedsLayoutForbidden; }
+    void setNeedsLayoutIsForbidden(bool flag) { m_setNeedsLayoutForbidden = flag; }
+#endif
+
     void addAbsoluteRectForLayer(LayoutRect& result);
     void setLayerNeedsFullRepaint();
     void setLayerNeedsFullRepaintForPositionedMovementLayout();
index 63f5d66..e4c979c 100644 (file)
@@ -256,18 +256,13 @@ void RenderQuote::computePreferredLogicalWidths(float lead)
     // the render tree then. When that's done, we also won't need to override
     // computePreferredLogicalWidths at all.
     // https://bugs.webkit.org/show_bug.cgi?id=104829
-    bool oldSetNeedsLayoutIsForbidden = isSetNeedsLayoutForbidden();
-    setNeedsLayoutIsForbidden(false);
+    SetLayoutNeededForbiddenScope layoutForbiddenScope(this, false);
 #endif
 
     if (!m_attached)
         attachQuote();
     setTextInternal(originalText());
 
-#ifndef NDEBUG
-    setNeedsLayoutIsForbidden(oldSetNeedsLayoutIsForbidden);
-#endif
-
     RenderText::computePreferredLogicalWidths(lead);
 }
 
index 4c9c330..8b3d73b 100644 (file)
@@ -263,7 +263,7 @@ void RenderTableSection::addCell(RenderTableCell* cell, RenderTableRow* row)
 int RenderTableSection::calcRowLogicalHeight()
 {
 #ifndef NDEBUG
-    setNeedsLayoutIsForbidden(true);
+    SetLayoutNeededForbiddenScope layoutForbiddenScope(this);
 #endif
 
     ASSERT(!needsLayout());
@@ -339,10 +339,6 @@ int RenderTableSection::calcRowLogicalHeight()
         m_rowPos[r + 1] = max(m_rowPos[r + 1], m_rowPos[r]);
     }
 
-#ifndef NDEBUG
-    setNeedsLayoutIsForbidden(false);
-#endif
-
     ASSERT(!needsLayout());
 
     statePusher.pop();
@@ -492,7 +488,7 @@ int RenderTableSection::distributeExtraLogicalHeightToRows(int extraLogicalHeigh
 void RenderTableSection::layoutRows()
 {
 #ifndef NDEBUG
-    setNeedsLayoutIsForbidden(true);
+    SetLayoutNeededForbiddenScope layoutForbiddenScope(this);
 #endif
 
     ASSERT(!needsLayout());
@@ -638,10 +634,6 @@ void RenderTableSection::layoutRows()
         }
     }
 
-#ifndef NDEBUG
-    setNeedsLayoutIsForbidden(false);
-#endif
-
     ASSERT(!needsLayout());
 
     setLogicalHeight(m_rowPos[totalRows]);
index c5e6e36..246f4b1 100644 (file)
@@ -85,19 +85,14 @@ void RenderMathMLOperator::computePreferredLogicalWidths()
     ASSERT(preferredLogicalWidthsDirty());
 
 #ifndef NDEBUG
-    // FIXME: Remove the setNeedsLayoutIsForbidden calls once mathml stops modifying the render tree here.
-    bool oldSetNeedsLayoutIsForbidden = isSetNeedsLayoutForbidden();
-    setNeedsLayoutIsForbidden(false);
+    // FIXME: Remove this once mathml stops modifying the render tree here.
+    SetLayoutNeededForbiddenScope layoutForbiddenScope(this, false);
 #endif
     
     // Check for an uninitialized operator.
     if (!firstChild())
         updateFromElement();
 
-#ifndef NDEBUG
-    setNeedsLayoutIsForbidden(oldSetNeedsLayoutIsForbidden);
-#endif
-
     RenderMathMLBlock::computePreferredLogicalWidths();
 }
 
index 17fa23e..7f72c7b 100644 (file)
@@ -188,9 +188,8 @@ void RenderMathMLRoot::computePreferredLogicalWidths()
     ASSERT(preferredLogicalWidthsDirty() && needsLayout());
     
 #ifndef NDEBUG
-    // FIXME: Remove the setNeedsLayoutIsForbidden calls once mathml stops modifying the render tree here.
-    bool oldSetNeedsLayoutIsForbidden = isSetNeedsLayoutForbidden();
-    setNeedsLayoutIsForbidden(false);
+    // FIXME: Remove this once mathml stops modifying the render tree here.
+    SetLayoutNeededForbiddenScope layoutForbiddenScope(this, false);
 #endif
     
     computeChildrenPreferredLogicalHeights();
@@ -226,10 +225,6 @@ void RenderMathMLRoot::computePreferredLogicalWidths()
     } else
         m_intrinsicPaddingStart = frontWidth;
 
-#ifndef NDEBUG
-    setNeedsLayoutIsForbidden(oldSetNeedsLayoutIsForbidden);
-#endif
-
     RenderMathMLBlock::computePreferredLogicalWidths();
     
     // Shrink our logical width to its probable value now without triggering unnecessary relayout of our children.
index d747359..25fb75b 100644 (file)
@@ -56,9 +56,8 @@ void RenderMathMLRow::computePreferredLogicalWidths()
     ASSERT(preferredLogicalWidthsDirty() && needsLayout());
 
 #ifndef NDEBUG
-    // FIXME: Remove the setNeedsLayoutIsForbidden calls once mathml stops modifying the render tree here.
-    bool oldSetNeedsLayoutIsForbidden = isSetNeedsLayoutForbidden();
-    setNeedsLayoutIsForbidden(false);
+    // FIXME: Remove this once mathml stops modifying the render tree here.
+    SetLayoutNeededForbiddenScope layoutForbiddenScope(this, false);
 #endif
 
     computeChildrenPreferredLogicalHeights();
@@ -84,10 +83,6 @@ void RenderMathMLRow::computePreferredLogicalWidths()
         }
     }
 
-#ifndef NDEBUG
-    setNeedsLayoutIsForbidden(oldSetNeedsLayoutIsForbidden);
-#endif
-
     RenderMathMLBlock::computePreferredLogicalWidths();
     
     // Shrink our logical width to its probable value now without triggering unnecessary relayout of our children.