[webkit-reviews] review denied: [Bug 45009] Add AudioDestinationNode files : [Attachment 66145] Patch

[bugzilla-daemon at webkit.org]

Kenneth Russell <kbr at google.com> has denied Chris Rogers <crogers at google.com>'s
request for review:
Bug 45009: Add AudioDestinationNode files
https://bugs.webkit.org/show_bug.cgi?id=45009

Attachment 66145: Patch
https://bugs.webkit.org/attachment.cgi?id=66145&action=review

------- Additional Comments from Kenneth Russell <kbr at google.com>
View in context:
https://bugs.webkit.org/attachment.cgi?id=66145&action=prettypatch

Generally looks fine; r- because of an inconsistency in the publicly visible
attributes. Also a couple of minor suggestions.

> WebCore/webaudio/AudioDestinationNode.cpp:50
> +    initialize();
I certainly hope that AudioNode initializes the m_isInitialized flag to false
in its constructor. It would be simpler if AudioNode were landed before this
class which depends on it.

> WebCore/webaudio/AudioDestinationNode.cpp:71
> +    m_isInitialized = true;
I would suggest just calling AudioNode::initialize() here and making
m_isInitialized private in AudioNode (assuming that's where it's declared).

> WebCore/webaudio/AudioDestinationNode.cpp:81
> +    m_isInitialized = false;
Same comment, but about calling AudioNode::uninitialize().

> WebCore/webaudio/AudioDestinationNode.idl:34
> +	   readonly attribute short numberOfChannels;
This attribute isn't currently in the Web Audio spec.