Move WebAudio source code to std::unique_ptr
Created attachment 221637 [details] Patch
Comment on attachment 221637 [details] Patch Clearing flags on attachment: 221637 Committed r162368: <http://trac.webkit.org/changeset/162368>
All reviewed patches have been landed. Closing bug.
Comment on attachment 221637 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=221637&action=review > Source/WebCore/Modules/webaudio/AudioNode.cpp:334 > - ASSERT(m_inputs.contains(input)); > - if (!m_inputs.contains(input)) > - return; > + for (const std::unique_ptr<AudioNodeInput>& savedInput : m_inputs) { > + if (input == savedInput.get()) { > + input->updateInternalBus(); > + return; > + } > + } > > - input->updateInternalBus(); > + ASSERT_NOT_REACHED(); The new code seems clearly worse than the old here. Is there something we can do about that? Maybe we can overload the contains function so it can be used with a raw pointer in a case like this? > Source/WebCore/platform/audio/HRTFDatabaseLoader.cpp:80 > - m_hrtfDatabase.clear(); > + m_hrtfDatabase.reset(); Normally we just use m_hrtfDatabase = nullptr; for a case like this. > Source/WebCore/platform/audio/HRTFKernel.h:60 > + return adoptRef(new HRTFKernel(std::forward<std::unique_ptr<FFTFrame>>(fftFrame), frameDelay, sampleRate)); I’m really surprised that we needed the template argument for std::forward here. I would expect just std::forward(fftFrame) or std::move(fftFrame) would work.
(In reply to comment #4) > The new code seems clearly worse than the old here. Is there something we can do about that? Maybe we can overload the contains function so it can be used with a raw pointer in a case like this? I'd actually like to get rid of Vector::contains since it's O(n). Forcing users to write out the full loop makes it more obvious that it's a linear operation. > > > Source/WebCore/platform/audio/HRTFDatabaseLoader.cpp:80 > > - m_hrtfDatabase.clear(); > > + m_hrtfDatabase.reset(); > > Normally we just use > > m_hrtfDatabase = nullptr; > > for a case like this. > > > Source/WebCore/platform/audio/HRTFKernel.h:60 > > + return adoptRef(new HRTFKernel(std::forward<std::unique_ptr<FFTFrame>>(fftFrame), frameDelay, sampleRate)); > > I’m really surprised that we needed the template argument for std::forward here. I would expect just std::forward(fftFrame) or std::move(fftFrame) would work. std::forward should only be used together with template type parameters. This should use move.