[webkit-reviews] review granted: [Bug 206929] Make sure MediaRecorder.requestData returns data with the new writer backend : [Attachment 401889] Adding deprecation macros

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted youenn fablet <youennf at gmail.com>'s
request for review:
Bug 206929: Make sure MediaRecorder.requestData returns data with the new
writer backend
https://bugs.webkit.org/show_bug.cgi?id=206929

Attachment 401889: Adding deprecation macros

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




--- Comment #5 from Darin Adler <darin at apple.com> ---
Comment on attachment 401889
  --> https://bugs.webkit.org/attachment.cgi?id=401889
Adding deprecation macros

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

>
Source/WebCore/platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm:
489
> +	       ALLOW_DEPRECATED_DECLARATIONS_BEGIN
> +	       [m_writer flush];
> +	       ALLOW_DEPRECATED_DECLARATIONS_END

Do we have to write code using a deprecated function? What’s the long term
strategy for the deprecation? Why can’t we take advantage of it now?

>
Source/WebCore/platform/mediarecorder/cocoa/MediaRecorderPrivateWriterCocoa.mm:
496
> +		   RefPtr<SharedBuffer> buffer;
> +		   if (weakThis)
> +		       buffer = WTFMove(m_data);
> +
> +		   completionHandler(WTFMove(buffer));

How about this version?

    if (!weakThis)
	return completionHandler({ });

    completionHandler(WTFMove(m_data));

Seems easier to read than the one with the local variable and double WTFMove?
And has the same early exit here as above. I probably would have written it
with nullptr and without using a "void return", but I figured we could stay
consistent.