Bug 125770 - [CSS Regions][CSS Shapes] ASSERTION FAILED: m_segmentRanges.size() < m_segments.size()
Summary: [CSS Regions][CSS Shapes] ASSERTION FAILED: m_segmentRanges.size() < m_segmen...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Zoltan Horvath
URL:
Keywords:
Depends on:
Blocks: 89256
  Show dependency treegraph
 
Reported: 2013-12-16 01:35 PST by Zoltan Horvath
Modified: 2014-02-07 09:42 PST (History)
5 users (show)

See Also:


Attachments
Patch (5.88 KB, patch)
2013-12-16 01:53 PST, Zoltan Horvath
no flags Details | Formatted Diff | Diff
Patch - address Darin's comment (5.92 KB, patch)
2013-12-16 13:06 PST, Zoltan Horvath
bjonesbe: review-
bjonesbe: commit-queue-
Details | Formatted Diff | Diff
Patch (6.47 KB, patch)
2014-01-06 15:34 PST, Zoltan Horvath
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zoltan Horvath 2013-12-16 01:35:40 PST
If we have an e.g. up-side-down triangle, then at the bottom part of the triangle the content doesn't fit, then it needs to be adjusted. The adjustment to the next region happens correctly, but the current region/shape doesn't get updated, which leads to the the shape mismatch, which leads to layout error / assertion.
Comment 1 Zoltan Horvath 2013-12-16 01:53:38 PST
Created attachment 219304 [details]
Patch
Comment 2 Darin Adler 2013-12-16 10:04:52 PST
Comment on attachment 219304 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=219304&action=review

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:1231
> +        RenderRegion* regionAtNewLogicalHeight = regionAtBlockOffset(logicalHeight());
> +        if (!currentRegion->isLastRegion() && currentRegion != regionAtNewLogicalHeight) {
> +            currentRegion = regionAtNewLogicalHeight;
> +            shapeInsideInfo = currentRegion->shapeInsideInfo();
> +        }

What if regionAtBlockOffset returns nullptr?
Comment 3 Zoltan Horvath 2013-12-16 13:06:12 PST
Created attachment 219345 [details]
Patch - address Darin's comment

> What if regionAtBlockOffset returns nullptr?

It should never happen at this point. I added an ASSERT for checking it.
Comment 4 Bem Jones-Bey 2014-01-06 12:00:06 PST
Comment on attachment 219345 [details]
Patch - address Darin's comment

View in context: https://bugs.webkit.org/attachment.cgi?id=219345&action=review

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:1226
> +        // flows into the next region with a shape, we need to update the current shape and region

This code seems like it belongs in the "overflowsToNextRegion" case above, as it is very strange to have two different places in the same function that move to the next region, and it also makes the code hard to follow. If it doesn't make the patch too big, I would prefer if this is refactored to make that possible. If not, then a FIXME comment here is ok, with a reference to a bug to do the refactor.
Comment 5 Zoltan Horvath 2014-01-06 15:34:29 PST
Created attachment 220463 [details]
Patch
Comment 6 Bem Jones-Bey 2014-01-06 15:50:28 PST
Comment on attachment 220463 [details]
Patch

That looks a lot better!
Comment 7 WebKit Commit Bot 2014-01-06 16:38:50 PST
Comment on attachment 220463 [details]
Patch

Clearing flags on attachment: 220463

Committed r161384: <http://trac.webkit.org/changeset/161384>
Comment 8 WebKit Commit Bot 2014-01-06 16:38:52 PST
All reviewed patches have been landed.  Closing bug.