WebKit Linux Leak (webkit_tests) failure on 9d961d0a083fe4ebd8c200c31b87d75e4727958b |
||||||||
Issue descriptionAccording to https://www.chromium.org/blink/sheriffing, I should update LeakExpectations After that, I'll re-assign to zqzhang@. /cc hajimehoshi@ and kouhei@ as they're mentioned in LeakExpectations My guess is that we're not calling removeEventListener() if a media element isn't played, which causes the leak.
,
Jul 13 2016
,
Jul 13 2016
Hi haraken, can you help me find the leak cause? So basically HTMLMediaElement has Member<ElementVisibilityObserver>, and ElementVisibilityObserver has Member<Element> referencing the element. I think there is a reference cycle, but they are not garbage-collected. Do you have any idea? If I use WeakMember<Element>, the leak is gone. (https://codereview.chromium.org/2144073002/)
,
Jul 14 2016
+szager to take a look
,
Jul 14 2016
I uploaded a different CL that might be preferable to adding WeakMember: https://codereview.chromium.org/2146383002
,
Jul 14 2016
,
Jul 14 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0adb9749b2374898b245206356ab73cd9224913e commit 0adb9749b2374898b245206356ab73cd9224913e Author: szager <szager@chromium.org> Date: Thu Jul 14 23:09:58 2016 ElementVisibilityObserver: get rid of reference cycle. BUG= 627539 R=mlamouri@chromium.org,zqzhang@chromium.org Review-Url: https://codereview.chromium.org/2146383002 Cr-Commit-Position: refs/heads/master@{#405619} [modify] https://crrev.com/0adb9749b2374898b245206356ab73cd9224913e/third_party/WebKit/Source/core/dom/ElementVisibilityObserver.cpp [modify] https://crrev.com/0adb9749b2374898b245206356ab73cd9224913e/third_party/WebKit/Source/core/dom/IntersectionObserver.h
,
Jul 18 2016
,
Jul 18 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/df5e915f2075aa324e59087cfca9848b6ff3f58a commit df5e915f2075aa324e59087cfca9848b6ff3f58a Author: zqzhang <zqzhang@chromium.org> Date: Mon Jul 18 15:20:31 2016 Using wrapWeakPersistent for VisibilityCallback in HTMLMediaElement This is a test patch that fixes a memory leak caused by the reference between HTMLMediaElement and ElementVisibilityObserver. The previous patch breaks the reference cycle but seems doesn't fix the leak. Using WeakPersistent is a workaround, but seems does not fix the leak entirely. Keeping https://crbug.com/627539 open to keep track of the remaining leaks. BUG= 628367 , 627539 Review-Url: https://codereview.chromium.org/2151243002 Cr-Commit-Position: refs/heads/master@{#405996} [modify] https://crrev.com/df5e915f2075aa324e59087cfca9848b6ff3f58a/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp
,
Jul 25 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/baf0ff22e47eb51e6cd4b0764d771151f7e0500b commit baf0ff22e47eb51e6cd4b0764d771151f7e0500b Author: sigbjornf <sigbjornf@opera.com> Date: Mon Jul 25 11:45:02 2016 Simplify ElementVisibilityObserver implementation. Recast ElementVisibilityObserver's VisibilityCallback in a more Blink-like manner by way of a Client interface. Thereby also addressing on-off-heap cycle, a leak source. Similarly, simplify the connection between ElementVisibilityObserver and IntersectionObserver -- have the former directly implement the IntersectionObserverCallback instead of indirectly using closure callbacks. R= BUG= 627539 Review-Url: https://codereview.chromium.org/2173353002 Cr-Commit-Position: refs/heads/master@{#407450} [modify] https://crrev.com/baf0ff22e47eb51e6cd4b0764d771151f7e0500b/third_party/WebKit/Source/core/dom/ElementVisibilityObserver.cpp [modify] https://crrev.com/baf0ff22e47eb51e6cd4b0764d771151f7e0500b/third_party/WebKit/Source/core/dom/ElementVisibilityObserver.h [modify] https://crrev.com/baf0ff22e47eb51e6cd4b0764d771151f7e0500b/third_party/WebKit/Source/core/dom/IntersectionObserver.h [modify] https://crrev.com/baf0ff22e47eb51e6cd4b0764d771151f7e0500b/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp [modify] https://crrev.com/baf0ff22e47eb51e6cd4b0764d771151f7e0500b/third_party/WebKit/Source/core/html/HTMLMediaElement.h
,
Aug 9 2016
,
Aug 12 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8c01587efe00372011ab1aac3987e9937c2da6ce commit 8c01587efe00372011ab1aac3987e9937c2da6ce Author: zqzhang <zqzhang@chromium.org> Date: Fri Aug 12 12:46:06 2016 Revert "Simplify ElementVisibilityObserver implementation." This reverts commit baf0ff22e47eb51e6cd4b0764d771151f7e0500b (https://codereview.chromium.org/2173353002/). The reason for this revert is that the previous commit does not fix the leak and does not provide real benefit to users of ElementVisibilityObserver. BUG= 627539 Review-Url: https://codereview.chromium.org/2245613002 Cr-Commit-Position: refs/heads/master@{#411622} [modify] https://crrev.com/8c01587efe00372011ab1aac3987e9937c2da6ce/third_party/WebKit/Source/core/dom/ElementVisibilityObserver.cpp [modify] https://crrev.com/8c01587efe00372011ab1aac3987e9937c2da6ce/third_party/WebKit/Source/core/dom/ElementVisibilityObserver.h [modify] https://crrev.com/8c01587efe00372011ab1aac3987e9937c2da6ce/third_party/WebKit/Source/core/dom/IntersectionObserver.h [modify] https://crrev.com/8c01587efe00372011ab1aac3987e9937c2da6ce/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp [modify] https://crrev.com/8c01587efe00372011ab1aac3987e9937c2da6ce/third_party/WebKit/Source/core/html/HTMLMediaElement.h
,
Aug 16 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/339881fcea518c6f7ff6e1a04a2a5f40e26de850 commit 339881fcea518c6f7ff6e1a04a2a5f40e26de850 Author: zqzhang <zqzhang@chromium.org> Date: Tue Aug 16 10:56:54 2016 Adding test to avoid memory leak when autoplayed video never become visible BUG= 627539 Review-Url: https://codereview.chromium.org/2251463002 Cr-Commit-Position: refs/heads/master@{#412210} [add] https://crrev.com/339881fcea518c6f7ff6e1a04a2a5f40e26de850/third_party/WebKit/LayoutTests/media/autoplay-never-visible.html
,
Aug 16 2016
zqzhang@, should this be fixed?
,
Aug 16 2016
I think we can mark this as fixed for now. The leak is very hard to trigger, and the tests does not catch it. Quoting my reply in the CL: > The leak seems to be super hard to trigger now. > > It only happens when I comment out `m_autoplayVisibilityObserver->stop()` in > HTMLMediaElement, and run `autoplay-muted` tests. > I narrowed down `autoplay-muted` tests and found that 3 of async_test/promise_test can trigger the leak when they are run at the same time, > and one test that should have failed due to commenting `stop()` does not fail. > > So my guess is that there exist some races among the async_tests/promise_tests > that caused the leak. This might also explain why the leak is flaky.
,
Oct 26 2017
|
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by bugdroid1@chromium.org
, Jul 12 2016