Changeset 40238 in webkit

Timestamp:

Jan 25, 2009 7:38:55 PM (15 years ago)

Author:

hyatt@apple.com

Message:

WebCore:

2009-01-25 David Hyatt <​hyatt@apple.com>

Fix for ​https://bugs.webkit.org/show_bug.cgi?id=23524, lots of missing content in table sections.

The new table code created a bug involving markAllDescendantsWithFloatsForLayout, namely that it could
end up marking ancestors of a block as needing layout when that block was still in the process of
doing a layout.

The fix is to add a parameter to markAllDescendantsWithFloatsForLayout that says whether or not
we are "mid-layout." If this flag is set, then the method will make sure to do only local dirtying
of objects to avoid accidentally marking a clean ancestor as needing layout again.

Ultimately the second parameter to setNeedsLayout and setChildNeedsLayout should just be removed,
with a check of whether or not we are mid-layout being done by those methods instead.

Reviewed by Oliver Hunt

Added fast/repaint/dynamic-table-vertical-alignment-change.html

• rendering/RenderBlock.cpp: (WebCore::RenderBlock::collapseMargins): (WebCore::RenderBlock::clearFloatsIfNeeded): (WebCore::RenderBlock::layoutBlockChildren): (WebCore::RenderBlock::markAllDescendantsWithFloatsForLayout):
• rendering/RenderBlock.h:
• rendering/RenderObject.cpp: (WebCore::RenderObject::removeFromObjectLists):
• rendering/RenderObject.h:
• rendering/RenderTableSection.cpp: (WebCore::RenderTableSection::layoutRows):

LayoutTests:

2009-01-25 David Hyatt <​hyatt@apple.com>

Add layout test for ​https://bugs.webkit.org/show_bug.cgi?id=23524.

Reviewed by Oliver Hunt

• fast/repaint/dynamic-table-vertical-alignment-change.html: Added.
• platform/mac/fast/repaint/dynamic-table-vertical-alignment-change-expected.checksum: Added.
• platform/mac/fast/repaint/dynamic-table-vertical-alignment-change-expected.png: Added.
• platform/mac/fast/repaint/dynamic-table-vertical-alignment-change-expected.txt: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/repaint/dynamic-table-vertical-alignment-change.html (added)
• LayoutTests/platform/mac/fast/repaint/dynamic-table-vertical-alignment-change-expected.checksum (added)
• LayoutTests/platform/mac/fast/repaint/dynamic-table-vertical-alignment-change-expected.png (added)
• LayoutTests/platform/mac/fast/repaint/dynamic-table-vertical-alignment-change-expected.txt (added)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/rendering/RenderBlock.cpp (modified) (5 diffs)
• WebCore/rendering/RenderBlock.h (modified) (1 diff)
• WebCore/rendering/RenderObject.cpp (modified) (2 diffs)
• WebCore/rendering/RenderObject.h (modified) (1 diff)
• WebCore/rendering/RenderSVGRoot.cpp (modified) (1 diff)
• WebCore/rendering/RenderTable.cpp (modified) (1 diff)
• WebCore/rendering/RenderTableSection.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2009-01-25  David Hyatt  <hyatt@apple.com>
+
+        Add layout test for https://bugs.webkit.org/show_bug.cgi?id=23524.
+
+        Reviewed by Oliver Hunt
+
+        * fast/repaint/dynamic-table-vertical-alignment-change.html: Added.
+        * platform/mac/fast/repaint/dynamic-table-vertical-alignment-change-expected.checksum: Added.
+        * platform/mac/fast/repaint/dynamic-table-vertical-alignment-change-expected.png: Added.
+        * platform/mac/fast/repaint/dynamic-table-vertical-alignment-change-expected.txt: Added.
+
 2009-01-25  Dan Bernstein  <mitz@apple.com>
 

trunk/WebCore/ChangeLog

@@ +1 @@
+2009-01-25  David Hyatt  <hyatt@apple.com>
+
+        Fix for https://bugs.webkit.org/show_bug.cgi?id=23524, lots of missing content in table sections.
+
+        The new table code created a bug involving  markAllDescendantsWithFloatsForLayout, namely that it could
+        end up marking ancestors of a block as needing layout when that block was still in the process of
+        doing a layout.
+
+        The fix is to add a parameter to markAllDescendantsWithFloatsForLayout that says whether or not
+        we are "mid-layout."  If this flag is set, then the method will make sure to do only local dirtying
+        of objects to avoid accidentally marking a clean ancestor as needing layout again.
+
+        Ultimately the second parameter to setNeedsLayout and setChildNeedsLayout should just be removed,
+        with a check of whether or not we are mid-layout being done by those methods instead.
+
+        Reviewed by Oliver Hunt
+
+        Added fast/repaint/dynamic-table-vertical-alignment-change.html
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::collapseMargins):
+        (WebCore::RenderBlock::clearFloatsIfNeeded):
+        (WebCore::RenderBlock::layoutBlockChildren):
+        (WebCore::RenderBlock::markAllDescendantsWithFloatsForLayout):
+        * rendering/RenderBlock.h:
+        * rendering/RenderObject.cpp:
+        (WebCore::RenderObject::removeFromObjectLists):
+        * rendering/RenderObject.h:
+        * rendering/RenderTableSection.cpp:
+        (WebCore::RenderTableSection::layoutRows):
+
 2009-01-25  Darin Adler  <darin@apple.com>
 

