Closed Bug 1165009 Opened 5 years ago Closed 5 years ago

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

Categories

(Core :: ImageLib, defect, critical)

Unspecified
Windows NT
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla41
Tracking Status
firefox40 --- fixed
firefox41 --- fixed

People

(Reporter: dholbert, Assigned: seth)

References

Details

(Keywords: crash, regression, topcrash)

Crash Data

Attachments

(1 file)

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
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.
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: nobody → seth
Status: NEW → ASSIGNED
Attachment #8606009 - Flags: review?(tnikkel) → review+
Thanks for the quick review!
^ 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.
This affects 40 as well, can we get it uplifted to Dev Edition?
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
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
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+
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.