Title: [199751] trunk
Revision
199751
Author
cdu...@apple.com
Date
2016-04-19 18:30:11 -0700 (Tue, 19 Apr 2016)

Log Message

AudioBufferSourceNode.buffer should be nullable
https://bugs.webkit.org/show_bug.cgi?id=156769

Reviewed by Darin Adler.

Source/WebCore:

AudioBufferSourceNode.buffer should be nullable as per the
specification:
https://webaudio.github.io/web-audio-api/#AudioBufferSourceNode

Our implementation was initially returning null when getting
AudioBufferSourceNode.buffer, which is correct. However, it would
throw a TypeError when trying to set the attribute to null. Our
implementation setter actually supported setting the buffer to
null but the custom bindings for the setter would not.

This patch does the following:
- Get rid of the custom bindings for the AudioBufferSourceNode.buffer
  setter. We can have the bindings generator generate the same code
  by using [StrictTypeChecking]. The custom bindinds were also throwing
  a TypeError if the input AudioBuffer had too many channels but this
  does not seem to be possible.
- Mark AudioBufferSourceNode.buffer as nullable in the IDL so that
  we no longer throw when the JS tries to assign null, but instead
  calls AudioBufferSourceNode::setBuffer(nullptr)

No new test, updated webaudio/audiobuffersource-channels.html

* CMakeLists.txt:
* Modules/webaudio/AudioBufferSourceNode.cpp:
(WebCore::AudioBufferSourceNode::setBuffer):
* Modules/webaudio/AudioBufferSourceNode.h:
* Modules/webaudio/AudioBufferSourceNode.idl:
* Modules/webaudio/AudioContext.h:
* WebCore.xcodeproj/project.pbxproj:
* bindings/js/JSAudioBufferSourceNodeCustom.cpp: Removed.

LayoutTests:

Update existing layout test to check that:
- AudioBufferSourceNode.buffer is initially null
- AudioBufferSourceNode.buffer can be set to null
- We cannot create an AudioBuffer that has too many channels

* webaudio/audiobuffersource-channels-expected.txt:
* webaudio/audiobuffersource-channels.html:

Modified Paths

Removed Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (199750 => 199751)


--- trunk/LayoutTests/ChangeLog	2016-04-20 01:19:59 UTC (rev 199750)
+++ trunk/LayoutTests/ChangeLog	2016-04-20 01:30:11 UTC (rev 199751)
@@ -1,3 +1,18 @@
+2016-04-19  Chris Dumez  <cdu...@apple.com>
+
+        AudioBufferSourceNode.buffer should be nullable
+        https://bugs.webkit.org/show_bug.cgi?id=156769
+
+        Reviewed by Darin Adler.
+
+        Update existing layout test to check that:
+        - AudioBufferSourceNode.buffer is initially null
+        - AudioBufferSourceNode.buffer can be set to null
+        - We cannot create an AudioBuffer that has too many channels
+
+        * webaudio/audiobuffersource-channels-expected.txt:
+        * webaudio/audiobuffersource-channels.html:
+
 2016-04-19  Brady Eidson  <beid...@apple.com>
 
         Modern IDB: Lots of IDB bindings cleanup (including making IDBVersionChangeEvent constructible).

Modified: trunk/LayoutTests/webaudio/audiobuffersource-channels-expected.txt (199750 => 199751)


--- trunk/LayoutTests/webaudio/audiobuffersource-channels-expected.txt	2016-04-20 01:19:59 UTC (rev 199750)
+++ trunk/LayoutTests/webaudio/audiobuffersource-channels-expected.txt	2016-04-20 01:30:11 UTC (rev 199751)
@@ -2,7 +2,8 @@
 
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
-PASS source.buffer = 57 threw exception TypeError: Value is not of type AudioBuffer.
+PASS source.buffer is null
+PASS source.buffer = 57 threw exception TypeError: The AudioBufferSourceNode.buffer attribute must be an instance of AudioBuffer.
 PASS Mono buffer can be set.
 PASS Stereo buffer can be set.
 PASS 3 channels buffer can be set.
