Bug 139393 - ASSERTION FAILED: opportunitiesInRun <= expansionOpportunityCount in WebCore::computeExpansionForJustifiedText
Summary: ASSERTION FAILED: opportunitiesInRun <= expansionOpportunityCount in WebCore:...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: zalan
URL:
Keywords: InRadar
Depends on:
Blocks: 116980
  Show dependency treegraph
 
Reported: 2014-12-08 05:47 PST by Renata Hodovan
Modified: 2016-08-30 10:59 PDT (History)
10 users (show)

See Also:


Attachments
Test case (113 bytes, text/html)
2014-12-08 05:47 PST, Renata Hodovan
no flags Details
Patch (3.75 KB, patch)
2016-08-25 16:27 PDT, zalan
mmaxfield: review-
Details | Formatted Diff | Diff
Patch (8.28 KB, patch)
2016-08-28 18:21 PDT, zalan
no flags Details | Formatted Diff | Diff
Patch (13.04 KB, patch)
2016-08-30 09:22 PDT, zalan
no flags Details | Formatted Diff | Diff
Patch (13.05 KB, patch)
2016-08-30 10:28 PDT, zalan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Renata Hodovan 2014-12-08 05:47:37 PST
Created attachment 242805 [details]
Test case

Run this test with debug WK:

<!DOCTYPE html>
<style>
:first-letter, * {
    white-space:pre-wrap;
}

</style>
<ruby>aa
    <rt>  </rt>
</ruby>

Note: at least 2 spaces are needed inside the <rt> tag.


Backtrace:

ASSERTION FAILED: opportunitiesInRun <= expansionOpportunityCount
../../Source/WebCore/rendering/RenderBlockLineLayout.cpp(595) : void WebCore::computeExpansionForJustifiedText(WebCore::BidiRun*, WebCore::BidiRun*, const WTF::Vector<unsigned int, 16u>&, unsigned int, float, float)

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fff98927700 (LWP 25064)]
0x00007fffedbca36f in WTFCrash () at ../../Source/WTF/wtf/Assertions.cpp:321
321	    *(int *)(uintptr_t)0xbbadbeef = 0;
#0  0x00007fffedbca36f in WTFCrash () at ../../Source/WTF/wtf/Assertions.cpp:321
#1  0x00007ffff397b317 in WebCore::computeExpansionForJustifiedText (firstRun=0x5f0e40, trailingSpaceRun=0x0, expansionOpportunities=..., expansionOpportunityCount=1, totalLogicalWidth=6, availableLogicalWidth=13) at ../../Source/WebCore/rendering/RenderBlockLineLayout.cpp:595
#2  0x00007ffff397c2c3 in WebCore::RenderBlockFlow::computeInlineDirectionPositionsForSegment (this=0x7093b0, lineBox=0x659910, lineInfo=..., textAlign=WebCore::JUSTIFY, logicalLeft=@0x7fffffff9ff4: 3.5, availableLogicalWidth=@0x7fffffff9ffc: 13, firstRun=0x5f0e40, trailingSpaceRun=0x0, textBoxDataMap=..., verticalPositionCache=..., wordMeasurements=...) at ../../Source/WebCore/rendering/RenderBlockLineLayout.cpp:777
#3  0x00007ffff397ba50 in WebCore::RenderBlockFlow::computeInlineDirectionPositionsForLine (this=0x7093b0, lineBox=0x659910, lineInfo=..., firstRun=0x5f0e40, trailingSpaceRun=0x0, reachedEnd=true, textBoxDataMap=..., verticalPositionCache=..., wordMeasurements=...) at ../../Source/WebCore/rendering/RenderBlockLineLayout.cpp:693
#4  0x00007ffff397cca1 in WebCore::RenderBlockFlow::createLineBoxesFromBidiRuns (this=0x7093b0, bidiLevel=0, bidiRuns=..., end=..., lineInfo=..., verticalPositionCache=..., trailingSpaceRun=0x0, wordMeasurements=...) at ../../Source/WebCore/rendering/RenderBlockLineLayout.cpp:993
#5  0x00007ffff397dc06 in WebCore::RenderBlockFlow::layoutRunsAndFloatsInRange (this=0x7093b0, layoutState=..., resolver=..., cleanLineStart=..., cleanLineBidiStatus=..., consecutiveHyphenatedLines=0) at ../../Source/WebCore/rendering/RenderBlockLineLayout.cpp:1185
#6  0x00007ffff397d277 in WebCore::RenderBlockFlow::layoutRunsAndFloats (this=0x7093b0, layoutState=..., hasInlineChild=true) at ../../Source/WebCore/rendering/RenderBlockLineLayout.cpp:1085
#7  0x00007ffff397faee in WebCore::RenderBlockFlow::layoutLineBoxes (this=0x7093b0, relayoutChildren=true, repaintLogicalTop=..., repaintLogicalBottom=...) at ../../Source/WebCore/rendering/RenderBlockLineLayout.cpp:1502
#8  0x00007ffff395f687 in WebCore::RenderBlockFlow::layoutInlineChildren (this=0x7093b0, relayoutChildren=true, repaintLogicalTop=..., repaintLogicalBottom=...) at ../../Source/WebCore/rendering/RenderBlockFlow.cpp:640
#9  0x00007ffff395e98a in WebCore::RenderBlockFlow::layoutBlock (this=0x7093b0, relayoutChildren=true, pageLogicalHeight=...) at ../../Source/WebCore/rendering/RenderBlockFlow.cpp:471
#10 0x00007ffff393456f in WebCore::RenderBlock::layout (this=0x7093b0) at ../../Source/WebCore/rendering/RenderBlock.cpp:931
#11 0x00007ffff3900519 in WebCore::RenderElement::layoutIfNeeded (this=0x7093b0) at ../../Source/WebCore/rendering/RenderElement.h:119
#12 0x00007ffff3ada344 in WebCore::RenderRubyRun::layoutSpecialExcludedChild (this=0x708530, relayoutChildren=true) at ../../Source/WebCore/rendering/RenderRubyRun.cpp:227
#13 0x00007ffff395f44a in WebCore::RenderBlockFlow::layoutBlockChildren (this=0x708530, relayoutChildren=true, maxFloatLogicalBottom=...) at ../../Source/WebCore/rendering/RenderBlockFlow.cpp:592
#14 0x00007ffff395e9ae in WebCore::RenderBlockFlow::layoutBlock (this=0x708530, relayoutChildren=true, pageLogicalHeight=...) at ../../Source/WebCore/rendering/RenderBlockFlow.cpp:473
#15 0x00007ffff393456f in WebCore::RenderBlock::layout (this=0x708530) at ../../Source/WebCore/rendering/RenderBlock.cpp:931
#16 0x00007ffff3ada376 in WebCore::RenderRubyRun::layout (this=0x708530) at ../../Source/WebCore/rendering/RenderRubyRun.cpp:233
#17 0x00007ffff3900519 in WebCore::RenderElement::layoutIfNeeded (this=0x708530) at ../../Source/WebCore/rendering/RenderElement.h:119
#18 0x00007ffff397faac in WebCore::RenderBlockFlow::layoutLineBoxes (this=0x6979d0, relayoutChildren=true, repaintLogicalTop=..., repaintLogicalBottom=...) at ../../Source/WebCore/rendering/RenderBlockLineLayout.cpp:1500
#19 0x00007ffff395f687 in WebCore::RenderBlockFlow::layoutInlineChildren (this=0x6979d0, relayoutChildren=true, repaintLogicalTop=..., repaintLogicalBottom=...) at ../../Source/WebCore/rendering/RenderBlockFlow.cpp:640
#20 0x00007ffff395e98a in WebCore::RenderBlockFlow::layoutBlock (this=0x6979d0, relayoutChildren=true, pageLogicalHeight=...) at ../../Source/WebCore/rendering/RenderBlockFlow.cpp:471
#21 0x00007ffff393456f in WebCore::RenderBlock::layout (this=0x6979d0) at ../../Source/WebCore/rendering/RenderBlock.cpp:931
#22 0x00007ffff395fa64 in WebCore::RenderBlockFlow::layoutBlockChild (this=0x5f65f0, child=..., marginInfo=..., previousFloatLogicalBottom=..., maxFloatLogicalBottom=...) at ../../Source/WebCore/rendering/RenderBlockFlow.cpp:699
#23 0x00007ffff395f581 in WebCore::RenderBlockFlow::layoutBlockChildren (this=0x5f65f0, relayoutChildren=true, maxFloatLogicalBottom=...) at ../../Source/WebCore/rendering/RenderBlockFlow.cpp:620
#24 0x00007ffff395e9ae in WebCore::RenderBlockFlow::layoutBlock (this=0x5f65f0, relayoutChildren=true, pageLogicalHeight=...) at ../../Source/WebCore/rendering/RenderBlockFlow.cpp:473
#25 0x00007ffff393456f in WebCore::RenderBlock::layout (this=0x5f65f0) at ../../Source/WebCore/rendering/RenderBlock.cpp:931
#26 0x00007ffff395fa64 in WebCore::RenderBlockFlow::layoutBlockChild (this=0x7d3570, child=..., marginInfo=..., previousFloatLogicalBottom=..., maxFloatLogicalBottom=...) at ../../Source/WebCore/rendering/RenderBlockFlow.cpp:699
#27 0x00007ffff395f581 in WebCore::RenderBlockFlow::layoutBlockChildren (this=0x7d3570, relayoutChildren=true, maxFloatLogicalBottom=...) at ../../Source/WebCore/rendering/RenderBlockFlow.cpp:620
#28 0x00007ffff395e9ae in WebCore::RenderBlockFlow::layoutBlock (this=0x7d3570, relayoutChildren=true, pageLogicalHeight=...) at ../../Source/WebCore/rendering/RenderBlockFlow.cpp:473
#29 0x00007ffff393456f in WebCore::RenderBlock::layout (this=0x7d3570) at ../../Source/WebCore/rendering/RenderBlock.cpp:931
#30 0x00007ffff3b2e84d in WebCore::RenderView::layoutContent (this=0x7d3570, state=...) at ../../Source/WebCore/rendering/RenderView.cpp:232
#31 0x00007ffff3b2ef1d in WebCore::RenderView::layout (this=0x7d3570) at ../../Source/WebCore/rendering/RenderView.cpp:357
#32 0x00007ffff369c389 in WebCore::FrameView::layout (this=0x7cd310, allowSubtree=true) at ../../Source/WebCore/page/FrameView.cpp:1313
#33 0x00007ffff3067dc5 in WebCore::Document::implicitClose (this=0x696980) at ../../Source/WebCore/dom/Document.cpp:2486
#34 0x00007ffff3547a0d in WebCore::FrameLoader::checkCallImplicitClose (this=0x79e748) at ../../Source/WebCore/loader/FrameLoader.cpp:898
#35 0x00007ffff3547779 in WebCore::FrameLoader::checkCompleted (this=0x79e748) at ../../Source/WebCore/loader/FrameLoader.cpp:844
#36 0x00007ffff35474e2 in WebCore::FrameLoader::finishedParsing (this=0x79e748) at ../../Source/WebCore/loader/FrameLoader.cpp:764
#37 0x00007ffff3070c99 in WebCore::Document::finishedParsing (this=0x696980) at ../../Source/WebCore/dom/Document.cpp:4615
#38 0x00007ffff33c6039 in WebCore::HTMLConstructionSite::finishedParsing (this=0x7cf148) at ../../Source/WebCore/html/parser/HTMLConstructionSite.cpp:395
#39 0x00007ffff3403a33 in WebCore::HTMLTreeBuilder::finished (this=0x7cf130) at ../../Source/WebCore/html/parser/HTMLTreeBuilder.cpp:3009
#40 0x00007ffff33ced4e in WebCore::HTMLDocumentParser::end (this=0x6e4780) at ../../Source/WebCore/html/parser/HTMLDocumentParser.cpp:439
#41 0x00007ffff33cee39 in WebCore::HTMLDocumentParser::attemptToRunDeferredScriptsAndEnd (this=0x6e4780) at ../../Source/WebCore/html/parser/HTMLDocumentParser.cpp:450
#42 0x00007ffff33cd8e7 in WebCore::HTMLDocumentParser::prepareToStopParsing (this=0x6e4780) at ../../Source/WebCore/html/parser/HTMLDocumentParser.cpp:165
#43 0x00007ffff33cee7c in WebCore::HTMLDocumentParser::attemptToEnd (this=0x6e4780) at ../../Source/WebCore/html/parser/HTMLDocumentParser.cpp:462
#44 0x00007ffff33cef33 in WebCore::HTMLDocumentParser::finish (this=0x6e4780) at ../../Source/WebCore/html/parser/HTMLDocumentParser.cpp:490
#45 0x00007ffff3538b7f in WebCore::DocumentWriter::end (this=0x73f8f0) at ../../Source/WebCore/loader/DocumentWriter.cpp:246
#46 0x00007ffff35248db in WebCore::DocumentLoader::finishedLoading (this=0x73f850, finishTime=0) at ../../Source/WebCore/loader/DocumentLoader.cpp:440
#47 0x00007ffff3524644 in WebCore::DocumentLoader::notifyFinished (this=0x73f850, resource=0x8d3260) at ../../Source/WebCore/loader/DocumentLoader.cpp:374
#48 0x00007ffff35d5370 in WebCore::CachedResource::checkNotify (this=0x8d3260) at ../../Source/WebCore/loader/cache/CachedResource.cpp:293
#49 0x00007ffff35d546e in WebCore::CachedResource::finishLoading (this=0x8d3260) at ../../Source/WebCore/loader/cache/CachedResource.cpp:309
#50 0x00007ffff35d1b63 in WebCore::CachedRawResource::finishLoading (this=0x8d3260, data=0x74e330) at ../../Source/WebCore/loader/cache/CachedRawResource.cpp:104
#51 0x00007ffff358594c in WebCore::SubresourceLoader::didFinishLoading (this=0x8d3920, finishTime=0) at ../../Source/WebCore/loader/SubresourceLoader.cpp:306
#52 0x00007ffff35816e1 in WebCore::ResourceLoader::didFinishLoading (this=0x8d3920, finishTime=0) at ../../Source/WebCore/loader/ResourceLoader.cpp:508
#53 0x00007ffff3f303e1 in WebCore::readCallback (asyncResult=0x68b1d0, data=0x8d4b40) at ../../Source/WebCore/platform/network/soup/ResourceHandleSoup.cpp:1300
#54 0x00007fffeb7ab7d6 in async_ready_callback_wrapper (source_object=0x7c66d0, res=0x68b1d0, user_data=user_data@entry=0x8d4b40) at ginputstream.c:523
#55 0x00007fffeb7d10d5 in g_task_return_now (task=0x68b1d0) at gtask.c:1077
#56 0x00007fffeb7d10f9 in complete_in_idle_cb (task=0x68b1d0) at gtask.c:1086
#57 0x00007fffeaa10a1d in g_main_dispatch (context=0x4780a0) at gmain.c:3064
#58 g_main_context_dispatch (context=context@entry=0x4780a0) at gmain.c:3663
#59 0x00007fffeaa10d88 in g_main_context_iterate (context=0x4780a0, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at gmain.c:3734
#60 0x00007fffeaa1104a in g_main_loop_run (loop=0x8eb810) at gmain.c:3928
#61 0x00007ffff45df9dc in WTF::RunLoop::run () at ../../Source/WTF/wtf/gtk/RunLoopGtk.cpp:59
#62 0x00007ffff2b44f82 in WebKit::ChildProcessMain<WebKit::WebProcess, WebKit::WebProcessMain> (argc=2, argv=0x7fffffffd978) at ../../Source/WebKit2/Shared/unix/ChildProcessMain.h:61
#63 0x00007ffff2b44de7 in WebKit::WebProcessMainUnix (argc=2, argv=0x7fffffffd978) at ../../Source/WebKit2/WebProcess/gtk/WebProcessMainGtk.cpp:73
#64 0x0000000000400891 in main (argc=2, argv=0x7fffffffd978) at ../../Source/WebKit2/WebProcess/EntryPoint/unix/WebProcessMain.cpp:44
Comment 1 Renata Hodovan 2015-01-29 06:42:51 PST
It's reported to Blink too. Although, it's not fixed there either but it worth to keep an eye on it: crbug.com/381574.
Comment 2 Renata Hodovan 2016-03-11 04:44:12 PST
It's still valid on ToT WebKit.
Validated on r197952 both with Mac and EFL builds.
Comment 3 Brent Fulgham 2016-08-04 11:53:56 PDT
Reproduces in r204037.
Comment 4 Radar WebKit Bug Importer 2016-08-04 11:54:10 PDT
<rdar://problem/27704243>
Comment 5 zalan 2016-08-25 16:27:36 PDT
Created attachment 287040 [details]
Patch
Comment 6 Dave Hyatt 2016-08-25 16:28:58 PDT
Comment on attachment 287040 [details]
Patch

