Changeset 134683 in webkit

Timestamp:

Nov 14, 2012 3:36:04 PM (11 years ago)

Author:

tony@chromium.org

Message:

Crash in flexbox when removing absolutely positioned children
​https://bugs.webkit.org/show_bug.cgi?id=100465

Reviewed by Ojan Vafai.

Source/WebCore:

We use m_numberOfChildrenOnFirstLine when computing baseline alignment.
This value gets set during flexbox layout. When we remove an absolutely
positioned child, we don't relayout and this value would get stale.

Change m_numberOfChildrenOnFirstLine to m_numberOfInFlowChildrenOnFirstLine
so the value doesn't get stale when we remove absolutely positioned children.
Also change the loop in firstLineBoxBaseline to bail if we run off the end of
the iterator.

Test: css3/flexbox/crash-removing-out-of-flow-child.html

• rendering/RenderFlexibleBox.cpp:

(WebCore::RenderFlexibleBox::RenderFlexibleBox):
(WebCore::RenderFlexibleBox::firstLineBoxBaseline):
(WebCore::RenderFlexibleBox::layoutBlock):
(WebCore::RenderFlexibleBox::repositionLogicalHeightDependentFlexItems):
(WebCore::RenderFlexibleBox::layoutAndPlaceChildren):

• rendering/RenderFlexibleBox.h:

LayoutTests:

Test case for the crash.

• css3/flexbox/crash-removing-out-of-flow-child-expected.txt: Added.
• css3/flexbox/crash-removing-out-of-flow-child.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/css3/flexbox/crash-removing-out-of-flow-child-expected.txt (added)
• LayoutTests/css3/flexbox/crash-removing-out-of-flow-child.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderFlexibleBox.cpp (modified) (6 diffs)
• Source/WebCore/rendering/RenderFlexibleBox.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2012-11-14  Tony Chang  <tony@chromium.org>
+
+        Crash in flexbox when removing absolutely positioned children
+        https://bugs.webkit.org/show_bug.cgi?id=100465
+
+        Reviewed by Ojan Vafai.
+
+        Test case for the crash.
+
+        * css3/flexbox/crash-removing-out-of-flow-child-expected.txt: Added.
+        * css3/flexbox/crash-removing-out-of-flow-child.html: Added.
+
 2012-11-14  Dirk Schulze  <krit@webkit.org>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-11-14  Tony Chang  <tony@chromium.org>
+
+        Crash in flexbox when removing absolutely positioned children
+        https://bugs.webkit.org/show_bug.cgi?id=100465
+
+        Reviewed by Ojan Vafai.
+
+        We use m_numberOfChildrenOnFirstLine when computing baseline alignment.
+        This value gets set during flexbox layout. When we remove an absolutely
+        positioned child, we don't relayout and this value would get stale.
+
+        Change m_numberOfChildrenOnFirstLine to m_numberOfInFlowChildrenOnFirstLine
+        so the value doesn't get stale when we remove absolutely positioned children.
+        Also change the loop in firstLineBoxBaseline to bail if we run off the end of
+        the iterator.
+
+        Test: css3/flexbox/crash-removing-out-of-flow-child.html
+
+        * rendering/RenderFlexibleBox.cpp:
+        (WebCore::RenderFlexibleBox::RenderFlexibleBox):
+        (WebCore::RenderFlexibleBox::firstLineBoxBaseline):
+        (WebCore::RenderFlexibleBox::layoutBlock):
+        (WebCore::RenderFlexibleBox::repositionLogicalHeightDependentFlexItems):
+        (WebCore::RenderFlexibleBox::layoutAndPlaceChildren):
+        * rendering/RenderFlexibleBox.h:
+
 2012-11-14  Joshua Bell  <jsbell@chromium.org>
 

trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp

@@ -133 +133 @@
 RenderFlexibleBox::RenderFlexibleBox(Node* node)
     : RenderBlock(node)
-    , m_numberOfChildrenOnFirstLine(0)
+    , m_numberOfInFlowChildrenOnFirstLine(-1)
 {
     setChildrenInline(false); // All of our children must be block-level.
@@ -255 +255 @@
     ASSERT(m_orderIterator);
 
-    if (isWritingModeRoot() || !m_numberOfChildrenOnFirstLine)
+    if (isWritingModeRoot() || m_numberOfInFlowChildrenOnFirstLine <= 0)
         return -1;
     RenderBox* baselineChild = 0;
-    RenderBox* child = m_orderIterator->first();
-    for (size_t childNumber = 0; childNumber < m_numberOfChildrenOnFirstLine; ++childNumber, child = m_orderIterator->next()) {
+    int childNumber = 0;
+    for (RenderBox* child = m_orderIterator->first(); child; child = m_orderIterator->next()) {
         if (child->isOutOfFlowPositioned())
             continue;
@@ -268 +268 @@
         if (!baselineChild)
             baselineChild = child;
+
+        ++childNumber;
+        if (childNumber == m_numberOfInFlowChildrenOnFirstLine)
+            break;
     }
 
@@ -321 +325 @@
     updateLogicalWidth();
 
+    m_numberOfInFlowChildrenOnFirstLine = -1;
     m_overflow.clear();
 
@@ -386 +391 @@
         flipForWrapReverse(iterator, lineContexts, crossAxisStartEdge);
     }
-
-    m_numberOfChildrenOnFirstLine = lineContexts.isEmpty() ? 0 : lineContexts[0].numberOfChildren;
 
     // direction:rtl + flex-direction:column means the cross-axis direction is flipped.
@@ -1133 +1136 @@
     }
 
+    if (m_numberOfInFlowChildrenOnFirstLine == -1)
+        m_numberOfInFlowChildrenOnFirstLine = seenInFlowPositionedChildren;
     lineContexts.append(LineContext(crossAxisOffset, maxChildCrossAxisExtent, children.size(), maxAscent));
     crossAxisOffset += maxChildCrossAxisExtent;

trunk/Source/WebCore/rendering/RenderFlexibleBox.h

@@ -147 +147 @@
 
     OwnPtr<OrderIterator> m_orderIterator;
-    size_t m_numberOfChildrenOnFirstLine;
+    int m_numberOfInFlowChildrenOnFirstLine;
 };