@@ -12,6 +13,10 @@
 PASS 7 channels buffer can be set.
 PASS 8 channels buffer can be set.
 PASS 9 channels buffer can be set.
+PASS context.createBuffer(64, 1024, context.sampleRate) threw exception Error: NotSupportedError: DOM Exception 9.
+PASS source.buffer is not null
+PASS source.buffer = null did not throw exception.
+PASS source.buffer is null
 PASS successfullyParsed is true
 
 TEST COMPLETE

Modified: trunk/LayoutTests/webaudio/audiobuffersource-channels.html (199750 => 199751)


--- trunk/LayoutTests/webaudio/audiobuffersource-channels.html	2016-04-20 01:19:59 UTC (rev 199750)
+++ trunk/LayoutTests/webaudio/audiobuffersource-channels.html	2016-04-20 01:30:11 UTC (rev 199751)
@@ -27,6 +27,8 @@
     context = new webkitAudioContext();
     source = context.createBufferSource();
 
+    shouldBeNull("source.buffer");
+
     // Make sure we can't set to something which isn't an AudioBuffer.
     shouldThrow("source.buffer = 57");
 
@@ -60,6 +62,12 @@
             testFailed(message);
         }
     }
+
+    shouldThrow("context.createBuffer(64, 1024, context.sampleRate)");
+
+    shouldNotBe("source.buffer", "null");
+    shouldNotThrow("source.buffer = null");
+    shouldBeNull("source.buffer");
         
     finishJSTest();
 }

Modified: trunk/Source/WebCore/CMakeLists.txt (199750 => 199751)


--- trunk/Source/WebCore/CMakeLists.txt	2016-04-20 01:19:59 UTC (rev 199750)
+++ trunk/Source/WebCore/CMakeLists.txt	2016-04-20 01:30:11 UTC (rev 199751)
@@ -1084,7 +1084,6 @@
     bindings/js/IDBBindingUtilities.cpp
     bindings/js/JSAnimationTimelineCustom.cpp
     bindings/js/JSAttrCustom.cpp
-    bindings/js/JSAudioBufferSourceNodeCustom.cpp
     bindings/js/JSAudioContextCustom.cpp
     bindings/js/JSAudioTrackCustom.cpp
     bindings/js/JSAudioTrackListCustom.cpp

Modified: trunk/Source/WebCore/ChangeLog (199750 => 199751)


--- trunk/Source/WebCore/ChangeLog	2016-04-20 01:19:59 UTC (rev 199750)
+++ trunk/Source/WebCore/ChangeLog	2016-04-20 01:30:11 UTC (rev 199751)
@@ -1,3 +1,41 @@
+2016-04-19  Chris Dumez  <cdu...@apple.com>
+
+        AudioBufferSourceNode.buffer should be nullable
+        https://bugs.webkit.org/show_bug.cgi?id=156769
+
+        Reviewed by Darin Adler.
+
+        AudioBufferSourceNode.buffer should be nullable as per the
+        specification:
+        https://webaudio.github.io/web-audio-api/#AudioBufferSourceNode
+
+        Our implementation was initially returning null when getting
+        AudioBufferSourceNode.buffer, which is correct. However, it would
+        throw a TypeError when trying to set the attribute to null. Our
+        implementation setter actually supported setting the buffer to
+        null but the custom bindings for the setter would not.
+
+        This patch does the following:
+        - Get rid of the custom bindings for the AudioBufferSourceNode.buffer
+          setter. We can have the bindings generator generate the same code
+          by using [StrictTypeChecking]. The custom bindinds were also throwing
+          a TypeError if the input AudioBuffer had too many channels but this
+          does not seem to be possible.
+        - Mark AudioBufferSourceNode.buffer as nullable in the IDL so that
+          we no longer throw when the JS tries to assign null, but instead
+          calls AudioBufferSourceNode::setBuffer(nullptr)
+
+        No new test, updated webaudio/audiobuffersource-channels.html
+
+        * CMakeLists.txt:
+        * Modules/webaudio/AudioBufferSourceNode.cpp:
+        (WebCore::AudioBufferSourceNode::setBuffer):
+        * Modules/webaudio/AudioBufferSourceNode.h:
+        * Modules/webaudio/AudioBufferSourceNode.idl:
+        * Modules/webaudio/AudioContext.h:
+        * WebCore.xcodeproj/project.pbxproj:
+        * bindings/js/JSAudioBufferSourceNodeCustom.cpp: Removed.
+
 2016-04-19  Brady Eidson  <beid...@apple.com>
 
         Modern IDB: Lots of IDB bindings cleanup (including making IDBVersionChangeEvent constructible).

