Closed Bug 1416774 Opened 7 years ago Closed 7 years ago

imgRequestProxy::CancelAndForgetObserver doesn't remove from imgCacheValidator proxy list

Categories

(Core :: Graphics: ImageLib, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox58 --- fixed
firefox59 --- fixed

People

(Reporter: aosmond, Assigned: aosmond)

References

Details

(Whiteboard: [gfx-noted])

Attachments

(1 file)

When I log into gmail with a debug build, I have been tripping an assert at: https://searchfox.org/mozilla-central/rev/fe75164c55321e011d9d13b6d05c1e00d0a640be/image/imgLoader.cpp#2954 Which happens after imgRequestProxy::CancelAndForgetObserver has been called on a proxy. It looks like we never remove cancelled proxies from the imgCacheValidator::mProxies array. Additionally we may clear imgRequestProxy::mDeferNotifications as part of imgRequest::RemoveProxy / ProgressTracker::RemoveObserver. This particular sequence of events should be harmless on release builds.
Assignee: nobody → aosmond
OS: Unspecified → All
Priority: -- → P3
Hardware: Unspecified → All
Whiteboard: [gfx-noted]
Status: NEW → ASSIGNED
Attachment #8927837 - Flags: review?(tnikkel) → review+
Pushed by aosmond@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/7e63329ef608 Ensure that imgRequestProxy::CancelAndForgetObserver removes itself from the cache validator. r=tnikkel
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Please request Beta approval on this.
Flags: needinfo?(aosmond)
Comment on attachment 8927837 [details] [diff] [review] 0001-Bug-1416774-Ensure-that-imgRequestProxy-CancelAndFor.patch Approval Request Comment [Feature/Bug causing the regression]: Bug 579122 introduced the assert we are tripping. [User impact if declined]: None, because on release builds this should have no impact beyond some minor accounting. This does fix intermittent test failures on debug however. [Is this code covered by automated tests?]: Yes. [Has the fix been verified in Nightly?]: Yes. [Needs manual test from QE? If yes, steps to reproduce]: No. [List of other uplifts needed for the feature/fix]: None. [Is the change risky?]: No. [Why is the change risky/not risky?]: Small, well contained and well understood solution. Since this is on the teardown of a request, even if we missed some notifications as a result of this, they will have no impact, since the request was destroyed anyways. [String changes made/needed]: None.
Flags: needinfo?(aosmond)
Attachment #8927837 - Flags: approval-mozilla-beta?
Comment on attachment 8927837 [details] [diff] [review] 0001-Bug-1416774-Ensure-that-imgRequestProxy-CancelAndFor.patch Fix an assert issue. Beta58+.
Attachment #8927837 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: