The dragged image should be the current frame only of the animated image
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 5 Oct 2016 01:52:35 +0000 (01:52 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 5 Oct 2016 01:52:35 +0000 (01:52 +0000)
https://bugs.webkit.org/show_bug.cgi?id=162109

Patch by Said Abou-Hallawa <sabouhallawa@apple.com> on 2016-10-04
Reviewed by Tim Horton.

Source/WebCore:

Instead of creating an NSImage with all the frames for the dragImage,
create an NSImage with the current frame only.

* dom/DataTransferMac.mm:
(WebCore::DataTransfer::createDragImage): Call snapshotNSImage() to create the dragImage.
* editing/cocoa/HTMLConverter.mm:
(fileWrapperForElement):  Call the Image function with its new name.
* platform/graphics/BitmapImage.h:
* platform/graphics/Image.h:
(WebCore::Image::nsImage): Rename getNSImage() to nsImage().
(WebCore::Image::snapshotNSImage): Returns the NSImage of the current frame.
(WebCore::Image::tiffRepresentation): Rename getTIFFRepresentation() to tiffRepresentation().
(WebCore::Image::getNSImage): Deleted.
(WebCore::Image::getTIFFRepresentation): Deleted.
* platform/graphics/mac/ImageMac.mm:
(WebCore::BitmapImage::tiffRepresentation): Rename getTIFFRepresentation() to tiffRepresentation().
(WebCore::BitmapImage::nsImage): Rename getNSImage() to nsImage().
(WebCore::BitmapImage::snapshotNSImage): Returns the NSImage of the current frame.
(WebCore::BitmapImage::getTIFFRepresentation): Deleted.
(WebCore::BitmapImage::getNSImage): Deleted.
* platform/mac/CursorMac.mm:
(WebCore::createCustomCursor): Call snapshotNSImage() since the cursor does not animate anyway.
* platform/mac/DragImageMac.mm:
(WebCore::createDragImageFromImage): Use snapshotNSImage() for the dragImage.
* platform/mac/PasteboardMac.mm:
(WebCore::Pasteboard::write): Call the Image function with its new name.

Source/WebKit/mac:

* DOM/DOM.mm:
(-[DOMElement image]): Call the Image function with its new name.
(-[DOMElement _imageTIFFRepresentation]): Ditto.
* Misc/WebElementDictionary.mm:
(-[WebElementDictionary _image]): Call the Image function with its new name.
* Misc/WebIconDatabase.mm:
(-[WebIconDatabase defaultIconWithSize:]): Call snapshotNSImage() to create the icon image.
(webGetNSImage): Call the Image function with its new name.
* WebCoreSupport/WebContextMenuClient.mm:
(WebContextMenuClient::imageForCurrentSharingServicePickerItem): Call snapshotNSImage() instead of nsImage()..
(WebContextMenuClient::contextMenuForEvent): Ditto.
* WebView/WebHTMLView.mm:
(-[WebHTMLView pasteboard:provideDataForType:]): Call the Image function with its new name.

Source/WebKit2:

* UIProcess/Cocoa/WebViewImpl.mm:
(WebKit::WebViewImpl::provideDataForPasteboard): Call the Image function with its new name.

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

17 files changed:
Source/WebCore/ChangeLog
Source/WebCore/dom/DataTransferMac.mm
Source/WebCore/editing/cocoa/HTMLConverter.mm
Source/WebCore/platform/graphics/BitmapImage.h
Source/WebCore/platform/graphics/Image.h
Source/WebCore/platform/graphics/mac/ImageMac.mm
Source/WebCore/platform/mac/CursorMac.mm
Source/WebCore/platform/mac/DragImageMac.mm
Source/WebCore/platform/mac/PasteboardMac.mm
Source/WebKit/mac/ChangeLog
Source/WebKit/mac/DOM/DOM.mm
Source/WebKit/mac/Misc/WebElementDictionary.mm
Source/WebKit/mac/Misc/WebIconDatabase.mm
Source/WebKit/mac/WebCoreSupport/WebContextMenuClient.mm
Source/WebKit/mac/WebView/WebHTMLView.mm
Source/WebKit2/ChangeLog
Source/WebKit2/UIProcess/Cocoa/WebViewImpl.mm

index 7658d6b..7d329ea 100644 (file)
@@ -1,3 +1,37 @@
+2016-10-04  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        The dragged image should be the current frame only of the animated image
+        https://bugs.webkit.org/show_bug.cgi?id=162109
+
+        Reviewed by Tim Horton.
+
+        Instead of creating an NSImage with all the frames for the dragImage,
+        create an NSImage with the current frame only.
+
+        * dom/DataTransferMac.mm:
+        (WebCore::DataTransfer::createDragImage): Call snapshotNSImage() to create the dragImage.
+        * editing/cocoa/HTMLConverter.mm:
+        (fileWrapperForElement):  Call the Image function with its new name.
+        * platform/graphics/BitmapImage.h:
+        * platform/graphics/Image.h:
+        (WebCore::Image::nsImage): Rename getNSImage() to nsImage().
+        (WebCore::Image::snapshotNSImage): Returns the NSImage of the current frame.
+        (WebCore::Image::tiffRepresentation): Rename getTIFFRepresentation() to tiffRepresentation().
+        (WebCore::Image::getNSImage): Deleted.
+        (WebCore::Image::getTIFFRepresentation): Deleted.
+        * platform/graphics/mac/ImageMac.mm:
+        (WebCore::BitmapImage::tiffRepresentation): Rename getTIFFRepresentation() to tiffRepresentation().
+        (WebCore::BitmapImage::nsImage): Rename getNSImage() to nsImage().
+        (WebCore::BitmapImage::snapshotNSImage): Returns the NSImage of the current frame.
+        (WebCore::BitmapImage::getTIFFRepresentation): Deleted.
+        (WebCore::BitmapImage::getNSImage): Deleted.
+        * platform/mac/CursorMac.mm:
+        (WebCore::createCustomCursor): Call snapshotNSImage() since the cursor does not animate anyway.
+        * platform/mac/DragImageMac.mm:
+        (WebCore::createDragImageFromImage): Use snapshotNSImage() for the dragImage.
+        * platform/mac/PasteboardMac.mm:
+        (WebCore::Pasteboard::write): Call the Image function with its new name.
+
 2016-10-04  Andy Estes  <aestes@apple.com>
 
         [iOS] Crash in WebResourceLoaderQuickLookDelegate when the client cancels the navigation to a QuickLook resource
index 7e8d967..94711a0 100644 (file)
@@ -49,7 +49,7 @@ DragImageRef DataTransfer::createDragImage(IntPoint& location) const
             location.setY(imageRect.height() - (elementRect.y() - imageRect.y() + m_dragLocation.y()));
         }
     } else if (m_dragImage) {
-        result = m_dragImage->image()->getNSImage();
+        result = m_dragImage->image()->snapshotNSImage();
         
         location = m_dragLocation;
         location.setY([result size].height - location.y());
index a162734..4aec974 100644 (file)
@@ -2464,7 +2464,7 @@ static RetainPtr<NSFileWrapper> fileWrapperForElement(HTMLImageElement& element)
     if (is<RenderImage>(renderer)) {
         auto* image = downcast<RenderImage>(*renderer).cachedImage();
         if (image && !image->errorOccurred()) {
-            RetainPtr<NSFileWrapper> wrapper = adoptNS([[NSFileWrapper alloc] initRegularFileWithContents:(NSData *)image->imageForRenderer(renderer)->getTIFFRepresentation()]);
+            RetainPtr<NSFileWrapper> wrapper = adoptNS([[NSFileWrapper alloc] initRegularFileWithContents:(NSData *)image->imageForRenderer(renderer)->tiffRepresentation()]);
             [wrapper setPreferredFilename:@"image.tiff"];
             return wrapper;
         }
index 5629c60..74c22a2 100644 (file)
@@ -96,11 +96,12 @@ public:
 
     // Accessors for native image formats.
 #if USE(APPKIT)
-    NSImage* getNSImage() override;
+    NSImage *nsImage() override;
+    RetainPtr<NSImage> snapshotNSImage() override;
 #endif
 
 #if PLATFORM(COCOA)
-    CFDataRef getTIFFRepresentation() override;
+    CFDataRef tiffRepresentation() override;
 #endif
 
 #if PLATFORM(WIN)
@@ -178,6 +179,10 @@ protected:
     bool notSolidColor() override;
 #endif
 
+#if PLATFORM(COCOA)
+    RetainPtr<CFDataRef> tiffRepresentation(const Vector<NativeImagePtr>&);
+#endif
+
 private:
     void clearTimer();
     void startTimer(double delay);
@@ -194,10 +199,10 @@ private:
     bool m_animationFinishedWhenCatchingUp { false };
 
 #if USE(APPKIT)
-    mutable RetainPtr<NSImage> m_nsImage; // A cached NSImage of frame 0. Only built lazily if someone actually queries for one.
+    mutable RetainPtr<NSImage> m_nsImage; // A cached NSImage of all the frames. Only built lazily if someone actually queries for one.
 #endif
 #if USE(CG)
-    mutable RetainPtr<CFDataRef> m_tiffRep; // Cached TIFF rep for frame 0. Only built lazily if someone queries for one.
+    mutable RetainPtr<CFDataRef> m_tiffRep; // Cached TIFF rep for all the frames. Only built lazily if someone queries for one.
 #endif
     RefPtr<Image> m_cachedImage;
 };
index 3ccf56a..a1d0952 100644 (file)
@@ -146,11 +146,12 @@ public:
     // Accessors for native image formats.
 
 #if USE(APPKIT)
-    virtual NSImage* getNSImage() { return nullptr; }
+    virtual NSImage *nsImage() { return nullptr; }
+    virtual RetainPtr<NSImage> snapshotNSImage() { return nullptr; }
 #endif
 
 #if PLATFORM(COCOA)
-    virtual CFDataRef getTIFFRepresentation() { return nullptr; }
+    virtual CFDataRef tiffRepresentation() { return nullptr; }
 #endif
 
 #if PLATFORM(WIN)
@@ -193,7 +194,7 @@ protected:
 
     // Supporting tiled drawing
     virtual Color singlePixelSolidColor() const { return Color(); }
-    
+
 private:
     RefPtr<SharedBuffer> m_encodedImageData;
     ImageObserver* m_imageObserver;
index 28e297b..2dba80a 100644 (file)
@@ -74,14 +74,9 @@ PassRefPtr<Image> Image::loadPlatformResource(const char *name)
     return Image::nullImage();
 }
 
