[webkit-reviews] review granted: [Bug 106803] [Chromium] Move AudioDestinationChromium into WebCore : [Attachment 182594] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jan 14 10:11:11 PST 2013


Adam Barth <abarth at webkit.org> has granted Mark Pilgrim (Google)
<pilgrim at chromium.org>'s request for review:
Bug 106803: [Chromium] Move AudioDestinationChromium into WebCore
https://bugs.webkit.org/show_bug.cgi?id=106803

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

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=182594&action=review


> Source/WebCore/platform/audio/chromium/AudioDestinationChromium.cpp:31
> +#if ENABLE(WEB_AUDIO)

We usually put a blank line after this line.

> Source/WebCore/platform/audio/chromium/AudioDestinationChromium.cpp:38
> +using namespace WebKit;

Can we skip this using directive?  In WebCore, it's nice to be able to see
what's used from the API.

> Source/WebCore/platform/audio/chromium/AudioDestinationChromium.cpp:45
> +// Size of the FIFO

There are lots of style errors in this file, but I guess I won't point them out
since you're just moving this code.

> Source/WebCore/platform/audio/chromium/AudioDestinationChromium.h:39
> +namespace WebKit { class WebAudioDevice; }

You probably don't need to forward declare this class given that you #include
public/WebAudioDevice.h


More information about the webkit-reviews mailing list