Closed Bug 730066 Opened 8 years ago Closed 8 years ago

Crash at mozilla::layers::ImageContainer::GetCurrentSize on printing http://www.mozilla.org/projects/firefox/prerelease.html

Categories

(Core :: Graphics, defect, critical)

13 Branch
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla14
Tracking Status
firefox13 --- affected

People

(Reporter: spacetime, Assigned: smichaud)

References

Details

(4 keywords)

Crash Data

Attachments

(3 files)

Attached file gdb.txt
User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/535.11 (KHTML, like Gecko) Chrome/17.0.963.56 Safari/535.11

Steps to reproduce:

1) Open www.mozilla.org/projects/firefox/prerelease.html
2) Close the browser (have Show windows and tabs from last session enabled) OR kill the firefox process
3) Recover the tabs (if killed), or the tab will be there already
4) Click on File->Print Preview

Nightly 13.0a1 (2012-02-24)
changeset:   87509:88aab8279621


Actual results:

Crash. With Segmentation fault on the terminal.
Crash window does not appear. gdb information (including backtrace) is in gdb.txt


Expected results:

Print Preview , no crashes
Confirmed
http://hg.mozilla.org/mozilla-central/rev/5e756e59a794
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120223 Firefox/13.0a1 ID:20120223031236

bp-ad045a11-30d5-4d85-b5eb-9739c2120223
Severity: normal → critical
Status: UNCONFIRMED → NEW
Crash Signature: [@ mozilla::ReentrantMonitorAutoEnter::ReentrantMonitorAutoEnter(mozilla::ReentrantMonitor&)]
Ever confirmed: true
Keywords: reproducible
OS: Linux → All
Keywords: crash
Regression window(m-c)
Works:
http://hg.mozilla.org/mozilla-central/rev/1cdef0321abd
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0a1) Gecko/20120202 Firefox/13.0a1 ID:20120202010526
Crashes:
http://hg.mozilla.org/mozilla-central/rev/005980552224
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0a1) Gecko/20120202 Firefox/13.0a1 ID:20120202022426
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=1cdef0321abd&tochange=005980552224


http://hg.mozilla.org/integration/mozilla-inbound/rev/b2f1b368e2f2
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0a1) Gecko/20120131 Firefox/13.0a1 ID:20120131180632
Crashes:
http://hg.mozilla.org/integration/mozilla-inbound/rev/67b0e13d7a62
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0a1) Gecko/20120131 Firefox/13.0a1 ID:20120131181932
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=b2f1b368e2f2&tochange=67b0e13d7a62

Regressed by:
67b0e13d7a62	Bas Schouten — Bug 715785: Make ImageContainers independent of LayerManagers. r=roc
Blocks: 715785
Crash Signature: [@ mozilla::ReentrantMonitorAutoEnter::ReentrantMonitorAutoEnter(mozilla::ReentrantMonitor&)] → [@ mozilla::ReentrantMonitorAutoEnter::ReentrantMonitorAutoEnter(mozilla::ReentrantMonitor&)] [@ mozilla::layers::ImageContainer::GetCurrentSize]
Component: Untriaged → Graphics
Product: Firefox → Core
QA Contact: untriaged → thebes
Hardware: x86_64 → All
(In reply to Rishab Arora from comment #0)
> Steps to reproduce:
> 
> 1) Open www.mozilla.org/projects/firefox/prerelease.html
> 2) Close the browser (have Show windows and tabs from last session enabled)
> OR kill the firefox process
> 3) Recover the tabs (if killed), or the tab will be there already
> 4) Click on File->Print Preview

I can reproduce this with simply selecting Ctrl+P and print on the page from 1) on all platforms.
I've confirmed Virgil's STR from comment #3 on OS X 10.6.8.

And I've confirmed Alice's report from comment #2 that these crashes were triggered by Bas Schouten's patch for bug 715785 (https://hg.mozilla.org/mozilla-central/rev/67b0e13d7a62).

Also, here's a more informative gdb stack trace, made on OS X 10.6.8 with a build made from today's mozilla-central code.
Apparently mozilla::layers::ImageContainer::GetCurrentSize() is getting called on a deleted ImageContainer object.
Summary: crash on print preview of recovered tab with video → Crash at mozilla::layers::ImageContainer::GetCurrentSize on printing http://www.mozilla.org/projects/firefox/prerelease.html
> Apparently mozilla::layers::ImageContainer::GetCurrentSize() is
> getting called on a deleted ImageContainer object.

Or maybe it's just a NULL ImageContainer object.
Keywords: regression
Comment #6 turns out to be right.

Here's a proof of concept patch that fixes these crashes in my tests on OS X 10.6.8.  I've started tryserver builds on all platforms, which should be available in a few hours.  Please test with them once they become available.

This patch isn't the best fix for these crashes -- at least not by itself.  Bas's patch for bug 715785 makes an assumption that turns out to be incorrect (that nsHTMLVideoElement::GetImageContainer() always returns a usable object), and there are other parts of the tree that would also need to be changed.

Others know this code much better than I do ... like Bas, to whom I'm going to assign this bug.
Bas, you know this code much better than I do :-)

Please reassign to someone else if that's appropriate.
Assignee: nobody → bas.schouten
Here are the tryserver builds made with my patch from comment #7.  Please try them out and report your results here.

http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/smichaud@pobox.com-76aee1ac1c4d/
Steven: I tried the build in Comment 9 on Mac 10.7.4 and Windows 7 x64 and can confirm that I do not get the crash following the STR in Comment 3.

In both cases I tested with the latest nightly on those systems and was able to generate a crash, so from what I can see the tryserver build does the trick.

(In reply to Steven Michaud from comment #9)
> Here are the tryserver builds made with my patch from comment #7.  Please
> try them out and report your results here.
> 
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/smichaud@pobox.com-
> 76aee1ac1c4d/
Also confirming on Linux. No crash with the Try Server build.

(In reply to Steven Michaud from comment #9)
> Here are the tryserver builds made with my patch from comment #7.  Please
> try them out and report your results here.
> 
> http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/smichaud@pobox.com-
> 76aee1ac1c4d/
Comment on attachment 607262 [details] [diff] [review]
Proof of concept patch

Apparently Bas doesn't have time to deal with this bug.  So I've taken
another look the the code myself, to see if I can do better than my
proof of concept patch in comment #7.

I can't.  Though my patch isn't what Bas originally intended, before
he removed the null check from his original patch (see bug 715785
comment #1), I still don't see anything wrong with it.

So Roc, I'll ask you to review.
Attachment #607262 - Flags: review?(roc)
Assignee: bas.schouten → smichaud
Printing layout/reftests/ogg-video/black140x100.ogv to PDF hits the same crash, so it should be easy to add a test.
Keywords: testcase
(In reply to comment #13)

On OS X the print and print preview dialogs are both native, so an automated test wouldn't work -- the dialog would never get closed.  (In fact the print-preview dialog is spawned from the print dialog.)
Yeah, and bug 675709.

Does the crash reproduce with reftest-print, or is that too different from actual printing?

Could you add a test and skip it on Mac?
https://hg.mozilla.org/mozilla-central/rev/736f897f8a0e
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14
Depends on: 778642
You need to log in before you can comment on or make changes to this bug.