Diff
Modified: trunk/LayoutTests/ChangeLog (168790 => 168791)
--- trunk/LayoutTests/ChangeLog 2014-05-14 06:32:56 UTC (rev 168790)
+++ trunk/LayoutTests/ChangeLog 2014-05-14 06:33:48 UTC (rev 168791)
@@ -1,3 +1,15 @@
+2014-05-13 Andrei Bucur <[email protected]>
+
+ [CSS Regions] Assertion failure in some cases with inline blocks
+ https://bugs.webkit.org/show_bug.cgi?id=132859
+
+ Reviewed by Mihnea Ovidenie.
+
+ Test that moving lines with inline blocks doesn't cause an assertion.
+
+ * fast/regions/inline-block-shifted-region-expected.txt: Added.
+ * fast/regions/inline-block-shifted-region.html: Added.
+
2014-05-13 Hans Muller <[email protected]>
[CSS Shapes] line height grows around polygon and incorrectly causes text to wrap to next line
Added: trunk/LayoutTests/fast/regions/inline-block-shifted-region-expected.txt (0 => 168791)
--- trunk/LayoutTests/fast/regions/inline-block-shifted-region-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/regions/inline-block-shifted-region-expected.txt 2014-05-14 06:33:48 UTC (rev 168791)
@@ -0,0 +1,3 @@
+Test for [CSS Regions] Assertion failure in some cases with inline blocks. The test passes if there is no crash.
+
+
Added: trunk/LayoutTests/fast/regions/inline-block-shifted-region.html (0 => 168791)
--- trunk/LayoutTests/fast/regions/inline-block-shifted-region.html (rev 0)
+++ trunk/LayoutTests/fast/regions/inline-block-shifted-region.html 2014-05-14 06:33:48 UTC (rev 168791)
@@ -0,0 +1,64 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style type="text/css">
+ .region {
+ -webkit-flow-from: flow;
+ height: 120px;
+ width: 300px;
+ float: left;
+ }
+
+ #content {
+ -webkit-flow-into: flow;
+ margin-top: 50px;
+ font-family: Ahem;
+ font-size: 12px;
+ }
+
+ #one {
+ background-color: green;
+ width: 100px;
+ height: 100px;
+ }
+ #two {
+ width: 10px;
+ height: 10px;
+ background-color: blue;
+ }
+
+ #inlineblock {
+ display: inline-block;
+ background-color: blue;
+ padding: 10px;
+ }
+
+ #boxonline {
+ background-color: green;
+ }
+
+ #outside {
+ border: thin solid red;
+ margin: 5px;
+ }
+</style>
+</head>
+<body>
+ <p>Test for <a href="" Regions] Assertion failure in some cases with inline blocks</a>. The test passes if there is no crash.</p>
+ <div class="region"></div>
+ <div class="region"></div>
+
+ <div id="content"><div id="outside">AAA<br/>AAA<br/>AAA<br/>AAA<br/><div id="inlineblock"><div id="boxonline">A</div></div></br>AAA</div></div>
+
+ <script>
+ if (window.testRunner)
+ testRunner.dumpAsText();
+
+ document.body.offsetTop;
+ document.getElementById("content").style.marginTop = "0px";
+ document.body.offsetTop;
+ document.body.removeChild(document.getElementById("content"));
+ document.body.offsetTop;
+ </script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (168790 => 168791)
--- trunk/Source/WebCore/ChangeLog 2014-05-14 06:32:56 UTC (rev 168790)
+++ trunk/Source/WebCore/ChangeLog 2014-05-14 06:33:48 UTC (rev 168791)
@@ -1,3 +1,51 @@
+2014-05-13 Andrei Bucur <[email protected]>
+
+ [CSS Regions] Assertion failure in some cases with inline blocks
+ https://bugs.webkit.org/show_bug.cgi?id=132859
+
+ Reviewed by Mihnea Ovidenie.
+
+ The patch hardens the conditions when the region range caches are
+ populated to avoid desynchronizations when objects move during layout.
+ This is true especially in the case of the boxes found inside
+ inline blocks, that get their range from the containing line.
+
+ There is a new function |computedRegionRangeForBox| that will always
+ return a region range for a box using a best effort algorithm. This should
+ be used only when there's no need to cache region information.
+
+ This change also allows better control over the lifecycle of the
+ |RenderBoxRegionInfo| objects stored on the regions. We can now iterate
+ over the full range of the box when cleaning up the region box info. The
+ same applies for the width change detection function.
+
+ Test: fast/regions/inline-block-shifted-region.html
+
+ * rendering/RenderBlockLineLayout.cpp:
+ (WebCore::RenderBlockFlow::updateRegionForLine): Don't set the containing
+ region if the block doesn't have a range. The returned value would not
+ be correctly clamped.
+ * rendering/RenderBox.cpp:
+ (WebCore::RenderBlock::hasRegionRangeInFlowThread):
+ * rendering/RenderBox.h:
+ * rendering/RenderFlowThread.cpp:
+ (WebCore::RenderFlowThread::removeRenderBoxRegionInfo): Iterate only over
+ the range of the box, not from the start of the region chain.
+ (WebCore::RenderFlowThread::logicalWidthChangedInRegionsForBlock): Same as
+ above.
+ (WebCore::RenderFlowThread::hasCachedRegionRangeForBox):
+ (WebCore::RenderFlowThread::getRegionRangeForBoxFromCachedInfo):
+ (WebCore::RenderFlowThread::getRegionRangeForBox):
+ (WebCore::RenderFlowThread::computedRegionRangeForBox): Best effort function
+ to determine the range of a box. It will always return something as long
+ as the flow thread has regions.
+ (WebCore::RenderFlowThread::objectShouldFragmentInFlowRegion): Use the new function
+ to determine the range.
+ * rendering/RenderFlowThread.h:
+ * rendering/RenderNamedFlowThread.cpp:
+ (WebCore::RenderNamedFlowThread::absoluteQuadsForBox): Use the new function to determine
+ the range.
+
2014-05-13 Simon Fraser <[email protected]>
Fix "ASSERTION FAILED: m_representation == PlatformLayerRepresentation" with UI-side compositing
Modified: trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp (168790 => 168791)
--- trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp 2014-05-14 06:32:56 UTC (rev 168790)
+++ trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp 2014-05-14 06:33:48 UTC (rev 168791)
@@ -1928,10 +1928,14 @@
{
ASSERT(lineBox);
- if (auto containingRegion = regionAtBlockOffset(lineBox->lineTopWithLeading()))
- lineBox->setContainingRegion(*containingRegion);
- else
+ if (!hasRegionRangeInFlowThread())
lineBox->clearContainingRegion();
+ else {
+ if (auto containingRegion = regionAtBlockOffset(lineBox->lineTopWithLeading()))
+ lineBox->setContainingRegion(*containingRegion);
+ else
+ lineBox->clearContainingRegion();
+ }
RootInlineBox* prevLineBox = lineBox->prevRootBox();
if (!prevLineBox)
Modified: trunk/Source/WebCore/rendering/RenderBox.cpp (168790 => 168791)
--- trunk/Source/WebCore/rendering/RenderBox.cpp 2014-05-14 06:32:56 UTC (rev 168790)
+++ trunk/Source/WebCore/rendering/RenderBox.cpp 2014-05-14 06:33:48 UTC (rev 168791)
@@ -150,6 +150,15 @@
return region;
}
+bool RenderBox::hasRegionRangeInFlowThread() const
+{
+ RenderFlowThread* flowThread = flowThreadContainingBlock();
+ if (!flowThread || !flowThread->hasValidRegionInfo())
+ return false;
+
+ return flowThread->hasCachedRegionRangeForBox(this);
+}
+
LayoutRect RenderBox::clientBoxRectInRegion(RenderRegion* region) const
{
if (!region)
Modified: trunk/Source/WebCore/rendering/RenderBox.h (168790 => 168791)
--- trunk/Source/WebCore/rendering/RenderBox.h 2014-05-14 06:32:56 UTC (rev 168790)
+++ trunk/Source/WebCore/rendering/RenderBox.h 2014-05-14 06:33:48 UTC (rev 168791)
@@ -383,6 +383,7 @@
LayoutRect borderBoxRectInRegion(RenderRegion*, RenderBoxRegionInfoFlags = CacheRenderBoxRegionInfo) const;
LayoutRect clientBoxRectInRegion(RenderRegion*) const;
RenderRegion* clampToStartAndEndRegions(RenderRegion*) const;
+ bool hasRegionRangeInFlowThread() const;
virtual LayoutUnit offsetFromLogicalTopOfFirstPage() const;
void positionLineBox(InlineElementBox&);
Modified: trunk/Source/WebCore/rendering/RenderFlowThread.cpp (168790 => 168791)
--- trunk/Source/WebCore/rendering/RenderFlowThread.cpp 2014-05-14 06:32:56 UTC (rev 168790)
+++ trunk/Source/WebCore/rendering/RenderFlowThread.cpp 2014-05-14 06:33:48 UTC (rev 168791)
@@ -578,12 +578,13 @@
RenderRegion* startRegion = nullptr;
RenderRegion* endRegion = nullptr;
- getRegionRangeForBox(box, startRegion, endRegion);
-
- for (auto& region : m_regionList) {
- region->removeRenderBoxRegionInfo(box);
- if (region == endRegion)
- break;
+ if (getRegionRangeForBox(box, startRegion, endRegion)) {
+ for (auto it = m_regionList.find(startRegion), end = m_regionList.end(); it != end; ++it) {
+ RenderRegion* region = *it;
+ region->removeRenderBoxRegionInfo(box);
+ if (region == endRegion)
+ break;
+ }
}
#ifndef NDEBUG
@@ -640,9 +641,11 @@
RenderRegion* startRegion = nullptr;
RenderRegion* endRegion = nullptr;
- getRegionRangeForBox(block, startRegion, endRegion);
+ if (!getRegionRangeForBox(block, startRegion, endRegion))
+ return;
- for (auto& region : m_regionList) {
+ for (auto it = m_regionList.find(startRegion), end = m_regionList.end(); it != end; ++it) {
+ RenderRegion* region = *it;
ASSERT(!region->needsLayout() || region->isRenderRegionSet());
// We have no information computed for this region so we need to do it.
@@ -754,14 +757,7 @@
{
ASSERT(box);
- if (m_regionRangeMap.contains(box))
- return true;
-
- InlineElementBox* boxWrapper = box->inlineBoxWrapper();
- if (boxWrapper && boxWrapper->root().containingRegion())
- return true;
-
- return false;
+ return m_regionRangeMap.contains(box);
}
bool RenderFlowThread::getRegionRangeForBoxFromCachedInfo(const RenderBox* box, RenderRegion*& startRegion, RenderRegion*& endRegion) const
@@ -779,13 +775,6 @@
return true;
}
- InlineElementBox* boxWrapper = box->inlineBoxWrapper();
- if (boxWrapper && boxWrapper->root().containingRegion()) {
- startRegion = endRegion = boxWrapper->root().containingRegion();
- ASSERT(m_regionList.contains(startRegion) && m_regionList.contains(endRegion));
- return true;
- }
-
return false;
}
@@ -805,12 +794,31 @@
if (getRegionRangeForBoxFromCachedInfo(box, startRegion, endRegion))
return true;
- // Check if the box is contained in an unsplittable box.
- // If the unsplittable box has region range, then the start and end region for the box
- // should be equal with the region for the unsplittable box if any.
+ return false;
+}
+
+bool RenderFlowThread::computedRegionRangeForBox(const RenderBox* box, RenderRegion*& startRegion, RenderRegion*& endRegion) const
+{
+ ASSERT(box);
+
+ startRegion = endRegion = nullptr;
+ if (!hasValidRegionInfo()) // We clear the ranges when we invalidate the regions.
+ return false;
+
+ if (getRegionRangeForBox(box, startRegion, endRegion))
+ return true;
+
+ // Search the region range using the information provided by the
+ // containing block chain.
RenderBox* cb = const_cast<RenderBox*>(box);
- RenderBox* cbToUse = nullptr;
- while (!cb->isRenderFlowThread() && !cbToUse) {
+ while (!cb->isRenderFlowThread()) {
+ InlineElementBox* boxWrapper = cb->inlineBoxWrapper();
+ if (boxWrapper && boxWrapper->root().containingRegion()) {
+ startRegion = endRegion = boxWrapper->root().containingRegion();
+ ASSERT(m_regionList.contains(startRegion));
+ return true;
+ }
+
// FIXME: Use the containingBlock() value once we patch all the layout systems to be region range aware
// (e.g. if we use containingBlock() the shadow controls of a video element won't get the range from the
// video box because it's not a block; they need to be patched separately).
@@ -818,17 +826,15 @@
cb = cb->parent()->enclosingBox();
ASSERT(cb);
- if (hasCachedRegionRangeForBox(cb))
- cbToUse = cb;
+ // If a box doesn't have a cached region range it usually means the box belongs to a line so startRegion should be equal with endRegion.
+ // FIXME: Find the cases when this startRegion should not be equal with endRegion and make sure these boxes have cached region ranges.
+ if (hasCachedRegionRangeForBox(cb)) {
+ startRegion = endRegion = const_cast<RenderFlowThread*>(this)->regionAtBlockOffset(cb, box->offsetFromLogicalTopOfFirstPage(), true, DisallowRegionAutoGeneration);
+ return true;
+ }
}
- // If a box doesn't have a cached region range it usually means the box belongs to a line so startRegion should be equal with endRegion.
- // FIXME: Find the cases when this startRegion should not be equal with endRegion and make sure these boxes have cached region ranges.
- if (cbToUse) {
- startRegion = endRegion = const_cast<RenderFlowThread*>(this)->regionAtBlockOffset(cbToUse, box->offsetFromLogicalTopOfFirstPage(), true, DisallowRegionAutoGeneration);
- return true;
- }
-
+ ASSERT_NOT_REACHED();
return false;
}
@@ -864,7 +870,7 @@
RenderRegion* enclosingBoxEndRegion = nullptr;
// If the box has no range, do not check regionInRange. Boxes inside inlines do not get ranges.
// Instead, the containing RootInlineBox will abort when trying to paint inside the wrong region.
- if (getRegionRangeForBox(enclosingBox, enclosingBoxStartRegion, enclosingBoxEndRegion)
+ if (computedRegionRangeForBox(enclosingBox, enclosingBoxStartRegion, enclosingBoxEndRegion)
&& !regionInRange(region, enclosingBoxStartRegion, enclosingBoxEndRegion))
return false;
Modified: trunk/Source/WebCore/rendering/RenderFlowThread.h (168790 => 168791)
--- trunk/Source/WebCore/rendering/RenderFlowThread.h 2014-05-14 06:32:56 UTC (rev 168790)
+++ trunk/Source/WebCore/rendering/RenderFlowThread.h 2014-05-14 06:33:48 UTC (rev 168791)
@@ -143,6 +143,7 @@
virtual void setRegionRangeForBox(const RenderBox*, RenderRegion*, RenderRegion*);
bool getRegionRangeForBox(const RenderBox*, RenderRegion*& startRegion, RenderRegion*& endRegion) const;
+ bool computedRegionRangeForBox(const RenderBox*, RenderRegion*& startRegion, RenderRegion*& endRegion) const;
bool hasCachedRegionRangeForBox(const RenderBox*) const;
// Check if the object is in region and the region is part of this flow thread.
Modified: trunk/Source/WebCore/rendering/RenderNamedFlowThread.cpp (168790 => 168791)
--- trunk/Source/WebCore/rendering/RenderNamedFlowThread.cpp 2014-05-14 06:32:56 UTC (rev 168790)
+++ trunk/Source/WebCore/rendering/RenderNamedFlowThread.cpp 2014-05-14 06:33:48 UTC (rev 168791)
@@ -820,7 +820,7 @@
RenderRegion* startRegion = nullptr;
RenderRegion* endRegion = nullptr;
// If the box doesn't have a range, we don't know how it is fragmented so fallback to the default behaviour.
- if (!getRegionRangeForBox(renderer, startRegion, endRegion))
+ if (!computedRegionRangeForBox(renderer, startRegion, endRegion))
return false;
for (auto iter = m_regionList.find(startRegion), end = m_regionList.end(); iter != end; ++iter) {
