Closed Bug 1259561 Opened 4 years ago Closed 4 years ago

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


(Core :: Layout, defect)

Not set



Tracking Status
firefox48 --- fixed


(Reporter: yury, Assigned: dholbert)





(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 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
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

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!
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in before you can comment on or make changes to this bug.