Use less explicit ref() / deref() calls in WebAudio code.
Created attachment 409498 [details] Patch
Comment on attachment 409498 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=409498&action=review > Source/WebCore/Modules/webaudio/MediaElementAudioSourceNode.cpp:88 > + auto protectedThis = makeRef(*this); Not sure I understand where the need for this comes from exactly. Not that I am arguing to not do it, just that I was wondering how I would have known it’s needed. > Source/WebCore/Modules/webaudio/OfflineAudioDestinationNode.cpp:102 > + m_renderThread = Thread::create("offline renderer", [this, protectedThis = WTFMove(protectedThis)]() mutable { Not sure anyone else agrees, but I’ve always been a fan of *not* capturing this in cases like this. I know it makes the code less elegant.
(In reply to Darin Adler from comment #2) > Comment on attachment 409498 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=409498&action=review > > > Source/WebCore/Modules/webaudio/MediaElementAudioSourceNode.cpp:88 > > + auto protectedThis = makeRef(*this); > > Not sure I understand where the need for this comes from exactly. Not that I > am arguing to not do it, just that I was wondering how I would have known > it’s needed. The pre-existing code was already ref-ing this but from inside the lock() function (and deref'ing in unlock()). I added the protector to maintain pre-existing behavior. > > > Source/WebCore/Modules/webaudio/OfflineAudioDestinationNode.cpp:102 > > + m_renderThread = Thread::create("offline renderer", [this, protectedThis = WTFMove(protectedThis)]() mutable { > > Not sure anyone else agrees, but I’ve always been a fan of *not* capturing > this in cases like this. I know it makes the code less elegant.
Committed r267505: <https://trac.webkit.org/changeset/267505> All reviewed patches have been landed. Closing bug and clearing flags on attachment 409498 [details].
<rdar://problem/69463911>