964132 - [RTSP] fix the cycle reference in RtpsMediaResource

Status: RESOLVED WONTFIX

(Core :: Audio/Video, defect)

Description

[Benjamin Chen [:bechen]]

There is a cycle reference: 
RtspMediaResource::mListener
Listener::mResource

We should use raw pointer here.
http://dxr.mozilla.org/mozilla-central/source/content/media/RtspMediaResource.h?from=rtspmediaresource#207

Comment 1

[Benjamin Chen [:bechen]]

Attachment: bug-964132.patch

Comment 2

[Steve Workman [:sworkman] (INACTIVE)]

Comment on attachment 8365757 [details] [diff] [review]
bug-964132.patch

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

r=me assuming the following is checked:

- What other objects maintain a ref to Listener? Could they outlive RtspMediaResource, and thus mean that Listener outlives RtspMediaResource?
- Are there enough null checks for Listener::mResource (assuming Listener can outlive RtspMediaResource)?
- Threading: RtspMediaResource is main thread only, right? More importantly, the Listener is main thread only, right? If not, you might need to lock mResource.

If you need to add some to the patch, just request the review again :)

Comment 3

[Benjamin Chen [:bechen]]

try server:
https://tbpl.mozilla.org/?tree=Try&rev=488f51fc5228

Comment 4

[Vincent Chang[:vchang][changyihsin]]

The listener is held by RtspControllerChild using nsCOMPtr and RtspController is held by RtspMediaResource only.

Comment 5

[Benjamin Chen [:bechen]]

Attachment: bug-964132.patch

r=sworkman

Comment 6

[Ken Chang[:kenkai]]

It blocks 1.4 feature.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/ab06f04ddb3d

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ab06f04ddb3d

Comment 9

[Ethan Tseng [:ethan]]

The patch in this bug caused the regression in bug 973840.

Comment 10

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

Proposed to back out this, for fixing bug 973840.

Comment 11

[Ken Chang[:kenkai]]

(In reply to Wesley Huang [:wesley_huang] from comment #10)
> Proposed to back out this, for fixing bug 973840.
Ethan is going to provide a patch for bug 973840. It seems no need to back out for this bug.

Comment 12

[Ethan Tseng [:ethan]]

(In reply to Ken Chang[:ken] from comment #11)
> Ethan is going to provide a patch for bug 973840. It seems no need to back
> out for this bug.
Actually the patch is already waiting for review. :)

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Backed out in bug 973840. Please resolve this as WONTFIX if no further work is intended for this bug.
https://hg.mozilla.org/integration/mozilla-inbound/rev/404edd4f2130

Comment 14

[Ethan Tseng [:ethan]]

Confirmed. No further work is intended for this bug.