shared/js/media/video_player.js leaks memory

RESOLVED FIXED

Status

Firefox OS
Gaia::Video
P1
normal
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: djf, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
The VideoPlayer class in shared/js/media/video_player.js registers event listeners on the window object for resize and visibilitychanged events and never unregisters them. This means that VideoPlayer objects can never be garbage collected because the window will always have a reference to every one that has been created.

Currently, I think that video_player.js is only used by media_frame.js and media_frame.js is only used by Gallery and Camera. And both of those apps only create a fixed number of permanent MediaFrame objects. So this memory leak is not currently affecting anything.

Given that we are replacing video_player.js with a web component, I suppose we don't have to fix this. But we should at least update the documentation with a big fat warning about not creating and discarding VideoPlayer objects.
(Reporter)

Comment 1

3 years ago
Russ,

Would you take this and update the documentation at the top of video_player.js, please? I'm bringing this to your attention so you'll know not to repeat my mistake of adding a resize handler in your web component.
Flags: needinfo?(rnicoletti)
Created attachment 8569329 [details] [review]
[gaia] russnicoletti:bug-1135278 > mozilla-b2g:master
Flags: needinfo?(rnicoletti)
Attachment #8569329 - Flags: review?(dflanagan)
(Reporter)

Comment 3

3 years ago
Comment on attachment 8569329 [details] [review]
[gaia] russnicoletti:bug-1135278 > mozilla-b2g:master

That's a nice, clear comment. Thanks.
Attachment #8569329 - Flags: review?(dflanagan) → review+

Updated

3 years ago
Flags: needinfo?(rnicoletti)
OS: Mac OS X → Gonk (Firefox OS)
Priority: -- → P1
Hardware: x86 → ARM

Comment 4

3 years ago
Russ, I saw this was merged to master by Mar 3. Could you help confirm this? and close bug.
Master: https://github.com/mozilla-b2g/gaia/commit/ea958a17fc51a0e735d5318291db46d499f2895b
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Flags: needinfo?(rnicoletti)
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.