CSSRegions: crash positioned object with inline containing block in flow thread
authormihnea@adobe.com <mihnea@adobe.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 19 Feb 2013 09:56:15 +0000 (09:56 +0000)
committermihnea@adobe.com <mihnea@adobe.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 19 Feb 2013 09:56:15 +0000 (09:56 +0000)
https://bugs.webkit.org/show_bug.cgi?id=108307

Reviewed by David Hyatt.

Source/WebCore:

The fix for https://bugs.webkit.org/show_bug.cgi?id=69896 allowed positioned blocks work
with variable width regions. However, the information needed for that is available only
when the container used for positioning is a block.

This patch ensures we are using this solution only when the container used for positioning
is a block. This needs to be revisited when we will extend support for other types of boxes
as mentioned in RenderBox::renderBoxRegionInfo.

Test: fast/regions/positioned-object-inline-cb-crash.html

* rendering/RenderBox.cpp:
(WebCore::RenderBox::renderBoxRegionInfo):
(WebCore::RenderBox::containingBlockLogicalWidthForPositioned):
(WebCore::RenderBox::computePositionedLogicalWidth): Make sure we are using containerBlocks
that are blocks. Add an assert that the type of containerBlock we are using can have
computed RenderBoxRegionInfo.
(WebCore::RenderBox::computePositionedLogicalHeight):
* rendering/RenderBoxModelObject.h:
(WebCore::RenderBoxModelObject::canHaveBoxInfoInRegion): This helper method
will return the boxes that may have computed RenderBoxRegionInfo. Currently,
returns true for blocks only.

LayoutTests:

* fast/regions/positioned-object-inline-cb-crash-expected.txt: Added.
* fast/regions/positioned-object-inline-cb-crash.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/regions/positioned-object-inline-cb-crash-expected.txt [new file with mode: 0644]
LayoutTests/fast/regions/positioned-object-inline-cb-crash.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderBox.cpp
Source/WebCore/rendering/RenderBoxModelObject.h

index 74070cd..d276c24 100644 (file)
@@ -1,3 +1,13 @@
+2013-02-19  Mihnea Ovidenie  <mihnea@adobe.com>
+
+        CSSRegions: crash positioned object with inline containing block in flow thread
+        https://bugs.webkit.org/show_bug.cgi?id=108307
+
+        Reviewed by David Hyatt.
+
+        * fast/regions/positioned-object-inline-cb-crash-expected.txt: Added.
+        * fast/regions/positioned-object-inline-cb-crash.html: Added.
+
 2013-02-19  Zan Dobersek  <zdobersek@igalia.com>
 
         Unreviewed GTK gardening.