Modified: trunk/Source/WebCore/Modules/webaudio/AudioBufferSourceNode.cpp (199750 => 199751)


--- trunk/Source/WebCore/Modules/webaudio/AudioBufferSourceNode.cpp	2016-04-20 01:19:59 UTC (rev 199750)
+++ trunk/Source/WebCore/Modules/webaudio/AudioBufferSourceNode.cpp	2016-04-20 01:30:11 UTC (rev 199751)
@@ -408,7 +408,7 @@
     m_lastGain = gain()->value();
 }
 
-bool AudioBufferSourceNode::setBuffer(AudioBuffer* buffer)
+void AudioBufferSourceNode::setBuffer(RefPtr<AudioBuffer>&& buffer)
 {
     ASSERT(isMainThread());
     
@@ -421,10 +421,8 @@
     if (buffer) {
         // Do any necesssary re-configuration to the buffer's number of channels.
         unsigned numberOfChannels = buffer->numberOfChannels();
+        ASSERT(numberOfChannels <= AudioContext::maxNumberOfChannels());
 
-        if (numberOfChannels > AudioContext::maxNumberOfChannels())
-            return false;
-
         output(0)->setNumberOfChannels(numberOfChannels);
 
         m_sourceChannels = std::make_unique<const float*[]>(numberOfChannels);
@@ -435,9 +433,7 @@
     }
 
     m_virtualReadIndex = 0;
-    m_buffer = buffer;
-    
-    return true;
+    m_buffer = WTFMove(buffer);
 }
 
 unsigned AudioBufferSourceNode::numberOfChannels()

Modified: trunk/Source/WebCore/Modules/webaudio/AudioBufferSourceNode.h (199750 => 199751)


--- trunk/Source/WebCore/Modules/webaudio/AudioBufferSourceNode.h	2016-04-20 01:19:59 UTC (rev 199750)
+++ trunk/Source/WebCore/Modules/webaudio/AudioBufferSourceNode.h	2016-04-20 01:30:11 UTC (rev 199751)
@@ -55,7 +55,7 @@
 
     // setBuffer() is called on the main thread.  This is the buffer we use for playback.
     // returns true on success.
-    bool setBuffer(AudioBuffer*);
+    void setBuffer(RefPtr<AudioBuffer>&&);
     AudioBuffer* buffer() { return m_buffer.get(); }
 
     // numberOfChannels() returns the number of output channels.  This value equals the number of channels from the buffer.

Modified: trunk/Source/WebCore/Modules/webaudio/AudioBufferSourceNode.idl (199750 => 199751)


