Avoid re-encoding action menu image data
authortimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 18 Nov 2014 22:54:52 +0000 (22:54 +0000)
committertimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 18 Nov 2014 22:54:52 +0000 (22:54 +0000)
https://bugs.webkit.org/show_bug.cgi?id=138817
<rdar://problem/18840382>

Reviewed by Anders Carlsson.

* Shared/mac/ActionMenuHitTestResult.h:
* Shared/mac/ActionMenuHitTestResult.mm:
(WebKit::ActionMenuHitTestResult::encode):
(WebKit::ActionMenuHitTestResult::decode):
Store and encode a SharedMemory with the raw encoded image data,
instead of re-painting the image into a ShareableBitmap.

* UIProcess/mac/WKActionMenuController.mm:
(-[WKActionMenuController _hitTestResultImage]):
(-[WKActionMenuController _defaultMenuItemsForImage]):
(-[WKActionMenuController _copyImage:]):
(-[WKActionMenuController _addImageToPhotos:]):
(-[WKActionMenuController _defaultMenuItems]):
(-[WKActionMenuController _canAddMediaToPhotos]): Deleted.
Build a temporary filename from a UUID and the image's desired extension.
Use the Image's encoded data instead of re-encoding it with CGImageDestination.
Build an image menu only if we have an image, URL, data, and extension.

* WebProcess/WebPage/mac/WebPageMac.mm:
(WebKit::WebPage::performActionMenuHitTestAtLocation):

* WebView/WebActionMenuController.mm:
(-[WebActionMenuController _defaultMenuItemsForImage:]):
(-[WebActionMenuController _addImageToPhotos:]):
Build a temporary filename from a UUID and the image's desired extension.
Use the Image's encoded data instead of re-encoding it with CGImageDestination.
Build an image menu only if we have an image, URL, data, and extension.

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

Source/WebKit/mac/ChangeLog
Source/WebKit/mac/WebView/WebActionMenuController.mm
Source/WebKit2/ChangeLog
Source/WebKit2/Shared/mac/ActionMenuHitTestResult.h
Source/WebKit2/Shared/mac/ActionMenuHitTestResult.mm
Source/WebKit2/UIProcess/mac/WKActionMenuController.mm
Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm

index ef785118d5c60314b19d150b15b1b738eb5d37aa..6ab673fc083c3ed9676f6302570cfb75958f8c0e 100644 (file)
@@ -1,3 +1,18 @@
+2014-11-18  Tim Horton  <timothy_horton@apple.com>
+
+        Avoid re-encoding action menu image data
+        https://bugs.webkit.org/show_bug.cgi?id=138817
+        <rdar://problem/18840382>
+
+        Reviewed by Anders Carlsson.
+
+        * WebView/WebActionMenuController.mm:
+        (-[WebActionMenuController _defaultMenuItemsForImage:]):
+        (-[WebActionMenuController _addImageToPhotos:]):
+        Build a temporary filename from a UUID and the image's desired extension.
+        Use the Image's encoded data instead of re-encoding it with CGImageDestination.
+        Build an image menu only if we have an image, URL, data, and extension.
+
 2014-11-18  Daniel Bates  <dabates@apple.com>
 
         [iOS] Libdispatch, Dyld, IOKit, Mach, NSPointerFunctions, MobileGestalt
index af47a40fa78955413c5d7ca528748d2b12b20b56..d1eafd73a4b1b98b4394dd4448200d15d559ceb4 100644 (file)
 #import "WebViewInternal.h"
 #import <ImageIO/ImageIO.h>
 #import <ImageKit/ImageKit.h>
+#import <WebCore/ArchiveResource.h>
 #import <WebCore/DataDetection.h>
 #import <WebCore/DataDetectorsSPI.h>
 #import <WebCore/DictionaryLookup.h>
+#import <WebCore/DocumentLoader.h>
 #import <WebCore/Editor.h>
 #import <WebCore/Element.h>
 #import <WebCore/EventHandler.h>
