Changeset 136324 in webkit

Timestamp:

Dec 2, 2012, 2:39:51 AM (12 years ago)

Author:

tony@chromium.org

Message:

Avoid a second layout of flex items in layoutAndPlaceChildren()
​https://bugs.webkit.org/show_bug.cgi?id=102352

Reviewed by Ojan Vafai.

Source/WebCore:

Avoid doing a second layout if we're going to get the same size as before.
This prevents us from doing an exponential number of layouts in some
common cases.

Test: css3/flexbox/stretch-after-sibling-size-change.html

• html/shadow/SliderThumbElement.cpp:

(WebCore::RenderSliderContainer::layout): Force a layout of the track, which positions the thumb.

• rendering/RenderFlexibleBox.cpp:

(WebCore::RenderFlexibleBox::needToStretchChild): Determine if a child is going to stretch.
(WebCore::RenderFlexibleBox::resetAutoMarginsAndLogicalTopInCrossAxis): Makes sure we're in a consistent state before
we apply auto margins.
(WebCore::RenderFlexibleBox::layoutAndPlaceChildren):

• rendering/RenderFlexibleBox.h: Add needToStretchChild.

LayoutTests:

Add a test case to make sure we relayout when a sibling is stretching.

• css3/flexbox/stretch-after-sibling-size-change-expected.txt: Added.
• css3/flexbox/stretch-after-sibling-size-change.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/css3/flexbox/stretch-after-sibling-size-change-expected.txt (added)
• LayoutTests/css3/flexbox/stretch-after-sibling-size-change.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/html/shadow/SliderThumbElement.cpp (modified) (2 diffs)
• Source/WebCore/rendering/RenderFlexibleBox.cpp (modified) (2 diffs)
• Source/WebCore/rendering/RenderFlexibleBox.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2012-12-02  Tony Chang  <tony@chromium.org>
+
+        Avoid a second layout of flex items in layoutAndPlaceChildren()
+        https://bugs.webkit.org/show_bug.cgi?id=102352
+
+        Reviewed by Ojan Vafai.
+
+        Add a test case to make sure we relayout when a sibling is stretching.
+
+        * css3/flexbox/stretch-after-sibling-size-change-expected.txt: Added.
+        * css3/flexbox/stretch-after-sibling-size-change.html: Added.
+
 2012-12-02  Yongjun Zhang  <yongjun_zhang@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-12-02  Tony Chang  <tony@chromium.org>
+
+        Avoid a second layout of flex items in layoutAndPlaceChildren()
+        https://bugs.webkit.org/show_bug.cgi?id=102352
+
+        Reviewed by Ojan Vafai.
+
+        Avoid doing a second layout if we're going to get the same size as before.
+        This prevents us from doing an exponential number of layouts in some
+        common cases.
+
+        Test: css3/flexbox/stretch-after-sibling-size-change.html
+
+        * html/shadow/SliderThumbElement.cpp:
+        (WebCore::RenderSliderContainer::layout): Force a layout of the track, which positions the thumb.
+        * rendering/RenderFlexibleBox.cpp:
+        (WebCore::RenderFlexibleBox::needToStretchChild): Determine if a child is going to stretch.
+        (WebCore::RenderFlexibleBox::resetAutoMarginsAndLogicalTopInCrossAxis): Makes sure we're in a consistent state before
+        we apply auto margins.
+        (WebCore::RenderFlexibleBox::layoutAndPlaceChildren):
+        * rendering/RenderFlexibleBox.h: Add needToStretchChild.
+
 2012-12-02  Yongjun Zhang  <yongjun_zhang@apple.com>
 

trunk/Source/WebCore/html/shadow/SliderThumbElement.cpp

@@ -179 +179 @@
 
     RenderBox* thumb = 0;
+    RenderBox* track = 0;
     if (input->sliderThumbElement() && input->sliderThumbElement()->renderer()) {
         thumb = toRenderBox(input->sliderThumbElement()->renderer());
-        // Reset the thumb location before layout.
-        thumb->setLocation(LayoutPoint());
+        track = toRenderBox(thumb->parent());
+        // Force a layout to reset the position of the thumb so the code below doesn't move the thumb to the wrong place.
+        // FIXME: Make a custom Render class for the track and move the thumb positioning code there.
+        track->setChildNeedsLayout(true, MarkOnlyThis);
     }
 
@@ -191 +194 @@
     if (!thumb)
         return;
-    RenderBox* track = toRenderBox(thumb->parent());
 
     double percentageOffset = sliderPosition(input).toDouble();

trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp

@@ -1066 +1066 @@
 }
 
+bool RenderFlexibleBox::needToStretchChild(RenderBox* child)
+{
+    if (alignmentForChild(child) != AlignStretch)
+        return false;
+
+    Length crossAxisLength = isHorizontalFlow() ? child->style()->height() : child->style()->width();
+    return crossAxisLength.isAuto();
+}
+
+void RenderFlexibleBox::resetAutoMarginsAndLogicalTopInCrossAxis(RenderBox* child)
+{
+    if (hasAutoMarginsInCrossAxis(child))
+        child->updateLogicalHeight();
+}
+
 void RenderFlexibleBox::layoutAndPlaceChildren(LayoutUnit& crossAxisOffset, const OrderedFlexItemList& children, const WTF::Vector<LayoutUnit>& childSizes, LayoutUnit availableFreeSpace, WTF::Vector<LineContext>& lineContexts)
 {
@@ -1091 +1106 @@
         setLogicalOverrideSize(child, childPreferredSize);
         // FIXME: Can avoid laying out here in some cases. See https://webkit.org/b/87905.
-        child->setChildNeedsLayout(true, MarkOnlyThis);
+        if (needToStretchChild(child) || childPreferredSize != mainAxisExtentForChild(child))
+            child->setChildNeedsLayout(true, MarkOnlyThis);
+        else {
+            // To avoid double applying margin changes in updateAutoMarginsInCrossAxis, we reset the margins here.
+            resetAutoMarginsAndLogicalTopInCrossAxis(child);
+        }
         child->layoutIfNeeded();
 

trunk/Source/WebCore/rendering/RenderFlexibleBox.h

@@ -135 +135 @@
     void freezeViolations(const WTF::Vector<Violation>&, LayoutUnit& availableFreeSpace, double& totalFlexGrow, double& totalWeightedFlexShrink, InflexibleFlexItemSize&);
 
+    void resetAutoMarginsAndLogicalTopInCrossAxis(RenderBox*);
+    bool needToStretchChild(RenderBox*);
     void setLogicalOverrideSize(RenderBox* child, LayoutUnit childPreferredSize);
     void prepareChildForPositionedLayout(RenderBox* child, LayoutUnit mainAxisOffset, LayoutUnit crossAxisOffset, PositionedLayoutMode);