At the time of writing, I did not know how to use the function. How about changing:

ModulesInitializer::CollectGarbageForAnimationWorklet =>
  ModulesInitializer::CollectGarbageForWorklets

And make that GC method do the clean up like this?

{
  AnimationWorkletThread::CollectAllGarbage()
  AudioWorkletThread::CollectAllGarbage()
}

Does it make sense?

Yeah, the change looks good.

By the way, I'm not sure if the leak detector really needs to run GC for audio worklets there. If it's necessary, the leak detector should warn memory leaks for the current implementation, right?

+yuzus@,keishi@: Any thoughts on this?

Yes. We probably have tests for audioworkletthreads so if an audio worklet thread is keeping objects alive it should have come up on the Leak bot. I did not write this code, but IIUC AnimationWorkletThread::CollectAllGarbage is necessary because unlike WorkerThreads they persist even when navigating to about:blank. Will AudioWorkletThread also persist when navigating to about:blank? If so I don't understand why this hasn't been detected as a leak (of kV8PerContextDataCounter or something) already.

Thank you for the clarification, keishi!

Regarding AudioWorkletThread, its backing thread is a per-process refcounted instance and terminated on navigation if other frames in the same renderer process don't use it. I guess there're no tests to run AudioWorklet in multiple iframes, so the leak detector have said nothing so far.

Regarding AnimationWorkletThread, its backing thread is also a per-process instance but it's not refcounted, so navigation doesn't terminate the backing thread. This should be the reason why the leak detector complains on animation worklet tests.

hongchan@: According to c#3-4, probably we have to run the GC function for AudioWorklet there. Can you make a patch like c#1?

I'll file a separate issue to make AnimationWorkletThread's backing thread ref-counted.

Filed  issue 892527 

BlinkLeakDetector will only work with one main frame. BlinkLeakDetector will run GC and count # of objects when there is one frame (displaying about:blank) in the process. I think this means, RefCount will guarantee that AudioWorkletThread is terminated, so we won't need the GC function for AudioWorklet.

Side not: yuzus@, I wonder if frames created with window.open() are closed before we do leak detection. Maybe it's causing false positives for real websites leak detection. Do you know?

Re #5:

Yeah, the CL is simple and easy, but per #8 we might not need to call the manual GC at all for the AudioWorkletThread. Do we still want to work on a CL like #1?

FYI WIP CL: https://chromium-review.googlesource.com/c/chromium/src/+/1265995

We can abandon it if it's not needed.

Due to its design, BlinkLeakDetector will only *ever* work with one main frame. So sorry but I don't think the CL is needed.

Thanks keishi@.

Then I will repurpose the CL to remove AudioWorkletThread::CollectAllGarbage().

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

commit 8c059b0f0d024c3a2cbeafa8a2e44416a4e6c46f
Author: Hongchan Choi <hongchan@chromium.org>
Date: Thu Oct 11 00:28:34 2018

Remove AudioWorkletThread::CollectAllGarbage()

nhiroki@ found that the method is never called and we decided to
remove the call. AudioWorkletThread is a ref-counted object, so
it does not need help of the blink garbage collector.

Bug:  892006 
Change-Id: I5ed17101f94ee68765e471624f599eede491dd03
Reviewed-on: https://chromium-review.googlesource.com/c/1265995
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Hongchan Choi <hongchan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598589}
[modify] https://crrev.com/8c059b0f0d024c3a2cbeafa8a2e44416a4e6c46f/third_party/blink/renderer/modules/webaudio/audio_worklet_thread.cc
[modify] https://crrev.com/8c059b0f0d024c3a2cbeafa8a2e44416a4e6c46f/third_party/blink/renderer/modules/webaudio/audio_worklet_thread.h

Thank you, hongchan@!