Add more informative messages for DOM exceptions.

Source/modules/webaudio/AudioContext.cpp

 /*
  * Copyright (C) 2010, Google Inc. All rights reserved.
  *
  * 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
@@ skipping 10 matching lines @@
  * (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 "modules/webaudio/AudioContext.h"
 
+#include "bindings/v8/ExceptionMessages.h"
 #include "bindings/v8/ExceptionState.h"
 #include "core/dom/Document.h"
 #include "core/dom/ExceptionCode.h"
 #include "core/html/HTMLMediaElement.h"
 #include "core/inspector/ScriptCallStack.h"
 #include "core/platform/audio/FFTFrame.h"
 #include "core/platform/audio/HRTFDatabaseLoader.h"
 #include "core/platform/audio/HRTFPanner.h"
 #include "modules/mediastream/MediaStream.h"
 #include "modules/webaudio/AnalyserNode.h"
@@ skipping 245 matching lines @@
     // of dealing with all of its ActiveDOMObjects at this point. uninitialize() can de-reference other
     // ActiveDOMObjects so let's schedule uninitialize() to be called later.
     // FIXME: see if there's a more direct way to handle this issue.
     callOnMainThread(stopDispatch, this);
 }
 
 PassRefPtr<AudioBuffer> AudioContext::createBuffer(unsigned numberOfChannels, size_t numberOfFrames, float sampleRate, ExceptionState& es)
 {
     RefPtr<AudioBuffer> audioBuffer = AudioBuffer::create(numberOfChannels, numberOfFrames, sampleRate);
     if (!audioBuffer.get()) {
-        es.throwUninformativeAndGenericDOMException(SyntaxError);
+        es.throwDOMException(
+            SyntaxError,
+            ExceptionMessages::failedToConstruct(
+                "AudioBuffer",
+                "invalid number of channels, frames, or sample rate."));

[Mike West, 2013/09/27 06:44:39]

For these kinds of errors, it would be nice to tell the developer which of the
numbers was invalid. That said, this is still strictly better than the previous
message. :)

[Raymond Toy (Google), 2013/09/27 16:24:10]

Agreed. I'll try to make this (and all others) more informative.  For this
particular one, it might be difficult to produce a more informative message
without access to the limits used in AudioBuffer::create.  (I'll need to check).

         return 0;
     }
 
     return audioBuffer;
 }
 
 PassRefPtr<AudioBuffer> AudioContext::createBuffer(ArrayBuffer* arrayBuffer, bool mixToMono, ExceptionState& es)
 {
     ASSERT(arrayBuffer);
     if (!arrayBuffer) {
-        es.throwUninformativeAndGenericDOMException(SyntaxError);
+        es.throwDOMException(
+            SyntaxError,
+            ExceptionMessages::failedToConstruct(
+                "AudioBuffer",
+                "invalid ArrayBuffer."));
         return 0;
     }
 
     RefPtr<AudioBuffer> audioBuffer = AudioBuffer::createFromAudioFileData(arrayBuffer->data(), arrayBuffer->byteLength(), mixToMono, sampleRate());
     if (!audioBuffer.get()) {
-        es.throwUninformativeAndGenericDOMException(SyntaxError);
+        es.throwDOMException(
+            SyntaxError,
+            ExceptionMessages::failedToConstruct(
+                "AudioBuffer",
+                "invalid audio data in ArrayBuffer."));
         return 0;
     }
 
     return audioBuffer;
 }
 
 void AudioContext::decodeAudioData(ArrayBuffer* audioData, PassRefPtr<AudioBufferCallback> successCallback, PassRefPtr<AudioBufferCallback> errorCallback, ExceptionState& es)
 {
     if (!audioData) {
-        es.throwUninformativeAndGenericDOMException(SyntaxError);
+        es.throwDOMException(
+            SyntaxError,
+            ExceptionMessages::failedToExecute(
+                "decodeAudioData",
+                "AudioContext",
+                "invalid ArrayBuffer for audioData."));
         return;
     }
     m_audioDecoder.decodeAsync(audioData, sampleRate(), successCallback, errorCallback);
 }
 
 PassRefPtr<AudioBufferSourceNode> AudioContext::createBufferSource()
 {
     ASSERT(isMainThread());
     lazyInitialize();
     RefPtr<AudioBufferSourceNode> node = AudioBufferSourceNode::create(this, m_destinationNode->sampleRate());
 
     // Because this is an AudioScheduledSourceNode, the context keeps a reference until it has finished playing.
     // When this happens, AudioScheduledSourceNode::finish() calls AudioContext::notifyNodeFinishedProcessing().
     refNode(node.get());
 
     return node;
 }
 
 PassRefPtr<MediaElementAudioSourceNode> AudioContext::createMediaElementSource(HTMLMediaElement* mediaElement, ExceptionState& es)
 {
-    ASSERT(mediaElement);

[Mike West, 2013/09/27 06:44:39]

Why drop the ASSERT?

[Raymond Toy (Google), 2013/09/27 16:24:10]

When running the webaudio layout tests in debug mode, this ASSERT asserts and
crashes the test. Since we check mediaElement in the next line, we would not be
missing anything, while still allowing us to test this case in the layout test.

     if (!mediaElement) {
-        es.throwUninformativeAndGenericDOMException(InvalidStateError);
+        es.throwDOMException(
+            InvalidStateError,
+            ExceptionMessages::failedToConstruct(
+                "MediaElementAudioSourceNode",
+                "invalid HTMLMedialElement."));
         return 0;
     }
 
     ASSERT(isMainThread());
     lazyInitialize();
 
     // First check if this media element already has a source node.
     if (mediaElement->audioSourceNode()) {
-        es.throwUninformativeAndGenericDOMException(InvalidStateError);
+        es.throwDOMException(
+            InvalidStateError,
+            ExceptionMessages::failedToConstruct(
+                "MediaElementAudioSourceNode",
+                "invalid HTMLMediaElement."));
         return 0;
     }
 
     RefPtr<MediaElementAudioSourceNode> node = MediaElementAudioSourceNode::create(this, mediaElement);
 
     mediaElement->setAudioSourceNode(node.get());
 
     refNode(node.get()); // context keeps reference until node is disconnected
     return node;
 }
 
 PassRefPtr<MediaStreamAudioSourceNode> AudioContext::createMediaStreamSource(MediaStream* mediaStream, ExceptionState& es)
 {
-    ASSERT(mediaStream);
     if (!mediaStream) {
-        es.throwUninformativeAndGenericDOMException(InvalidStateError);
+        es.throwDOMException(
+            InvalidStateError,
+            ExceptionMessages::failedToConstruct("MediaStreamAudioSourceNode"));

[Mike West, 2013/09/27 06:44:39]

"invalid MediaStream source"?

         return 0;
     }
 
     ASSERT(isMainThread());
     lazyInitialize();
 
     AudioSourceProvider* provider = 0;
 
     MediaStreamTrackVector audioTracks = mediaStream->getAudioTracks();
 
@@ skipping 34 matching lines @@
     return createScriptProcessor(bufferSize, numberOfInputChannels, 2, es);
 }
 
 PassRefPtr<ScriptProcessorNode> AudioContext::createScriptProcessor(size_t bufferSize, size_t numberOfInputChannels, size_t numberOfOutputChannels, ExceptionState& es)
 {
     ASSERT(isMainThread());
     lazyInitialize();
     RefPtr<ScriptProcessorNode> node = ScriptProcessorNode::create(this, m_destinationNode->sampleRate(), bufferSize, numberOfInputChannels, numberOfOutputChannels);
 
     if (!node.get()) {
-        es.throwUninformativeAndGenericDOMException(SyntaxError);
+        es.throwDOMException(
+            SyntaxError,
+            ExceptionMessages::failedToConstruct(
+                "ScriptProcessorNode",
+                "invalid buffer size, or number of input/outputs."));
         return 0;
     }
 
     refNode(node.get()); // context keeps reference until we stop making javascript rendering callbacks
     return node;
 }
 
 PassRefPtr<BiquadFilterNode> AudioContext::createBiquadFilter()
 {
     ASSERT(isMainThread());
@@ skipping 66 matching lines @@
 }
 
 PassRefPtr<ChannelSplitterNode> AudioContext::createChannelSplitter(size_t numberOfOutputs, ExceptionState& es)
 {
     ASSERT(isMainThread());
     lazyInitialize();
 
     RefPtr<ChannelSplitterNode> node = ChannelSplitterNode::create(this, m_destinationNode->sampleRate(), numberOfOutputs);
 
     if (!node.get()) {
-        es.throwUninformativeAndGenericDOMException(SyntaxError);
+        es.throwDOMException(
+            SyntaxError,
+            ExceptionMessages::failedToConstruct(
+                "ChannelSplitterNode",
+                "invalid number of outputs."));

[Mike West, 2013/09/27 06:44:39]

"The number of outputs (" + String::number(numberOfOutputs) ") is invalid."?

It would be nice if you could give a pointer in the message as to what would be
valid: "We only support 17 outputs." or something. :)

         return 0;
     }
 
     return node;
 }
 
 PassRefPtr<ChannelMergerNode> AudioContext::createChannelMerger(ExceptionState& es)
 {
     const unsigned ChannelMergerDefaultNumberOfInputs = 6;
     return createChannelMerger(ChannelMergerDefaultNumberOfInputs, es);
 }
 
 PassRefPtr<ChannelMergerNode> AudioContext::createChannelMerger(size_t numberOfInputs, ExceptionState& es)
 {
     ASSERT(isMainThread());
     lazyInitialize();
 
     RefPtr<ChannelMergerNode> node = ChannelMergerNode::create(this, m_destinationNode->sampleRate(), numberOfInputs);
 
     if (!node.get()) {
-        es.throwUninformativeAndGenericDOMException(SyntaxError);
+        es.throwDOMException(
+            SyntaxError,
+            ExceptionMessages::failedToConstruct(
+                "ChannelMergerNode",
+                "invalid number of inputs."));

[Mike West, 2013/09/27 06:44:39]

Same as for outputs.

         return 0;
     }
 
     return node;
 }
 
 PassRefPtr<OscillatorNode> AudioContext::createOscillator()
 {
     ASSERT(isMainThread());
     lazyInitialize();
 
     RefPtr<OscillatorNode> node = OscillatorNode::create(this, m_destinationNode->sampleRate());
 
     // Because this is an AudioScheduledSourceNode, the context keeps a reference until it has finished playing.
     // When this happens, AudioScheduledSourceNode::finish() calls AudioContext::notifyNodeFinishedProcessing().
     refNode(node.get());
 
     return node;
 }
 
 PassRefPtr<PeriodicWave> AudioContext::createPeriodicWave(Float32Array* real, Float32Array* imag, ExceptionState& es)
 {
     ASSERT(isMainThread());
 
     if (!real || !imag || (real->length() != imag->length())) {
-        es.throwUninformativeAndGenericDOMException(SyntaxError);
+        es.throwDOMException(
+            SyntaxError,
+            ExceptionMessages::failedToConstruct(
+                "WaveTable",
+                "invalid real or imag arrays for wave table."));

[Mike West, 2013/09/27 06:44:39]

One option would be to split this up into multiple exceptions: "Invalid real
array", "Invalid imaginary array", "The real array and imaginary array must be
the same length (8 vs 7)."

         return 0;
     }
 
     lazyInitialize();
     return PeriodicWave::create(sampleRate(), real, imag);
 }
 
 void AudioContext::notifyNodeFinishedProcessing(AudioNode* node)
 {
     ASSERT(isAudioThread());
@@ skipping 380 matching lines @@
 }
 
 void AudioContext::decrementActiveSourceCount()
 {
     atomicDecrement(&m_activeSourceCount);
 }
 
 } // namespace WebCore
 
 #endif // ENABLE(WEB_AUDIO)