OfflineAudioContext constructor should not throw when given a bad buffer length. We should instead throw later on when trying to start rendering.
Created attachment 408864 [details] Patch
Created attachment 408875 [details] Patch
The GTK/WPE error does not make much sense to me. Maybe it needs a clean build.
Comment on attachment 408875 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=408875&action=review > Source/WebCore/Modules/webaudio/OfflineAudioDestinationNode.cpp:44 > +OfflineAudioDestinationNode::OfflineAudioDestinationNode(BaseAudioContext& context, unsigned numberOfChannels, AudioBuffer* renderTarget) could be a RefPtr<AudioBuffer>&&? > Source/WebCore/Modules/webaudio/WebKitOfflineAudioContext.cpp:39 > +inline WebKitOfflineAudioContext::WebKitOfflineAudioContext(Document& document, Ref<AudioBuffer>&& renderTarget) inline not needed
retrying gtk build in case it works.
(In reply to Chris Dumez from comment #3) > The GTK/WPE error does not make much sense to me. Maybe it needs a clean > build. It was a build issue masked by unified builds; after updating the patch from bug #216599 it was possible for me to add this patch on top and have builds complete for both the GTK and WPE ports.
Comment on attachment 408875 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=408875&action=review > Source/WebCore/Modules/webaudio/WebKitAudioContext.cpp:88 > +WebKitAudioContext::WebKitAudioContext(Document& document, Ref<AudioBuffer>&& renderTarget) > + : BaseAudioContext(document, renderTarget->numberOfChannels(), WTFMove(renderTarget)) I think this could be problematic, is it defined that the move happens after the ->numberOfChannels(). This has caused issues in the past with GCC. I think it would be safer if this receives the number of channels as a parameter too.
(In reply to Carlos Garcia Campos from comment #7) > Comment on attachment 408875 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=408875&action=review > > > Source/WebCore/Modules/webaudio/WebKitAudioContext.cpp:88 > > +WebKitAudioContext::WebKitAudioContext(Document& document, Ref<AudioBuffer>&& renderTarget) > > + : BaseAudioContext(document, renderTarget->numberOfChannels(), WTFMove(renderTarget)) > > I think this could be problematic, is it defined that the move happens after > the ->numberOfChannels(). This has caused issues in the past with GCC. I > think it would be safer if this receives the number of channels as a > parameter too. A WTFMove() is just a cast though so it should not matter, even if the order is not guaranteed. The parameter type is an rvalue reference so the move constructor will not run until the constructor actually saves it into its data member.
Created attachment 408923 [details] Patch
(In reply to Adrian Perez from comment #6) > (In reply to Chris Dumez from comment #3) > > The GTK/WPE error does not make much sense to me. Maybe it needs a clean > > build. > > It was a build issue masked by unified builds; after updating the patch > from bug #216599 it was possible for me to add this patch on top and have > builds complete for both the GTK and WPE ports. It does not look like it did the trick. GTK/WPE bots still seem unhappy.
Created attachment 408924 [details] Patch
Comment on attachment 408924 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=408924&action=review > Source/WebCore/Modules/webaudio/WebKitOfflineAudioContext.h:35 > + ~WebKitOfflineAudioContext(); Looks like adding this destructor fixed the WPE build.
Comment on attachment 408924 [details] Patch Clearing flags on attachment: 408924 Committed r267147: <https://trac.webkit.org/changeset/267147>
All reviewed patches have been landed. Closing bug.
<rdar://problem/68992445>