Closed Bug 942690 Opened 6 years ago Closed 6 years ago

Loading whatwg.org HTML Living Standard logs many nsTextFrame NS_ASSERTION failures

Categories

(Core :: Layout: Text and Fonts, defect, P4)

x86
macOS
defect

Tracking

()

RESOLVED FIXED
mozilla29

People

(Reporter: cpeterson, Assigned: smontagu)

References

()

Details

(Keywords: assertion, regression)

Attachments

(2 files)

Loading the HTML Living Standard page in a debug build of Firefox, logs many nsTextFrame assertion failures:

http://www.whatwg.org/specs/web-apps/current-work/

[40533] ###!!! ASSERTION: negative length: 'GetContentEnd() - mContentOffset >= 0', file mozilla/central/layout/generic/nsTextFrame.h, line 437
[40533] ###!!! ASSERTION: negative length: 'GetContentEnd() - mContentOffset >= 0', file mozilla/central/layout/generic/nsTextFrame.h, line 437
[40533] ###!!! ASSERTION: negative length: 'GetContentEnd() - mContentOffset >= 0', file mozilla/central/layout/generic/nsTextFrame.h, line 437
[40533] ###!!! ASSERTION: negative length: 'GetContentEnd() - mContentOffset >= 0', file mozilla/central/layout/generic/nsTextFrame.h, line 437
[40533] ###!!! ASSERTION: integer overflow: 'mMaxTextLength < UINT32_MAX - aFrame->GetContentLength()', file mozilla/central/layout/generic/nsTextFrame.cpp, line 1550
[40533] ###!!! ASSERTION: Should have been cleared: 'mBreakSinks.IsEmpty()', file mozilla/central/layout/generic/nsTextFrame.cpp, line 903
[40533] ###!!! ASSERTION: Should have been cleared: 'mMappedFlows.IsEmpty()', file mozilla/central/layout/generic/nsTextFrame.cpp, line 906
[40533] ###!!! ASSERTION: Should have Reset() before destruction!: 'mCurrentWord.Length() == 0', file mozilla/central/content/base/src/nsLineBreaker.cpp, line 25
Is this a regression?
Priority: -- → P4
I believe this is a regression because I can reproduce these assertions in a Nightly 28 debug build (2013-11-25-mozilla-central-debug), but NOT a Aurora 27 debug build (2013-11-24-mozilla-aurora-debug).
I bisected Nightly 28 debug builds and the regression build is 2013-11-19. Here is the pushlog from 11-18 to 11-19:

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=d58ab6f6ca0a&tochange=ba9ecdea3a90

Could these nsTextFrame assertion failures be a regression from layering bug 919144?
I'd be more inclined to suspect one of these bidi-related patches:

bdf16bdfce5a	Simon Montagu — Optimize bidi resolution on blocks without mixed-direction text. Bug 646359, r=roc
ec7b5c159c31	Simon Montagu — Bug 936935: Mark lines dirty more accurately in Bidi resolution, r=roc

(though even if the culprit is one of these, it's entirely possible it's just exposing a pre-existing bug.)

Any chance you could bisect inbound builds, to narrow things down further?
I bisected inbound builds to this regression range, which includes smontagu's bidi patches:

https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=6f5bbe846139&tochange=bdf16bdfce5a
Attached file Minimized testcase
Assignee: nobody → smontagu
Attached patch Patch v.0Splinter Review
This rolls back the change that caused the assertions in the first place, but it's possible that we want to fix it differently. The semantics of NS_FRAME_IS_BIDI are a bit vague: in theory it's supposed to mean "this frame has a bidi continuation or is a bidi continuation, but nsTextFrame::GetInFlowContentLength uses it to mean "this frame might have a bidi continuation somewhere in its continuation chain". It might improve performance to have another flag that really means that, and set it more precisely
Attachment #8338524 - Flags: feedback?(roc)
Duplicate of this bug: 944195
Sorry to be annoying, but I was hesitating between two or three different options and I'm not sure what you mean by "feedback+".

Do you think we should just check in attachment 8338524 [details] [diff] [review] and move on, or might it be worth having a better way for nsTextFrame::GetInFlowContentLength to know whether to check bidi continuations than what we currently do (or did before bug 646359), i.e. set NS_FRAME_IS_BIDI on every frame that goes through bidi resolution?
https://hg.mozilla.org/mozilla-central/rev/0d4dc34efd9b
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
You need to log in before you can comment on or make changes to this bug.