375495 - Crashing while Printing

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Colin Barrett [:cbarrett]]

Attachment: fix v1.0

In addition to not printing (bug 368933) we are also prone to crashing when we de-reference a null surface. Somehow.

Stuart Parmenter wrote the attached patch on my machine while we were going through trying to debug bug 368933.

Comment 1

[Josh Aas]

Comment on attachment 259767 [details] [diff] [review]
fix v1.0

I'm not sure if we want to do that, requesting review from Pav.

Comment 2

[Stuart Parmenter]

Does the crash actually still happen?  We shouldn't ever hit that situation and I'd like to know why we do before we bandaid over it.  Also don't use tabs in patches.

Comment 3

[Colin Barrett [:cbarrett]]

Sigh, Xcode's defaults are tabs.

I'm not sure how we're getting into that situation. Could nulling the surface in EndPage (because we need a new surface for each page), cause this sort of thing to happen?

Comment 4

[Stuart Parmenter]

We shouldn't be using the same rendering context for each page per simplepageiterator code in layout...  Can you please debug further?

Comment 5

[Colin Barrett [:cbarrett]]

Attachment: fix v2.0

Figured it out, here's the patch.

To figure this out I had to understand both the gecko print loop (nsSimplePageSequence::PrintNextPage) and the Carbon one (http://developer.apple.com/documentation/Carbon/Conceptual/CPM_Concepts/cpm_chap2/chapter_2_section_6.html). That should be useful in case this ever breaks again somehow.

Re-ordered the calls in nsThebesDeviceContext::BeginPage so that the PrintManager calls get done in the right order (BeginPage, GetCGContext, EndPage). The earlier patch was just kind of masking this.

For future reference, the problem was that we didn't have a valid mPrintingSurface when BeginPage was called by nsSimplePageSequence. CreateRenderingContext had a hack to re-create the QuartzSurface after we null it in EndPage, since each Surface represents one page on the Mac. However, just moving the call to that in nsSimplePageSequence above BeginPage is wrong, because on the Mac (and perhaps elsewhere), we need to call BeginPage before we can get the rendering context.

Pav checked and it didn't seem like reordering those calls would have any effect on Linux or Windows.

Comment 6

[Josh Aas]

checked in on trunk