Compute starting and ending regions for boxes and clamp to them. This is because any descendants should be considered overflow out of the block, and so ultimately will render in the region range that the containing block begins/ends in.
Created attachment 109983 [details] Patch
Comment on attachment 109983 [details] Patch Minusing. I left some code commented out.
Created attachment 109985 [details] Patch
Attachment 109983 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1 Source/WebCore/rendering/RenderRegion.h:70: The parameter name "box" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/rendering/RenderFlowThread.cpp:652: Should have a space between // and comment [whitespace/comments] [4] Source/WebCore/rendering/RenderFlowThread.cpp:686: Should have a space between // and comment [whitespace/comments] [4] Total errors found: 3 in 9 files If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 109985 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1 Source/WebCore/rendering/RenderRegion.h:70: The parameter name "box" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 9 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 109985 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=109985&action=review > Source/WebCore/rendering/RenderBlock.cpp:1231 > + setLogicalHeight(INT_MAX / 2); // FIXME: With the eventual refactoring of logical height computation to be region-aware, we can avoid this hack. This is better written using numeric_limits<LayoutUnit>::max(). > Source/WebCore/rendering/RenderFlowThread.cpp:574 > + if (region == endRegion) > + break; Perhaps this should be part of the loop condition? > Source/WebCore/rendering/RenderFlowThread.cpp:653 > + if (region == endRegion) > + break; Ditto > Source/WebCore/rendering/RenderFlowThread.cpp:687 > + if (region == endRegion) > + break; Ditto > Source/WebCore/rendering/RenderFlowThread.cpp:765 > + RenderRegion* endRegion = box->contentLogicalHeight() < 0 ? lastRegion() : renderRegionForLine(offsetFromLogicalTopOfFirstPage + box->logicalHeight(), box, true); As discussed on IRC, please remove the impossible < 0 case. > Source/WebCore/rendering/RenderFlowThread.cpp:783 > + if (region == range->endRegion()) > + break; Perhaps this should be part of the loop condition? > Source/WebCore/rendering/RenderFlowThread.cpp:790 > + m_regionRangeMap.set(box, range); This isn’t super-hot code, so it might not make any practical difference, but the more efficient idiom for this is to do an add() at the beginning instead of a get(). Then you just set the new region into the iterator from add(), saving a hash lookup. > Source/WebCore/rendering/RenderFlowThread.h:122 > + void regionRangeForBox(const RenderBox*, RenderRegion*& startRegion, RenderRegion*& endRegion) const; Perhaps this should be called getRegionRangeForBox, since it returns the result in out parameters. > Source/WebCore/rendering/RenderFlowThread.h:171 > + // A maps from RenderBox A maps? > Source/WebCore/rendering/RenderRegion.h:70 > + void removeRenderBoxRegionInfo(const RenderBox* box); You should drop the parameter name here.
Fixed in r96842.