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)
Core
Graphics: ImageLib
Tracking
()
RESOLVED
FIXED
mozilla59
People
(Reporter: aosmond, Assigned: aosmond)
References
Details
(Whiteboard: [gfx-noted])
Attachments
(1 file)
3.19 KB,
patch
|
tnikkel
:
review+
gchang
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
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 | ||
Updated•7 years ago
|
Assignee: nobody → aosmond
OS: Unspecified → All
Priority: -- → P3
Hardware: Unspecified → All
Whiteboard: [gfx-noted]
Assignee | ||
Comment 1•7 years ago
|
||
Attachment #8927837 -
Flags: review?(tnikkel)
Assignee | ||
Updated•7 years ago
|
Status: NEW → ASSIGNED
Updated•7 years ago
|
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
Comment 4•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox59:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Comment 5•7 years ago
|
||
Please request Beta approval on this.
status-firefox58:
--- → affected
Flags: needinfo?(aosmond)
Assignee | ||
Comment 6•7 years ago
|
||
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 7•7 years ago
|
||
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+
Comment 8•7 years ago
|
||
bugherder uplift |
You need to log in
before you can comment on or make changes to this bug.
Description
•