diff --git a/LayoutTests/fast/regions/positioned-object-inline-cb-crash-expected.txt b/LayoutTests/fast/regions/positioned-object-inline-cb-crash-expected.txt
new file mode 100644 (file)
index 0000000..87be0f6
--- /dev/null
@@ -0,0 +1,2 @@
+Test for WebKit Bug 108307 Crash when layout a positioned object with inline containing block inside a flow thread.The test passes if it does not crash or assert.PASS
+
diff --git a/LayoutTests/fast/regions/positioned-object-inline-cb-crash.html b/LayoutTests/fast/regions/positioned-object-inline-cb-crash.html
new file mode 100644 (file)
index 0000000..24c4e08
--- /dev/null
@@ -0,0 +1,25 @@
+<!doctype html>
+<html>
+    <head>
+        <style>
+            #container { -webkit-flow-into: flow; }
+            p { position: absolute; }
+        </style>
+    </head>
+    <body>
+        <p>Test for <a href="https://bugs.webkit.org/show_bug.cgi?id=108307">WebKit Bug 108307</a> Crash when layout a positioned object with inline containing block inside a flow thread.</p>
+        <p>The test passes if it does not crash or assert.</p>
+        <p>PASS</p>
+        <div id="container">
+            <span style="position: relative;">
+                <div style="display: inline-block; -webkit-writing-mode: vertical-rl;">
+                    <p></p>
+                </div>
+            </span>
+        </div>
+        <script>
+            if (window.testRunner)
+                window.testRunner.dumpAsText();
+        </script>
+    </body>
+</html>
index 0550746..6426e79 100644 (file)
@@ -1,3 +1,32 @@
+2013-02-19  Mihnea Ovidenie  <mihnea@adobe.com>
+
+        CSSRegions: crash positioned object with inline containing block in flow thread
+        https://bugs.webkit.org/show_bug.cgi?id=108307
+
+        Reviewed by David Hyatt.
+
+        The fix for https://bugs.webkit.org/show_bug.cgi?id=69896 allowed positioned blocks work
+        with variable width regions. However, the information needed for that is available only
+        when the container used for positioning is a block.
+
+        This patch ensures we are using this solution only when the container used for positioning
+        is a block. This needs to be revisited when we will extend support for other types of boxes
+        as mentioned in RenderBox::renderBoxRegionInfo.
+
+        Test: fast/regions/positioned-object-inline-cb-crash.html
+
+        * rendering/RenderBox.cpp:
+        (WebCore::RenderBox::renderBoxRegionInfo):
+        (WebCore::RenderBox::containingBlockLogicalWidthForPositioned):
+        (WebCore::RenderBox::computePositionedLogicalWidth): Make sure we are using containerBlocks
+        that are blocks. Add an assert that the type of containerBlock we are using can have
+        computed RenderBoxRegionInfo.
+        (WebCore::RenderBox::computePositionedLogicalHeight):
+        * rendering/RenderBoxModelObject.h:
+        (WebCore::RenderBoxModelObject::canHaveBoxInfoInRegion): This helper method
+        will return the boxes that may have computed RenderBoxRegionInfo. Currently,
+        returns true for blocks only.
+
 2013-02-19  Ryosuke Niwa  <rniwa@webkit.org>
 
         Yet anther JSC build fix after r143304.
