DataTransfer.files contains multiple files when pasting a single image with multiple...
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 22 May 2020 04:42:47 +0000 (04:42 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 22 May 2020 04:42:47 +0000 (04:42 +0000)
https://bugs.webkit.org/show_bug.cgi?id=212245
<rdar://problem/60240436>

Reviewed by Tim Horton.

Source/WebCore:

When pasting or dropping a single image that is backed by multiple representations in NSPasteboard (or
UIPasteboard), we currently report more than one `File` to the page via `DataTransfer.files`. This is because
`Pasteboard::read(PasteboardFileReader&)`, which is responsible for converting the contents of the pasteboard
into a list of files, currently iterates over every pasteboard type and adds each of them as a file. This is
wrong when an item has multiple type representations.

To differentiate the case where a single item has multiple representations from the case where it has multiple
pasteboard items, we use `allPasteboardItemInfo()` instead to grab a per-item list of types from the pasteboard
on Cocoa platforms, and only create at most 1 file per item using the highest fidelity type that contains data.

Test: PasteImage.PasteImageWithMultipleRepresentations

* platform/cocoa/PasteboardCocoa.mm:
(WebCore::Pasteboard::read):

Tools:

* DumpRenderTree/mac/DumpRenderTreePasteboard.mm:
(-[LocalPasteboard _clearContentsWithoutUpdatingChangeCount]):
(-[LocalPasteboard _addTypesWithoutUpdatingChangeCount:owner:]):
(-[LocalPasteboard writeObjects:]):
(-[LocalPasteboard pasteboardItems]):

Adjust DumpRenderTree's LocalPasteboard so that it lazily populates the pasteboard when constructing
NSPasteboardItems. To do this, we need to make a few adjustments:

1.      When reifying NSPasteboardItems from LocalPasteboard, ask the owner (WebHTMLView) to provide pasteboard
        data for each pasteboard type that was promised by WebKit, but was not eagerly written to the pasteboard.

2.      Cache pasteboard items that were created, so that we don't repeatedly ask WebHTMLView to provide
        pasteboard data. WebHTMLView doesn't currently support this, and suffers from a bug where TIFF data may
        only be provided once. This was fixed for WebKit2, but not for WebKit1.

3.      Maintain a separate hash list of original pasteboard types (which may not be UTIs) that were handed to
        LocalPasteboard by WebKit. We use these original types in step (1).

* TestWebKitAPI/Tests/WebKitCocoa/PasteImage.mm:

Add a new API test to verify that one file is exposed via the DataTranfer when the pasteboard contains a single
image with two image representations, but two files are exposed when the pasteboard contains two images, each
with a single representation.

(writeImageDataToPasteboard):

Overload this helper method with two additional variants: one that takes a dictionary of pasteboard types to
data, and another that takes an array of dictionaries, each representing a single item's types and data.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@262047 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/platform/cocoa/PasteboardCocoa.mm
Tools/ChangeLog
Tools/DumpRenderTree/mac/DumpRenderTreePasteboard.mm
Tools/TestWebKitAPI/Tests/WebKitCocoa/PasteImage.mm

index 5ee394a..daf77fc 100644 (file)
@@ -1,3 +1,26 @@
+2020-05-21  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        DataTransfer.files contains multiple files when pasting a single image with multiple representations
+        https://bugs.webkit.org/show_bug.cgi?id=212245
+        <rdar://problem/60240436>
+
+        Reviewed by Tim Horton.
+
+        When pasting or dropping a single image that is backed by multiple representations in NSPasteboard (or
+        UIPasteboard), we currently report more than one `File` to the page via `DataTransfer.files`. This is because
+        `Pasteboard::read(PasteboardFileReader&)`, which is responsible for converting the contents of the pasteboard
+        into a list of files, currently iterates over every pasteboard type and adds each of them as a file. This is
+        wrong when an item has multiple type representations.
+
+        To differentiate the case where a single item has multiple representations from the case where it has multiple
+        pasteboard items, we use `allPasteboardItemInfo()` instead to grab a per-item list of types from the pasteboard
+        on Cocoa platforms, and only create at most 1 file per item using the highest fidelity type that contains data.
+
+        Test: PasteImage.PasteImageWithMultipleRepresentations
+
+        * platform/cocoa/PasteboardCocoa.mm:
+        (WebCore::Pasteboard::read):
+
 2020-05-21  Simon Fraser  <simon.fraser@apple.com>
 
         Fix rare scrolling thread crash firing the m_delayedRenderingUpdateDetectionTimer timer
index 0f4faaf..ba1ac1c 100644 (file)
@@ -224,24 +224,27 @@ void Pasteboard::read(PasteboardFileReader& reader)
         return;
     }
 
