Bug 190519 – [MediaStream] Consolidate all image conversion and resizing into one class

• Component: WebRTC
• Status: RESOLVED FIXED

[Eric Carlson]

Consolidate all image conversion and resizing into one class

[Radar WebKit Bug Importer]

<rdar://problem/45224307>

[Eric Carlson]

Created attachment 352200 [details]
Patch

[youenn fablet]

Comment on attachment 352200 [details]
Patch

Looks like a good refactoring!
Some nits below.
I haven't looked at why iOS/Mac are not compiling.

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

> Source/WebCore/platform/MediaSample.h:96
> +    virtual uint32_t videoPixelFormat() const { return 0; }

It feels like it should be a MediaSampleObjC method only.

> Source/WebCore/platform/graphics/avfoundation/objc/MediaSampleAVFObjC.h:67
> +    uint32_t videoPixelFormat() const override;

s/override/final/

> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.h:31
> +#include "MediaSample.h"

forward declare MediaSample?

> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.h:32
> +#include <CoreMedia/CMTime.h>

Do we need this one?

> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.mm:51
> +

line to remove?

> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.mm:101
> +RetainPtr<CVPixelBufferRef> ImageTransferSessionVT::createPixelBuffer(CVPixelBufferRef sourceBuffer, const IntSize& size)

s/createPixelBuffer/convertPixelBuffer/ ?

Ditto for other methods.

> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.mm:104
> +        return nullptr;

I wonder whether we should special case when sourceBuffer has the right size and right format.
We should then be able to directly return sourceBuffer.

> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.mm:131
> +    return convertedPixelBuffer;

Probably can be rewritten as:
return createPixelBuffer(CMSampleBufferGetImageBuffer(sourceBuffer), size);
Maybe sourceBuffer check is needed though?

> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.mm:134
> +RetainPtr<CMSampleBufferRef> ImageTransferSessionVT::createCMSampleBuffer(CMSampleBufferRef sourceBuffer, const IntSize& size)

s/createCMSampleBuffer/convertCMSampleBuffer/?

> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.mm:137
> +        return nullptr;

Already done within next call to createPixelBuffer.

> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.mm:143
> +    CMVideoFormatDescriptionRef formatDescription = nullptr;

Should we store formatDescription in a retain pointer to ensure we do not leak it in case of early return?

> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.mm:185
> +    if (!m_cgImageBufferPool || m_cgImagePoolSize != imageSize) {

Do we need a m_cgImageBufferPool?
It seems we will only use one ARGB pixel buffer at a time since we will convert it later on.
Except maybe if we optimize the case of same-size same format case.

> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.mm:200
> +        CVReturn status = CVPixelBufferPoolCreate(kCFAllocatorDefault, pixelBufferPoolOptions, pixelBufferOptions, &bufferPool);

auto

> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.mm:209
> +    CVReturn status = CVPixelBufferPoolCreatePixelBuffer(kCFAllocatorDefault, m_cgImageBufferPool.get(), &rgbBuffer);

auto

> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.mm:233
> +    if (bufferSize != m_size) {

If the format is not the right one, we should probably call createPixelBuffer.

> Source/WebCore/platform/mediastream/RealtimeMediaSource.cpp:168
> +    RefPtr<MediaSample> protectedMediaSample = &mediaSample;

It seems we should only do this assignment when using m_imageTransferSession.

> Source/WebCore/platform/mediastream/RealtimeMediaSource.h:228
> +    std::unique_ptr<ImageTransferSessionVT> m_imageTransferSession;

I am not sure whether this should be added at that level, RealtimeMediaSource is an abstract class.
Maybe we should keep it at the leaf classes for now.
We might want to have it in RealtimeIncomingVideoSourceCocoa as well (this patch or another).

[Eric Carlson]

Created attachment 352514 [details]
Patch

[Eric Carlson]

Comment on attachment 352200 [details]
Patch

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

>> Source/WebCore/platform/MediaSample.h:96
>> +    virtual uint32_t videoPixelFormat() const { return 0; }
> 
> It feels like it should be a MediaSampleObjC method only.

Changed.

>> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.h:31
>> +#include "MediaSample.h"
> 
> forward declare MediaSample?

We do now.

>> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.h:32
>> +#include <CoreMedia/CMTime.h>
> 
> Do we need this one?

No, removed.

>> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.mm:51
>> +
> 
> line to remove?

Removed.

>> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.mm:101
>> +RetainPtr<CVPixelBufferRef> ImageTransferSessionVT::createPixelBuffer(CVPixelBufferRef sourceBuffer, const IntSize& size)
> 
> s/createPixelBuffer/convertPixelBuffer/ ?
> 
> Ditto for other methods.

Good point, changed.

>> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.mm:104
>> +        return nullptr;
> 
> I wonder whether we should special case when sourceBuffer has the right size and right format.
> We should then be able to directly return sourceBuffer.

Yes, fixed this and convertCMSampleBuffer.

>> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.mm:131
>> +    return convertedPixelBuffer;
> 
> Probably can be rewritten as:
> return createPixelBuffer(CMSampleBufferGetImageBuffer(sourceBuffer), size);
> Maybe sourceBuffer check is needed though?

Changed.

>> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.mm:137
>> +        return nullptr;
> 
> Already done within next call to createPixelBuffer.

Changed.

>> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.mm:143
>> +    CMVideoFormatDescriptionRef formatDescription = nullptr;
> 
> Should we store formatDescription in a retain pointer to ensure we do not leak it in case of early return?

I moved the call to CMVideoFormatDescriptionCreateForImageBuffer to just before it is used.

>> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.mm:185
>> +    if (!m_cgImageBufferPool || m_cgImagePoolSize != imageSize) {
> 
> Do we need a m_cgImageBufferPool?
> It seems we will only use one ARGB pixel buffer at a time since we will convert it later on.
> Except maybe if we optimize the case of same-size same format case.

We probably don't need it, removed.

>> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.mm:200
>> +        CVReturn status = CVPixelBufferPoolCreate(kCFAllocatorDefault, pixelBufferPoolOptions, pixelBufferOptions, &bufferPool);
> 
> auto

Fixed.

>> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.mm:209
>> +    CVReturn status = CVPixelBufferPoolCreatePixelBuffer(kCFAllocatorDefault, m_cgImageBufferPool.get(), &rgbBuffer);
> 
> auto

Fixed.

>> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.mm:233
>> +    if (bufferSize != m_size) {
> 
> If the format is not the right one, we should probably call createPixelBuffer.

Good point, fixed.

>> Source/WebCore/platform/mediastream/RealtimeMediaSource.cpp:168
>> +    RefPtr<MediaSample> protectedMediaSample = &mediaSample;
> 
> It seems we should only do this assignment when using m_imageTransferSession.

I moved this logic to the derived classes.

>> Source/WebCore/platform/mediastream/RealtimeMediaSource.h:228
>> +    std::unique_ptr<ImageTransferSessionVT> m_imageTransferSession;
> 
> I am not sure whether this should be added at that level, RealtimeMediaSource is an abstract class.
> Maybe we should keep it at the leaf classes for now.
> We might want to have it in RealtimeIncomingVideoSourceCocoa as well (this patch or another).

Lets change RealtimeIncomingVideoSourceCocoa in a followup.

[Eric Carlson]

Created attachment 352544 [details]
Patch

[EWS Watchlist]

Attachment 352544 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.h:51:  The parameter name "rotation" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.h:52:  The parameter name "rotation" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.h:55:  The parameter name "rotation" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/platform/mediastream/RealtimeMediaSource.h:168:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
ERROR: Source/WebCore/platform/mediastream/RealtimeMediaSource.h:169:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
ERROR: Source/WebCore/platform/mediastream/RealtimeMediaSource.h:206:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 6 in 25 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Eric Carlson]

Created attachment 352545 [details]
Patch

[EWS Watchlist]

Attachment 352545 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.h:51:  The parameter name "rotation" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.h:52:  The parameter name "rotation" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.h:55:  The parameter name "rotation" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/platform/mediastream/RealtimeMediaSource.h:203:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 4 in 25 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Eric Carlson]

Created attachment 352546 [details]
Patch

[EWS Watchlist]

Attachment 352546 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.h:51:  The parameter name "rotation" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.h:52:  The parameter name "rotation" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.h:55:  The parameter name "rotation" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/platform/mediastream/RealtimeMediaSource.h:203:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 4 in 24 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Eric Carlson]

Created attachment 352570 [details]
Patch

[EWS Watchlist]

Attachment 352570 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.h:51:  The parameter name "rotation" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.h:52:  The parameter name "rotation" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.h:55:  The parameter name "rotation" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/platform/mediastream/RealtimeMediaSource.h:203:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 4 in 24 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[youenn fablet]

Comment on attachment 352570 [details]
Patch

It seems some of the minor changes could impact tests, for instance deviceID.

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

> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.h:31
> +#include "MediaSampleAVFObjc.h"

Is this one needed here? It is already included in the .mm

> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.h:55
> +    RefPtr<MediaSample> createMediaSample(IOSurfaceRef, const MediaTime&, const IntSize&, MediaSample::VideoRotation rotation = MediaSample::VideoRotation::None, bool mirrored = false);

s/rotation =/=/ here and above.

> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.h:59
> +    WEBCORE_EXPORT ImageTransferSessionVT(uint32_t);

explicit. Maybe add pixelFormat since this is not clear without.
I wonder whether we should consider typing the pixelFormat like we do for ObjectIdentifier.

> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.mm:62
> +    VTSessionSetProperty(transferSession, kVTPixelTransferPropertyKey_ScalingMode, kVTScalingMode_Trim);

Should we check status for all of these and return nullptr in create in case this fails?
Or output some debug/release logging if failing?

> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.mm:131
> +        RELEASE_LOG(Media, "ImageTransferSessionVT::createPixelBuffer: CMSampleBufferGetImageBuffer returned nullptr");

No need for this release logging, error case will be logged in convertPixelBuffer.

> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.mm:258
> +    return createCMSampleBuffer(pixelBuffer.get(), sampleTime, size);

There will be some redundant checks related to size.
Not a big deal but maybe a helper function could be used to share the same code between the two createCMSampleBuffer variants?

> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.mm:284
> +    int32_t extendedBottom = roundUpToMacroblockMultiple(height) - height;

auto as well.
Maybe add a comment to explain this?

> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.mm:295
> +    }

