Closed Bug 1267271 Opened 8 years ago Closed 8 years ago

Crash at SkBitmap::getAddr8 SkBitmap::extractAlpha mozilla::gfx::DrawTargetSkia::DrawSurfaceWithShadow mozilla::dom::AdjustedTargetForShadow::~AdjustedTargetForShadow mozilla::dom::AdjustedTarget::~AdjustedTarget

Categories

(Core :: Graphics, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox47 --- unaffected
firefox48 --- fixed
firefox49 --- fixed

People

(Reporter: cbook, Assigned: lsalzman)

References

()

Details

(Keywords: crash)

Attachments

(2 files)

Attached file crash stack
found via bughunter and reproduced on a win7 debug build based on todays m-c tip

Steps to reproduce:
-> Load https://asicw.playngonetwork.com/Casino/PlayMobile?pid=185&gid=pimped&lang=zh_CN&ticket=52640-592P537D497W148L

--> Crash
It appears that we're trying to draw a shadow for an empty surface. Skia is calling tmpBitmap.getAddr(0, 0) to get a result to the bitmap pixels, even though it will never actually use them because of the empty size. It is doing that right around here: https://dxr.mozilla.org/mozilla-central/source/gfx/skia/skia/src/core/SkBitmap.cpp#1049

The simplest fix for this is just to bail out before we have get there and end up calling extractAlpha on an empty bitmap.
Assignee: nobody → lsalzman
Status: NEW → ASSIGNED
Attachment #8744978 - Flags: review?(mchang)
Attachment #8744978 - Flags: review?(mchang) → review+
https://hg.mozilla.org/mozilla-central/rev/a5d69f1eaa2a
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Comment on attachment 8744978 [details] [diff] [review]
skip empty surfaces in DrawTargetSkia::DrawSurfaceWithShadow

Approval Request Comment
[Feature/regressing bug #]: bug 1259621, 48+
[User impact if declined]: Possible canvas crash regression introduced by bug 1259621 against 48+. Fix needed for bug 1266106 as well. 
[Describe test coverage new/current, TreeHerder]: mochitest, reftest
[Risks and why]: Basically none. Just skips drawing something in a case we shouldn't even be trying to.
[String/UUID change made/needed]: None
Attachment #8744978 - Flags: approval-mozilla-aurora?
See Also: → 1266106
Comment on attachment 8744978 [details] [diff] [review]
skip empty surfaces in DrawTargetSkia::DrawSurfaceWithShadow

Approved for Aurora, check 'er in.
Attachment #8744978 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(In reply to Al Billings [:abillings] from comment #5)
> Comment on attachment 8744978 [details] [diff] [review]
> skip empty surfaces in DrawTargetSkia::DrawSurfaceWithShadow
> 
> Approved for Aurora, check 'er in.

This makes sense. Abilling beat me to it, ;) let's uplift to Aurora48.
Blocks: 1266106
You need to log in before you can comment on or make changes to this bug.