Closed
Bug 1165009
Opened 9 years ago
Closed 9 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 :: Graphics: ImageLib, defect)
Tracking
()
RESOLVED
FIXED
mozilla41
People
(Reporter: dholbert, Assigned: seth)
References
Details
(Keywords: crash, regression, topcrash)
Crash Data
Attachments
(1 file)
1.10 KB,
patch
|
tnikkel
:
review+
lizzard
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
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•9 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•9 years ago
|
||
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•9 years ago
|
Assignee: nobody → seth
Status: NEW → ASSIGNED
Updated•9 years ago
|
Attachment #8606009 -
Flags: review?(tnikkel) → review+
Assignee | ||
Comment 3•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9f3849ec0117
Assignee | ||
Comment 4•9 years ago
|
||
Thanks for the quick review!
Assignee | ||
Comment 5•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/7ec1d89d5986
Assignee | ||
Comment 6•9 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•9 years ago
|
||
This affects 40 as well, can we get it uplifted to Dev Edition?
status-firefox40:
--- → affected
Assignee | ||
Comment 8•9 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?
Comment 9•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/7ec1d89d5986
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Comment 10•9 years ago
|
||
This is now by far the top crash in Dev Edition 40 with about 9% of all crashes in browser/content processes.
Reporter | ||
Comment 11•9 years ago
|
||
(sledru, are you doing DevEdition approvals? The sooner we can get this approved to land there, the better, per comment 10.)
Reporter | ||
Updated•9 years ago
|
Flags: needinfo?(sledru)
Comment 12•9 years ago
|
||
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•9 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.
Description
•