r=me
Comment 7 Myles C. Maxfield 2016-08-25 16:34:44 PDT
Comment on attachment 287040 [details]
Patch

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

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:939
> +        expansionOpportunityCount -= expansionOpportunities.last()--;

Yuck no please ☹️😱😵
Comment 8 Myles C. Maxfield 2016-08-25 16:36:03 PDT
Comment on attachment 287040 [details]
Patch

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

>> Source/WebCore/rendering/RenderBlockLineLayout.cpp:939
>> +        expansionOpportunityCount -= expansionOpportunities.last()--;
> 
> Yuck no please ☹️😱😵

Why do you know that expansionOpportunities.last() will either be 0 or 1?

I think this patch is incorrect.
Comment 9 Myles C. Maxfield 2016-08-25 16:55:13 PDT
Comment on attachment 287040 [details]
Patch

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

>>> Source/WebCore/rendering/RenderBlockLineLayout.cpp:939
>>> +        expansionOpportunityCount -= expansionOpportunities.last()--;
>> 
>> Yuck no please ☹️😱😵
> 
> Why do you know that expansionOpportunities.last() will either be 0 or 1?
> 
> I think this patch is incorrect.

How do we get into a situation where platform/ code thinks we are after an expansion opportunity, but there's nothing in the array?

I think the broken code here is deeper. Perhaps there should be an ASSERT() here instead.
Comment 10 zalan 2016-08-28 18:21:50 PDT
Created attachment 287246 [details]
Patch
Comment 11 Myles C. Maxfield 2016-08-29 13:24:50 PDT
Comment on attachment 287246 [details]
Patch

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

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:946
> +            ASSERT(expansionOpportunities.at(lastValidExpansionOpportunitiesIndex));