--- trunk/Source/WebCore/Modules/webaudio/AudioBufferSourceNode.idl	2016-04-20 01:19:59 UTC (rev 199750)
+++ trunk/Source/WebCore/Modules/webaudio/AudioBufferSourceNode.idl	2016-04-20 01:30:11 UTC (rev 199751)
@@ -27,7 +27,7 @@
     Conditional=WEB_AUDIO,
     JSGenerateToJSObject,
 ] interface AudioBufferSourceNode : AudioNode {
-    [CustomSetter, SetterRaisesException] attribute AudioBuffer buffer;
+    [StrictTypeChecking] attribute AudioBuffer? buffer;
 
     const unsigned short UNSCHEDULED_STATE = 0;
     const unsigned short SCHEDULED_STATE = 1;

Modified: trunk/Source/WebCore/Modules/webaudio/AudioContext.h (199750 => 199751)


--- trunk/Source/WebCore/Modules/webaudio/AudioContext.h	2016-04-20 01:19:59 UTC (rev 199750)
+++ trunk/Source/WebCore/Modules/webaudio/AudioContext.h	2016-04-20 01:30:11 UTC (rev 199751)
@@ -204,8 +204,8 @@
     // Returns true if this thread owns the context's lock.
     bool isGraphOwner() const;
 
-    // Returns the maximum numuber of channels we can support.
-    static unsigned maxNumberOfChannels() { return MaxNumberOfChannels;}
+    // Returns the maximum number of channels we can support.
+    static unsigned maxNumberOfChannels() { return MaxNumberOfChannels; }
 
     class AutoLocker {
     public:

Modified: trunk/Source/WebCore/WebCore.xcodeproj/project.pbxproj (199750 => 199751)


--- trunk/Source/WebCore/WebCore.xcodeproj/project.pbxproj	2016-04-20 01:19:59 UTC (rev 199750)
+++ trunk/Source/WebCore/WebCore.xcodeproj/project.pbxproj	2016-04-20 01:30:11 UTC (rev 199751)
@@ -7093,7 +7093,6 @@
 		FDEA6243152102E200479DF0 /* JSOscillatorNode.h in Headers */ = {isa = PBXBuildFile; fileRef = FDEA6241152102E200479DF0 /* JSOscillatorNode.h */; };
 		FDEA6246152102FC00479DF0 /* JSPeriodicWave.cpp in Sources */ = {isa = PBXBuildFile; fileRef = FDEA6244152102FC00479DF0 /* JSPeriodicWave.cpp */; };
 		FDEA6247152102FC00479DF0 /* JSPeriodicWave.h in Headers */ = {isa = PBXBuildFile; fileRef = FDEA6245152102FC00479DF0 /* JSPeriodicWave.h */; };
-		FDEAAAF312B02EE400DCF33B /* JSAudioBufferSourceNodeCustom.cpp in Sources */ = {isa = PBXBuildFile; fileRef = FDEAAAEF12B02EE400DCF33B /* JSAudioBufferSourceNodeCustom.cpp */; };
 		FDEAAAF412B02EE400DCF33B /* JSAudioContextCustom.cpp in Sources */ = {isa = PBXBuildFile; fileRef = FDEAAAF012B02EE400DCF33B /* JSAudioContextCustom.cpp */; };
 		FDF09DC81399B62200688E5B /* JSBiquadFilterNode.cpp in Sources */ = {isa = PBXBuildFile; fileRef = FDF09DC61399B62200688E5B /* JSBiquadFilterNode.cpp */; };
 		FDF09DC91399B62200688E5B /* JSBiquadFilterNode.h in Headers */ = {isa = PBXBuildFile; fileRef = FDF09DC71399B62200688E5B /* JSBiquadFilterNode.h */; };
@@ -15208,7 +15207,6 @@
 		FDEA6241152102E200479DF0 /* JSOscillatorNode.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = JSOscillatorNode.h; sourceTree = "<group>"; };
 		FDEA6244152102FC00479DF0 /* JSPeriodicWave.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = JSPeriodicWave.cpp; sourceTree = "<group>"; };
 		FDEA6245152102FC00479DF0 /* JSPeriodicWave.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = JSPeriodicWave.h; sourceTree = "<group>"; };
-		FDEAAAEF12B02EE400DCF33B /* JSAudioBufferSourceNodeCustom.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = JSAudioBufferSourceNodeCustom.cpp; sourceTree = "<group>"; };
 		FDEAAAF012B02EE400DCF33B /* JSAudioContextCustom.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = JSAudioContextCustom.cpp; sourceTree = "<group>"; };
 		FDF09DC61399B62200688E5B /* JSBiquadFilterNode.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = JSBiquadFilterNode.cpp; sourceTree = "<group>"; };
 		FDF09DC71399B62200688E5B /* JSBiquadFilterNode.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = JSBiquadFilterNode.h; sourceTree = "<group>"; };
@@ -22408,7 +22406,6 @@
 			children = (
 				1221E0721C03E4C2006A1A00 /* JSAnimationTimelineCustom.cpp */,
 				BC2ED6BB0C6BD2F000920BFF /* JSAttrCustom.cpp */,
-				FDEAAAEF12B02EE400DCF33B /* JSAudioBufferSourceNodeCustom.cpp */,
 				FDEAAAF012B02EE400DCF33B /* JSAudioContextCustom.cpp */,
 				BE6DF70E171CA2DA00DD52B8 /* JSAudioTrackCustom.cpp */,
 				BE6DF710171CA2DA00DD52B8 /* JSAudioTrackListCustom.cpp */,
@@ -30133,7 +30130,6 @@
 				FDF7E9C313AC21DB00A51EAC /* JSAudioBufferCallback.cpp in Sources */,
 				FDA15E9F12B03EE1003A583A /* JSAudioBufferSourceNode.cpp in Sources */,
 				69A6CBAC1C6BE42C00B836E9 /* AccessibilitySVGElement.cpp in Sources */,
-				FDEAAAF312B02EE400DCF33B /* JSAudioBufferSourceNodeCustom.cpp in Sources */,
 				FDA15EA512B03EE1003A583A /* JSAudioContext.cpp in Sources */,
 				FDEAAAF412B02EE400DCF33B /* JSAudioContextCustom.cpp in Sources */,
 				FDA15EA712B03EE1003A583A /* JSAudioDestinationNode.cpp in Sources */,

Deleted: trunk/Source/WebCore/bindings/js/JSAudioBufferSourceNodeCustom.cpp (199750 => 199751)


--- trunk/Source/WebCore/bindings/js/JSAudioBufferSourceNodeCustom.cpp	2016-04-20 01:19:59 UTC (rev 199750)
+++ trunk/Source/WebCore/bindings/js/JSAudioBufferSourceNodeCustom.cpp	2016-04-20 01:30:11 UTC (rev 199751)
@@ -1,56 +0,0 @@
-/*
- * Copyright (C) 2010, Google Inc. All rights reserved.
- * Copyright (C) 2012 Samsung Electronics
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1.  Redistributions of source code must retain the above copyright
- *    notice, this list of conditions and the following disclaimer.
- * 2.  Redistributions in binary form must reproduce the above copyright
- *    notice, this list of conditions and the following disclaimer in the
- *    documentation and/or other materials provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS'' AND ANY
- * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
- * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
- * DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS BE LIABLE FOR ANY
- * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
- * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
- * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
- * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
- * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#include "config.h"
-
-#if ENABLE(WEB_AUDIO)
-
-#include "JSAudioBufferSourceNode.h"
-
-#include "AudioBuffer.h"
-#include "AudioBufferSourceNode.h"
-#include "JSAudioBuffer.h"
-#include <runtime/Error.h>
-#include <runtime/JSCJSValueInlines.h>
-
-using namespace JSC;
-
-namespace WebCore {
-
-void JSAudioBufferSourceNode::setBuffer(ExecState& state, JSValue value)
-{
-    AudioBuffer* buffer = JSAudioBuffer::toWrapped(value);
-    if (!buffer) {
-        state.vm().throwException(&state, createTypeError(&state, "Value is not of type AudioBuffer"));
-        return;
-    }
-    
-    if (!wrapped().setBuffer(buffer))
-        state.vm().throwException(&state, createTypeError(&state, "AudioBuffer unsupported number of channels"));
-}
-
-} // namespace WebCore
-
-#endif // ENABLE(WEB_AUDIO)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to