@@ -56,6 +58,7 @@
 #import <WebCore/Range.h>
 #import <WebCore/RenderElement.h>
 #import <WebCore/RenderObject.h>
+#import <WebCore/SharedBuffer.h>
 #import <WebCore/SoftLinking.h>
 #import <WebCore/TextCheckerClient.h>
 #import <WebKitSystemInterface.h>
@@ -328,11 +331,15 @@ static IntRect elementBoundingBoxInWindowCoordinatesFromNode(Node* node)
 
     RetainPtr<NSMenuItem> shareItem = [self _createActionMenuItemForTag:WebActionMenuItemTagShareImage];
     if (Image* image = _hitTestResult.image()) {
-        RetainPtr<CGImageRef> cgImage = image->getCGImageRef();
-        RetainPtr<NSImage> nsImage = adoptNS([[NSImage alloc] initWithCGImage:cgImage.get() size:NSZeroSize]);
-        _sharingServicePicker = adoptNS([[NSSharingServicePicker alloc] initWithItems:@[ nsImage.get() ]]);
-        [_sharingServicePicker setDelegate:self];
-        [shareItem setSubmenu:[_sharingServicePicker menu]];
+        RefPtr<SharedBuffer> buffer = image->data();
+        if (buffer) {
+            RetainPtr<NSData> nsData = [NSData dataWithBytes:buffer->data() length:buffer->size()];
+            RetainPtr<NSImage> nsImage = adoptNS([[NSImage alloc] initWithData:nsData.get()]);
+            _sharingServicePicker = adoptNS([[NSSharingServicePicker alloc] initWithItems:@[ nsImage.get() ]]);
+            [_sharingServicePicker setDelegate:self];
+            [shareItem setSubmenu:[_sharingServicePicker menu]];
+        } else
+            [shareItem setEnabled:NO];
     }
 
     return @[ copyImageItem.get(), addToPhotosItem.get(), saveToDownloadsItem.get(), shareItem.get() ];
@@ -404,19 +411,23 @@ static NSString *pathToPhotoOnDisk(NSString *suggestedFilename)
     if (!image)
         return;
 
-    RetainPtr<CGImageRef> cgImage = image->getCGImageRef();
+    String imageExtension = image->filenameExtension();
+    if (imageExtension.isEmpty())
+        return;
 
-    dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
-        NSString * const suggestedFilename = @"image.jpg";
+    RefPtr<SharedBuffer> buffer = image->data();
+    if (!buffer)
+        return;
+    RetainPtr<NSData> nsData = [NSData dataWithBytes:buffer->data() length:buffer->size()];
+    RetainPtr<NSString> suggestedFilename = [[[NSProcessInfo processInfo] globallyUniqueString] stringByAppendingPathExtension:imageExtension];
 
-        NSString *filePath = pathToPhotoOnDisk(suggestedFilename);
+    dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
+        NSString *filePath = pathToPhotoOnDisk(suggestedFilename.get());
         if (!filePath)
             return;
 
         NSURL *fileURL = [NSURL fileURLWithPath:filePath];
