[webkit-reviews] review granted: [Bug 190101] [macOS] Update Pasteboard::read to prioritize native representations over TIFF : [Attachment 449845] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has granted Wenson Hsieh
<wenson_hsieh at apple.com>'s request for review:
Bug 190101: [macOS] Update Pasteboard::read to prioritize native
representations over TIFF
https://bugs.webkit.org/show_bug.cgi?id=190101

Attachment 449845: Patch

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




--- Comment #17 from Ryosuke Niwa <rniwa at webkit.org> ---
Comment on attachment 449845
  --> https://bugs.webkit.org/attachment.cgi?id=449845
Patch

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

> Source/WebCore/platform/mac/PasteboardMac.mm:442
> +    Vector<PasteboardItemInfo> relevantItems;

relevant items sounds very generic vague.
How about itemsForIndexByFidelity?
Alternatively, it might be better to create a lambda here
that each case below uses:

e.g.
auto platformTypesFromItems = [](auto& items) {
    HashSet<String> types;
    for (auto& item : types) {
	for (auto& type : item.platformTypesByFidelity)
	    nonTranscodedTypes.add(type);
    }
    return types;
}

if (itemIndex) {
    if (auto itemInfo = strategy.informationForItemAtIndex(*itemIndex,
m_pasteboardName, m_changeCount, context())) {
	types = itemInfo->platformTypesByFidelity;
	nonTranscodedTypes = platformTypesFromItems(*itemInfo);
    }
} else {
    strategy.getTypes(types, m_pasteboardName, context());
    if (auto allItems = strategy.allPasteboardItemInfo(m_pasteboardName,
m_changeCount, context()))
	nonTranscodedTypes = platformTypesFromItems(*itemInfo);
}

> Source/WebCore/platform/mac/PasteboardMac.mm:537
> +	   { String(legacyTIFFPasteboardType()), "image/tiff"_s },

Doesn't this prefer TIFF and PDF over PNG/JPEG if both are written into the
pasteboard?
Perhaps that warrants an explanation in the change log since this patch is
about preferring PNG/JPEG.

> Source/WebCore/platform/mac/PasteboardMac.mm:571
> +    // If the original data could not be read directly, fall back by trying
to transcode to any of the above types.

This comment repeats what the code says.
Can we instead explain when this would happen?
Why may we not be able to read the original data?