Changeset 152572 in webkit

Timestamp:

Jul 11, 2013 9:17:37 AM (11 years ago)

Author:

abucur@adobe.com

Message:

[CSS Regions] In a region chain with auto-height regions, lines get their length based only on the first region
​https://bugs.webkit.org/show_bug.cgi?id=118531

Reviewed by Alexandru Chiculita.

Source/WebCore:

When computing the height a flow thread it's possible to overflow the maximum LayoutUnit and obtain a negative value.
This leads to invalid results during layout when computing the region range and the RenderBoxRegion info for the
descendant boxes of the flow thread.

This issue appears especially during the auto-height algorithm because it initializes the auto-height regions
height with the LayoutUnit::max() / 2 value. Summing two such regions overflows and results in a negative value.

The fix clamps the maximum height of the flow thread to LayoutUnit::max() / 2. This doesn't affect the auto-height
algorithm because regionAtBlockOffset() will still return the correct values as the auto-height regions content
is established and their height updated.

Tests: fast/regions/autoheight-correct-region-for-lines-2.html

fast/regions/autoheight-correct-region-for-lines.html

• rendering/RenderBlock.cpp:

(WebCore::RenderBlock::updateRegionsAndShapesBeforeChildLayout):

• rendering/RenderFlowThread.cpp:

(WebCore::RenderFlowThread::computeLogicalHeight):
(WebCore::RenderFlowThread::setRegionRangeForBox):
(WebCore::RenderFlowThread::updateRegionsFlowThreadPortionRect):

• rendering/RenderFlowThread.h:
• rendering/RenderRegion.cpp:

(WebCore::RenderRegion::maxPageLogicalHeight):

LayoutTests:

Add tests verifying the lines widths are correctly computed in auto-height regions with and without a max-height set.

• fast/regions/autoheight-correct-region-for-lines-2-expected.html: Added.
• fast/regions/autoheight-correct-region-for-lines-2.html: Added.
• fast/regions/autoheight-correct-region-for-lines-expected.html: Added.
• fast/regions/autoheight-correct-region-for-lines.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/regions/autoheight-correct-region-for-lines-2-expected.html (added)
• LayoutTests/fast/regions/autoheight-correct-region-for-lines-2.html (added)
• LayoutTests/fast/regions/autoheight-correct-region-for-lines-expected.html (added)
• LayoutTests/fast/regions/autoheight-correct-region-for-lines.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderBlock.cpp (modified) (1 diff)
• Source/WebCore/rendering/RenderFlowThread.cpp (modified) (3 diffs)
• Source/WebCore/rendering/RenderFlowThread.h (modified) (1 diff)
• Source/WebCore/rendering/RenderRegion.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2013-07-11  Andrei Bucur  <abucur@adobe.com>
+
+        [CSS Regions] In a region chain with auto-height regions, lines get their length based only on the first region
+        https://bugs.webkit.org/show_bug.cgi?id=118531
+
+        Reviewed by Alexandru Chiculita.
+
+        Add tests verifying the lines widths are correctly computed in auto-height regions with and without a max-height set.
+
+        * fast/regions/autoheight-correct-region-for-lines-2-expected.html: Added.
+        * fast/regions/autoheight-correct-region-for-lines-2.html: Added.
+        * fast/regions/autoheight-correct-region-for-lines-expected.html: Added.
+        * fast/regions/autoheight-correct-region-for-lines.html: Added.
+
 2013-07-11  Radu Stavila  <stavila@adobe.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2013-07-11  Andrei Bucur  <abucur@adobe.com>