trunk/WebCore/rendering/RenderBlock.cpp

@@ -1067 +1067 @@
 
         if (!child->avoidsFloats() && child->containsFloats())
-            child->markAllDescendantsWithFloatsForLayout();
+            static_cast<RenderBlock*>(child)->markAllDescendantsWithFloatsForLayout();
 
         // Our guess was wrong. Make the child lay itself out again.
@@ -1122 +1122 @@
         child->setChildNeedsLayout(true, false);
     if (!child->avoidsFloats() && child->containsFloats())
-        child->markAllDescendantsWithFloatsForLayout();
+        static_cast<RenderBlock*>(child)->markAllDescendantsWithFloatsForLayout();
     child->layoutIfNeeded();
 }
@@ -1343 +1343 @@
 
         if (markDescendantsWithFloats)
-            child->markAllDescendantsWithFloatsForLayout();
+            static_cast<RenderBlock*>(child)->markAllDescendantsWithFloatsForLayout();
 
         if (child->isRenderBlock())
@@ -3053 +3053 @@
 }
 
-void RenderBlock::markAllDescendantsWithFloatsForLayout(RenderBox* floatToRemove)
-{
-    setChildNeedsLayout(true);
+void RenderBlock::markAllDescendantsWithFloatsForLayout(RenderBox* floatToRemove, bool inLayout)
+{
+    setChildNeedsLayout(true, !inLayout);
 
     if (floatToRemove)
@@ -3065 +3065 @@
             if (isBlockFlow() && !child->isFloatingOrPositioned() &&
                 ((floatToRemove ? child->containsFloat(floatToRemove) : child->containsFloats()) || child->shrinkToAvoidFloats()))
-                child->markAllDescendantsWithFloatsForLayout(floatToRemove);
+                static_cast<RenderBlock*>(child)->markAllDescendantsWithFloatsForLayout(floatToRemove, inLayout);
         }
     }

trunk/WebCore/rendering/RenderBlock.h

@@ -178 +178 @@
     void clearFloats();
     int getClearDelta(RenderBox* child);
-    virtual void markAllDescendantsWithFloatsForLayout(RenderBox* floatToRemove = 0);
+    void markAllDescendantsWithFloatsForLayout(RenderBox* floatToRemove = 0, bool inLayout = true);
     void markPositionedObjectsForLayout();
 

trunk/WebCore/rendering/RenderObject.cpp

@@ -507 +507 @@
 {
     return (style()->top().isAuto() && style()->bottom().isAuto()) || style()->top().isStatic();
-}
-
-void RenderObject::markAllDescendantsWithFloatsForLayout(RenderBox*)
-{
 }
 
@@ -2272 +2268 @@
 
         if (outermostBlock)
-            outermostBlock->markAllDescendantsWithFloatsForLayout(toRenderBox(this));
+            outermostBlock->markAllDescendantsWithFloatsForLayout(toRenderBox(this), false);
     }
 

trunk/WebCore/rendering/RenderObject.h

@@ -363 +363 @@
     RenderObject* hoverAncestor() const;
 
-    virtual void markAllDescendantsWithFloatsForLayout(RenderBox* floatToRemove = 0);
     void markContainingBlocksForLayout(bool scheduleRelayout = true, RenderObject* newRoot = 0);
     void setNeedsLayout(bool b, bool markParents = true);

trunk/WebCore/rendering/RenderSVGRoot.cpp

@@ -108 +108 @@
     for (RenderObject* child = firstChild(); child; child = child->nextSibling()) {
         if (selfNeedsLayout()) // either bounds or transform changed, force kids to relayout
-            child->setNeedsLayout(true);
+            child->setNeedsLayout(true, false);
         
         child->layoutIfNeeded();

trunk/WebCore/rendering/RenderTable.cpp

@@ -296 +296 @@
     for (RenderObject* child = firstChild(); child; child = child->nextSibling()) {
         // FIXME: What about a form that has a display value that makes it a table section?
-        if (child->needsLayout() && !(child->element() && child->element()->hasTagName(formTag)))
+        if (child->needsLayout() && !(child->element() && child->element()->hasTagName(formTag) && !child->isTableSection()))
             child->layout();
         if (child->isTableSection()) {

trunk/WebCore/rendering/RenderTableSection.cpp

@@ -588 +588 @@
     }
 
+    ASSERT(!needsLayout());
+
     statePusher.pop();