Add more informative messages for DOM exceptions.

Source/modules/webaudio/AudioNode.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/AudioNode.h"
 
+#include "bindings/v8/ExceptionMessages.h"
 #include "bindings/v8/ExceptionState.h"
 #include "core/dom/ExceptionCode.h"
 #include "modules/webaudio/AudioContext.h"
 #include "modules/webaudio/AudioNodeInput.h"
 #include "modules/webaudio/AudioNodeOutput.h"
 #include "modules/webaudio/AudioParam.h"
 #include "wtf/Atomics.h"
 #include "wtf/MainThread.h"
 
 #if DEBUG_AUDIONODE_REFERENCES
@@ skipping 82 matching lines @@
         return m_outputs[i].get();
     return 0;
 }
 
 void AudioNode::connect(AudioNode* destination, unsigned outputIndex, unsigned inputIndex, ExceptionState& es)
 {
     ASSERT(isMainThread());
     AudioContext::AutoLocker locker(context());
 
     if (!destination) {
-        es.throwUninformativeAndGenericDOMException(SyntaxError);
+        es.throwDOMException(
+            SyntaxError,
+            ExceptionMessages::failedToExecute(
+                "connect",
+                "AudioNode",
+                "invalid destination node."));
         return;
     }
 
     // Sanity check input and output indices.
     if (outputIndex >= numberOfOutputs()) {
-        es.throwUninformativeAndGenericDOMException(IndexSizeError);
+        es.throwDOMException(
+            IndexSizeError,
+            ExceptionMessages::failedToExecute(
+                "connect",
+                "AudioNode",
+                "invalid output index."));

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

"The provided output index (8) is larger than the number of outputs (3)."

         return;
     }
 
     if (destination && inputIndex >= destination->numberOfInputs()) {
-        es.throwUninformativeAndGenericDOMException(IndexSizeError);
+        es.throwDOMException(
+            IndexSizeError,
+            ExceptionMessages::failedToExecute(
+                "connect",
+                "AudioNode",
+                "invalid input index."));

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

Same as above.

         return;
     }
 
     if (context() != destination->context()) {
-        es.throwUninformativeAndGenericDOMException(SyntaxError);
+        es.throwDOMException(
+            SyntaxError,
+            ExceptionMessages::failedToExecute(
+                "connect",
+                "AudioNode",
+                "invalid destination context."));
         return;
     }
 
     AudioNodeInput* input = destination->input(inputIndex);
     AudioNodeOutput* output = this->output(outputIndex);
     input->connect(output);
 
     // Let context know that a connection has been made.
     context()->incrementConnectionCount();
 }
 
 void AudioNode::connect(AudioParam* param, unsigned outputIndex, ExceptionState& es)
 {
     ASSERT(isMainThread());
     AudioContext::AutoLocker locker(context());
 
     if (!param) {
-        es.throwUninformativeAndGenericDOMException(SyntaxError);
+        es.throwDOMException(
+            SyntaxError,
+            ExceptionMessages::failedToExecute(
+                "connect",
+                "AudioNode",
+                "invalid AudioParam."));
         return;
     }
 
     if (outputIndex >= numberOfOutputs()) {
-        es.throwUninformativeAndGenericDOMException(IndexSizeError);
+        es.throwDOMException(
+            IndexSizeError,
+            ExceptionMessages::failedToExecute(
+                "connect",
+                "AudioNode",
+                "invalid output index."));
         return;
     }
 
     if (context() != param->context()) {
-        es.throwUninformativeAndGenericDOMException(SyntaxError);
+        es.throwDOMException(
+            SyntaxError,
+            ExceptionMessages::failedToExecute(
+                "connect",
+                "AudioNode",
+                "destination context is different."));
         return;
     }
 
     AudioNodeOutput* output = this->output(outputIndex);
     param->connect(output);
 }
 
 void AudioNode::disconnect(unsigned outputIndex, ExceptionState& es)
 {
     ASSERT(isMainThread());
     AudioContext::AutoLocker locker(context());
 
     // Sanity check input and output indices.
     if (outputIndex >= numberOfOutputs()) {
-        es.throwUninformativeAndGenericDOMException(IndexSizeError);
+        es.throwDOMException(
+            IndexSizeError,
+            ExceptionMessages::failedToExecute(
+                "disconnect",
+                "AudioNode",
+                "invalid output index."));

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

Same as above.

         return;
     }
 
     AudioNodeOutput* output = this->output(outputIndex);
     output->disconnectAll();
 }
 
 unsigned long AudioNode::channelCount()
 {
     return m_channelCount;
 }
 
 void AudioNode::setChannelCount(unsigned long channelCount, ExceptionState& es)
 {
     ASSERT(isMainThread());
     AudioContext::AutoLocker locker(context());
 
     if (channelCount > 0 && channelCount <= AudioContext::maxNumberOfChannels()) {
         if (m_channelCount != channelCount) {
             m_channelCount = channelCount;
             if (m_channelCountMode != Max)
                 updateChannelsForInputs();
         }
     } else {
-        es.throwUninformativeAndGenericDOMException(InvalidStateError);
+        es.throwDOMException(
+            InvalidStateError,
+            ExceptionMessages::failedToSet(
+                "channelCount",
+                "AudioNode",
+                "invalid number of channels."));

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

Perhaps split this into "The channel count may not be 0." and "The provided
channel count (x) is greater than the maximum number of channels supported (y)."

     }
 }
 
 String AudioNode::channelCountMode()
 {
     switch (m_channelCountMode) {
     case Max:
         return "max";
     case ClampedMax:
         return "clamped-max";
     case Explicit:
         return "explicit";
     }
     ASSERT_NOT_REACHED();
     return "";
 }
 
 void AudioNode::setChannelCountMode(const String& mode, ExceptionState& es)
 {
     ASSERT(isMainThread());
     AudioContext::AutoLocker locker(context());
 
     ChannelCountMode oldMode = m_channelCountMode;
 
-    if (mode == "max")
+    if (mode == "max") {
         m_channelCountMode = Max;
-    else if (mode == "clamped-max")
+    } else if (mode == "clamped-max") {
         m_channelCountMode = ClampedMax;
-    else if (mode == "explicit")
+    } else if (mode == "explicit") {
         m_channelCountMode = Explicit;
-    else
-        es.throwUninformativeAndGenericDOMException(InvalidStateError);
+    } else {
+        es.throwDOMException(
+            InvalidStateError,
+            ExceptionMessages::failedToSet(
+                "channelCountMode",
+                "AudioNode",
+                "must be \"max\", \"clamped-max\", or \"explicit\"."));

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

I'd suggest including the developer's input here: "The provided mode '" + mode +
"' is invalid. Only 'x', 'y', and 'z' are supported."

I've also generally been using single-quotes to denote code in error messages,
but that's pure preference without any real rationale behind it.

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

Agreed.
I originally had single quotes, but kind of wanted to distinguish how you print
messages vs the values.  Currently we print something like

Failed to set `channelCountMode' of `AudioNode': must be "max", "clamped-max",
"explicit".

Not sure if this distinction is necessary. Single quotes are fine with me.

+    }
 
     if (m_channelCountMode != oldMode)
         updateChannelsForInputs();
 }
 
 String AudioNode::channelInterpretation()
 {
     switch (m_channelInterpretation) {
     case AudioBus::Speakers:
         return "speakers";
     case AudioBus::Discrete:
         return "discrete";
     }
     ASSERT_NOT_REACHED();
     return "";
 }
 
 void AudioNode::setChannelInterpretation(const String& interpretation, ExceptionState& es)
 {
     ASSERT(isMainThread());
     AudioContext::AutoLocker locker(context());
 
-    if (interpretation == "speakers")
+    if (interpretation == "speakers") {
         m_channelInterpretation = AudioBus::Speakers;
-    else if (interpretation == "discrete")
+    } else if (interpretation == "discrete") {
         m_channelInterpretation = AudioBus::Discrete;
-    else
-        es.throwUninformativeAndGenericDOMException(InvalidStateError);
+    } else {
+        es.throwDOMException(
+            InvalidStateError,
+            ExceptionMessages::failedToSet(
+                "channelInterpretation",
+                "AudioNode",
+                "must be \"speakers\" or \"discrete\"."));

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

Same as above.

+    }
 }
 
 void AudioNode::updateChannelsForInputs()
 {
     for (unsigned i = 0; i < m_inputs.size(); ++i)
         input(i)->changedOutputs();
 }
 
 const AtomicString& AudioNode::interfaceName() const
 {
@@ skipping 231 matching lines @@
         fprintf(stderr, "%d: %d\n", i, s_nodeCount[i]);
 
     fprintf(stderr, "===========================\n\n\n");
 }
 
 #endif // DEBUG_AUDIONODE_REFERENCES
 
 } // namespace WebCore
 
 #endif // ENABLE(WEB_AUDIO)