Last Comment Bug 665218 - Crash [@ gfxContext::gfxContext ] when printing boarding passes on American Airlines
: Crash [@ gfxContext::gfxContext ] when printing boarding passes on American A...
Status: VERIFIED FIXED
[#1 Mac topcrash in 6 Beta with 10% o...
: crash, testcase
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: 5 Branch
: x86 Mac OS X
: -- critical (vote)
: mozilla8
Assigned To: Mats Palmgren (:mats)
:
Mentors:
Depends on: 684622
Blocks: 639168
  Show dependency treegraph
 
Reported: 2011-06-17 16:16 PDT by Curtis Koenig [:curtisk-use curtis.koenig+bzATgmail.com]]
Modified: 2013-12-27 14:28 PST (History)
12 users (show)
mats: in‑testsuite+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
fixed
fixed


Attachments
testcase (142 bytes, text/html)
2011-07-07 03:10 PDT, Martijn Wargers [:mwargers] (not working for Mozilla)
no flags Details
stack from a debug build (3.50 KB, text/plain)
2011-07-16 19:28 PDT, Mats Palmgren (:mats)
no flags Details
fix (1.31 KB, patch)
2011-07-16 19:47 PDT, Mats Palmgren (:mats)
no flags Details | Diff | Splinter Review
crashtest.diff (772 bytes, patch)
2011-07-16 19:50 PDT, Mats Palmgren (:mats)
no flags Details | Diff | Splinter Review
fix (1.34 KB, patch)
2011-07-16 20:32 PDT, Mats Palmgren (:mats)
no flags Details | Diff | Splinter Review
fix (1.31 KB, patch)
2011-07-18 15:23 PDT, Mats Palmgren (:mats)
roc: review+
asa: approval‑mozilla‑aurora+
asa: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description Curtis Koenig [:curtisk-use curtis.koenig+bzATgmail.com]] 2011-06-17 16:16:16 PDT
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]] 2011-06-17 16:33:34 PDT
Tested in the QA lab and was able to print, must be something wrong with my particular box.
Comment 3 Boris Zbarsky [:bz] 2011-06-21 10:28:14 PDT
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) 2011-06-21 22:32:13 PDT
Regression? Any chance of a regression window?
Comment 5 Martijn Wargers [:mwargers] (not working for Mozilla) 2011-07-07 03:10:59 PDT
Created attachment 544430 [details]
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 [:mwargers] (not working for Mozilla) 2011-07-07 03:12:21 PDT
I meant, that I'm crashing with the testcase, while trying to print.
Comment 7 Scoobidiver (away) 2011-07-15 13:37:52 PDT
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 (:mats) 2011-07-16 19:28:02 PDT
Created attachment 546369 [details]
stack from a debug build
Comment 9 Mats Palmgren (:mats) 2011-07-16 19:47:38 PDT
Created attachment 546370 [details] [diff] [review]
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 (:mats) 2011-07-16 19:50:55 PDT
Created attachment 546371 [details] [diff] [review]
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 (:mats) 2011-07-16 20:32:00 PDT
Created attachment 546375 [details] [diff] [review]
fix

Doing it in BeginPage works better... 
(last patch printed the page upside down, oops)
Comment 13 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2011-07-17 16:32:50 PDT
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] 2011-07-18 14:56:36 PDT
We're considering this for Beta so please help us understand the risk as you review it. Thanks.
Comment 15 Mats Palmgren (:mats) 2011-07-18 15:23:57 PDT
Created attachment 546645 [details] [diff] [review]
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) (Exited; email my personal email if necessary) 2011-07-18 17:54:42 PDT
Comment on attachment 546645 [details] [diff] [review]
fix

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

Risk seems extremely low.
Comment 19 Scoobidiver (away) 2011-07-23 02:56:31 PDT
Aurora and Beta landing?
Comment 20 Jesse Ruderman 2011-08-01 12:35:42 PDT
Filed bug 675709 on adding a way to test this.
Comment 21 Asa Dotzler [:asa] 2011-08-01 14:41:50 PDT
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) (Exited; email my personal email if necessary) 2011-08-01 15:47:41 PDT
http://hg.mozilla.org/releases/mozilla-beta/rev/7fc29f37f62d
Comment 24 George Carstoiu 2011-08-05 05:23:53 PDT
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 26 Phil Ringnalda (:philor) 2013-05-18 18:14:28 PDT
https://hg.mozilla.org/mozilla-central/rev/88e4547bdbcf

Note You need to log in before you can comment on or make changes to this bug.