I do not understand well this code, in particular why we have m_ioSurfaceBufferAttributes sometimes not having any kCVPixelBufferExtendedPixelsRightKey/kCVPixelBufferExtendedPixelsBottomKey

> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.mm:325
> +        return nullptr;

These checks are done within below createPixelBuffer call.

> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.mm:343
> +        return nullptr;

Isn't setSize called within convertCMSampleBuffer?

> Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.h:106
> +    void processNewFrame(RefPtr<MediaSample>);

RefPtr<MediaSample>&& probably.
Could we make it a Ref<MediaSample>&&?

> Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.mm:564
> +    scheduleDeferredTask([this, sample = WTFMove(sample)] {

sample = sample.releaseNonNull() should do the trick to make it a Ref

> Source/WebCore/platform/mediastream/mac/DisplayCaptureSourceCocoa.cpp:174
> +    auto frame = generateFrame();

Can frame be null?
If not, we can probably remove the if checks within the switchOn lambdas.

> Source/WebCore/platform/mediastream/mac/MockRealtimeVideoSourceMac.mm:85
> +        return;

Put this if inside the previous if.
I wonder whether we need release logging here, maybe ImageTransferSessionVT::create is already doing that for us.

> Source/WebCore/platform/mediastream/mac/MockRealtimeVideoSourceMac.mm:90
> +        return;

Ditto here for logging.

> Source/WebCore/platform/mediastream/mac/MockRealtimeVideoSourceMac.mm:93
> +    dispatchMediaSampleToObservers(*sampleBuffer.get());

s/.get()// ?

[Eric Carlson]

Created attachment 352576 [details]
Patch

[EWS Watchlist]

Attachment 352576 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.h:51:  The parameter name "rotation" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.h:52:  The parameter name "rotation" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.h:55:  The parameter name "rotation" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/platform/mediastream/mac/DisplayCaptureSourceCocoa.cpp:188:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/WebCore/platform/mediastream/RealtimeMediaSource.h:203:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 5 in 24 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Eric Carlson]

Created attachment 352596 [details]
Patch for landing.

[EWS Watchlist]

Attachment 352596 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.h:51:  The parameter name "rotation" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.h:52:  The parameter name "rotation" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.h:55:  The parameter name "rotation" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/platform/mediastream/RealtimeMediaSource.h:203:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 4 in 24 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Eric Carlson]

