[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