-    auto cocoaTypes = readTypesWithSecurityCheck();
-    HashSet<const char*> existingMIMEs;
-    for (auto& cocoaType : cocoaTypes) {
-        auto imageType = cocoaTypeToImageType(cocoaType);
-        const char* mimeType = imageTypeToMIMEType(imageType);
-        if (!mimeType)
-            continue;
-        if (existingMIMEs.contains(mimeType))
-            continue;
-        auto buffer = readBufferForTypeWithSecurityCheck(cocoaType);
+    auto allInfo = allPasteboardItemInfo();
+    if (!allInfo)
+        return;
+
+    for (size_t itemIndex = 0; itemIndex < allInfo->size(); ++itemIndex) {
+        auto& info = allInfo->at(itemIndex);
+        for (auto cocoaType : info.platformTypesByFidelity) {
+            auto imageType = cocoaTypeToImageType(cocoaType);
+            auto* mimeType = imageTypeToMIMEType(imageType);
+            if (!mimeType)
+                continue;
+            auto buffer = readBuffer(itemIndex, cocoaType);
 #if PLATFORM(MAC)
-        if (buffer && imageType == ImageType::TIFF)
-            buffer = convertTIFFToPNG(buffer.releaseNonNull());
+            if (buffer && imageType == ImageType::TIFF)
+                buffer = convertTIFFToPNG(buffer.releaseNonNull());
 #endif
-        if (!buffer)
-            continue;
-        existingMIMEs.add(mimeType);
-        reader.readBuffer(imageTypeToFakeFilename(imageType), mimeType, buffer.releaseNonNull());
+            if (buffer) {
+                reader.readBuffer(imageTypeToFakeFilename(imageType), mimeType, buffer.releaseNonNull());
+                break;
+            }
+        }
     }
 }
 
index 90d08be..c073346 100644 (file)
@@ -1,3 +1,41 @@
+2020-05-21  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        DataTransfer.files contains multiple files when pasting a single image with multiple representations
+        https://bugs.webkit.org/show_bug.cgi?id=212245
+        <rdar://problem/60240436>
+
+        Reviewed by Tim Horton.
+
+        * DumpRenderTree/mac/DumpRenderTreePasteboard.mm:
+        (-[LocalPasteboard _clearContentsWithoutUpdatingChangeCount]):
+        (-[LocalPasteboard _addTypesWithoutUpdatingChangeCount:owner:]):
+        (-[LocalPasteboard writeObjects:]):
+        (-[LocalPasteboard pasteboardItems]):
+
+        Adjust DumpRenderTree's LocalPasteboard so that it lazily populates the pasteboard when constructing
+        NSPasteboardItems. To do this, we need to make a few adjustments:
+
+        1.      When reifying NSPasteboardItems from LocalPasteboard, ask the owner (WebHTMLView) to provide pasteboard
+                data for each pasteboard type that was promised by WebKit, but was not eagerly written to the pasteboard.
+
+        2.      Cache pasteboard items that were created, so that we don't repeatedly ask WebHTMLView to provide
+                pasteboard data. WebHTMLView doesn't currently support this, and suffers from a bug where TIFF data may
+                only be provided once. This was fixed for WebKit2, but not for WebKit1.
+
+        3.      Maintain a separate hash list of original pasteboard types (which may not be UTIs) that were handed to
+                LocalPasteboard by WebKit. We use these original types in step (1).
+
+        * TestWebKitAPI/Tests/WebKitCocoa/PasteImage.mm:
+
+        Add a new API test to verify that one file is exposed via the DataTranfer when the pasteboard contains a single
+        image with two image representations, but two files are exposed when the pasteboard contains two images, each
+        with a single representation.
+
+        (writeImageDataToPasteboard):
+
+        Overload this helper method with two additional variants: one that takes a dictionary of pasteboard types to
+        data, and another that takes an array of dictionaries, each representing a single item's types and data.
+
 2020-05-21  Robin Morisset  <rmorisset@apple.com>
 
         Various compile-time boolean flags could/should be marked constexpr
