[webkit-reviews] review granted: [Bug 177728] Merge readFilenames() and read(PasteboardFileReader) : [Attachment 322340] Fixed Windows build

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sun Oct 1 18:52:16 PDT 2017


Sam Weinig <sam at webkit.org> has granted Ryosuke Niwa <rniwa at webkit.org>'s
request for review:
Bug 177728: Merge readFilenames() and read(PasteboardFileReader)
https://bugs.webkit.org/show_bug.cgi?id=177728

Attachment 322340: Fixed Windows build

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




--- Comment #6 from Sam Weinig <sam at webkit.org> ---
Comment on attachment 322340
  --> https://bugs.webkit.org/attachment.cgi?id=322340
Fixed Windows build

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

> Source/WebCore/editing/WebCorePasteboardFileReader.cpp:45
> +    files.append(File::create(Blob::create(WTFMove(data), type), filename));

Is it really necessary to do this File wrapping a Blob dance? Given that a File
isa Blob, I think you should be able to avoid it.

> Source/WebCore/platform/cocoa/PasteboardCocoa.mm:107
> +#if PLATFORM(MAC)
> +	   return "image/png"; // For Web compatibility, we pretend to have PNG
instead.
> +#else
>	   ASSERT_NOT_REACHED();
> -	   return nullptr;
> +	   return nullptr; // Don't support TIFF on iOS for now.
> +#endif

This would be slightly nicer as a #if HAVE(TIFF)

> Source/WebCore/platform/cocoa/PasteboardCocoa.mm:153
> +    Vector<String> cocoaTypes = readTypesWithSecurityCheck();

auto?

> Source/WebCore/platform/cocoa/PasteboardCocoa.mm:154
> +    if (!cocoaTypes.size())

I prefer this as cocoaTypes.isEmpty()

> Source/WebCore/platform/cocoa/PasteboardCocoa.mm:184
> +    Vector<String> cocoaTypes = readTypesWithSecurityCheck();

auto?

> Source/WebCore/platform/cocoa/PasteboardCocoa.mm:188
> +	   const char* mimeType = imageTypeToMIMEType(imageType);

auto?

> Source/WebCore/platform/win/PasteboardWin.cpp:314
> +struct PasteboardFileCounter final : public PasteboardFileReader {
> +
> +    void readFilename(const String&) final { ++count; }

Extra newline.

> Source/WebCore/platform/win/PasteboardWin.cpp:355
> +    Vector<String> list = m_dragDataMap.get(cfHDropFormat()->cfFormat);

auto? Or just put the call to get in loop header:

for (auto& filename : m_dragDataMap.get(cfHDropFormat()->cfFormat))


More information about the webkit-reviews mailing list