Closed
Bug 919051
Opened 11 years ago
Closed 11 years ago
Media Recording - memory leak when record a media stream without any tracks
Categories
(Core :: Audio/Video: Recording, defect)
Tracking
()
People
(Reporter: jruderman, Assigned: rlin)
Details
(Keywords: memory-leak, testcase)
Attachments
(3 files, 2 obsolete files)
|
293 bytes,
text/html
|
Details | |
|
1.51 KB,
text/plain
|
Details | |
|
3.95 KB,
patch
|
Details | Diff | Splinter Review |
1. Run a debug build with XPCOM_MEM_LEAK_LOG=2
2. Load the testcase
3. Quit
Result: trace-refcnt reports leaked MediaStream and other objects.
|
Reporter | |
Comment 1•11 years ago
|
||
|
Assignee | |
Updated•11 years ago
|
Assignee: nobody → rlin
|
||
Updated•11 years ago
|
Blocks: MediaRecording
|
|
Assignee | |
Comment 2•11 years ago
|
||
The Media Recorder get the media stream object without tracks and enter recording state.
I will fix it.
|
|
||
Comment 3•11 years ago
|
||
FWIW - this is good candidate to get a mochitest with btw. You could just create a test that models the exact test case included with an additional check that the InvalidStateError fires when calling start a 2nd time.
|
|
Assignee | |
Comment 4•11 years ago
|
||
I will let media recorder throw the InvalidStateError exception when we got this kind of source.
BTW, correct the comment 2, now it failed on principal check (no source can be checked) and throw security error without destroy the TrackUnionStream object, this unclear message would confuse UA.
|
|
Assignee | |
Updated•11 years ago
|
Summary: MediaRecorder leak → Media Recording - memory leak when record a media stream without any tracks
|
|
Assignee | |
Comment 5•11 years ago
|
||
patch for this bug, also include the test case.
Attachment #809795 -
Flags: review?(roc)
Attachment #809795 -
Flags: review?(jsmith)
|
|
||
Comment 6•11 years ago
|
||
Comment on attachment 809795 [details] [diff] [review]
patch v1
Review of attachment 809795 [details] [diff] [review]:
-----------------------------------------------------------------
review+ with nits
::: content/media/test/test_mediarecorder_record_nosrc.html
@@ +5,5 @@
> + <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
> + <script type="text/javascript" src="manifest.js"></script>
> +</head>
> +<body>
> +<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=897776">Mozill
Nit - the href link should point to bug 919051
@@ +14,5 @@
> +function startTest() {
> + var mediastream = document.createElement('video').mozCaptureStream();
> +
> + var mr = new MediaRecorder(mediastream);
> + ok(true, 'create MediaRecorder');
Nit - I would make this an info statement
Attachment #809795 -
Flags: review?(jsmith) → review+
Attachment #809795 -
Flags: review?(roc) → review+
|
|
Assignee | |
Comment 7•11 years ago
|
||
Check in patch, carry reviewer, merge with today's mc change.
try result
https://tbpl.mozilla.org/?tree=Try&rev=13fbf4296439
|
|
Assignee | |
Comment 8•11 years ago
|
||
this one, remove two useless header include.
Attachment #811036 -
Attachment is obsolete: true
|
|
Assignee | |
Updated•11 years ago
|
Keywords: checkin-needed
|
||
Updated•11 years ago
|
Attachment #809795 -
Attachment is obsolete: true
|
|
||
Comment 9•11 years ago
|
||
Flags: in-testsuite+
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
|
|
||
Comment 11•11 years ago
|
||
Verified through successful landing of mochitest.
Status: RESOLVED → VERIFIED
|
||
Comment 12•11 years ago
|
||
bug 924724, which is koi+, depends on the change here. Need to uplift this patch
blocking-b2g: --- → koi?
|
|
||
Comment 13•11 years ago
|
||
(In reply to C.J. Ku[:CJKu] from comment #12)
> bug 924724, which is koi+, depends on the change here. Need to uplift this
> patch
That shouldn't influence if this bug is a blocker or not. What you need to do on bug 924724 is build a branch-specific patch to fix the issue independent of this bug.
I don't think this bug is a blocker either - it's not a likely use case a developer will do & memory leak impact isn't critical.
blocking-b2g: koi? → -
|
|
||
Comment 14•11 years ago
|
||
If the branch-specific patch ends up being "roll these changes into that patch", it would be preferable to just land these changes under this bug number. Also, we have a lot of historical precedent for "blocks a blocker" approvals.
|
||
Updated•11 years ago
|
Component: Video/Audio → Video/Audio: Recording
|
|
||
Updated•11 years ago
|
No longer blocks: MediaRecording
You need to log in
before you can comment on or make changes to this bug.
Description
•