-        auto dest = adoptCF(CGImageDestinationCreateWithURL((CFURLRef)fileURL, kUTTypeJPEG, 1, nullptr));
-        CGImageDestinationAddImage(dest.get(), cgImage.get(), nullptr);
-        CGImageDestinationFinalize(dest.get());
+        [nsData writeToURL:fileURL atomically:NO];
 
         dispatch_async(dispatch_get_main_queue(), ^{
             // This API provides no way to report failure, but if 18420778 is fixed so that it does, we should handle this.
@@ -832,7 +843,8 @@ static DictionaryPopupInfo performDictionaryLookupForRange(Frame* frame, Range&
         return [self _defaultMenuItemsForVideo];
     }
 
-    if (_hitTestResult.image() && !_hitTestResult.absoluteImageURL().isEmpty()) {
+    Image* image = _hitTestResult.image();
+    if (image && !_hitTestResult.absoluteImageURL().isEmpty() && !image->filenameExtension().isEmpty() && image->data() && !image->data()->isEmpty()) {
         _type = WebActionMenuImage;
         return [self _defaultMenuItemsForImage];
     }
index 359d52b8733acd873f86d341beaea9048bc2355b..250fc361e850a24c196ec3f2a52ca70df51d83d5 100644 (file)
@@ -1,3 +1,32 @@
+2014-11-18  Tim Horton  <timothy_horton@apple.com>
+
+        Avoid re-encoding action menu image data
+        https://bugs.webkit.org/show_bug.cgi?id=138817
+        <rdar://problem/18840382>
+
+        Reviewed by Anders Carlsson.
+
+        * Shared/mac/ActionMenuHitTestResult.h:
+        * Shared/mac/ActionMenuHitTestResult.mm:
+        (WebKit::ActionMenuHitTestResult::encode):
+        (WebKit::ActionMenuHitTestResult::decode):
+        Store and encode a SharedMemory with the raw encoded image data,
+        instead of re-painting the image into a ShareableBitmap.
+
+        * UIProcess/mac/WKActionMenuController.mm:
+        (-[WKActionMenuController _hitTestResultImage]):
+        (-[WKActionMenuController _defaultMenuItemsForImage]):
+        (-[WKActionMenuController _copyImage:]):
+        (-[WKActionMenuController _addImageToPhotos:]):
+        (-[WKActionMenuController _defaultMenuItems]):
+        (-[WKActionMenuController _canAddMediaToPhotos]): Deleted.
+        Build a temporary filename from a UUID and the image's desired extension.
+        Use the Image's encoded data instead of re-encoding it with CGImageDestination.
+        Build an image menu only if we have an image, URL, data, and extension.
+
+        * WebProcess/WebPage/mac/WebPageMac.mm:
+        (WebKit::WebPage::performActionMenuHitTestAtLocation):
+
 2014-11-18  Eric Carlson  <eric.carlson@apple.com>
 
         [iOS] allow host application to opt-out of alternate fullscreen
index 9d1ca90c53f55e21a58ce9cd722e73ca7083bc04..e7be170442f839ee22ca1277f91b3a238543ef49 100644 (file)
@@ -26,7 +26,9 @@
 #ifndef ActionMenuHitTestResult_h
 #define ActionMenuHitTestResult_h
 
+#include "DataReference.h"
 #include "ShareableBitmap.h"
+#include "SharedMemory.h"
 #include "TextIndicator.h"
 #include "WebHitTestResult.h"
 #include <WebCore/FloatRect.h>
@@ -50,7 +52,8 @@ struct ActionMenuHitTestResult {
     WebHitTestResult::Data hitTestResult;
 
     String lookupText;
-    RefPtr<ShareableBitmap> image;
+    RefPtr<SharedMemory> imageSharedMemory;
+    String imageExtension;
 
     RetainPtr<DDActionContext> actionContext;
     WebCore::FloatRect detectedDataBoundingBox;
index 1dfbc32b497f2077d578eed628bcee4fc73d5faf..367c7aefc5d77ddb7a43a0455238a5ce437ca5f6 100644 (file)
@@ -42,14 +42,12 @@ void ActionMenuHitTestResult::encode(IPC::ArgumentEncoder& encoder) const
     encoder << hitTestLocationInViewCooordinates;
     encoder << hitTestResult;
     encoder << lookupText;
+    encoder << imageExtension;
 
-    ShareableBitmap::Handle handle;
-
-    // FIXME: We should consider sharing the raw original resource data so that metadata and whatnot are preserved.
-    if (image)
-        image->createHandle(handle, SharedMemory::ReadOnly);
-
-    encoder << handle;
+    SharedMemory::Handle imageHandle;
+    if (imageSharedMemory && imageSharedMemory->size())
+        imageSharedMemory->createHandle(imageHandle, SharedMemory::ReadOnly);
+    encoder << imageHandle;
 
     bool hasActionContext = actionContext;
     encoder << hasActionContext;
@@ -82,12 +80,15 @@ bool ActionMenuHitTestResult::decode(IPC::ArgumentDecoder& decoder, ActionMenuHi
     if (!decoder.decode(actionMenuHitTestResult.lookupText))
         return false;
 
-    ShareableBitmap::Handle handle;
-    if (!decoder.decode(handle))
+    if (!decoder.decode(actionMenuHitTestResult.imageExtension))
+        return false;
+
+    SharedMemory::Handle imageHandle;
+    if (!decoder.decode(imageHandle))
         return false;
 
-    if (!handle.isNull())
-        actionMenuHitTestResult.image = ShareableBitmap::create(handle, SharedMemory::ReadOnly);
+    if (!imageHandle.isNull())
+        actionMenuHitTestResult.imageSharedMemory = SharedMemory::create(imageHandle, SharedMemory::ReadOnly);
 
     bool hasActionContext;
     if (!decoder.decode(hasActionContext))
index 682158ae8e00f22eba7d46e295bd54031a6edc18..0064bef82eb4d0a3693ff1c8062d9bbeb84a0c4b 100644 (file)
@@ -458,6 +458,16 @@ static bool targetSizeFitsInAvailableSpace(NSSize targetSize, NSSize availableSp
 
 #pragma mark Image actions
 
+- (NSImage *)_hitTestResultImage
+{
+    RefPtr<SharedMemory> imageSharedMemory = _hitTestResult.imageSharedMemory;
+    if (!imageSharedMemory)
+        return nil;
+
+    RetainPtr<NSImage> nsImage = adoptNS([[NSImage alloc] initWithData:[NSData dataWithBytes:imageSharedMemory->data() length:imageSharedMemory->size()]]);
+    return nsImage.autorelease();
+}
+
 - (NSArray *)_defaultMenuItemsForImage
 {
     RetainPtr<NSMenuItem> copyImageItem = [self _createActionMenuItemForTag:kWKContextActionItemTagCopyImage];
@@ -469,10 +479,8 @@ static bool targetSizeFitsInAvailableSpace(NSSize targetSize, NSSize availableSp
     RetainPtr<NSMenuItem> saveToDownloadsItem = [self _createActionMenuItemForTag:kWKContextActionItemTagSaveImageToDownloads];
     RetainPtr<NSMenuItem> shareItem = [self _createActionMenuItemForTag:kWKContextActionItemTagShareImage];
 
-    if (RefPtr<ShareableBitmap> bitmap = _hitTestResult.image) {
-        RetainPtr<CGImageRef> image = bitmap->makeCGImage();
-        RetainPtr<NSImage> nsImage = adoptNS([[NSImage alloc] initWithCGImage:image.get() size:NSZeroSize]);
-        _sharingServicePicker = adoptNS([[NSSharingServicePicker alloc] initWithItems:@[ nsImage.get() ]]);
+    if (RetainPtr<NSImage> image = [self _hitTestResultImage]) {
+        _sharingServicePicker = adoptNS([[NSSharingServicePicker alloc] initWithItems:@[ image.get() ]]);
         [_sharingServicePicker setDelegate:self];
         [shareItem setSubmenu:[_sharingServicePicker menu]];
     }
@@ -482,14 +490,12 @@ static bool targetSizeFitsInAvailableSpace(NSSize targetSize, NSSize availableSp
 
 - (void)_copyImage:(id)sender
 {
-    RefPtr<ShareableBitmap> bitmap = _hitTestResult.image;
-    if (!bitmap)
+    RetainPtr<NSImage> image = [self _hitTestResultImage];
+    if (!image)
         return;
 
-    RetainPtr<CGImageRef> image = bitmap->makeCGImage();
-    RetainPtr<NSImage> nsImage = adoptNS([[NSImage alloc] initWithCGImage:image.get() size:NSZeroSize]);
     [[NSPasteboard generalPasteboard] clearContents];
-    [[NSPasteboard generalPasteboard] writeObjects:@[ nsImage.get() ]];
+    [[NSPasteboard generalPasteboard] writeObjects:@[ image.get() ]];
 }
 
 - (void)_saveImageToDownloads:(id)sender
@@ -553,22 +559,20 @@ static NSString *pathToPhotoOnDisk(NSString *suggestedFilename)
     if (![self _canAddMediaToPhotos])
         return;
 
-    RefPtr<ShareableBitmap> bitmap = _hitTestResult.image;
-    if (!bitmap)
+    RefPtr<SharedMemory> imageSharedMemory = _hitTestResult.imageSharedMemory;
+    if (!imageSharedMemory->size() || _hitTestResult.imageExtension.isEmpty())
         return;
-    RetainPtr<CGImageRef> image = bitmap->makeCGImage();
 
-    dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
-        NSString * const suggestedFilename = @"image.jpg";
+    RetainPtr<NSData> imageData = adoptNS([[NSData alloc] initWithBytes:imageSharedMemory->data() length:imageSharedMemory->size()]);
+    RetainPtr<NSString> suggestedFilename = [[[NSProcessInfo processInfo] globallyUniqueString] stringByAppendingPathExtension:_hitTestResult.imageExtension];
 
-        NSString *filePath = pathToPhotoOnDisk(suggestedFilename);
+    dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
+        NSString *filePath = pathToPhotoOnDisk(suggestedFilename.get());
         if (!filePath)
             return;
 
         NSURL *fileURL = [NSURL fileURLWithPath:filePath];
-        auto dest = adoptCF(CGImageDestinationCreateWithURL((CFURLRef)fileURL, kUTTypeJPEG, 1, nullptr));
-        CGImageDestinationAddImage(dest.get(), image.get(), nullptr);
-        CGImageDestinationFinalize(dest.get());
+        [imageData writeToURL:fileURL atomically:NO];
 
         dispatch_async(dispatch_get_main_queue(), ^{
             // This API provides no way to report failure, but if 18420778 is fixed so that it does, we should handle this.
@@ -909,7 +913,7 @@ static NSString *pathToPhotoOnDisk(NSString *suggestedFilename)
         return [self _defaultMenuItemsForVideo];
     }
 
-    if (!hitTestResult->absoluteImageURL().isEmpty() && _hitTestResult.image) {
+    if (!hitTestResult->absoluteImageURL().isEmpty() && _hitTestResult.imageSharedMemory && !_hitTestResult.imageExtension.isEmpty()) {
         _type = kWKActionMenuImage;
         return [self _defaultMenuItemsForImage];
     }
index 59785c3f4a3ac23ccc5f601ec6405231e72ef041..08c2bfa60c784fb4b7f90f7409c716832d5059bf 100644 (file)
@@ -1009,9 +1009,13 @@ void WebPage::performActionMenuHitTestAtLocation(WebCore::FloatPoint locationInV
     m_lastActionMenuHitTestResult = hitTestResult;
 
     if (Image* image = hitTestResult.image()) {
-        actionMenuResult.image = ShareableBitmap::createShareable(IntSize(image->size()), ShareableBitmap::SupportsAlpha);
-        if (actionMenuResult.image)
-            actionMenuResult.image->createGraphicsContext()->drawImage(image, ColorSpaceDeviceRGB, IntPoint());
+        RefPtr<SharedBuffer> buffer = image->data();
+        String imageExtension = image->filenameExtension();
+        if (!imageExtension.isEmpty() && buffer) {
+            actionMenuResult.imageSharedMemory = SharedMemory::create(buffer->size());
+            memcpy(actionMenuResult.imageSharedMemory->data(), buffer->data(), buffer->size());
+            actionMenuResult.imageExtension = imageExtension;
+        }
     }
 
     bool pageOverlayDidOverrideDataDetectors = false;