-CFDataRef BitmapImage::getTIFFRepresentation()
+RetainPtr<CFDataRef> BitmapImage::tiffRepresentation(const Vector<NativeImagePtr>& nativeImages)
 {
-    if (m_tiffRep)
-        return m_tiffRep.get();
-
-    auto nativeImages = this->framesNativeImages();
-
-    // If framesImages.size() is zero, we know for certain this image doesn't have valid data
+    // If nativeImages.size() is zero, we know for certain this image doesn't have valid data
     // Even though the call to CGImageDestinationCreateWithData will fail and we'll handle it gracefully,
     // in certain circumstances that call will spam the console with an error message
     if (!nativeImages.size())
@@ -97,24 +92,50 @@ CFDataRef BitmapImage::getTIFFRepresentation()
         CGImageDestinationAddImage(destination.get(), nativeImage.get(), 0);
 
     CGImageDestinationFinalize(destination.get());
+    return data;
+}
+
+CFDataRef BitmapImage::tiffRepresentation()
+{
+    if (m_tiffRep)
+        return m_tiffRep.get();
+
+    auto data = tiffRepresentation(framesNativeImages());
+    if (!data)
+        return nullptr;
 
     m_tiffRep = data;
     return m_tiffRep.get();
+
+    
 }
 
 #if USE(APPKIT)