index f931c6a..9807736 100644 (file)
 @interface LocalPasteboard : NSPasteboard {
     RetainPtr<id> _owner;
     RetainPtr<NSString> _pasteboardName;
-    RetainPtr<NSMutableArray<NSPasteboardItem *>> _writtenPasteboardItems;
+    RetainPtr<NSArray<NSPasteboardItem *>> _cachedPasteboardItems;
     NSInteger _changeCount;
 
     ListHashSet<RetainPtr<CFStringRef>, WTF::RetainPtrObjectHash<CFStringRef>> _types;
+    ListHashSet<RetainPtr<CFStringRef>, WTF::RetainPtrObjectHash<CFStringRef>> _originalTypes;
     HashMap<RetainPtr<CFStringRef>, RetainPtr<CFDataRef>, WTF::RetainPtrObjectHash<CFStringRef>, WTF::RetainPtrObjectHashTraits<CFStringRef>> _data;
 }
 
@@ -144,8 +145,9 @@ static RetainPtr<CFStringRef> toUTI(NSString *type)
 
 - (void)_clearContentsWithoutUpdatingChangeCount
 {
-    _writtenPasteboardItems = nil;
+    _cachedPasteboardItems = nil;
     _types.clear();
+    _originalTypes.clear();
     _data.clear();
 }
 
@@ -168,8 +170,10 @@ static RetainPtr<CFStringRef> toUTI(NSString *type)
 {
     _owner = newOwner;
 
-    for (NSString *type in newTypes)
+    for (NSString *type in newTypes) {
         _types.add(toUTI(type));
+        _originalTypes.add((__bridge CFStringRef)type);
+    }
 }
 
 - (NSInteger)changeCount
@@ -239,10 +243,10 @@ static RetainPtr<CFStringRef> toUTI(NSString *type)
 
 - (BOOL)writeObjects:(NSArray<id <NSPasteboardWriting>> *)objects
 {
-    _writtenPasteboardItems = adoptNS([[NSMutableArray<NSPasteboardItem *> alloc] initWithCapacity:objects.count]);
+    auto items = adoptNS([[NSMutableArray<NSPasteboardItem *> alloc] initWithCapacity:objects.count]);
     for (id <NSPasteboardWriting> object in objects) {
         ASSERT([object isKindOfClass:NSPasteboardItem.class]);
-        [_writtenPasteboardItems addObject:(NSPasteboardItem *)object];
+        [items addObject:(NSPasteboardItem *)object];
         for (NSString *type in [object writableTypesForPasteboard:self]) {
             [self addTypes:@[ type ] owner:self];
 
@@ -254,21 +258,29 @@ static RetainPtr<CFStringRef> toUTI(NSString *type)
         }
     }
 
+    _cachedPasteboardItems = items.get();
     return YES;
 }
 
 - (NSArray<NSPasteboardItem *> *)pasteboardItems
 {
-    if (_writtenPasteboardItems)
-        return _writtenPasteboardItems.get();
+    if (_cachedPasteboardItems)
+        return _cachedPasteboardItems.get();
 
     auto item = adoptNS([[NSPasteboardItem alloc] init]);
+    for (auto& type : _originalTypes) {
+        if (!_data.contains(toUTI((__bridge NSString *)type.get())))
+            [_owner pasteboard:self provideDataForType:(__bridge NSString *)type.get()];
+    }
+
     for (const auto& typeAndData : _data) {
         NSData *data = (__bridge NSData *)typeAndData.value.get();
         NSString *type = (__bridge NSString *)typeAndData.key.get();
         [item setData:data forType:[NSPasteboard _modernPasteboardType:type]];
     }
-    return @[ item.get() ];
+
+    _cachedPasteboardItems = @[ item.get() ];
+    return _cachedPasteboardItems.get();
 }
 
 @end