I think we should add an ASSERT() as well as a conditional to FontCascade::expansionOpportunityCountInternal() where we decrement "count" (to make sure it doesn't underflow).

Ultimately, it looks like FontCascade::expansionOpportunityCountInternal() doesn't really handle empty runs well.

> Source/WebCore/rendering/RenderBlockLineLayout.cpp:948
> +            expansionOpportunityCount--;

This whole thing seems pretty broken because WidthIterator::WidthIterator() and ComplexTextController::ComplexTextController() directly call FontCascade::expansionOpportunityCount(), but they don't decrement the results in this same way. In the future, we should probably use applyExpansionBehavior() to actually forbid the trailing expansion at the InlineBox level, so later paint calls will appropriately handle the removal of the last opportunity.

Anyway, that doesn't need to be done in this patch.
Comment 12 zalan 2016-08-30 09:22:42 PDT
Created attachment 287398 [details]
Patch
Comment 13 WebKit Commit Bot 2016-08-30 10:27:23 PDT
Comment on attachment 287398 [details]
Patch

Rejecting attachment 287398 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-03', 'validate-changelog', '--check-oops', '--non-interactive', 287398, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Myles Maxfield found in /Volumes/Data/EWS/WebKit/LayoutTests/ChangeLog does not appear to be a valid reviewer according to contributors.json.
/Volumes/Data/EWS/WebKit/LayoutTests/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://webkit-queues.webkit.org/results/1973988
Comment 14 zalan 2016-08-30 10:28:45 PDT
Created attachment 287405 [details]
Patch
Comment 15 WebKit Commit Bot 2016-08-30 10:59:35 PDT
Comment on attachment 287405 [details]
Patch

Clearing flags on attachment: 287405

Committed r205186: <http://trac.webkit.org/changeset/205186>
Comment 16 WebKit Commit Bot 2016-08-30 10:59:41 PDT
All reviewed patches have been landed.  Closing bug.