[webkit-reviews] review denied: [Bug 213801] Added PannerNode constructor according to spec : [Attachment 403328] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Jul 1 16:15:11 PDT 2020


Chris Dumez <cdumez at apple.com> has denied Clark Wang <clark_wang at apple.com>'s
request for review:
Bug 213801: Added PannerNode constructor according to spec
https://bugs.webkit.org/show_bug.cgi?id=213801

Attachment 403328: Patch

https://bugs.webkit.org/attachment.cgi?id=403328&action=review




--- Comment #13 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 403328
  --> https://bugs.webkit.org/attachment.cgi?id=403328
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=403328&action=review

> Source/WebCore/ChangeLog:35
> +2020-06-30  Peng Liu  <peng.liu6 at apple.com>

This changelog is not related to your change.

> Source/WebCore/Modules/webaudio/AudioNode.cpp:122
> +AudioNode::AudioNode(BaseAudioContext& context, AudioNodeOptions options)

const AudioNodeOptions&

> Source/WebCore/Modules/webaudio/AudioNode.h:64
> +    AudioNode(BaseAudioContext&, AudioNodeOptions);

const AudioNodeOptions&

> Source/WebCore/Modules/webaudio/AudioNode.h:217
> +    volatile bool m_isInitialized { false };

Well you fixed this one to use bracket initialization but left all the other
data members using = initialization :(

> Source/WebCore/Modules/webaudio/PannerNode.cpp:92
> +ExceptionOr<Ref<PannerNode>> PannerNode::create(BaseAudioContext& context,
PannerOptions options)

const PannerOptions&

> Source/WebCore/Modules/webaudio/PannerNode.h:76
> +    static ExceptionOr<Ref<PannerNode>> create(BaseAudioContext&,
PannerOptions = { });

const PannerOptional& = { }

> Source/WebCore/Modules/webaudio/PannerNode.h:161
> +    float m_lastGain = -1.0;

Bracket initialization.

> Source/WebCore/Modules/webaudio/PannerNode.idl:-28
> -    EnabledBySetting=ModernUnprefixedWebAudio,

It is definitely wrong to drop the EnabledBySetting...


More information about the webkit-reviews mailing list