crash in mozilla::gfx::BaseRect<int, mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits>, mozilla::gfx::IntPointTyped<mozilla::gfx::UnknownUnits>, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits>, [...]

RESOLVED FIXED in Firefox 40

Status

()

Core
ImageLib
--
critical
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: dholbert, Assigned: seth)

Tracking

({crash, regression, topcrash})

Trunk
mozilla41
Unspecified
Windows NT
crash, regression, topcrash
Points:
---

Firefox Tracking Flags

(firefox40 fixed, firefox41 fixed)

Details

(crash signature)

Attachments

(1 attachment)

This bug was filed from the Socorro interface and is 
report bp-d5eb0ad3-1e00-42d4-a2ea-dd7372150512.
=============================================================

We've got a top crash, on this "mAnim" deref I think:
> 973     if (aNewFrameCount > 1) {
> 974       mAnim->UnionFirstFrameRefreshArea(aNewRefreshArea);
> 975     }

http://hg.mozilla.org/mozilla-central/annotate/502e1a5e722f/image/src/RasterImage.cpp#l962

The "Graph" tab [1] shows that this first appeared in a 20150509 nightly build. "hg log" on RasterImage.cpp shows that bug 1155332 was one of the most recent changes, and it falls into the right regression range to have caused this. (It hit central on 5/8, so it would have first shipped in the 5/9 Nightly.)  So, tentatively assuming this is a regression from that.


[1] https://crash-stats.mozilla.com/report/list?product=Firefox&range_value=7&range_unit=days&date=2015-05-14&signature=mozilla%3A%3Agfx%3A%3ABaseRect%3Cint%2C+mozilla%3A%3Agfx%3A%3AIntRectTyped%3Cmozilla%3A%3Agfx%3A%3AUnknownUnits%3E%2C+mozilla%3A%3Agfx%3A%3AIntPointTyped%3Cmozilla%3A%3Agfx%3A%3AUnknownUnits%3E%2C+mozilla%3A%3Agfx%3A%3AIntSizeTyped%3Cmozilla%3A%3Agfx%3A%3AUnknownUnits%3E%2C+mozilla%3A%3Agfx%3A%3AIntMarginTyped%3Cmozilla%3A%3Agfx%3A%3AUnknownUnits...&version=Firefox%3A41.0a1#tab-graph
(Assignee)

Comment 1

3 years ago
Ugh. This one is straightforward to fix, but I'm nervous that there may be more fallout from that bug. It's such a fundamental change that it's kinda hard to be sure that we've eliminated all potential issues.
(Assignee)

Comment 2

3 years ago
Created attachment 8606009 [details] [diff] [review]
Bail in RasterImage::OnAddedFrame if we hit an error during decoding

We normally check mError before touching mAnim, but OnAddedFrame is a case where
we don't, and that resulted in this crash. This patch fixes that.
Attachment #8606009 - Flags: review?(tnikkel)
(Assignee)

Updated

3 years ago
Assignee: nobody → seth
Status: NEW → ASSIGNED
Attachment #8606009 - Flags: review?(tnikkel) → review+
(Assignee)

Comment 4

3 years ago
Thanks for the quick review!
(Assignee)

Comment 6

3 years ago
^ I went ahead and pushed this in the hope that it'll be able to make it into the next Nightly, even though try hasn't finished. Given that it's a top crash, the risk of a potential backout seemed worth it, especially since the patch is so simple.

Comment 7

3 years ago
This affects 40 as well, can we get it uplifted to Dev Edition?
status-firefox40: --- → affected
(Assignee)

Comment 8

3 years ago
Comment on attachment 8606009 [details] [diff] [review]
Bail in RasterImage::OnAddedFrame if we hit an error during decoding

Approval Request Comment
[Feature/regressing bug #]: Bug 1155332.
[User impact if declined]: This is a top crash.
[Describe test coverage new/current, TreeHerder]: Try is green. Presumably on m-c by tonight.
[Risks and why]: Very, very low risk.
[String/UUID change made/needed]: None.
Attachment #8606009 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/7ec1d89d5986
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox41: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41

Comment 10

3 years ago
This is now by far the top crash in Dev Edition 40 with about 9% of all crashes in browser/content processes.
(sledru, are you doing DevEdition approvals? The sooner we can get this approved to land there, the better, per comment 10.)
Comment on attachment 8606009 [details] [diff] [review]
Bail in RasterImage::OnAddedFrame if we hit an error during decoding

Approved for uplift to aurora - topcrash, regression, has been ok on m-c over the weekend.
Attachment #8606009 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Comment 13

3 years ago
Thanks for the approval! Now if only mozilla-aurora wasn't closed.

Clearing needinfo for sledru since we're good to go on the approval side.
Flags: needinfo?(sledru)
You need to log in before you can comment on or make changes to this bug.