Closed Bug 964132 Opened 11 years ago Closed 11 years ago

[RTSP] fix the cycle reference in RtpsMediaResource

Categories

(Core :: Audio/Video, defect)

Product:
Core
Component:
Audio/Video
Platform:
x86_64
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED WONTFIX
Project Flags:
blocking-b2g 1.4+

People

(Reporter: bechen, Assigned: bechen)

References

Details

Attachments

(1 file, 1 obsolete file)

bug-964132.patch
(deleted), patch
bechen
: review+
Details | Diff | Splinter Review
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
Attached patch bug-964132.patch (obsolete) (deleted) — — Splinter Review
Attachment #8365757 - Flags: review?(sworkman)
Blocks: 945603
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 :)
Attachment #8365757 - Flags: review?(sworkman) → review+
The listener is held by RtspControllerChild using nsCOMPtr and RtspController is held by RtspMediaResource only.
Attached patch bug-964132.patch (deleted) — — Splinter Review
r=sworkman
Attachment #8365757 - Attachment is obsolete: true
Attachment #8370523 - Flags: review+
Keywords: checkin-needed
It blocks 1.4 feature.
blocking-b2g: --- → 1.4+
https://hg.mozilla.org/mozilla-central/rev/ab06f04ddb3d
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.4 S1 (14feb)
The patch in this bug caused the regression in bug 973840.
Blocks: 973840
Proposed to back out this, for fixing bug 973840.
(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.
(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. :)
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
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: 1.4 S1 (14feb) → ---
Component: General → Video/Audio
Product: Firefox OS → Core
Confirmed. No further work is intended for this bug.
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: