Closed Bug 1416774 Opened 3 years ago Closed 3 years ago
Request Proxy::Cancel And Forget Observer doesn't remove from img Cache Validator proxy list
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
Attachment #8927837 - Flags: review?(tnikkel) → review+
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/7e63329ef608 Ensure that imgRequestProxy::CancelAndForgetObserver removes itself from the cache validator. r=tnikkel
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.
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.