New issue
Advanced search Search tips

Issue 832043 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Webaudio webkit_layout_tests failing on chromium.webkit/WebKit Linux Trusty Leak

Project Member Reported by sheriff-...@appspot.gserviceaccount.com, Apr 12 2018

Issue description

Filed by sheriff-o-matic@appspot.gserviceaccount.com on behalf of grunell@chromium.org

Webaudio webkit_layout_tests failing on chromium.webkit/WebKit Linux Trusty Leak

Builders failed on: 
- WebKit Linux Trusty Leak: 
  https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20Trusty%20Leak

 
Components: Blink>WebAudio
Labels: -Pri-2 -Sheriff-Chromium OS-Linux Pri-1 Type-Bug-Regression
Owner: rtoy@chromium.org
Status: Assigned (was: Available)
Seems to be caused by https://chromium-review.googlesource.com/996387

Comment 2 by rtoy@chromium.org, Apr 12 2018

Status: Started (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Apr 13 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cecfc61c401ecdee47132344a6b5347dfb1c85f0

commit cecfc61c401ecdee47132344a6b5347dfb1c85f0
Author: Raymond Toy <rtoy@chromium.org>
Date: Fri Apr 13 01:50:49 2018

Fix leaks in WebAudio layout tests

When the context is closing, the nodes that were processing their
tails need to be stopped.  This disables the output of the node, which
might cause other nodes to disable their outputs, and these nodes are
placed on |finished_tail_processing_handlers_|.  These also need to be
handled when finishing tail processing.  So loop around processing
|finished_tail_processing_handlers_| and |tail_processing_handlers_|
until disabling of outputs no longer adds anything to either of these.


Bug:  832043 ,  828826 
Test: run-webkit-tests --enable-leak-detection no longer detects leaks
Change-Id: I6ca195352b240d343095c208c9b70b0c5553c925
Reviewed-on: https://chromium-review.googlesource.com/1010833
Commit-Queue: Raymond Toy <rtoy@chromium.org>
Reviewed-by: Hongchan Choi <hongchan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550468}
[modify] https://crrev.com/cecfc61c401ecdee47132344a6b5347dfb1c85f0/third_party/blink/renderer/modules/webaudio/deferred_task_handler.cc
[modify] https://crrev.com/cecfc61c401ecdee47132344a6b5347dfb1c85f0/third_party/blink/renderer/modules/webaudio/deferred_task_handler.h

Cc: -grunell@chromium.org

Comment 5 by rtoy@chromium.org, Apr 13 2018

Status: Fixed (was: Started)

Comment 6 by rtoy@chromium.org, Apr 13 2018

Labels: Merge-Request-67
The CL in c#3 fixes an actual memory leak where AudioNodes were not getting collected as expected.


Project Member

Comment 7 by sheriffbot@chromium.org, Apr 14 2018

Labels: -Merge-Request-67 Merge-Approved-67 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M67. Please go ahead and merge the CL to branch 3396 manually. Please contact milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 8 by gov...@chromium.org, Apr 15 2018

Pls merge your change to M67 branch 3396 by 1:00 PM PT Monday (04/16/18) so we can pick it up for next M67 dev release. Thank you.
Project Member

Comment 9 by bugdroid1@chromium.org, Apr 16 2018

Labels: -merge-approved-67 merge-merged-3396
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7b70b324da48af23dfd7a05c8ef4a96eab57ed65

commit 7b70b324da48af23dfd7a05c8ef4a96eab57ed65
Author: Raymond Toy <rtoy@chromium.org>
Date: Mon Apr 16 14:31:40 2018

Fix leaks in WebAudio layout tests

When the context is closing, the nodes that were processing their
tails need to be stopped.  This disables the output of the node, which
might cause other nodes to disable their outputs, and these nodes are
placed on |finished_tail_processing_handlers_|.  These also need to be
handled when finishing tail processing.  So loop around processing
|finished_tail_processing_handlers_| and |tail_processing_handlers_|
until disabling of outputs no longer adds anything to either of these.


Bug:  832043 ,  828826 
Test: run-webkit-tests --enable-leak-detection no longer detects leaks
Change-Id: I6ca195352b240d343095c208c9b70b0c5553c925
Reviewed-on: https://chromium-review.googlesource.com/1010833
Commit-Queue: Raymond Toy <rtoy@chromium.org>
Reviewed-by: Hongchan Choi <hongchan@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#550468}(cherry picked from commit cecfc61c401ecdee47132344a6b5347dfb1c85f0)
Reviewed-on: https://chromium-review.googlesource.com/1014164
Reviewed-by: Raymond Toy <rtoy@chromium.org>
Cr-Commit-Position: refs/branch-heads/3396@{#14}
Cr-Branched-From: 9ef2aa869bc7bc0c089e255d698cca6e47d6b038-refs/heads/master@{#550428}
[modify] https://crrev.com/7b70b324da48af23dfd7a05c8ef4a96eab57ed65/third_party/blink/renderer/modules/webaudio/deferred_task_handler.cc
[modify] https://crrev.com/7b70b324da48af23dfd7a05c8ef4a96eab57ed65/third_party/blink/renderer/modules/webaudio/deferred_task_handler.h

Project Member

Comment 10 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-testbranch
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cecfc61c401ecdee47132344a6b5347dfb1c85f0

commit cecfc61c401ecdee47132344a6b5347dfb1c85f0
Author: Raymond Toy <rtoy@chromium.org>
Date: Fri Apr 13 01:50:49 2018

Fix leaks in WebAudio layout tests

When the context is closing, the nodes that were processing their
tails need to be stopped.  This disables the output of the node, which
might cause other nodes to disable their outputs, and these nodes are
placed on |finished_tail_processing_handlers_|.  These also need to be
handled when finishing tail processing.  So loop around processing
|finished_tail_processing_handlers_| and |tail_processing_handlers_|
until disabling of outputs no longer adds anything to either of these.


Bug:  832043 ,  828826 
Test: run-webkit-tests --enable-leak-detection no longer detects leaks
Change-Id: I6ca195352b240d343095c208c9b70b0c5553c925
Reviewed-on: https://chromium-review.googlesource.com/1010833
Commit-Queue: Raymond Toy <rtoy@chromium.org>
Reviewed-by: Hongchan Choi <hongchan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550468}
[modify] https://crrev.com/cecfc61c401ecdee47132344a6b5347dfb1c85f0/third_party/blink/renderer/modules/webaudio/deferred_task_handler.cc
[modify] https://crrev.com/cecfc61c401ecdee47132344a6b5347dfb1c85f0/third_party/blink/renderer/modules/webaudio/deferred_task_handler.h

Sign in to add a comment