[webkit-reviews] review denied: [Bug 208090] [GPUP] Implement Modern EME API in the GPU Process : [Attachment 391623] Part 2: WebKit + SharedBuffer

[bugzilla-daemon at webkit.org]

Alex Christensen <achristensen at apple.com> has denied Jer Noble
<jer.noble at apple.com>'s request for review:
Bug 208090: [GPUP] Implement Modern EME API in the GPU Process
https://bugs.webkit.org/show_bug.cgi?id=208090

Attachment 391623: Part 2: WebKit + SharedBuffer

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




--- Comment #24 from Alex Christensen <achristensen at apple.com> ---
Comment on attachment 391623
  --> https://bugs.webkit.org/attachment.cgi?id=391623
Part 2: WebKit + SharedBuffer

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

> Source/WebKit/Platform/IPC/SharedBufferDataReference.h:36
>      SharedBufferDataReference() = default;
> +    SharedBufferDataReference(RefPtr<WebCore::SharedBuffer>&& buffer)

Can we remove these? The default constructor was only needed in a time when
decoding needed to instantiate a default constructed object, which is not the
case any more.
Can we make m_buffer a Ref?

> Source/WebKit/Scripts/webkit/messages.py:230
> +	   'WebKit::RemoteCDMIdentifier',

This change seems unrelated to the rest of this patch.

> Source/WebKit/Shared/WebCoreArgumentCoders.cpp:3211
> +Optional<RefPtr<SharedBuffer>>
ArgumentCoder<RefPtr<WebCore::SharedBuffer>>::decode(Decoder& decoder)

Optional<Ref<SharedBuffer>> ArgumentCoder<Ref<...

> Source/WebKit/Shared/WebCoreArgumentCoders.h:835
> +    static Optional<RefPtr<WebCore::SharedBuffer>> decode(Decoder&);

Optional<Ref<...

> Source/WebKit/WebProcess/Storage/WebServiceWorkerFetchTaskClient.cpp:132
> +    m_connection->send(Messages::ServiceWorkerFetchTask::DidReceiveData { {
SharedBuffer::create(data, size) }, static_cast<int64_t>(size) },
m_fetchIdentifier);

This is an unnecessary copy of the data.

> Source/WebKit/WebProcess/WebPage/WebPage.cpp:3503
> +	   dataReference = SharedBuffer::create(data.get());

This is a great improvement.

> Source/WebKit/WebProcess/WebPage/WebPage.cpp:4922
> +    send(Messages::WebPageProxy::DataCallback({
SharedBuffer::create(pdfPageData.get()) }, callbackID));

Great!