index 28ef3d6..6810c5d 100644 (file)
@@ -2228,8 +2228,7 @@ RenderBoxRegionInfo* RenderBox::renderBoxRegionInfo(RenderRegion* region, Layout
     // No cached value was found, so we have to compute our insets in this region.
     // FIXME: For now we limit this computation to normal RenderBlocks. Future patches will expand
     // support to cover all boxes.
-    if (!inRenderFlowThread() || isFloating() || isReplaced() || isInline() || hasColumns()
-        || isTableCell() || !isBlockFlow() || isRenderFlowThread())
+    if (!inRenderFlowThread() || !canHaveBoxInfoInRegion() || isRenderFlowThread())
         return 0;
 
     RenderFlowThread* flowThread = enclosingRenderFlowThread();
@@ -2772,29 +2771,27 @@ LayoutUnit RenderBox::containingBlockLogicalWidthForPositioned(const RenderBoxMo
         return containingBlockLogicalHeightForPositioned(containingBlock, false);
 
     if (containingBlock->isBox()) {
+        if (!inRenderFlowThread())
+            return toRenderBox(containingBlock)->clientLogicalWidth();
+
         const RenderBlock* cb = toRenderBlock(containingBlock);
-        LayoutUnit result = cb->clientLogicalWidth();
-        if (inRenderFlowThread()) {
-            RenderBoxRegionInfo* boxInfo = 0;
-            if (!region) {
-                if (containingBlock->isRenderFlowThread() && !checkForPerpendicularWritingMode)
-                    return toRenderFlowThread(containingBlock)->contentLogicalWidthOfFirstRegion();
-                if (isWritingModeRoot()) {
-                    LayoutUnit cbPageOffset = offsetFromLogicalTopOfFirstPage - logicalTop();
-                    RenderRegion* cbRegion = cb->regionAtBlockOffset(cbPageOffset);
-                    if (cbRegion) {
-                        cbRegion = cb->clampToStartAndEndRegions(cbRegion);
-                        boxInfo = cb->renderBoxRegionInfo(cbRegion, cbPageOffset);
-                    }
+        RenderBoxRegionInfo* boxInfo = 0;
+        if (!region) {
+            if (containingBlock->isRenderFlowThread() && !checkForPerpendicularWritingMode)
+                return toRenderFlowThread(containingBlock)->contentLogicalWidthOfFirstRegion();
+            if (isWritingModeRoot()) {
+                LayoutUnit cbPageOffset = offsetFromLogicalTopOfFirstPage - logicalTop();
+                RenderRegion* cbRegion = cb->regionAtBlockOffset(cbPageOffset);
+                if (cbRegion) {
+                    cbRegion = cb->clampToStartAndEndRegions(cbRegion);
+                    boxInfo = cb->renderBoxRegionInfo(cbRegion, cbPageOffset);
                 }
-            } else if (region && enclosingRenderFlowThread()->isHorizontalWritingMode() == containingBlock->isHorizontalWritingMode()) {
-                RenderRegion* containingBlockRegion = cb->clampToStartAndEndRegions(region);
-                boxInfo = cb->renderBoxRegionInfo(containingBlockRegion, offsetFromLogicalTopOfFirstPage - logicalTop());
             }
-            if (boxInfo)
-                return max<LayoutUnit>(0, result - (cb->logicalWidth() - boxInfo->logicalWidth()));
+        } else if (region && enclosingRenderFlowThread()->isHorizontalWritingMode() == containingBlock->isHorizontalWritingMode()) {
+            RenderRegion* containingBlockRegion = cb->clampToStartAndEndRegions(region);
+            boxInfo = cb->renderBoxRegionInfo(containingBlockRegion, offsetFromLogicalTopOfFirstPage - logicalTop());
         }
-        return result;
+        return (boxInfo) ? max<LayoutUnit>(0, cb->clientLogicalWidth() - (cb->logicalWidth() - boxInfo->logicalWidth())) : cb->clientLogicalWidth();
     }
 
     ASSERT(containingBlock->isRenderInline() && containingBlock->isInFlowPositioned());
@@ -3029,7 +3026,9 @@ void RenderBox::computePositionedLogicalWidth(LogicalExtentComputedValues& compu
     computedValues.m_extent += bordersPlusPadding;
     
     // Adjust logicalLeft if we need to for the flipped version of our writing mode in regions.
-    if (inRenderFlowThread() && !region && isWritingModeRoot() && isHorizontalWritingMode() == containerBlock->isHorizontalWritingMode()) {
+    // FIXME: Add support for other types of objects as containerBlock, not only RenderBlock.
+    if (inRenderFlowThread() && !region && isWritingModeRoot() && isHorizontalWritingMode() == containerBlock->isHorizontalWritingMode() && containerBlock->isRenderBlock()) {
+        ASSERT(containerBlock->canHaveBoxInfoInRegion());
         LayoutUnit logicalLeftPos = computedValues.m_position;
         const RenderBlock* cb = toRenderBlock(containerBlock);
         LayoutUnit cbPageOffset = offsetFromLogicalTopOfFirstPage - logicalTop();
@@ -3345,7 +3344,9 @@ void RenderBox::computePositionedLogicalHeight(LogicalExtentComputedValues& comp
     computedValues.m_extent += bordersPlusPadding;
     
     // Adjust logicalTop if we need to for perpendicular writing modes in regions.
-    if (inRenderFlowThread() && isHorizontalWritingMode() != containerBlock->isHorizontalWritingMode()) {
+    // FIXME: Add support for other types of objects as containerBlock, not only RenderBlock.
+    if (inRenderFlowThread() && isHorizontalWritingMode() != containerBlock->isHorizontalWritingMode() && containerBlock->isRenderBlock()) {
+        ASSERT(containerBlock->canHaveBoxInfoInRegion());
         LayoutUnit logicalTopPos = computedValues.m_position;
         const RenderBlock* cb = toRenderBlock(containerBlock);
         LayoutUnit cbPageOffset = cb->offsetFromLogicalTopOfFirstPage() - logicalLeft();
index bb70d46..475d8e4 100644 (file)
@@ -164,6 +164,8 @@ public:
 
     virtual void setSelectionState(SelectionState s);
 
+    bool canHaveBoxInfoInRegion() const { return !isFloating() && !isReplaced() && !isInline() && !hasColumns() && !isTableCell() && isBlockFlow(); }
+
 #if USE(ACCELERATED_COMPOSITING)
     void contentChanged(ContentChangeType);
     bool hasAcceleratedCompositing() const;