1167590 - Mark imgRequestProxy::mListener as MOZ_UNSAFE_REF

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Comment 1

[Nika Layzell [:nika] (ni? for response)]

Attachment: Mark imgRequestProxy::mListener as MOZ_UNSAFE_REF

Comment 2

[Timothy Nikkel (:tnikkel)]

Comment on attachment 8609364 [details] [diff] [review]
Mark imgRequestProxy::mListener as MOZ_UNSAFE_REF

Seth would probably be a better reviewer for imagelib things.

Comment 3

[Seth Fowler [:seth] [:s2h]]

Comment on attachment 8609364 [details] [diff] [review]
Mark imgRequestProxy::mListener as MOZ_UNSAFE_REF

Review of attachment 8609364 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry for the slow review; I've been unavailable to varying degrees for the past two weeks.

::: image/src/imgRequestProxy.h
@@ +203,5 @@
>  
>    // mListener is only promised to be a weak ref (see imgILoader.idl),
>    // but we actually keep a strong ref to it until we've seen our
>    // first OnStopRequest.
> +  imgINotificationObserver* MOZ_UNSAFE_REF("Callers of methods setting mListener "

I'm not sure that "Callers of methods setting mListener" is quite the right way to phrase this. Just say something like "Observers must call Cancel() or CancelAndForgetObserver() before they are destroyed."

Comment 4

[Nika Layzell [:nika] (ni? for response)]

Attachment: Mark imgRequestProxy::mListener as MOZ_UNSAFE_REF

No problem. Updated patch with changed phrasing.

Comment 5

[Carsten Book [:Tomcat]]

Hi,

this failed to apply:
applying Bug-1167590---Mark-imgRequestProxymListener-as-MOZ.patch
unable to find 'image/src/imgRequestProxy.h' for patching
1 out of 1 hunks FAILED -- saving rejects to file image/src/imgRequestProxy.h.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working directory
errors during apply, please fix and refresh Bug-1167590---Mark-imgRequestProxymListener-as-MOZ.patch

Comment 6

[Nika Layzell [:nika] (ni? for response)]

Attachment: Mark imgRequestProxy::mListener as MOZ_UNSAFE_REF

Should apply cleanly on mozilla-central now.

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1f62853f6c43

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1f62853f6c43