index 152e1ac..e5465c8 100644 (file)
 #endif
 
 #if PLATFORM(MAC)
+
 void writeImageDataToPasteboard(NSString *type, NSData *data)
 {
-    [[NSPasteboard generalPasteboard] declareTypes:@[type] owner:nil];
-    [[NSPasteboard generalPasteboard] setData:data forType:type];
+    [NSPasteboard.generalPasteboard declareTypes:@[type] owner:nil];
+    [NSPasteboard.generalPasteboard setData:data forType:type];
+}
+
+void writeImageDataToPasteboard(NSDictionary <NSString *, NSData *> *typesAndData)
+{
+    [NSPasteboard.generalPasteboard declareTypes:typesAndData.allKeys owner:nil];
+    for (NSString *type in typesAndData)
+        [NSPasteboard.generalPasteboard setData:typesAndData[type] forType:type];
+}
+
+void writeImageDataToPasteboard(NSArray<NSDictionary <NSString *, NSData *> *> *items)
+{
+    [NSPasteboard.generalPasteboard clearContents];
+    auto pasteboardItems = adoptNS([[NSMutableArray alloc] initWithCapacity:items.count]);
+    for (NSDictionary<NSString *, NSData *> *typesAndData in items) {
+        auto pasteboardItem = adoptNS([[NSPasteboardItem alloc] init]);
+        for (NSString *type in typesAndData)
+            [pasteboardItem setData:typesAndData[type] forType:type];
+        [pasteboardItems addObject:pasteboardItem.get()];
+    }
+    [NSPasteboard.generalPasteboard writeObjects:pasteboardItems.get()];
 }
+
 #else
+
 void writeImageDataToPasteboard(NSString *type, NSData *data)
 {
-    [[UIPasteboard generalPasteboard] setItems:@[@{ type: data }]];
+    UIPasteboard.generalPasteboard.items = @[ @{ type: data } ];
+}
+
+void writeImageDataToPasteboard(NSDictionary <NSString *, NSData *> *typesAndData)
+{
+    UIPasteboard.generalPasteboard.items = @[ typesAndData ];
+}
+
+void writeImageDataToPasteboard(NSArray<NSDictionary <NSString *, NSData *> *> *items)
+{
+    UIPasteboard.generalPasteboard.items = items;
 }
+
 #endif
 
 @interface TestWKWebView (PasteImage)
@@ -137,6 +171,30 @@ TEST(PasteImage, PastePNGImage)
     EXPECT_WK_STREQ("200", [webView stringByEvaluatingJavaScript:@"imageElement.width"]);
 }
 
+TEST(PasteImage, PasteImageWithMultipleRepresentations)
+{
+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 400, 400)]);
+    [webView synchronouslyLoadTestPageNamed:@"paste-image"];
+
+    auto pngData = [NSData dataWithContentsOfFile:[[NSBundle mainBundle] pathForResource:@"sunset-in-cupertino-200px" ofType:@"png" inDirectory:@"TestWebKitAPI.resources"]];
+    auto jpegData = [NSData dataWithContentsOfFile:[[NSBundle mainBundle] pathForResource:@"sunset-in-cupertino-600px" ofType:@"jpg" inDirectory:@"TestWebKitAPI.resources"]];
+    writeImageDataToPasteboard(@{
+        (__bridge NSString *)kUTTypePNG : pngData,
+        (__bridge NSString *)kUTTypeJPEG : jpegData
+    });
+    [webView paste:nil];
+
+    EXPECT_WK_STREQ("1", [webView stringByEvaluatingJavaScript:@"dataTransfer.files.length"]);
+
+    writeImageDataToPasteboard(@[
+        @{ (__bridge NSString *)kUTTypePNG : pngData },
+        @{ (__bridge NSString *)kUTTypeJPEG : jpegData }
+    ]);
+    [webView paste:nil];
+
+    EXPECT_WK_STREQ("2", [webView stringByEvaluatingJavaScript:@"dataTransfer.files.length"]);
+}
+
 TEST(PasteImage, RevealSelectionAfterPastingImage)
 {
     auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 400, 400)]);