[webkit-reviews] review denied: [Bug 80073] WebKit2: remove the last NSPasteboard access from the WebProcess : [Attachment 129782] Patch

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has denied Enrica Casucci <enrica at apple.com>'s
request for review:
Bug 80073: WebKit2: remove the last NSPasteboard access from the WebProcess
https://bugs.webkit.org/show_bug.cgi?id=80073

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

------- Additional Comments from Sam Weinig <sam at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=129782&action=review


> Source/WebKit2/UIProcess/API/mac/PageClientImpl.mm:272
> +void PageClientImpl::setPromisedData(const String& pasteboardName,
PassRefPtr<WebCore::SharedBuffer> imageBuffer, const String& filename, const
String& extension, const String& title, const String& url, const String&
visibleUrl, WTF::PassRefPtr<WebCore::SharedBuffer> archiveBuffer)
> +{
> +    RefPtr<WebCore::Image> image = BitmapImage::create();

The WebCore::'s should not be necessary here.  We also usually make sure that
all the letters in URL are the same case.

>>> Source/WebKit2/UIProcess/API/mac/WKView.mm:76
>>> +#import <WebKit/WebKitNSStringExtras.h>
>> 
>> Can we really include WebKit files from WebKit2???
> 
> We do it in some other places too.

We really should not be doing this.  We should move those things down to
WebCore if we need them.

> Source/WebKit2/UIProcess/mac/WebPageProxyMac.mm:301
> +    RefPtr<SharedMemory> sharedMemoryImage =
SharedMemory::create(imageHandle, SharedMemory::ReadOnly);
> +    RefPtr<SharedBuffer> imageBuffer =
WebCore::SharedBuffer::create(static_cast<unsigned char
*>(sharedMemoryImage->data()), imageSize);
> +    RefPtr<SharedBuffer> archiveBuffer;
> +    
> +    if (!archiveHandle.isNull()) {
> +	   RefPtr<SharedMemory> sharedMemoryArchive =
SharedMemory::create(archiveHandle, SharedMemory::ReadOnly);;
> +	   archiveBuffer = WebCore::SharedBuffer::create(static_cast<unsigned
char *>(sharedMemoryArchive->data()), archiveSize);
> +    }
> +    m_pageClient->setPromisedData(pasteboardName, imageBuffer, filename,
extension, title, url, visibleURL, archiveBuffer);

The WebCore:: should not be necessary.	And the * is on the wrong side for
static_cast<unsigned char *>.