Closed Bug 1259561 Opened 8 years ago Closed 8 years ago

Viewing 756-page-long PDF file causes "nsBlockReflowContext:" warnings in debug builds

Categories

(Core :: Layout, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox48 --- fixed

People

(Reporter: yury, Assigned: dholbert)

References

()

Details

Attachments

(1 file, 1 obsolete file)

When viewing large PDFs (let's say PDF32000_2008.pdf) in debug builds of Firefox we are getting lots of:

nsBlockReflowContext: Block(div)(1)@12a433c38 metrics=83760,64139460!

Yes, we are really getting size 1396x1068991 (there are 756 pages/divs that are 1414 px tall).

Can we increase CRAZY_COORD (see https://dxr.mozilla.org/mozilla-central/source/layout/generic/nsContainerFrame.h#37) by the order of magnitude?
Summary: Viewing PDF files causes "nsBlockReflowContext:" warnings in debug builds → Viewing 756-page-long PDF file causes "nsBlockReflowContext:" warnings in debug builds
I think the assumption of this warning is that, if we're laying out a ~700-page document, we think something is likely to have gone wrong (and the warning is trying to catch the "gone wrong" thing early).

Maybe this assumption isn't valid anymore, though?

I think I'd be OK upping this value by an order of magnitude.  Note that this will put CRAZY_COORD will be just over 1/4 of nscoord_MAX, which is pretty close, but still a useful threshold to start warning at probably.
Showing my work on the "approximately 1/4th of nscoord_MAX" estimate:
 * CRAZY_COORD will now be 60 * ten million = 6 * 10^8.
 * nscoord_MAX is 2^31.
 * So:  (2^31) / (6 * 10^8) is about 3.6
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Attachment #8734922 - Flags: review?(mats)
Same patch, now with a commit message.
Attachment #8734922 - Attachment is obsolete: true
Attachment #8734922 - Flags: review?(mats)
Attachment #8734923 - Flags: review?(mats)
Comment on attachment 8734923 [details] [diff] [review]
fix v2: increase CRAZY_COORD by an order of magnitude

r=mats
Attachment #8734923 - Flags: review?(mats) → review+
Personally, I don't find the associated warnings useful at all, so I'd r+ a patch
that simply removed them.  But perhaps others find them useful...
I'm neutral on them, so I won't move towards removing them at this point. (They'll arguably be a bit more useful now, too, now that they'll be less easy to trip.)

Thanks for the review!
https://hg.mozilla.org/mozilla-central/rev/5eecbb62c31e
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.