+
+        [CSS Regions] In a region chain with auto-height regions, lines get their length based only on the first region
+        https://bugs.webkit.org/show_bug.cgi?id=118531
+
+        Reviewed by Alexandru Chiculita.
+
+        When computing the height a flow thread it's possible to overflow the maximum LayoutUnit and obtain a negative value.
+        This leads to invalid results during layout when computing the region range and the RenderBoxRegion info for the
+        descendant boxes of the flow thread.
+    
+        This issue appears especially during the auto-height algorithm because it initializes the auto-height regions
+        height with the LayoutUnit::max() / 2 value. Summing two such regions overflows and results in a negative value.
+
+        The fix clamps the maximum height of the flow thread to LayoutUnit::max() / 2. This doesn't affect the auto-height
+        algorithm because regionAtBlockOffset() will still return the correct values as the auto-height regions content
+        is established and their height updated.
+
+        Tests: fast/regions/autoheight-correct-region-for-lines-2.html
+               fast/regions/autoheight-correct-region-for-lines.html
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::updateRegionsAndShapesBeforeChildLayout):
+        * rendering/RenderFlowThread.cpp:
+        (WebCore::RenderFlowThread::computeLogicalHeight):
+        (WebCore::RenderFlowThread::setRegionRangeForBox):
+        (WebCore::RenderFlowThread::updateRegionsFlowThreadPortionRect):
+        * rendering/RenderFlowThread.h:
+        * rendering/RenderRegion.cpp:
+        (WebCore::RenderRegion::maxPageLogicalHeight):
+
 2013-07-11  Timothy Hatcher  <timothy@apple.com>
 

trunk/Source/WebCore/rendering/RenderBlock.cpp

@@ -1497 +1497 @@
     // Compute the maximum logical height content may cause this block to expand to
     // FIXME: These should eventually use the const computeLogicalHeight rather than updateLogicalHeight
-    setLogicalHeight(LayoutUnit::max() / 2);
+    setLogicalHeight(RenderFlowThread::maxLogicalHeight());
     updateLogicalHeight();
 

trunk/Source/WebCore/rendering/RenderFlowThread.cpp

@@ -252 +252 @@
     computedValues.m_extent = 0;
 
+    const LayoutUnit maxFlowSize = RenderFlowThread::maxLogicalHeight();
     for (RenderRegionList::const_iterator iter = m_regionList.begin(); iter != m_regionList.end(); ++iter) {
         RenderRegion* region = *iter;
         ASSERT(!region->needsLayout() || region->isRenderRegionSet());
 
-        computedValues.m_extent += region->logicalHeightOfAllFlowThreadContent();
+        LayoutUnit distanceToMaxSize = maxFlowSize - computedValues.m_extent;
+        computedValues.m_extent += std::min(distanceToMaxSize, region->logicalHeightOfAllFlowThreadContent());
+
+        // If we reached the maximum size there's no point in going further.
+        if (computedValues.m_extent == maxFlowSize)
+            return;
     }
 }
@@ -686 +692 @@
         return;
 
+    ASSERT(box->logicalHeight() >= 0);
+
     // FIXME: Not right for differing writing-modes.
     RenderRegion* startRegion = regionAtBlockOffset(offsetFromLogicalTopOfFirstPage, true);
@@ -876 +884 @@
 
         LayoutUnit regionLogicalWidth = region->pageLogicalWidth();
-        LayoutUnit regionLogicalHeight = std::min<LayoutUnit>(LayoutUnit::max() / 2 - logicalHeight, region->logicalHeightOfAllFlowThreadContent());
+        LayoutUnit regionLogicalHeight = std::min<LayoutUnit>(RenderFlowThread::maxLogicalHeight() - logicalHeight, region->logicalHeightOfAllFlowThreadContent());
 
         LayoutRect regionRect(style()->direction() == LTR ? LayoutUnit() : logicalWidth() - regionLogicalWidth, logicalHeight, regionLogicalWidth, regionLogicalHeight);

trunk/Source/WebCore/rendering/RenderFlowThread.h

@@ -169 +169 @@
     LayoutUnit offsetFromLogicalTopOfFirstRegion(const RenderBlock*) const;
 
+    // Used to estimate the maximum height of the flow thread.
+    static LayoutUnit maxLogicalHeight() { return LayoutUnit::max() / 2; }
+
 protected:
     virtual const char* renderName() const = 0;

trunk/Source/WebCore/rendering/RenderRegion.cpp

@@ -82 +82 @@
     ASSERT(m_flowThread);
     ASSERT(hasAutoLogicalHeight() && !m_flowThread->inConstrainedLayoutPhase());
-    return style()->logicalMaxHeight().isUndefined() ? LayoutUnit::max() / 2 : computeReplacedLogicalHeightUsing(style()->logicalMaxHeight());
+    return style()->logicalMaxHeight().isUndefined() ? RenderFlowThread::maxLogicalHeight() : computeReplacedLogicalHeightUsing(style()->logicalMaxHeight());
 }