[webkit-reviews] review granted: [Bug 227205] Merge AudioFileReaderMac and AudioFileReaderIOS into AudioFileReaderCocoa : [Attachment 431832] Patch

[bugzilla-daemon at webkit.org]

Chris Dumez <cdumez at apple.com> has granted Jean-Yves Avenard [:jya]
<jean-yves.avenard at apple.com>'s request for review:
Bug 227205: Merge AudioFileReaderMac and AudioFileReaderIOS into
AudioFileReaderCocoa
https://bugs.webkit.org/show_bug.cgi?id=227205

Attachment 431832: Patch

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




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

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

r=me with nits.

> Source/WebCore/platform/audio/cocoa/AudioFileReaderCocoa.cpp:103
> +    : m_data(nullptr)

These data members should use inline initialization in the header. Also please
use nullptr instead of 0 for pointers.

> Source/WebCore/platform/audio/cocoa/AudioFileReaderCocoa.cpp:108
> +    RetainPtr<CFStringRef> filePathString =
adoptCF(CFStringCreateWithCString(kCFAllocatorDefault, filePath,
kCFStringEncodingUTF8));

auto

> Source/WebCore/platform/audio/cocoa/AudioFileReaderCocoa.cpp:109
> +    RetainPtr<CFURLRef> urlRef =
adoptCF(CFURLCreateWithFileSystemPath(kCFAllocatorDefault,
filePathString.get(), kCFURLPOSIXPathStyle, false));

auto

> Source/WebCore/platform/audio/cocoa/AudioFileReaderCocoa.cpp:118
> +    , m_audioFileID(0)

These should use inline initialization in the header

> Source/WebCore/platform/audio/cocoa/AudioFileReaderCocoa.cpp:125
> +	   m_extAudioFileRef = 0;

= nullptr;

> Source/WebCore/platform/audio/cocoa/AudioFileReaderCocoa.cpp:143
> +    AudioFileReader* audioFileReader =
static_cast<AudioFileReader*>(clientData);

auto

> Source/WebCore/platform/audio/cocoa/AudioFileReaderCocoa.cpp:145
> +    size_t dataSize = audioFileReader->dataSize();

auto

> Source/WebCore/platform/audio/cocoa/AudioFileReaderCocoa.cpp:146
> +    const void* data = audioFileReader->data();

auto

> Source/WebCore/platform/audio/cocoa/AudioFileReaderCocoa.h:47
> +    explicit AudioFileReader(const void* data, size_t dataSize);

We usually don't use explicit for constructors with 2 mandatory parameters.