665218 - Crash [@ gfxContext::gfxContext ] when printing boarding passes on American Airlines

Status: VERIFIED FIXED

(Core :: Graphics, defect)

Description

[Curtis Koenig [:curtisk-use curtis.koenig+bzATgmail.com]]]

Steps to reproduce:
1) go to https://www.aa.com/reservation/flightCheckInViewReservationsAccess.do?v_locale=en_US&v_mobileUAFlag=AA
2) Enter information for flight (first name, last name, record locator)
3) Print boarding passes
4) On new window that opens choose either print boarding passes or print without offers
5) Printer dialogue opens
6) Click print button
7) Browser crashes items do not print

Expected Results:
* Boarding passes should print and browser should not crash

Steps attempted to fix:
* Print from a clean profile also crashes
* Reset printer information

Crashes from about:crashes
bp-14d498f9-bacc-4968-be01-209c721106172011/06/174:07 
PMbp-7424235d-b3a3-4174-ae98-4892e21106172011/06/173:59 
PMbp-190cc4b0-22b5-496d-b357-6dbae2110617

Comment 1

[Curtis Koenig [:curtisk-use curtis.koenig+bzATgmail.com]]]

Tested in the QA lab and was able to print, must be something wrong with my particular box.

Comment 2

[Mats Palmgren (inactive)]

bp-14d498f9-bacc-4968-be01-209c72110617
bp-7424235d-b3a3-4174-ae98-4892e2110617
bp-190cc4b0-22b5-496d-b357-6dbae2110617

Comment 3

[Boris Zbarsky [:bzbarsky]]

This is crashing on a null-deref; I bet the surface passed to the gfxContext constructor is null or its CairoSurface() is null...

Comment 4

[Timothy Nikkel (:tnikkel)]

Regression? Any chance of a regression window?

Comment 5

[Martijn Wargers (dead)]

Attachment: testcase

I'm crashing with this stacktrace in current trunk build on the Mac:
https://crash-stats.mozilla.com/report/index/203028a6-a5e0-4394-91ed-0feac2110707
0 	XUL 	gfxContext::gfxContext 	gfx/thebes/gfxContext.cpp:64
1 	XUL 	nsRenderingContext::Init 	gfx/src/nsRenderingContext.cpp:91
2 	XUL 	nsDeviceContext::CreateRenderingContext 	gfx/src/nsDeviceContext.cpp:442
3 	XUL 	PresShell::GetReferenceRenderingContext 	layout/base/nsPresShell.cpp:3808
4 	XUL 	PresShell::DoReflow 	layout/base/nsPresShell.cpp:7867
5 	XUL 	PresShell::ProcessReflowCommands 	layout/base/nsPresShell.cpp:8076
6 	XUL 	PresShell::FlushPendingNotifications 	layout/base/nsPresShell.cpp:4802
7 	XUL 	nsRefreshDriver::Notify 	layout/base/nsRefreshDriver.cpp:398
8 	CoreGraphics 	CGEventSourceCounterForEventType 	
9 	XUL 	js::mjit::FrameState::ensureTypeSynced 	js/src/assembler/assembler/AssemblerBuffer.h:76

Comment 6

[Martijn Wargers (dead)]

I meant, that I'm crashing with the testcase, while trying to print.

Comment 7

[Scoobidiver (away)]

It is #1 top browser crasher on Mac OS X in 6.0 with 10% of all crashes.

There is a correlation with PrintingPrivate:
100% (93/93) vs.  20% (129/639) PrintingPrivate

Comment 8

[Mats Palmgren (inactive)]

Attachment: stack from a debug build

Comment 9

[Mats Palmgren (inactive)]

Attachment: fix

On MacOSX, mPrintingSurface is nulled out in nsDeviceContext::EndPage
and re-created in BeginPage.  The reflow comes in-between, so mPrintingSurface
is null and we crash.  There are other methods that don't null-check
mPrintingSurface so it seems more robust to just re-create it directly.

Comment 10

[Mats Palmgren (inactive)]

Attachment: crashtest.diff

I tried to make Martijn's testcase into a crashtest but it doesn't
crash when run by the reftest framework.  It does crash when I load
it directly and try to print, so I don't know what the problem is...

Comment 11

[Mats Palmgren (inactive)]

Attachment: fix

Doing it in BeginPage works better... 
(last patch printed the page upside down, oops)

Comment 12

[Mats Palmgren (inactive)]

Builds for testing:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-1126e44f4649/

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 546375 [details] [diff] [review]
fix

Review of attachment 546375 [details] [diff] [review]:
-----------------------------------------------------------------

I don't understand this patch. Why set mPrintingSurface to null at all? The following call to GetSurfaceForPrinter will overwrite it anyway.

::: gfx/src/nsDeviceContext.cpp
@@ +638,5 @@
> +    // We need to release the CGContextRef in the surface here as these
> +    // CGContextRefs are only good for one page.
> +    mPrintingSurface = nsnull;
> +    // We need to get a new surface for each page on the Mac.
> +    // We do this here rather than in BeginPage so it's always non-null.

rather than in EndPage

Comment 14

[Asa Dotzler [:asa]]

We're considering this for Beta so please help us understand the risk as you review it. Thanks.

Comment 15

[Mats Palmgren (inactive)]

Attachment: fix

> Why set mPrintingSurface to null at all?

My intention was to make it clear that we're deliberately
releasing the existing surface, but maybe it's just confusing.
Fair enough, the comment should make that clear I hope.

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 546645 [details] [diff] [review]
fix

Review of attachment 546645 [details] [diff] [review]:
-----------------------------------------------------------------

Risk seems extremely low.

Comment 17

[Mats Palmgren (inactive)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/be8e253130ff

Comment 18

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/be8e253130ff

Comment 19

[Scoobidiver (away)]

Aurora and Beta landing?

Comment 20

[Jesse Ruderman]

Filed bug 675709 on adding a way to test this.

Comment 21

[Asa Dotzler [:asa]]

Comment on attachment 546645 [details] [diff] [review]
fix

Please hurry with the beta landing. Time is short for our (hopefully) final Beta build.

Comment 22

[Robert O'Callahan (:roc) (email my personal email if necessary)]

http://hg.mozilla.org/releases/mozilla-beta/rev/7fc29f37f62d

Comment 23

[Mats Palmgren (inactive)]

http://hg.mozilla.org/releases/mozilla-aurora/rev/822f8a8e65aa

Comment 24

[George Carstoiu]

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:6.0) Gecko/20100101 Firefox/6.0 - beta 5

Verified issue on Mac OS X 10.6 and 10.7 on all three channels (Nightly, Aurora, Beta) using the attached testcase - crash no longer reproducible

Setting status to Verified Fixed.

Comment 25

[Mats Palmgren (inactive)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/88e4547bdbcf

Comment 26

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/88e4547bdbcf