- Revision
- 135750
- Author
- commit-qu...@webkit.org
- Date
- 2012-11-26 12:00:49 -0800 (Mon, 26 Nov 2012)
Log Message
[CSS Regions] Add Region info for RootLineBoxes and pack the pagination data
https://bugs.webkit.org/show_bug.cgi?id=101332
Patch by Andrei Bucur <abu...@adobe.com> on 2012-11-26
Reviewed by David Hyatt.
Source/WebCore:
Currently the pagination information for lines is spread between the RootInlineBox and InlineFlowBox classes, consuming memory even though
the boxes were not the result of an pagination layout. To overcome this, a new struct (LineFragmentationData) is created that wraps all the data,
including two new members, the containing Region for the line and a boolean that states if the line was laid out in a Region or not.
The flag is necessary because the sanitize function on LineFragmentationData resets the containing Region to 0 if the Region was removed from
chain (so a value of 0 for the containing Region means two things). The sanitize function should prevent access to an invalid address.
The containing Region is used to detect if a line changed the Region where it resides. This will be helpful especially when implementing region
styling for layout properties (e.g. the font-size property https://bugs.webkit.org/show_bug.cgi?id=95559 ).
A line can change the region when it is shifted inside the containing block or if the entire block moves. This means it's better to delegate
the task of updating the containing Region to the block.
Tests: fast/regions/line-containing-region-crash.html
* rendering/InlineFlowBox.cpp:
(SameSizeAsInlineFlowBox):
* rendering/InlineFlowBox.h:
(WebCore::InlineFlowBox::InlineFlowBox):
(InlineFlowBox):
* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::lineWidthForPaginatedLineChanged):
* rendering/RenderBlockLineLayout.cpp:
(WebCore::RenderBlock::layoutRunsAndFloatsInRange):
(WebCore::RenderBlock::linkToEndLineIfNeeded):
(WebCore::RenderBlock::determineStartPosition):
* rendering/RootInlineBox.cpp:
(WebCore::RootInlineBox::RootInlineBox):
(WebCore::RootInlineBox::setContainingRegion):
(WebCore):
(WebCore::RootInlineBox::LineFragmentationData::sanitize): This is an O(1) function that checks if the containig Region is still valid pointer.
* rendering/RootInlineBox.h:
(WebCore):
(WebCore::RootInlineBox::paginationStrut):
(WebCore::RootInlineBox::setPaginationStrut):
(WebCore::RootInlineBox::isFirstAfterPageBreak):
(WebCore::RootInlineBox::setIsFirstAfterPageBreak):
(WebCore::RootInlineBox::paginatedLineWidth):
(WebCore::RootInlineBox::setPaginatedLineWidth):
(RootInlineBox):
(WebCore::RootInlineBox::containingRegion):
(WebCore::RootInlineBox::hasContainingRegion): Use this to determine if the line has a region or not.
(WebCore::RootInlineBox::ensureLineFragmentationData):
(LineFragmentationData):
(WebCore::RootInlineBox::LineFragmentationData::LineFragmentationData):
LayoutTests:
The test checks if there is a crash when doing a line layout if:
- the flow has no region
- the flow has a region but the lines have no containing region
- the flow has no region but the lines have a containing region
* fast/regions/line-containing-region-crash-expected.txt: Added.
* fast/regions/line-containing-region-crash.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (135749 => 135750)
--- trunk/LayoutTests/ChangeLog 2012-11-26 19:57:40 UTC (rev 135749)
+++ trunk/LayoutTests/ChangeLog 2012-11-26 20:00:49 UTC (rev 135750)
@@ -1,3 +1,18 @@
+2012-11-26 Andrei Bucur <abu...@adobe.com>
+
+ [CSS Regions] Add Region info for RootLineBoxes and pack the pagination data
+ https://bugs.webkit.org/show_bug.cgi?id=101332
+
+ Reviewed by David Hyatt.
+
+ The test checks if there is a crash when doing a line layout if:
+ - the flow has no region
+ - the flow has a region but the lines have no containing region
+ - the flow has no region but the lines have a containing region
+
+ * fast/regions/line-containing-region-crash-expected.txt: Added.
+ * fast/regions/line-containing-region-crash.html: Added.
+
2012-11-26 Michelangelo De Simone <michelang...@webkit.org>
[CSS Shaders] Add IDL file and bindings for mix function
Added: trunk/LayoutTests/fast/regions/line-containing-region-crash-expected.txt (0 => 135750)
--- trunk/LayoutTests/fast/regions/line-containing-region-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/regions/line-containing-region-crash-expected.txt 2012-11-26 20:00:49 UTC (rev 135750)
@@ -0,0 +1,8 @@
+Test for WebKit Bug 101332 Crash
+
+The test passes if it does not crash or assert.
+
+PASS
+
+Hello Crash!
+
Added: trunk/LayoutTests/fast/regions/line-containing-region-crash.html (0 => 135750)
--- trunk/LayoutTests/fast/regions/line-containing-region-crash.html (rev 0)
+++ trunk/LayoutTests/fast/regions/line-containing-region-crash.html 2012-11-26 20:00:49 UTC (rev 135750)
@@ -0,0 +1,38 @@
+<!doctype html>
+<html>
+<head>
+ <style>
+ #content {
+ width: 500px;
+ height: 150px;
+ -webkit-flow-into: flow;
+ }
+ .region {
+ -webkit-flow-from: flow;
+ width: 500px;
+ height: 500px;
+ }
+ </style>
+</head>
+<body>
+ <p>Test for <a href="" Bug 101332</a> Crash </p>
+ <p>The test passes if it does not crash or assert.</p>
+ <p>PASS</p>
+ <div id="content">
+ Hello Crash!
+ </div>
+ <div id="region"></div>
+ <script>
+ if (window.testRunner)
+ window.testRunner.dumpAsText();
+ var content = document.getElementById("content");
+ content.style.height = "200px";
+ document.body.offsetTop;
+ var region = document.getElementById("region");
+ region.className = "region";
+ document.body.offsetTop;
+ region.className = "noregion";
+ document.body.offsetTop;
+ </script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (135749 => 135750)
--- trunk/Source/WebCore/ChangeLog 2012-11-26 19:57:40 UTC (rev 135749)
+++ trunk/Source/WebCore/ChangeLog 2012-11-26 20:00:49 UTC (rev 135750)
@@ -1,3 +1,54 @@
+2012-11-26 Andrei Bucur <abu...@adobe.com>
+
+ [CSS Regions] Add Region info for RootLineBoxes and pack the pagination data
+ https://bugs.webkit.org/show_bug.cgi?id=101332
+
+ Reviewed by David Hyatt.
+
+ Currently the pagination information for lines is spread between the RootInlineBox and InlineFlowBox classes, consuming memory even though
+ the boxes were not the result of an pagination layout. To overcome this, a new struct (LineFragmentationData) is created that wraps all the data,
+ including two new members, the containing Region for the line and a boolean that states if the line was laid out in a Region or not.
+ The flag is necessary because the sanitize function on LineFragmentationData resets the containing Region to 0 if the Region was removed from
+ chain (so a value of 0 for the containing Region means two things). The sanitize function should prevent access to an invalid address.
+ The containing Region is used to detect if a line changed the Region where it resides. This will be helpful especially when implementing region
+ styling for layout properties (e.g. the font-size property https://bugs.webkit.org/show_bug.cgi?id=95559 ).
+ A line can change the region when it is shifted inside the containing block or if the entire block moves. This means it's better to delegate
+ the task of updating the containing Region to the block.
+
+ Tests: fast/regions/line-containing-region-crash.html
+
+ * rendering/InlineFlowBox.cpp:
+ (SameSizeAsInlineFlowBox):
+ * rendering/InlineFlowBox.h:
+ (WebCore::InlineFlowBox::InlineFlowBox):
+ (InlineFlowBox):
+ * rendering/RenderBlock.cpp:
+ (WebCore::RenderBlock::lineWidthForPaginatedLineChanged):
+ * rendering/RenderBlockLineLayout.cpp:
+ (WebCore::RenderBlock::layoutRunsAndFloatsInRange):
+ (WebCore::RenderBlock::linkToEndLineIfNeeded):
+ (WebCore::RenderBlock::determineStartPosition):
+ * rendering/RootInlineBox.cpp:
+ (WebCore::RootInlineBox::RootInlineBox):
+ (WebCore::RootInlineBox::setContainingRegion):
+ (WebCore):
+ (WebCore::RootInlineBox::LineFragmentationData::sanitize): This is an O(1) function that checks if the containig Region is still valid pointer.
+ * rendering/RootInlineBox.h:
+ (WebCore):
+ (WebCore::RootInlineBox::paginationStrut):
+ (WebCore::RootInlineBox::setPaginationStrut):
+ (WebCore::RootInlineBox::isFirstAfterPageBreak):
+ (WebCore::RootInlineBox::setIsFirstAfterPageBreak):
+ (WebCore::RootInlineBox::paginatedLineWidth):
+ (WebCore::RootInlineBox::setPaginatedLineWidth):
+ (RootInlineBox):
+ (WebCore::RootInlineBox::containingRegion):
+ (WebCore::RootInlineBox::hasContainingRegion): Use this to determine if the line has a region or not.
+ (WebCore::RootInlineBox::ensureLineFragmentationData):
+ (LineFragmentationData):
+ (WebCore::RootInlineBox::LineFragmentationData::LineFragmentationData):
+
+
2012-11-26 Michelangelo De Simone <michelang...@webkit.org>
[CSS Shaders] Add IDL file and bindings for mix function
Modified: trunk/Source/WebCore/rendering/InlineFlowBox.cpp (135749 => 135750)
--- trunk/Source/WebCore/rendering/InlineFlowBox.cpp 2012-11-26 19:57:40 UTC (rev 135749)
+++ trunk/Source/WebCore/rendering/InlineFlowBox.cpp 2012-11-26 20:00:49 UTC (rev 135750)
@@ -48,7 +48,7 @@
struct SameSizeAsInlineFlowBox : public InlineBox {
void* pointers[5];
- uint32_t bitfields : 24;
+ uint32_t bitfields : 23;
};
COMPILE_ASSERT(sizeof(InlineFlowBox) == sizeof(SameSizeAsInlineFlowBox), InlineFlowBox_should_stay_small);
Modified: trunk/Source/WebCore/rendering/InlineFlowBox.h (135749 => 135750)
--- trunk/Source/WebCore/rendering/InlineFlowBox.h 2012-11-26 19:57:40 UTC (rev 135749)
+++ trunk/Source/WebCore/rendering/InlineFlowBox.h 2012-11-26 20:00:49 UTC (rev 135750)
@@ -52,7 +52,6 @@
, m_baselineType(AlphabeticBaseline)
, m_hasAnnotationsBefore(false)
, m_hasAnnotationsAfter(false)
- , m_isFirstAfterPageBreak(false)
#ifndef NDEBUG
, m_hasBadChildList(false)
#endif
@@ -325,7 +324,6 @@
// If the line contains any ruby runs, then this will be true.
unsigned m_hasAnnotationsBefore : 1;
unsigned m_hasAnnotationsAfter : 1;
- unsigned m_isFirstAfterPageBreak : 1;
unsigned m_lineBreakBidiStatusEor : 5; // WTF::Unicode::Direction
unsigned m_lineBreakBidiStatusLastStrong : 5; // WTF::Unicode::Direction
Modified: trunk/Source/WebCore/rendering/RenderBlock.cpp (135749 => 135750)
--- trunk/Source/WebCore/rendering/RenderBlock.cpp 2012-11-26 19:57:40 UTC (rev 135749)
+++ trunk/Source/WebCore/rendering/RenderBlock.cpp 2012-11-26 20:00:49 UTC (rev 135750)
@@ -7245,7 +7245,14 @@
if (!inRenderFlowThread())
return false;
- return rootBox->paginatedLineWidth() != availableLogicalWidthForContent(rootBox->lineTopWithLeading() + lineDelta);
+ RenderRegion* currentRegion = regionAtBlockOffset(rootBox->lineTopWithLeading() + lineDelta);
+ // Just bail if we still don't have a region.
+ if (!rootBox->hasContainingRegion() && !currentRegion)
+ return false;
+ // Just bail if the region didn't change.
+ if (rootBox->hasContainingRegion() && rootBox->containingRegion() == currentRegion)
+ return false;
+ return rootBox->paginatedLineWidth() != availableLogicalWidthForContent(currentRegion, offsetFromLogicalTopOfFirstPage());
}
LayoutUnit RenderBlock::offsetFromLogicalTopOfFirstPage() const
Modified: trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp (135749 => 135750)
--- trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp 2012-11-26 19:57:40 UTC (rev 135749)
+++ trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp 2012-11-26 20:00:49 UTC (rev 135750)
@@ -1508,6 +1508,9 @@
setLogicalHeight(lineBox->lineBottomWithLeading());
}
+
+ if (inRenderFlowThread())
+ lineBox->setContainingRegion(regionAtBlockOffset(lineBox->lineTopWithLeading()));
}
}
@@ -1562,6 +1565,8 @@
layoutState.updateRepaintRangeFromBox(line, delta);
line->adjustBlockDirectionPosition(delta);
}
+ if (inRenderFlowThread())
+ line->setContainingRegion(regionAtBlockOffset(line->lineTopWithLeading()));
if (Vector<RenderBox*>* cleanLineFloats = line->floatsPtr()) {
Vector<RenderBox*>::iterator end = cleanLineFloats->end();
for (Vector<RenderBox*>::iterator f = cleanLineFloats->begin(); f != end; ++f) {
@@ -1599,6 +1604,8 @@
LayoutRect logicalLayoutOverflow(0, blockLogicalHeight, 1, bottomLayoutOverflow - blockLogicalHeight);
LayoutRect logicalVisualOverflow(0, blockLogicalHeight, 1, bottomVisualOverflow - blockLogicalHeight);
trailingFloatsLineBox->setOverflowFromLogicalRects(logicalLayoutOverflow, logicalVisualOverflow, trailingFloatsLineBox->lineTop(), trailingFloatsLineBox->lineBottom());
+ if (inRenderFlowThread())
+ trailingFloatsLineBox->setContainingRegion(regionAtBlockOffset(trailingFloatsLineBox->lineTopWithLeading()));
}
const FloatingObjectSet& floatingObjectSet = m_floatingObjects->set();
@@ -1793,6 +1800,8 @@
layoutState.updateRepaintRangeFromBox(curr, paginationDelta);
curr->adjustBlockDirectionPosition(paginationDelta);
}
+ if (inRenderFlowThread())
+ curr->setContainingRegion(regionAtBlockOffset(curr->lineTopWithLeading()));
}
// If a new float has been inserted before this line or before its last known float, just do a full layout.
Modified: trunk/Source/WebCore/rendering/RootInlineBox.cpp (135749 => 135750)
--- trunk/Source/WebCore/rendering/RootInlineBox.cpp 2012-11-26 19:57:40 UTC (rev 135749)
+++ trunk/Source/WebCore/rendering/RootInlineBox.cpp 2012-11-26 20:00:49 UTC (rev 135750)
@@ -33,6 +33,7 @@
#include "PaintInfo.h"
#include "RenderArena.h"
#include "RenderBlock.h"
+#include "RenderFlowThread.h"
#include "RenderView.h"
#include "VerticalPositionCache.h"
#include <wtf/unicode/Unicode.h>
@@ -52,8 +53,6 @@
, m_lineBottom(0)
, m_lineTopWithLeading(0)
, m_lineBottomWithLeading(0)
- , m_paginationStrut(0)
- , m_paginatedLineWidth(0)
{
setIsHorizontal(block->isHorizontalWritingMode());
}
@@ -251,6 +250,30 @@
}
}
+void RootInlineBox::setContainingRegion(RenderRegion* region)
+{
+ ASSERT(!isDirty());
+ ASSERT(block()->inRenderFlowThread());
+ LineFragmentationData* fragmentationData = ensureLineFragmentationData();
+ fragmentationData->m_containingRegion = region;
+ fragmentationData->m_hasContainingRegion = !!region;
+}
+
+RootInlineBox::LineFragmentationData* RootInlineBox::LineFragmentationData::sanitize(const RenderBlock* block)
+{
+ ASSERT(block->inRenderFlowThread());
+ if (!m_containingRegion)
+ return this;
+
+ RenderFlowThread* flowThread = block->enclosingRenderFlowThread();
+ const RenderRegionList& regionList = flowThread->renderRegionList();
+ // For pointer types the hash function is |safeToCompareToEmptyOrDeleted|. There shouldn't be any problems if m_containingRegion was deleted.
+ if (!regionList.contains(m_containingRegion))
+ m_containingRegion = 0;
+
+ return this;
+}
+
LayoutUnit RootInlineBox::alignBoxesInBlockDirection(LayoutUnit heightOfBlock, GlyphOverflowAndFallbackFontsMap& textBoxDataMap, VerticalPositionCache& verticalPositionCache)
{
#if ENABLE(SVG)
Modified: trunk/Source/WebCore/rendering/RootInlineBox.h (135749 => 135750)
--- trunk/Source/WebCore/rendering/RootInlineBox.h 2012-11-26 19:57:40 UTC (rev 135749)
+++ trunk/Source/WebCore/rendering/RootInlineBox.h 2012-11-26 20:00:49 UTC (rev 135750)
@@ -28,6 +28,7 @@
class EllipsisBox;
class HitTestResult;
+class RenderRegion;
struct BidiStatus;
struct GapRects;
@@ -53,15 +54,19 @@
LayoutUnit lineTopWithLeading() const { return m_lineTopWithLeading; }
LayoutUnit lineBottomWithLeading() const { return m_lineBottomWithLeading; }
- LayoutUnit paginationStrut() const { return m_paginationStrut; }
- void setPaginationStrut(LayoutUnit s) { m_paginationStrut = s; }
+ LayoutUnit paginationStrut() const { return m_fragmentationData ? m_fragmentationData->m_paginationStrut : LayoutUnit(0); }
+ void setPaginationStrut(LayoutUnit strut) { ensureLineFragmentationData()->m_paginationStrut = strut; }
- bool isFirstAfterPageBreak() const { return m_isFirstAfterPageBreak; }
- void setIsFirstAfterPageBreak(bool isFirstAfterPageBreak) { m_isFirstAfterPageBreak = isFirstAfterPageBreak; }
+ bool isFirstAfterPageBreak() const { return m_fragmentationData ? m_fragmentationData->m_isFirstAfterPageBreak : false; }
+ void setIsFirstAfterPageBreak(bool isFirstAfterPageBreak) { ensureLineFragmentationData()->m_isFirstAfterPageBreak = isFirstAfterPageBreak; }
- LayoutUnit paginatedLineWidth() const { return m_paginatedLineWidth; }
- void setPaginatedLineWidth(LayoutUnit width) { m_paginatedLineWidth = width; }
+ LayoutUnit paginatedLineWidth() const { return m_fragmentationData ? m_fragmentationData->m_paginatedLineWidth : LayoutUnit(0); }
+ void setPaginatedLineWidth(LayoutUnit width) { ensureLineFragmentationData()->m_paginatedLineWidth = width; }
+ RenderRegion* containingRegion() const { return m_fragmentationData ? m_fragmentationData->sanitize(block())->m_containingRegion : 0; }
+ bool hasContainingRegion() const { return m_fragmentationData ? m_fragmentationData->m_hasContainingRegion : false; }
+ void setContainingRegion(RenderRegion*);
+
LayoutUnit selectionTop() const;
LayoutUnit selectionBottom() const;
LayoutUnit selectionHeight() const { return max<LayoutUnit>(0, selectionBottom() - selectionTop()); }
@@ -193,6 +198,15 @@
LayoutUnit beforeAnnotationsAdjustment() const;
+ struct LineFragmentationData;
+ LineFragmentationData* ensureLineFragmentationData()
+ {
+ if (!m_fragmentationData)
+ m_fragmentationData = adoptPtr(new LineFragmentationData());
+
+ return m_fragmentationData.get();
+ }
+
// This folds into the padding at the end of InlineFlowBox on 64-bit.
unsigned m_lineBreakPos;
@@ -207,9 +221,33 @@
LayoutUnit m_lineTopWithLeading;
LayoutUnit m_lineBottomWithLeading;
- LayoutUnit m_paginationStrut;
- LayoutUnit m_paginatedLineWidth;
+ struct LineFragmentationData {
+ WTF_MAKE_NONCOPYABLE(LineFragmentationData); WTF_MAKE_FAST_ALLOCATED;
+ public:
+ LineFragmentationData()
+ : m_containingRegion(0)
+ , m_paginationStrut(0)
+ , m_paginatedLineWidth(0)
+ , m_isFirstAfterPageBreak(false)
+ , m_hasContainingRegion(false)
+ {
+ }
+
+ LineFragmentationData* sanitize(const RenderBlock*);
+
+ // It should not be assumed the |containingRegion| is always valid.
+ // It can also be 0 if the flow has no region chain or an invalid pointer if the region is no longer in the chain.
+ // Use |sanitize| to filter an invalid region.
+ RenderRegion* m_containingRegion;
+ LayoutUnit m_paginationStrut;
+ LayoutUnit m_paginatedLineWidth;
+ unsigned m_isFirstAfterPageBreak : 1;
+ unsigned m_hasContainingRegion : 1; // We need to keep this to differentiate between the case of a void region and an invalid region.
+ };
+
+ OwnPtr<LineFragmentationData> m_fragmentationData;
+
// Floats hanging off the line are pushed into this vector during layout. It is only
// good for as long as the line has not been marked dirty.
OwnPtr<Vector<RenderBox*> > m_floats;
