Closed Bug 1256515 Opened 4 years ago Closed 4 years ago

crashes in mozilla::dom::CanvasRenderingContext2D::DrawWindow

Categories

(Core :: Canvas: 2D, defect, critical)

x86
Windows NT
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla48
Tracking Status
firefox46 --- fixed
firefox47 --- fixed
firefox48 --- fixed

People

(Reporter: dbaron, Assigned: lsalzman)

Details

(Keywords: crash, topcrash, Whiteboard: gfx-noted)

Crash Data

Attachments

(2 files)

This bug was filed from the Socorro interface and is 
report bp-5fb7b977-09ea-4d86-b545-44faa2160314.
=============================================================

After bug 1246775 there are still a decent number of crashes on nightly in DrawWindow.

I took a look at the minidump for this one, and looking at the minidump agrees with the naive interpretation of looking at the crash address and the stack, which is that |snapshot| is null and we're trying to dereference it.
Flags: needinfo?(lsalzman)
Whiteboard: gfx-noted
(In reply to David Baron [:dbaron] ⌚️UTC-7 from comment #0)
> This bug was filed from the Socorro interface and is 
> report bp-5fb7b977-09ea-4d86-b545-44faa2160314.
> =============================================================
> 
> After bug 1246775 there are still a decent number of crashes on nightly in
> DrawWindow.
> 
> I took a look at the minidump for this one, and looking at the minidump
> agrees with the naive interpretation of looking at the crash address and the
> stack, which is that |snapshot| is null and we're trying to dereference it.

The cause here is different from bug 1246775. In that bug, we were sending mTarget off to the void, never to return, so that when we tried to copy to it, kaboom.

In this case, the source (drawDT) is apparently invalid, so Snapshot()ing it fails. This can only seemingly happen in DrawTargetCairo, and the report at least confirms this. The only return this can also happen is if the Cairo surface or context has an error status, which the bug report also indicates in the metadata - here an unfortunate CAIRO_STATUS_NO_MEMORY. So we've OOM'd.

I'll write up a patch to at least check for this and bail, since we're already doing similar if the map fails nearby.
Flags: needinfo?(lsalzman)
Cairo has determined, in its wisdom, that the surface and/or context wrapped in DrawTargetCairo are in error status, having occurred somewhere in the black box of drawing the window contents. Cairo has already spat out an error to the log by the time we get here, so there is not much more we can do than just try not to crash violently. This patch at least attempts to ensure that.
Assignee: nobody → lsalzman
Status: NEW → ASSIGNED
Attachment #8730856 - Flags: review?(bas)
Attachment #8730856 - Flags: review?(bas) → review+
https://hg.mozilla.org/mozilla-central/rev/22a9fe577704
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
can we uplift this simple crash fix to beta?
Comment on attachment 8730856 [details] [diff] [review]
bail out in CanvasRenderingContext2D::DrawWindow if snapshotting draw target fails

Approval Request Comment
[Feature/regressing bug #]: Unknown, but goes back to at least 2014 and probably affects as far back as 45.
[User impact if declined]: Potential canvas drawWindow crashes (would be xul only)
[Describe test coverage new/current, TreeHerder]: mochitest, reftests
[Risks and why]: Low risk, just turns an unintentional crash into a JS error, and its usage is confined to XUL anyway.
[String/UUID change made/needed]: None
Attachment #8730856 - Flags: approval-mozilla-beta?
Attachment #8730856 - Flags: approval-mozilla-aurora?
Comment on attachment 8730856 [details] [diff] [review]
bail out in CanvasRenderingContext2D::DrawWindow if snapshotting draw target fails

Crash fix, please uplift to aurora and beta.
Attachment #8730856 - Flags: approval-mozilla-beta?
Attachment #8730856 - Flags: approval-mozilla-beta+
Attachment #8730856 - Flags: approval-mozilla-aurora?
Attachment #8730856 - Flags: approval-mozilla-aurora+
Fix parameter name/syntax error when applied against 46b.

Approval Request Comment
[Feature/regressing bug #]: Affects as far back as 45.
[User impact if declined]: Crashes in canvas drawWindow (xul only)
[Describe test coverage new/current, TreeHerder]: mochitests, reftests
[Risks and why]: Low risk, turns crash into JS error, confined to XUL.
[String/UUID change made/needed]: None
Flags: needinfo?(lsalzman)
Attachment #8735556 - Flags: review+
Attachment #8735556 - Flags: approval-mozilla-beta?
Comment on attachment 8735556 [details] [diff] [review]
bail out in CanvasRenderingContext2D::DrawWindow if snapshotting draw target fails (46b)

Rebased for 46, Let's see if we can get this crash fix in beta 6.
Attachment #8735556 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
The rebased patch ended up without mentioning the bug number, and I didn't catch that until after I pushed it. Oops.
You need to log in before you can comment on or make changes to this bug.