Bug 69267

Summary: Assertion failure (m_nestedIsolateCount >= 1) in BidiResolver::exitIsolate()
Product: WebKit Reporter: mitz
Component: Layout and RenderingAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: eric, rniwa, zmo
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
URL: data:text/html,%3Cbr%3E%3Cspan%20style=%22unicode-bidi:%20-webkit-isolate;%22%3E%3C/span%3E
Bug Depends on: 69275    
Bug Blocks: 73310    
Attachments:
Description Flags
work in progress
none
Duplicated Text Test Case
none
fixes the bug
none
fixes the bug
none
Fixed bidi-isolate-hang-with-neutral-expected.html eric: review+

mitz
Reported 2011-10-03 10:17:09 PDT
To reproduce, navigate to the URL. Results: ASSERTION FAILED: m_nestedIsolateCount >= 1 Source/WebCore/platform/text/BidiResolver.h(203) : void WebCore::BidiResolver<WebCore::InlineIterator, WebCore::BidiRun>::exitIsolate() 1 WebCore::BidiResolver<WebCore::InlineIterator, WebCore::BidiRun>::exitIsolate() 2 _ZN7WebCoreL28notifyObserverWillExitObjectINS_12BidiResolverINS_14InlineIteratorENS_7BidiRunEEEEEvPT_PNS_12RenderObjectE 3 _ZN7WebCoreL14bidiNextSharedINS_12BidiResolverINS_14InlineIteratorENS_7BidiRunEEEEEPNS_12RenderObjectES6_S6_PT_NS_19EmptyInlineBehaviorEPb 4 _ZN7WebCoreL28bidiNextSkippingEmptyInlinesINS_12BidiResolverINS_14InlineIteratorENS_7BidiRunEEEEEPNS_12RenderObjectES6_S6_PT_ 5 WebCore::InlineIterator::increment(WebCore::BidiResolver<WebCore::InlineIterator, WebCore::BidiRun>*) 6 WebCore::BidiResolver<WebCore::InlineIterator, WebCore::BidiRun>::increment() 7 WebCore::RenderBlock::LineBreaker::skipLeadingWhitespace(WebCore::BidiResolver<WebCore::InlineIterator, WebCore::BidiRun>&, WebCore::LineInfo&, WebCore::RenderBlock::FloatingObject*, WebCore::LineWidth&) 8 WebCore::RenderBlock::LineBreaker::nextLineBreak(WebCore::BidiResolver<WebCore::InlineIterator, WebCore::BidiRun>&, WebCore::LineInfo&, std::pair<WebCore::RenderText*, WebCore::LazyLineBreakIterator>&, WebCore::RenderBlock::FloatingObject*, unsigned int) 9 WebCore::RenderBlock::layoutRunsAndFloatsInRange(WebCore::LineLayoutState&, WebCore::BidiResolver<WebCore::InlineIterator, WebCore::BidiRun>&, WebCore::InlineIterator const&, WebCore::BidiStatus const&, unsigned int) 10 WebCore::RenderBlock::layoutRunsAndFloats(WebCore::LineLayoutState&, bool) 11 WebCore::RenderBlock::layoutInlineChildren(bool, int&, int&) 12 WebCore::RenderBlock::layoutBlock(bool, int, WebCore::RenderBlock::BlockLayoutPass) 13 WebCore::RenderBlock::layout() 14 WebCore::RenderBlock::layoutBlockChild(WebCore::RenderBox*, WebCore::RenderBlock::MarginInfo&, int&, int&) 15 WebCore::RenderBlock::layoutBlockChildren(bool, int&) 16 WebCore::RenderBlock::layoutBlock(bool, int, WebCore::RenderBlock::BlockLayoutPass) 17 WebCore::RenderBlock::layout() 18 WebCore::RenderBlock::layoutBlockChild(WebCore::RenderBox*, WebCore::RenderBlock::MarginInfo&, int&, int&) 19 WebCore::RenderBlock::layoutBlockChildren(bool, int&) 20 WebCore::RenderBlock::layoutBlock(bool, int, WebCore::RenderBlock::BlockLayoutPass) 21 WebCore::RenderBlock::layout() 22 WebCore::RenderView::layout() 23 WebCore::FrameView::layout(bool) 24 WebCore::Document::implicitClose() 25 WebCore::FrameLoader::checkCallImplicitClose() 26 WebCore::FrameLoader::checkCompleted() 27 WebCore::FrameLoader::finishedParsing() 28 WebCore::Document::finishedParsing() 29 WebCore::HTMLTreeBuilder::finished() 30 WebCore::HTMLDocumentParser::end() 31 WebCore::HTMLDocumentParser::attemptToRunDeferredScriptsAndEnd()
Attachments
work in progress (5.83 KB, patch)
2011-11-29 15:04 PST, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
Duplicated Text Test Case (286 bytes, text/html)
2011-11-29 15:17 PST, Levi Weintraub 		no flags
Details
fixes the bug (11.37 KB, patch)
2011-11-29 17:57 PST, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
fixes the bug (11.36 KB, patch)
2011-11-29 17:59 PST, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
Fixed bidi-isolate-hang-with-neutral-expected.html (11.35 KB, patch)
2011-11-29 18:01 PST, Ryosuke Niwa 		eric: review+
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Eric Seidel (no email)
Comment 1 2011-11-28 11:23:16 PST
Dupe of bug 69275?
Ryosuke Niwa
Comment 2 2011-11-28 11:42:59 PST
This is probably a dupe of the bug 73116. I don't hit the assertion anymore. http://trac.webkit.org/changeset/101180
mitz
Comment 3 2011-11-28 11:45:12 PST
I can reproduce this in r101263.
Ryosuke Niwa
Comment 4 2011-11-29 14:03:00 PST
*** Bug 73250 has been marked as a duplicate of this bug. ***
Ryosuke Niwa
Comment 5 2011-11-29 15:04:31 PST
Created attachment 117053 [details] work in progress
Levi Weintraub
Comment 6 2011-11-29 15:17:29 PST
Created attachment 117056 [details] Duplicated Text Test Case This patch also fixes one case where text runs are duplicated when using isolate. There are still other duplication vectors, and this test case will crash without this patch.
Ryosuke Niwa
Comment 7 2011-11-29 17:57:01 PST
Created attachment 117084 [details] fixes the bug
Ryosuke Niwa
Comment 8 2011-11-29 17:59:29 PST
Created attachment 117085 [details] fixes the bug
Ryosuke Niwa
Comment 9 2011-11-29 18:01:34 PST
Created attachment 117086 [details] Fixed bidi-isolate-hang-with-neutral-expected.html
Eric Seidel (no email)
Comment 10 2011-11-30 11:34:25 PST
Comment on attachment 117086 [details] Fixed bidi-isolate-hang-with-neutral-expected.html View in context: https://bugs.webkit.org/attachment.cgi?id=117086&action=review > Source/WebCore/platform/text/BidiResolver.h:180 > + // FIXME: Rename setPosition to reflect the fact it doesn't update m_nestedIsolateCount Didn't you do this? > Source/WebCore/platform/text/BidiResolver.h:186 > + void setPosition(const Iterator& position, unsigned nestedIsolatedCount) > + { > + m_current = position; > + m_nestedIsolateCount = nestedIsolatedCount; > + } Should we be keeping this value on some Position object instead? It seems like we'll have trouble keeping these in sync. Why do we need the nestedIsolateCount again?
Ryosuke Niwa
Comment 11 2011-11-30 11:36:51 PST
Comment on attachment 117086 [details] Fixed bidi-isolate-hang-with-neutral-expected.html View in context: https://bugs.webkit.org/attachment.cgi?id=117086&action=review >> Source/WebCore/platform/text/BidiResolver.h:180 >> + // FIXME: Rename setPosition to reflect the fact it doesn't update m_nestedIsolateCount > > Didn't you do this? Oops, I just forgot to remove it. >> Source/WebCore/platform/text/BidiResolver.h:186 > > Should we be keeping this value on some Position object instead? It seems like we'll have trouble keeping these in sync. Why do we need the nestedIsolateCount again? We need it to detect whether we're inside an isolated content or not. Walking the render tree in each one of those places is much more expensive.
Ryosuke Niwa
Comment 12 2011-11-30 14:32:27 PST
Thanks for the review! Great to fix this assertion.
Ryosuke Niwa
Comment 13 2011-11-30 14:33:27 PST
Committed r101556: <http://trac.webkit.org/changeset/101556>
Note You need to log in before you can comment on or make changes to this bug.