Closed
Bug 803568
Opened 12 years ago
Closed 11 years ago
Nightly crashed when see pdf file in fullscreen (presentation mode) or zooming
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
mozilla21
People
(Reporter: raul.malea, Assigned: milan)
References
()
Details
(Keywords: crash, topcrash)
Crash Data
Attachments
(3 files, 6 obsolete files)
4.40 KB,
patch
|
bas.schouten
:
review+
bas.schouten
:
checkin+
|
Details | Diff | Splinter Review |
4.40 KB,
patch
|
akeybl
:
approval-mozilla-aurora+
RyanVM
:
checkin+
|
Details | Diff | Splinter Review |
4.40 KB,
patch
|
akeybl
:
approval-mozilla-beta+
RyanVM
:
checkin+
|
Details | Diff | Splinter Review |
1. Opened this pdf file: http://www.primariahd.ro/fisiere/module_fisiere/4191/Dispoz%20ordinara%20SEPTEMBRIE.pdf 2a. Switch to Presentation Mode or 2b. Alternate zoom (automatic, page width, 75%, etc.) 3. Nightly crashed Crash reports: https://crash-stats.mozilla.com/report/index/bp-37527549-85df-430d-ab85-dd5032121019 https://crash-stats.mozilla.com/report/index/bp-c3397d2f-6204-42d1-9683-18b7c2121019 https://crash-stats.mozilla.com/report/index/bp-8dd9614e-3dd4-484c-adba-222c32121019
Reporter | ||
Comment 1•12 years ago
|
||
Version: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/19.0 Firefox/19.0 ID:20121019030551 Extensions: about:me 0.5 Adblock Plus 2.1.2 Add-on Compatibility Reporter 1.1 Adobe Acrobat - Create PDF 1.2 Adobe Acrobat 10.1.4.38 Adobe Acrobat 11.0.0.379 ANIMATED - Snowflake Village 1320945460 [DISABLED] British English Dictionary 1.19.1 Bugzilla Tweaks 1.12.1 Cheevos 1.4 Collusion 0.16.3 [DISABLED] Dicționar românesc de corectare ortografică. 1.12 Download Statusbar 0.9.10 fireblur 1260925626 [DISABLED] Google Earth Plugin 6.2.0.5788 Google Shortcuts 2.1.7.1 Google Update 1.3.21.123 HP Smart Web Printing 4.5 [DISABLED] Implicit 19.0a1 Java Deployment Toolkit 7.0.70.11 10.7.2.11 Java(TM) Platform SE 7 U7 10.7.2.11 Microsoft Office 2010 14.0.4730.1010 Microsoft Office 2010 14.0.4761.1000 Mozilla QA Companion 1.2.3 Mozilla Reps Companion 1.1 MozMill 1.5.19 Mozmill Crowd 0.1.5 Nightly Tester Tools 3.3 NoScript 2.5.8 NVIDIA 3D Vision 7.17.13.697 NVIDIA 3D VISION 7.17.13.697 Puppy Dogs... 1291695684 [DISABLED] Sage 1.4.12 Shockwave Flash 11.4.402.287 Silverlight Plug-In 5.1.10411.0 Submit Word 1.1.0 Test Pilot 1.2.2 VLC Web Plugin 2.0.2.0 Yahoo Application State Plugin 1.0.0.7 Build config: about:buildconfig Build Machine w64-ix-slave98 Source Built from http://hg.mozilla.org/mozilla-central/rev/0ff60bfb3442 Build platform target i686-pc-mingw32 Build tools Compiler Version Compiler flags e:/builds/moz2_slave/m-cen-w32-ntly/build/obj-firefox/_virtualenv/Scripts/python.exe -O e:/builds/moz2_slave/m-cen-w32-ntly/build/build/cl.py cl 16.00.30319.01 -TC -nologo -W3 -Gy -Fdgenerated.pdb -we4553 -DNDEBUG -DTRIMMED -Zi -UDEBUG -DNDEBUG -GL -wd4624 -wd4952 -O1 -Oy- e:/builds/moz2_slave/m-cen-w32-ntly/build/obj-firefox/_virtualenv/Scripts/python.exe -O e:/builds/moz2_slave/m-cen-w32-ntly/build/build/cl.py cl 16.00.30319.01 -TP -nologo -W3 -Gy -Fdgenerated.pdb -wd4345 -wd4351 -wd4800 -we4553 -GR- -DNDEBUG -DTRIMMED -Zi -UDEBUG -DNDEBUG -GL -wd4624 -wd4952 -O1 -Oy- Configure arguments --enable-update-channel=nightly --enable-update-packaging --enable-jemalloc --enable-signmar --enable-profiling --enable-js-diagnostics
Reporter | ||
Updated•12 years ago
|
Crash Signature: _VEC_memcpy | mozilla::gfx::CopyDataToCairoSurface
See Also: → 800523
Comment 2•12 years ago
|
||
It doesn't crash for me with a new profile. Can you narrow down the extension that causes the issue (probably something similar to ImageTweak)?
Severity: major → critical
Crash Signature: _VEC_memcpy | mozilla::gfx::CopyDataToCairoSurface → [@ _VEC_memcpy | mozilla::gfx::CopyDataToCairoSurface ]
Component: PDF Viewer → Graphics
Keywords: crash
Product: Firefox → Core
Hardware: x86_64 → x86
Version: 19 Branch → Trunk
Comment 3•12 years ago
|
||
Forget comment 2, Firefox crashes when toggling full screen and playing with zoom.
Reporter | ||
Comment 4•12 years ago
|
||
(In reply to Scoobidiver from comment #3) > Forget comment 2, Firefox crashes when toggling full screen and playing with > zoom. Yes, confirmed this. In Safe Mode Firefox crashed when zooming in pdf files . Try + / - zoom more times. New crash reports: http://crash-stats.mozilla.com/report/index/bp-c37ab190-5d84-420d-8aed-11b8e2121019 http://crash-stats.mozilla.com/report/index/bp-5d2447a9-86e2-467b-acb6-030d32121019 In Safe mode fullscreen working fine .
Comment 5•12 years ago
|
||
It's #14 top browser crasher in 19.0a2.
tracking-firefox19:
--- → ?
Keywords: topcrash
Comment 6•12 years ago
|
||
Will track this topcrasher and since PDF.js is something we're looking to ship in 19, Jet can you get someone on this?
Assignee: nobody → bugs
Assignee | ||
Updated•12 years ago
|
Assignee: bugs → milan
Assignee | ||
Comment 7•12 years ago
|
||
Are these errors in any way useful: it JavaScript "debugger" keyword. JS call stack... 0 CanvasGraphics_putBinaryImageData() ["resource://pdf.js/build/pdf.js":3237] this = [object Object] 1 CanvasGraphics_paintImageXObject() ["resource://pdf.js/build/pdf.js":3216] this = [object Object] 2 CanvasGraphics_executeOperatorList() ["resource://pdf.js/build/pdf.js":2254] this = [object Object] 3 next() ["resource://pdf.js/build/pdf.js":1651] <failed to get 'this' value> 4 pdfViewcContinueCallback() ["resource://pdf.js/web/viewer.js":2079] <failed to get 'this' value> 5 anonymous() ["resource://pdf.js/build/pdf.js":1644] <failed to get 'this' value> 6 PDFPageProxy_display() ["resource://pdf.js/build/pdf.js":1659] this = [object Object] 7 pageDisplayReadyPromise() ["resource://pdf.js/build/pdf.js":1560] this = [object Object] 8 Promise_resolve() ["resource://pdf.js/build/pdf.js":1171] this = [object Object] 9 pageSetTimeout() ["resource://pdf.js/build/pdf.js":1585] this = [object Window]
Assignee | ||
Comment 8•12 years ago
|
||
Seems that CanvasRenderingContext2D is bad. Takes a lot longer (for me at least) to crash in the debugger, which could be us releasing the memory while we still need it?
Assignee | ||
Comment 9•12 years ago
|
||
No need to be full screen, it will crash in the window as well.
Assignee | ||
Comment 10•12 years ago
|
||
When we fall far enough behind (because of repeated +/- zoom requests), Cairo seems to fail to give us back a valid surface, at least on Windows. Just check for null pointers. Brendan, I've added you because of previous association with PDF, but I may be way off base. I will open another bug for the same page actually not rendering under windows.
Attachment #696053 -
Flags: review?(bdahl)
Attachment #696053 -
Flags: review?(bas)
Comment 11•12 years ago
|
||
see also bug 825037
Comment 12•12 years ago
|
||
Comment on attachment 696053 [details] [diff] [review] Check for Cairo surface creation failure Review of attachment 696053 [details] [diff] [review]: ----------------------------------------------------------------- I'd like to have a better understanding of why this happens. I'd like to see a little more inline comments as to what we've tested so far. Having said that, as a band-aid this patch seems fine to me.
Attachment #696053 -
Flags: review?(bas) → review+
Assignee | ||
Comment 13•12 years ago
|
||
I imagine it has to do with us trying to allocate >4k x > 8k texture repeatedly, but I don't know Cairo. I'm looking at the fact that it never displays, even if it does not hit this crash. This document is made of 1-bit/1-channel textures.
Assignee | ||
Comment 14•12 years ago
|
||
If the image is 8192 we're fine, if it's 8193, we're not. See 824981.
Comment 16•11 years ago
|
||
Comment on attachment 696053 [details] [diff] [review] Check for Cairo surface creation failure Review of attachment 696053 [details] [diff] [review]: ----------------------------------------------------------------- I can review code in the pdf.js extension, but this isn't my area so I'm removing the review request.
Attachment #696053 -
Flags: review?(bdahl)
Assignee | ||
Comment 17•11 years ago
|
||
[Security approval request comment] How easily could an exploit be constructed based on the patch? A graphics expert could figure out how to have the Cairo image surface creation fail if they know about image size limits. Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? There are no code comments for the null pointer checks, no check-in comments beyond what the code does - check for the failure. Which older supported branches are affected by this flaw? 19 has this problem. If not all supported branches, which bug introduced the flaw? This is a new feature (PDF support). Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? Yes, same, simple change, no additional risk, look for the attachment. How likely is this patch to cause regressions; how much testing does it need? Very unlikely. All we are doing is making sure null pointers don't get used.
Attachment #696053 -
Attachment is obsolete: true
Attachment #698787 -
Flags: sec-approval?
Attachment #698787 -
Flags: review+
Assignee | ||
Comment 18•11 years ago
|
||
Aurora (19) matching patch. r=me, same change as reviewed.
Attachment #698790 -
Flags: review+
Comment 19•11 years ago
|
||
Comment on attachment 698787 [details] [diff] [review] Check for Cairo surface creation failure (removed code comment) This doesn't need sec approval as it is not a rated security bug.
Attachment #698787 -
Flags: sec-approval?
Assignee | ||
Comment 20•11 years ago
|
||
Since this wasn't a security bug (I mixed up a couple), here are more code comments as per original review. Bas, if this is OK, can you also check it in? Also, this is tracked for 19, does that mean I should produce aurora and beta patches and ask for the approval?
Attachment #698787 -
Attachment is obsolete: true
Attachment #698790 -
Attachment is obsolete: true
Attachment #700016 -
Flags: review?(bas)
Attachment #700016 -
Flags: checkin?(bas)
Assignee | ||
Comment 21•11 years ago
|
||
[Approval Request Comment] Bug caused by (feature/regressing bug #): Introduction of the pdf.js viewer as the default User impact if declined: crash Testing completed (on m-c, etc.): m-c, m-a, m-b Risk to taking this patch (and alternatives if risky): low; we are stopping the code from accessing a nullptr String or UUID changes made by this patch: none
Attachment #700406 -
Flags: review?(bas)
Attachment #700406 -
Flags: checkin?
Attachment #700406 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 22•11 years ago
|
||
[Approval Request Comment] Bug caused by (feature/regressing bug #): Introduction of the pdf.js viewer as the default User impact if declined: crash Testing completed (on m-c, etc.): m-c, m-a, m-b Risk to taking this patch (and alternatives if risky): low; we are stopping the code from accessing a nullptr String or UUID changes made by this patch: none
Attachment #700408 -
Flags: review?(bas)
Attachment #700408 -
Flags: checkin?
Attachment #700408 -
Flags: approval-mozilla-beta?
Comment 23•11 years ago
|
||
Comment on attachment 700016 [details] [diff] [review] Check for Cairo surface creation failure (with better code comments) Review of attachment 700016 [details] [diff] [review]: ----------------------------------------------------------------- It's not entirely clear to be how this could happen the way it's described in the comments. Having said that the comments are descriptive enough that they allow further investigation and refer to this bug, so it should be easily enough to investigate further later. What I think is happening is that with a smaller size than 8193, we're not using Cairo drawtargets and -that-'s why we're not hitting the bug there. In any case if these checks help this bug we should just land them, as they can't hurt. ::: gfx/2d/DrawTargetCairo.cpp @@ +178,5 @@ > + // dimension (4k on older cards) could fail, so we have to be ready for the above > + // operation failing. > + if (cairo_surface_status(surf)) { > + return nullptr; > + } As mentioned above, I believe this could fail, but probably for different reasons.
Attachment #700016 -
Flags: review?(bas) → review+
Comment 24•11 years ago
|
||
Since this has r+, can we land this on mozilla-central as soon as possible? We'd like to uplift to Aurora/Beta given the fact that we've only got a handful of betas for FF19 before release.
Comment 25•11 years ago
|
||
Reached out to Bas/Milan on email & Bas confirmed he will land this by EOD today as there was some pending review on the comments with the code here.
Assignee | ||
Comment 26•11 years ago
|
||
To speed things up, I will submit new patches, with Bas' more detailed comments, so that he can land them tomorrow (time difference kicking in).
Assignee | ||
Comment 27•11 years ago
|
||
The code comments mention the bug and that this is a band-aid.
Attachment #700016 -
Attachment is obsolete: true
Attachment #700016 -
Flags: checkin?(bas)
Attachment #702542 -
Flags: review?(bas)
Attachment #702542 -
Flags: checkin?(bas)
Assignee | ||
Comment 28•11 years ago
|
||
[Approval Request Comment] Bug caused by (feature/regressing bug #): Introduction of the pdf.js viewer as the default User impact if declined: crash Testing completed (on m-c, etc.): m-c, m-a, m-b Risk to taking this patch (and alternatives if risky): low; we are stopping the code from accessing a nullptr String or UUID changes made by this patch: none
Attachment #700406 -
Attachment is obsolete: true
Attachment #700406 -
Flags: review?(bas)
Attachment #700406 -
Flags: checkin?
Attachment #700406 -
Flags: approval-mozilla-aurora?
Attachment #702544 -
Flags: review?(bas)
Attachment #702544 -
Flags: checkin?(bas)
Attachment #702544 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 29•11 years ago
|
||
[Approval Request Comment] Bug caused by (feature/regressing bug #): Introduction of the pdf.js viewer as the default User impact if declined: crash Testing completed (on m-c, etc.): m-c, m-a, m-b Risk to taking this patch (and alternatives if risky): low; we are stopping the code from accessing a nullptr String or UUID changes made by this patch: none
Attachment #700408 -
Attachment is obsolete: true
Attachment #700408 -
Flags: review?(bas)
Attachment #700408 -
Flags: checkin?
Attachment #700408 -
Flags: approval-mozilla-beta?
Attachment #702545 -
Flags: review?(bas)
Attachment #702545 -
Flags: checkin?(bas)
Attachment #702545 -
Flags: approval-mozilla-beta?
Comment 30•11 years ago
|
||
Waiting on review and check in to mozilla-central before approving for Aurora/Beta.
Updated•11 years ago
|
Attachment #702542 -
Flags: review?(bas) → review+
Comment 31•11 years ago
|
||
Comment on attachment 702542 [details] [diff] [review] Check for Cairo surface creation failure (central) https://hg.mozilla.org/integration/mozilla-inbound/rev/152d3caaf16a
Attachment #702542 -
Flags: checkin?(bas) → checkin+
Updated•11 years ago
|
status-firefox19:
--- → affected
status-firefox20:
--- → affected
status-firefox21:
--- → affected
tracking-firefox20:
--- → +
Comment 32•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/152d3caaf16a
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Updated•11 years ago
|
Comment 33•11 years ago
|
||
Comment on attachment 702544 [details] [diff] [review] Check for Cairo surface creation failure (aurora) This patch looks identical to the m-c checkin. Approving for Aurora/Beta.
Attachment #702544 -
Flags: review?(bas)
Attachment #702544 -
Flags: approval-mozilla-aurora?
Attachment #702544 -
Flags: approval-mozilla-aurora+
Updated•11 years ago
|
Attachment #702545 -
Flags: review?(bas)
Attachment #702545 -
Flags: approval-mozilla-beta?
Attachment #702545 -
Flags: approval-mozilla-beta+
Comment 34•11 years ago
|
||
FWIW, it looks so far like the fix has helped on trunk and no crashes of this signature have been seen yet with builds after the checkin.
Comment 35•11 years ago
|
||
https://hg.mozilla.org/releases/mozilla-beta/rev/3ec9a94061f9 https://hg.mozilla.org/releases/mozilla-aurora/rev/1a2b008b524b
Comment 36•11 years ago
|
||
Correction: https://hg.mozilla.org/releases/mozilla-beta/rev/cb91e0df071e
Updated•11 years ago
|
Comment 37•11 years ago
|
||
The pdf isn't loading correctly due to https://bugzilla.mozilla.org/show_bug.cgi?id=728571 Raul, can you please verify if it's still reproducing on FF 19.0b3? http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/19.0b3-candidates/build1/
Reporter | ||
Comment 38•11 years ago
|
||
In Nightly Mozilla/5.0 (Windows NT 6.1; WOW64; rv:21.0) Gecko/20130123 Firefox/21.0 ID:20130123052720 pdf loading correctly (e.g. http://www.primariahd.ro/fisiere/module_fisiere/4506/PROCES%20VERBAL%20EXTRA%2018.01.2013.pdf). No crash.
Comment 39•11 years ago
|
||
I can reproduce this issue quite easily on Firefox 19 beta 1, but no matter how much I tried it didn't reproduce on beta 4. Marking bug as verified on 19 considering this and the fact that there are no post-fix crashes in Socorro https://crash-stats.mozilla.com/report/list?query_search=signature&query_type=contains&reason_type=contains&range_value=4&range_unit=weeks&hang_type=any&process_type=any&signature=_VEC_memcpy%20%7C%20mozilla%3A%3Agfx%3A%3ACopyDataToCairoSurface.
Updated•11 years ago
|
Attachment #702544 -
Flags: checkin?(bas) → checkin+
Updated•11 years ago
|
Attachment #702545 -
Flags: checkin?(bas) → checkin+
Comment 40•11 years ago
|
||
Verified as fixed on Friefox 20 Beta 1 (20130220104816): Mozilla/5.0 (Windows NT 6.1; rv:20.0) Gecko/20100101 Firefox/20.0 Mozilla/5.0 (X11; Linux i686; rv:20.0) Gecko/20100101 Firefox/20.0 Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:20.0) Gecko/20100101 Firefox/20.0 No new crashes in Socorro either.
Comment 41•11 years ago
|
||
Verified as fixed on Friefox 21 Beta 1 (20130401192816): Mozilla/5.0 (Windows NT 6.1; rv:21.0) Gecko/20100101 Firefox/21.0 Mozilla/5.0 (X11; Linux i686; rv:21.0) Gecko/20100101 Firefox/21.0 Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:21.0) Gecko/20100101 Firefox/21.0 No new crashes in Socorro either.
You need to log in
before you can comment on or make changes to this bug.
Description
•