[webkit-reviews] review granted: [Bug 188496] [Attachment Support] Augment _WKAttachment SPI to handle NSFileWrappers in addition to NSData : [Attachment 347529] Guard API::Attachment with ENABLE_ATTACHMENT_ELEMENT

[bugzilla-daemon at webkit.org]

Tim Horton <thorton at apple.com> has granted Wenson Hsieh
<wenson_hsieh at apple.com>'s request for review:
Bug 188496: [Attachment Support] Augment _WKAttachment SPI to handle
NSFileWrappers in addition to NSData
https://bugs.webkit.org/show_bug.cgi?id=188496

Attachment 347529: Guard API::Attachment with ENABLE_ATTACHMENT_ELEMENT

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




--- Comment #16 from Tim Horton <thorton at apple.com> ---
Comment on attachment 347529
  --> https://bugs.webkit.org/attachment.cgi?id=347529
Guard API::Attachment with ENABLE_ATTACHMENT_ELEMENT

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

> Source/WebCore/ChangeLog:88
> +	   (): Deleted.

Oh really

> Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm:686
> +		   FileSystem::getFileSize(path, fileSize);
> +		   auto contentType = File::contentTypeForFile(path);

How does this all go if we don't have access to the file?

> Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:4428
> +	       auto typeIdentifier =
adoptCF(UTTypeCreatePreferredIdentifierForTag(kUTTagClassFilenameExtension,
(CFStringRef)pathExtension, NULL));
> +	       contentType = (NSString
*)CFAutorelease(UTTypeCopyPreferredTagWithClass(typeIdentifier.get(),
kUTTagClassMIMEType));

Maybe use UTIUtilities for less typing?

> Source/WebKit/UIProcess/API/Cocoa/_WKAttachment.mm:73
> +    if (![_fileWrapper isRegularFile])
> +	   return nil;

What other kinds of files are there? How sure are we that we don't want them?

> Source/WebKit/UIProcess/API/Cocoa/_WKAttachment.mm:99
> +    auto typeIdentifier =
adoptCF(UTTypeCreatePreferredIdentifierForTag(kUTTagClassFilenameExtension,
(CFStringRef)extension, nil));

Moar UTIUtilities

> Source/WebKit/UIProcess/API/Cocoa/_WKAttachment.mm:169
> +    auto fileSize = [[[fileWrapper fileAttributes] objectForKey:NSFileSize]
unsignedLongLongValue];

Subscripts instead of objectForKey?

> Source/WebKit/UIProcess/API/Cocoa/_WKAttachment.mm:174
> +    _attachment->updateAttributes(fileSize, contentType, [fileWrapper
preferredFilename], [capturedBlock = makeBlockPtr(completionHandler)] (auto
error) {

fileWrapper.preferredFilename? (dots!)