-NSImage* BitmapImage::getNSImage()
+NSImage* BitmapImage::nsImage()
 {
     if (m_nsImage)
         return m_nsImage.get();
 
-    CFDataRef data = getTIFFRepresentation();
+    CFDataRef data = tiffRepresentation();
     if (!data)
-        return 0;
+        return nullptr;
     
     m_nsImage = adoptNS([[NSImage alloc] initWithData:(NSData*)data]);
     return m_nsImage.get();
 }
+
+RetainPtr<NSImage> BitmapImage::snapshotNSImage()
+{
+    auto nativeImage = this->nativeImageForCurrentFrame();
+    if (!nativeImage)
+        return nullptr;
+
+    auto data = tiffRepresentation({ nativeImage });
+    if (!data)
+        return nullptr;
+
+    return adoptNS([[NSImage alloc] initWithData:(NSData*)data.get()]);
+}
 #endif
 
 }
index 2cc4024..a6d7832 100644 (file)
@@ -48,9 +48,9 @@ static RetainPtr<NSCursor> createCustomCursor(Image* image, const IntPoint& hotS
 #endif
 {
     // FIXME: The cursor won't animate.  Not sure if that's a big deal.
-    NSImage* nsImage = image->getNSImage();
+    auto nsImage = image->snapshotNSImage();
     if (!nsImage)
-        return 0;
+        return nullptr;
     BEGIN_BLOCK_OBJC_EXCEPTIONS;
 
 #if ENABLE(MOUSE_CURSOR_SCALE)
@@ -78,7 +78,7 @@ static RetainPtr<NSCursor> createCustomCursor(Image* image, const IntPoint& hotS
     [[[nsImage representations] objectAtIndex:0] setSize:expandedSize];
 #endif
 
-    return adoptNS([[NSCursor alloc] initWithImage:nsImage hotSpot:hotSpot]);
+    return adoptNS([[NSCursor alloc] initWithImage:nsImage.get() hotSpot:hotSpot]);
     END_BLOCK_OBJC_EXCEPTIONS;
     return nullptr;
 }
index 7697c7b..bfcb72f 100644 (file)
@@ -121,7 +121,7 @@ RetainPtr<NSImage> createDragImageFromImage(Image* image, ImageOrientationDescri
 
 #pragma clang diagnostic push
 #pragma clang diagnostic ignored "-Wdeprecated-declarations"
-            [image->getNSImage() drawInRect:destRect fromRect:NSMakeRect(0, 0, size.width(), size.height()) operation:NSCompositeSourceOver fraction:1.0];
+            [image->snapshotNSImage() drawInRect:destRect fromRect:NSMakeRect(0, 0, size.width(), size.height()) operation:NSCompositeSourceOver fraction:1.0];
 #pragma clang diagnostic pop
             [rotatedDragImage.get() unlockFocus];
 
@@ -129,7 +129,7 @@ RetainPtr<NSImage> createDragImageFromImage(Image* image, ImageOrientationDescri
         }
     }
 
-    RetainPtr<NSImage> dragImage = adoptNS([image->getNSImage() copy]);
+    auto dragImage = image->snapshotNSImage();
     [dragImage.get() setSize:(NSSize)size];
     return dragImage;
 }
index df31216..3fec2d0 100644 (file)
@@ -255,7 +255,7 @@ static void writeFileWrapperAsRTFDAttachment(NSFileWrapper *wrapper, const Strin
 
 void Pasteboard::write(const PasteboardImage& pasteboardImage)
 {
-    CFDataRef imageData = pasteboardImage.image->getTIFFRepresentation();
+    CFDataRef imageData = pasteboardImage.image->tiffRepresentation();
     if (!imageData)
         return;
 
index 5a86cbf..7b313c9 100644 (file)
@@ -1,3 +1,24 @@
+2016-10-04  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        The dragged image should be the current frame only of the animated image
+        https://bugs.webkit.org/show_bug.cgi?id=162109
+
+        Reviewed by Tim Horton.
+
+        * DOM/DOM.mm:
+        (-[DOMElement image]): Call the Image function with its new name.
+        (-[DOMElement _imageTIFFRepresentation]): Ditto.
+        * Misc/WebElementDictionary.mm:
+        (-[WebElementDictionary _image]): Call the Image function with its new name.
+        * Misc/WebIconDatabase.mm:
+        (-[WebIconDatabase defaultIconWithSize:]): Call snapshotNSImage() to create the icon image.
+        (webGetNSImage): Call the Image function with its new name.
+        * WebCoreSupport/WebContextMenuClient.mm:
+        (WebContextMenuClient::imageForCurrentSharingServicePickerItem): Call snapshotNSImage() instead of nsImage()..
+        (WebContextMenuClient::contextMenuForEvent): Ditto.
+        * WebView/WebHTMLView.mm:
+        (-[WebHTMLView pasteboard:provideDataForType:]): Call the Image function with its new name.
+
 2016-10-02  Ryan Haddad  <ryanhaddad@apple.com>
 
         Unreviewed, rolling out r206683.
index c7dcd20..91efc93 100644 (file)
@@ -670,7 +670,7 @@ id <DOMEventTarget> kit(EventTarget* eventTarget)
     auto* cachedImage = downcast<RenderImage>(*renderer).cachedImage();
     if (!cachedImage || cachedImage->errorOccurred())
         return nil;
-    return cachedImage->imageForRenderer(renderer)->getNSImage();
+    return cachedImage->imageForRenderer(renderer)->nsImage();
 }
 
 #endif
@@ -698,7 +698,7 @@ id <DOMEventTarget> kit(EventTarget* eventTarget)
     auto* cachedImage = downcast<RenderImage>(*renderer).cachedImage();
     if (!cachedImage || cachedImage->errorOccurred())
         return nil;
-    return (NSData *)cachedImage->imageForRenderer(renderer)->getTIFFRepresentation();
+    return (NSData *)cachedImage->imageForRenderer(renderer)->tiffRepresentation();
 }
 
 #endif
index 2126f75..de09aa6 100644 (file)
@@ -204,7 +204,7 @@ static NSString* NSStringOrNil(String coreString)
 - (NSImage *)_image
 {
     Image* image = _result->image();
-    return image ? image->getNSImage() : nil;
+    return image ? image->nsImage() : nil;
 }
 
 - (NSValue *)_imageRect
index 30b9b7e..862db48 100644 (file)
@@ -163,7 +163,7 @@ static WebIconDatabaseClient* defaultClient()
     ASSERT(size.height);
     
     Image* image = iconDatabase().defaultIcon(IntSize(size));
-    return image ? image->getNSImage() : nil;
+    return image ? image->snapshotNSImage().autorelease() : nil;
 }
 
 - (NSImage *)defaultIconForURL:(NSString *)URL withSize:(NSSize)size
@@ -470,7 +470,7 @@ NSImage *webGetNSImage(Image* image, NSSize size)
     // to WebCore::Image at some point.
     if (!image)
         return nil;
-    NSImage* nsImage = image->getNSImage();
+    NSImage* nsImage = image->nsImage();
     if (!nsImage)
         return nil;
     if (!NSEqualSizes([nsImage size], size)) {
index 08d435b..914d8e0 100644 (file)
@@ -226,7 +226,7 @@ RetainPtr<NSImage> WebContextMenuClient::imageForCurrentSharingServicePickerItem
     if (!image)
         return nil;
 
-    return [[image->getNSImage() retain] autorelease];
+    return image->snapshotNSImage();
 }
 #endif
 
@@ -242,7 +242,7 @@ NSMenu *WebContextMenuClient::contextMenuForEvent(NSEvent *event, NSView *view,
     if (Image* image = page->contextMenuController().context().controlledImage()) {
         ASSERT(page->contextMenuController().context().hitTestResult().innerNode());
 
-        RetainPtr<NSItemProvider> itemProvider = adoptNS([[NSItemProvider alloc] initWithItem:image->getNSImage() typeIdentifier:@"public.image"]);
+        RetainPtr<NSItemProvider> itemProvider = adoptNS([[NSItemProvider alloc] initWithItem:image->snapshotNSImage().autorelease() typeIdentifier:@"public.image"]);
 
         bool isContentEditable = page->contextMenuController().context().hitTestResult().innerNode()->isContentEditable();
         m_sharingServicePickerController = adoptNS([[WebSharingServicePickerController alloc] initWithItems:@[ itemProvider.get() ] includeEditorServices:isContentEditable client:this style:NSSharingServicePickerStyleRollover]);
index 8043bdf..0613810 100644 (file)
@@ -2210,7 +2210,7 @@ static bool mouseEventIsPartOfClickOrDrag(NSEvent *event)
         [archive release];
     } else if ([type isEqual:NSTIFFPboardType] && [self promisedDragTIFFDataSource]) {
         if (Image* image = [self promisedDragTIFFDataSource]->image())
-            [pasteboard setData:(NSData *)image->getTIFFRepresentation() forType:NSTIFFPboardType];
+            [pasteboard setData:(NSData *)image->tiffRepresentation() forType:NSTIFFPboardType];
         [self setPromisedDragTIFFDataSource:0];
     }
 }
index 7a06059..fbe76b3 100644 (file)
@@ -1,3 +1,13 @@
+2016-10-04  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        The dragged image should be the current frame only of the animated image
+        https://bugs.webkit.org/show_bug.cgi?id=162109
+
+        Reviewed by Tim Horton.
+
+        * UIProcess/Cocoa/WebViewImpl.mm:
+        (WebKit::WebViewImpl::provideDataForPasteboard): Call the Image function with its new name.
+
 2016-10-04  Ryosuke Niwa  <rniwa@webkit.org>
 
         Revert a change erroneously committed in r206795.
index 8ae36be..175c892 100644 (file)
@@ -3038,7 +3038,7 @@ void WebViewImpl::provideDataForPasteboard(NSPasteboard *pasteboard, NSString *t
     // FIXME: need to support NSRTFDPboardType
 
     if ([type isEqual:NSTIFFPboardType] && m_promisedImage) {
-        [pasteboard setData:(NSData *)m_promisedImage->getTIFFRepresentation() forType:NSTIFFPboardType];
+        [pasteboard setData:(NSData *)m_promisedImage->tiffRepresentation() forType:NSTIFFPboardType];
         m_promisedImage = nullptr;
     }
 }