Created attachment 352623 [details]
patch for landing

[Eric Carlson]

Comment on attachment 352570 [details]
Patch

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

>> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.h:31
>> +#include "MediaSampleAVFObjc.h"
> 
> Is this one needed here? It is already included in the .mm

An include is needed for the VideoRotation enum.

>> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.h:55
>> +    RefPtr<MediaSample> createMediaSample(IOSurfaceRef, const MediaTime&, const IntSize&, MediaSample::VideoRotation rotation = MediaSample::VideoRotation::None, bool mirrored = false);
> 
> s/rotation =/=/ here and above.

Fixed.

>> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.h:59
>> +    WEBCORE_EXPORT ImageTransferSessionVT(uint32_t);
> 
> explicit. Maybe add pixelFormat since this is not clear without.
> I wonder whether we should consider typing the pixelFormat like we do for ObjectIdentifier.

Fixed.

>> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.mm:62
>> +    VTSessionSetProperty(transferSession, kVTPixelTransferPropertyKey_ScalingMode, kVTScalingMode_Trim);
> 
> Should we check status for all of these and return nullptr in create in case this fails?
> Or output some debug/release logging if failing?

Logging added.

>> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.mm:284
>> +    int32_t extendedBottom = roundUpToMacroblockMultiple(height) - height;
> 
> auto as well.
> Maybe add a comment to explain this?

Fixed.

>> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.mm:325
>> +        return nullptr;
> 
> These checks are done within below createPixelBuffer call.

Fixed.

>> Source/WebCore/platform/graphics/cv/ImageTransferSessionVT.mm:343
>> +        return nullptr;
> 
> Isn't setSize called within convertCMSampleBuffer?

Indeed, removed.

>> Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.h:106
>> +    void processNewFrame(RefPtr<MediaSample>);
> 
> RefPtr<MediaSample>&& probably.
> Could we make it a Ref<MediaSample>&&?

Fixed.

>> Source/WebCore/platform/mediastream/mac/AVVideoCaptureSource.mm:564
>> +    scheduleDeferredTask([this, sample = WTFMove(sample)] {
> 
> sample = sample.releaseNonNull() should do the trick to make it a Ref

OK

>> Source/WebCore/platform/mediastream/mac/DisplayCaptureSourceCocoa.cpp:174
>> +    auto frame = generateFrame();
> 
> Can frame be null?
> If not, we can probably remove the if checks within the switchOn lambdas.

Yes, it can be NULL.

>> Source/WebCore/platform/mediastream/mac/MockRealtimeVideoSourceMac.mm:85
>> +        return;
> 
> Put this if inside the previous if.
> I wonder whether we need release logging here, maybe ImageTransferSessionVT::create is already doing that for us.

Fixed.

>> Source/WebCore/platform/mediastream/mac/MockRealtimeVideoSourceMac.mm:90
>> +        return;
> 
> Ditto here for logging.

createMediaSample will log in the event of a failure.

>> Source/WebCore/platform/mediastream/mac/MockRealtimeVideoSourceMac.mm:93
>> +    dispatchMediaSampleToObservers(*sampleBuffer.get());
> 
> s/.get()// ?

Removed.

[EWS Watchlist]

Attachment 352623 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/mediastream/RealtimeMediaSource.h:203:  Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line.  [build/webcore_export] [4]
Total errors found: 1 in 24 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Commit Bot]

Comment on attachment 352623 [details]
patch for landing

Clearing flags on attachment: 352623

Committed r237236: <https://trac.webkit.org/changeset/237236>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Eric Carlson]

Reopening to attach new patch.

[Eric Carlson]

Created attachment 352660 [details]
Fix iOSMac build

[WebKit Commit Bot]

Comment on attachment 352660 [details]
Fix iOSMac build

Rejecting attachment 352660 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'validate-changelog', '--check-oops', '--non-interactive', 352660, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

/Volumes/Data/EWS/WebKit/Source/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: https://webkit-queues.webkit.org/results/9644368

[Ryan Haddad]

Comment on attachment 352660 [details]
Fix iOSMac build

Clearing flags on attachment: 352660

Committed r237240: <https://trac.webkit.org/changeset/237240>

[Ryan Haddad]

All reviewed patches have been landed.  Closing bug.