Issue metadata
Sign in to add a comment
|
Memory leak when continuously playing video
Reported by
syed.hus...@appspace.com,
Jan 13 2017
|
||||||||||||||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (X11; CrOS x86_64 9000.50.0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/56.0.2924.53 Safari/537.36 Platform: 9000.50.0 (Official Build) beta-channel panther Steps to reproduce the problem: 1. Install CRX from here : https://www.dropbox.com/s/5e9l2yei0ts58p1/Appspace-videotest.crx?dl=0 2. Open app, it should play a video showing mountain continuously 3. Open task manager, the memory will climb up gradually until App restart itself What is the expected behavior? Memory is at the more or less the same level What went wrong? The memory climb up gradually until the App restart itself. In Kiosk mode, it will freeze the App. Did this work before? Yes 53 Chrome version: 56.0.2924.53 Channel: n/a OS Version: 9000.50.0 Flash Version: Shockwave Flash 24.0 r0
,
Jan 13 2017
Providing a heap trace is likely very useful; see https://chromium.googlesource.com/chromium/src/+/master/docs/memory-infra/heap_profiler.md
,
Jan 14 2017
The video file is located inside the crx and is referenced in video source using relative URL. I'll try to provide the heap profile.
,
Jan 16 2017
Additional info, I've managed to replicate it successfully using Asus Chromebox CN60 running the beta firmware 56
,
Jan 17 2017
Any idea if this can be reproduced on a website outside a CRX? Absent such a repro I'll remove Component:Blink - assume it's a CrOS-app specific issue. /cc abodenha. Please re-add Component:Blink if this is found to reproduce on a web page on any chrome platform. Since this is reported as a regression, our best bet is probably do try to do a bisect. How long do you typically need to leave the video playing for before the increase in memory is obvious? If it's a long time then it may be challenging to bisect.
,
Jan 18 2017
It's not reproducible when running it as a website. The memory does climb up but eventually will go back down. The increase in memory is pretty much directly when the app starts. If you run the CRX in desktop mode, eventually the app will restart itself - I believe because the app was out of memory. I haven't run this in kiosk mode.
,
Jan 24 2017
since this is not html5 video on Chrome, but repro on an CrOS app. remove chrome media team.
,
Feb 3 2017
Hi, is there any progress/update on this? Anything that I can help in order to get this going?
,
Feb 3 2017
I can confirm the same behavior as the author of this thread. Nonstop video playback eats up memory inside CRX.
,
Feb 20 2017
,
Feb 21 2017
,
Feb 21 2017
steel@, could you find an owner for this? The app that could repro the problem is also published to cws: https://chrome.google.com/webstore/detail/appspace/ibdfmgnhfbdhbepdiellpehbkkagildf
,
Feb 23 2017
,
Feb 25 2017
Seems to be https://chromium.googlesource.com/chromium/src/+/2c68be804b1f752a88b2c0e2e7a086945ef0494c When created, MediaControlsMediaEventListener adds itself as a listener for the document's fullscreenchange event [1], but never gets removed, which means that the event listener remains reachable (and thus lingers on) throughout document's lifetime. This becomes very noticeable linked in this issue - the app keeps creating video elements, each accompanied by its MediaControlsMediaEventListener - the app plays a video clip in a loop by discarding current video element and replacing it with a newly created one when the video clip reaches the end. [1] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/html/shadow/MediaControlsMediaEventListener.cpp?rcl=1aa3f1c709ebdc77c75fa90263b983d181af91e1&l=30
,
Mar 1 2017
,
Mar 1 2017
,
Mar 1 2017
URGENT - PTAL ASAP. A friendly reminder that M57 Stable is launch is coming soon! Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and get it merged into the release branch by Friday (03/03), 5:00 PM PST in order to make into the desktop Stable final build cut. Thank you! Know that this issue shouldn't block the release? Remove the ReleaseBlock-Stable label or move to M58. Thank you.
,
Mar 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8daac32c17f494d496fb2ca3768d67d9bd77db5e commit 8daac32c17f494d496fb2ca3768d67d9bd77db5e Author: mlamouri <mlamouri@chromium.org> Date: Wed Mar 01 22:10:03 2017 Media: fix memory leak because of the document holding on an EventListener. MediaControls checks if they are part of a document or not in order to unregister most of its event listeners when removed from a document to avoid leaking memory until the document is closed. The approach taken in this CL is the most "web based" because the controls are using events. Another approach would be to use internal callbacks (from ContainerNode) or even have a `addWeakEventListener` method in EventTarget. The latter might be an interesting solution. BUG= 680898 R=zqzhang@chromium.org,ojan@chromium.org Review-Url: https://codereview.chromium.org/2725873002 Cr-Commit-Position: refs/heads/master@{#454060} [modify] https://crrev.com/8daac32c17f494d496fb2ca3768d67d9bd77db5e/third_party/WebKit/Source/core/BUILD.gn [modify] https://crrev.com/8daac32c17f494d496fb2ca3768d67d9bd77db5e/third_party/WebKit/Source/core/html/shadow/MediaControls.cpp [modify] https://crrev.com/8daac32c17f494d496fb2ca3768d67d9bd77db5e/third_party/WebKit/Source/core/html/shadow/MediaControls.h [add] https://crrev.com/8daac32c17f494d496fb2ca3768d67d9bd77db5e/third_party/WebKit/Source/core/html/shadow/MediaControlsLeakTest.cpp [modify] https://crrev.com/8daac32c17f494d496fb2ca3768d67d9bd77db5e/third_party/WebKit/Source/core/html/shadow/MediaControlsMediaEventListener.cpp [modify] https://crrev.com/8daac32c17f494d496fb2ca3768d67d9bd77db5e/third_party/WebKit/Source/core/html/shadow/MediaControlsMediaEventListener.h [modify] https://crrev.com/8daac32c17f494d496fb2ca3768d67d9bd77db5e/third_party/WebKit/Source/core/html/shadow/MediaControlsOrientationLockDelegate.cpp [modify] https://crrev.com/8daac32c17f494d496fb2ca3768d67d9bd77db5e/third_party/WebKit/Source/core/html/shadow/MediaControlsOrientationLockDelegate.h
,
Mar 2 2017
,
Mar 2 2017
Rechecked the issue on chrome version 58.0.3028.0 on Windows, MAC (Unsigned Build) and ubuntu OS as per the steps provided. Memory fluctuated between 18200 KB to 19200 KB, but did not observe more spike. Played the app continuously for 20 Minutes. After 20 minutes memory slowly increased to 21344 KB. App neither restarted itself. Assuming this to be fixed, @ mlamouri: Request you to please confirm if this is expected. Thanks.!
,
Mar 2 2017
ranjitkan@, what's the numbers for earlier versions?
,
Mar 2 2017
@mlamouri: Just rechecked on Dev 58.0.3026.3 & Beta 57.0.2987.88 and observed that on launching the app, memory goes straight away to 56995 KB and within 2 minutes it reached 65233 KB on Windows 10 and MAC 10.12.3 But again did not observed an APP restart. Thanks.!
,
Mar 2 2017
App restart would probably happen because the computer went OOM. It would happen if you would run the app for long enough on a resource constrained machine. Thank you for confirming this is fixed! :) As soon as I have the green light, I will merge this to M57.
,
Mar 2 2017
Approving merge to M57 branch 2987 based on comment #22 and #23. Also per chat with mlamouri@, this is a safe merge. There should be no visible differences to the end user apart from saving memory. Please merge ASAP. Thank you.
,
Mar 6 2017
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Mar 7 2017
Adding TE-Verified labels for M58 as per the confirmation in comment#23
,
Mar 7 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e724b2c3898091f20dd9e9c302cb12c86cdf5e5f commit e724b2c3898091f20dd9e9c302cb12c86cdf5e5f Author: Mounir Lamouri <mlamouri@chromium.org> Date: Tue Mar 07 15:10:44 2017 Media: fix memory leak because of the document holding on an EventListener. MediaControls checks if they are part of a document or not in order to unregister most of its event listeners when removed from a document to avoid leaking memory until the document is closed. The approach taken in this CL is the most "web based" because the controls are using events. Another approach would be to use internal callbacks (from ContainerNode) or even have a `addWeakEventListener` method in EventTarget. The latter might be an interesting solution. BUG= 680898 R=zqzhang@chromium.org,ojan@chromium.org Review-Url: https://codereview.chromium.org/2725873002 Cr-Commit-Position: refs/heads/master@{#454060} (cherry picked from commit 8daac32c17f494d496fb2ca3768d67d9bd77db5e) Review-Url: https://codereview.chromium.org/2736853003 . Cr-Commit-Position: refs/branch-heads/2987@{#783} Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943} [modify] https://crrev.com/e724b2c3898091f20dd9e9c302cb12c86cdf5e5f/third_party/WebKit/Source/core/BUILD.gn [modify] https://crrev.com/e724b2c3898091f20dd9e9c302cb12c86cdf5e5f/third_party/WebKit/Source/core/html/shadow/MediaControls.cpp [modify] https://crrev.com/e724b2c3898091f20dd9e9c302cb12c86cdf5e5f/third_party/WebKit/Source/core/html/shadow/MediaControls.h [add] https://crrev.com/e724b2c3898091f20dd9e9c302cb12c86cdf5e5f/third_party/WebKit/Source/core/html/shadow/MediaControlsLeakTest.cpp [modify] https://crrev.com/e724b2c3898091f20dd9e9c302cb12c86cdf5e5f/third_party/WebKit/Source/core/html/shadow/MediaControlsMediaEventListener.cpp [modify] https://crrev.com/e724b2c3898091f20dd9e9c302cb12c86cdf5e5f/third_party/WebKit/Source/core/html/shadow/MediaControlsMediaEventListener.h [modify] https://crrev.com/e724b2c3898091f20dd9e9c302cb12c86cdf5e5f/third_party/WebKit/Source/core/html/shadow/MediaControlsOrientationLockDelegate.cpp [modify] https://crrev.com/e724b2c3898091f20dd9e9c302cb12c86cdf5e5f/third_party/WebKit/Source/core/html/shadow/MediaControlsOrientationLockDelegate.h
,
Mar 7 2017
,
Mar 8 2017
Verified on Mac os 10.12.2 , ubuntu 14.04 and windows 7 using M57 #57.0.2987.96 and issue is fixed. Played app for 5 minutes and observed memory was at the same level and didn't increase drastically. Attached screenshot for reference. Adding TE-Verified labels. Thanks!
,
Mar 8 2017
I have been watching this with great interest. Can you confirm when this will be ready for public deployment? We have customers with Chromebox/bits and waiting for the M57 release. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by rch@chromium.org
, Jan 13 2017