[iOS DnD] Web process uses too much memory when beginning a drag on a very large...
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 17 Jul 2017 23:17:09 +0000 (23:17 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 17 Jul 2017 23:17:09 +0000 (23:17 +0000)
https://bugs.webkit.org/show_bug.cgi?id=174585
<rdar://problem/33302541>

Reviewed by Tim Horton.

Source/WebCore:

Currently, attempting to drag a very large image fails, either due to us telling CoreGraphics to create an image
buffer that is too large, or because the web process exceeds its memory limit and gets jetsamed. There are two
places where we can optimize our memory use during the drag initialization sequence, and this patch improves
both.

First, on iOS, we attempt to encode and send over a WebCore::Image in the PasteboardImage when writing to the
item providers upon starting a drag. Currently, this Image is only used in the drag and drop codepath, in
PlatformPasteboard::writeObjectRepresentations, to grab the size of the image being written for the purpose of
specifying estimated display size. Serializing and deserializing an Image calls into Image::nativeImage, which
attempts to draw the contents of the image into a buffer so that it can be shipped across to the UI process.
Instead, we can simply compute the size in the web process while we already have the Image, and simply send that
across. For copy/paste, this doesn't result in any behavior change, since we don't use the PasteboardImage's
image in the first place.

Secondly, when starting a drag, we try to allocate create an image buffer the size of the WebCore::Image for the
purpose of generating the drag preview. Instead, this patch establishes a limit on the size of this drag preview
image, such that if the Image's size is larger, we'll scale down the drag preview image to be the maximum
allowed size.

Test: DataInteractionTests.CanStartDragOnEnormousImage.

* editing/ios/EditorIOS.mm:
(WebCore::Editor::writeImageToPasteboard):
* platform/Pasteboard.h:
* platform/graphics/GeometryUtilities.cpp:
(WebCore::sizeWithAreaAndAspectRatio):

Introduce a new helper function to compute a size with the given aspect ratio and area.

* platform/graphics/GeometryUtilities.h:
* platform/ios/DragImageIOS.mm:
(WebCore::createDragImageFromImage):
* platform/ios/PlatformPasteboardIOS.mm:
(WebCore::PlatformPasteboard::writeObjectRepresentations):

Source/WebKit:

Add IPC support for serializing/deserializing the size of an image written to the pasteboard. See WebCore
ChangeLogs for more details.

* Shared/WebCoreArgumentCoders.cpp:
(IPC::ArgumentCoder<PasteboardImage>::encode):
(IPC::ArgumentCoder<PasteboardImage>::decode):

Tools:

Adds a new test verifying that we don't try to allocate any image buffer equal to the true size of the image
being dragged when initiating a drag.

* TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* TestWebKitAPI/Tests/WebKit2Cocoa/enormous.svg: Added.
* TestWebKitAPI/Tests/ios/DataInteractionTests.mm:
(TestWebKitAPI::TEST):
* TestWebKitAPI/cocoa/TestWKWebView.h:

Add a new -synchronouslyLoadHTMLString: helper that works like -synchronouslyLoadTestPage:, but takes markup.

* TestWebKitAPI/cocoa/TestWKWebView.mm:
(-[TestWKWebView synchronouslyLoadHTMLString:]):

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

15 files changed:
Source/WebCore/ChangeLog
Source/WebCore/editing/ios/EditorIOS.mm
Source/WebCore/platform/Pasteboard.h
Source/WebCore/platform/graphics/GeometryUtilities.cpp
Source/WebCore/platform/graphics/GeometryUtilities.h
Source/WebCore/platform/ios/DragImageIOS.mm
Source/WebCore/platform/ios/PlatformPasteboardIOS.mm
Source/WebKit/ChangeLog
Source/WebKit/Shared/WebCoreArgumentCoders.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
Tools/TestWebKitAPI/Tests/WebKit2Cocoa/enormous.svg [new file with mode: 0644]
Tools/TestWebKitAPI/Tests/ios/DataInteractionTests.mm
Tools/TestWebKitAPI/cocoa/TestWKWebView.h
Tools/TestWebKitAPI/cocoa/TestWKWebView.mm

index 0af2654..9837bf6 100644 (file)
@@ -1,3 +1,46 @@
+2017-07-17  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [iOS DnD] Web process uses too much memory when beginning a drag on a very large image
+        https://bugs.webkit.org/show_bug.cgi?id=174585
+        <rdar://problem/33302541>
+
+        Reviewed by Tim Horton.
+
+        Currently, attempting to drag a very large image fails, either due to us telling CoreGraphics to create an image
+        buffer that is too large, or because the web process exceeds its memory limit and gets jetsamed. There are two
+        places where we can optimize our memory use during the drag initialization sequence, and this patch improves
+        both.
+
+        First, on iOS, we attempt to encode and send over a WebCore::Image in the PasteboardImage when writing to the
+        item providers upon starting a drag. Currently, this Image is only used in the drag and drop codepath, in
+        PlatformPasteboard::writeObjectRepresentations, to grab the size of the image being written for the purpose of
+        specifying estimated display size. Serializing and deserializing an Image calls into Image::nativeImage, which
+        attempts to draw the contents of the image into a buffer so that it can be shipped across to the UI process.
+        Instead, we can simply compute the size in the web process while we already have the Image, and simply send that
+        across. For copy/paste, this doesn't result in any behavior change, since we don't use the PasteboardImage's
+        image in the first place.
+
+        Secondly, when starting a drag, we try to allocate create an image buffer the size of the WebCore::Image for the
+        purpose of generating the drag preview. Instead, this patch establishes a limit on the size of this drag preview
+        image, such that if the Image's size is larger, we'll scale down the drag preview image to be the maximum
+        allowed size.
+
+        Test: DataInteractionTests.CanStartDragOnEnormousImage.
+
+        * editing/ios/EditorIOS.mm:
+        (WebCore::Editor::writeImageToPasteboard):
+        * platform/Pasteboard.h:
+        * platform/graphics/GeometryUtilities.cpp:
+        (WebCore::sizeWithAreaAndAspectRatio):
+
+        Introduce a new helper function to compute a size with the given aspect ratio and area.
+
+        * platform/graphics/GeometryUtilities.h:
+        * platform/ios/DragImageIOS.mm:
+        (WebCore::createDragImageFromImage):
+        * platform/ios/PlatformPasteboardIOS.mm:
+        (WebCore::PlatformPasteboard::writeObjectRepresentations):
+
 2017-07-17  Michael Catanzaro  <mcatanzaro@igalia.com>
 
         [CMake] Include most CMake modules from WebKitCommon.cmake
index c261741..c57eff2 100644 (file)
@@ -199,9 +199,10 @@ void Editor::writeImageToPasteboard(Pasteboard& pasteboard, Element& imageElemen
 {
     PasteboardImage pasteboardImage;
 
+    RefPtr<Image> image;
     CachedImage* cachedImage;
-    getImage(imageElement, pasteboardImage.image, cachedImage);
-    if (!pasteboardImage.image)
+    getImage(imageElement, image, cachedImage);
+    if (!image)
         return;
     ASSERT(cachedImage);
 
@@ -209,6 +210,7 @@ void Editor::writeImageToPasteboard(Pasteboard& pasteboard, Element& imageElemen
     pasteboardImage.url.url = url.isEmpty() ? imageSourceURL : url;
     pasteboardImage.url.title = title;
     pasteboardImage.suggestedName = imageSourceURL.lastPathComponent();
+    pasteboardImage.imageSize = image->size();
     pasteboardImage.resourceMIMEType = pasteboard.resourceMIMEType(cachedImage->response().mimeType());
     pasteboardImage.resourceData = cachedImage->resourceBuffer();
 
index 15ab556..01c7e0f 100644 (file)
@@ -117,6 +117,7 @@ struct PasteboardImage {
     Vector<RefPtr<SharedBuffer>> clientData;
 #endif
     String suggestedName;
+    FloatSize imageSize;
 };
 
 // For reading from the pasteboard.
index 5f296ac..8a72165 100644 (file)
@@ -146,6 +146,12 @@ FloatRect smallestRectWithAspectRatioAroundRect(float aspectRatio, const FloatRe
     return destRect;
 }
 
+FloatSize sizeWithAreaAndAspectRatio(float area, float aspectRatio)
+{
+    auto scaledWidth = std::sqrt(area * aspectRatio);
+    return { scaledWidth, scaledWidth / aspectRatio };
+}
+
 bool ellipseContainsPoint(const FloatPoint& center, const FloatSize& radii, const FloatPoint& point)
 {
     FloatPoint transformedPoint(point);
index 86ca197..ef2e9ae 100644 (file)
@@ -47,6 +47,8 @@ FloatRect mapRect(const FloatRect&, const FloatRect& srcRect, const FloatRect& d
 WEBCORE_EXPORT FloatRect largestRectWithAspectRatioInsideRect(float aspectRatio, const FloatRect&);
 WEBCORE_EXPORT FloatRect smallestRectWithAspectRatioAroundRect(float aspectRatio, const FloatRect&);
 
+FloatSize sizeWithAreaAndAspectRatio(float area, float aspectRatio);
+
 // Compute a rect that encloses all points covered by the given rect if it were rotated a full turn around (0,0).
 FloatRect boundsOfRotatingRect(const FloatRect&);
 
index c388f5f..4fd4aa0 100644 (file)
@@ -34,6 +34,7 @@
 #import "FontCascade.h"
 #import "FontPlatformData.h"
 #import "Frame.h"
+#import "GeometryUtilities.h"
 #import "GraphicsContext.h"
 #import "Image.h"
 #import "NotImplemented.h"
@@ -88,18 +89,26 @@ DragImageRef scaleDragImage(DragImageRef image, FloatSize scale)
     return imageCopy.CGImage;
 }
 
-DragImageRef createDragImageFromImage(Image * _Nullable image, ImageOrientationDescription orientation)
+static float maximumAllowedDragImageArea = 600 * 1024;
+
+DragImageRef createDragImageFromImage(Image* image, ImageOrientationDescription orientation)
 {
-    if (!image)
+    if (!image || !image->width() || !image->height())
         return nil;
 
+    float adjustedImageScale = 1;
     CGSize imageSize(image->size());
+    if (imageSize.width * imageSize.height > maximumAllowedDragImageArea) {
+        auto adjustedSize = roundedIntSize(sizeWithAreaAndAspectRatio(maximumAllowedDragImageArea, imageSize.width / imageSize.height));
+        adjustedImageScale = adjustedSize.width() / imageSize.width;
+        imageSize = adjustedSize;
+    }
 
     RetainPtr<UIGraphicsImageRenderer> render = adoptNS([allocUIGraphicsImageRendererInstance() initWithSize:imageSize]);
     UIImage *imageCopy = [render.get() imageWithActions:^(UIGraphicsImageRendererContext *rendererContext) {
         GraphicsContext context(rendererContext.CGContext);
         context.translate(0, imageSize.height);
-        context.scale({ 1, -1 });
+        context.scale({ adjustedImageScale, -adjustedImageScale });
         ImagePaintingOptions paintingOptions;
         paintingOptions.m_orientationDescription = orientation;
         context.drawImage(*image, FloatPoint(), paintingOptions);
index 3456591..677db16 100644 (file)
@@ -290,16 +290,14 @@ void PlatformPasteboard::writeObjectRepresentations(const PasteboardImage& paste
     for (size_t i = 0, size = types.size(); i < size; ++i)
         [itemsToRegister addData:data[i]->createNSData().get() forType:types[i]];
 
-    if (auto image = pasteboardImage.image) {
-        NSString *mimeType = pasteboardImage.resourceMIMEType;
-        if (UTTypeIsDeclared((CFStringRef)mimeType)) {
-            auto imageData = pasteboardImage.resourceData->createNSData();
-            [itemsToRegister addData:imageData.get() forType:mimeType];
-        } else if (auto nativeImage = image->nativeImage()) {
-            if (auto uiImage = adoptNS([allocUIImageInstance() initWithCGImage:nativeImage.get()]))
-                [itemsToRegister addRepresentingObject:uiImage.get()];
-        }
-        [itemsToRegister setEstimatedDisplayedSize:image->size()];
+    if (pasteboardImage.resourceData && !pasteboardImage.resourceMIMEType.isEmpty()) {
+        auto utiOrMIMEType = pasteboardImage.resourceMIMEType.createCFString();
+        if (!UTTypeIsDeclared(utiOrMIMEType.get()))
+            utiOrMIMEType = UTTypeCreatePreferredIdentifierForTag(kUTTagClassMIMEType, utiOrMIMEType.get(), nil);
+
+        auto imageData = pasteboardImage.resourceData->createNSData();
+        [itemsToRegister addData:imageData.get() forType:(NSString *)utiOrMIMEType.get()];
+        [itemsToRegister setEstimatedDisplayedSize:pasteboardImage.imageSize];
         [itemsToRegister setSuggestedName:pasteboardImage.suggestedName];
     }
 
index 09bffdb..857081d 100644 (file)
@@ -1,3 +1,18 @@
+2017-07-17  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [iOS DnD] Web process uses too much memory when beginning a drag on a very large image
+        https://bugs.webkit.org/show_bug.cgi?id=174585
+        <rdar://problem/33302541>
+
+        Reviewed by Tim Horton.
+
+        Add IPC support for serializing/deserializing the size of an image written to the pasteboard. See WebCore
+        ChangeLogs for more details.
+
+        * Shared/WebCoreArgumentCoders.cpp:
+        (IPC::ArgumentCoder<PasteboardImage>::encode):
+        (IPC::ArgumentCoder<PasteboardImage>::decode):
+
 2017-07-17  Konstantin Tokarev  <annulen@yandex.ru>
 
         Unreviewed attempt to fix Mac cmake build
index 295ab08..130d63d 100644 (file)
@@ -1491,6 +1491,7 @@ void ArgumentCoder<PasteboardImage>::encode(Encoder& encoder, const PasteboardIm
     encoder << pasteboardImage.url.title;
     encoder << pasteboardImage.resourceMIMEType;
     encoder << pasteboardImage.suggestedName;
+    encoder << pasteboardImage.imageSize;
     if (pasteboardImage.resourceData)
         encodeSharedBuffer(encoder, pasteboardImage.resourceData.get());
     encodeClientTypesAndData(encoder, pasteboardImage.clientTypes, pasteboardImage.clientData);
@@ -1508,6 +1509,8 @@ bool ArgumentCoder<PasteboardImage>::decode(Decoder& decoder, PasteboardImage& p
         return false;
     if (!decoder.decode(pasteboardImage.suggestedName))
         return false;
+    if (!decoder.decode(pasteboardImage.imageSize))
+        return false;
     if (!decodeSharedBuffer(decoder, pasteboardImage.resourceData))
         return false;
     if (!decodeClientTypesAndData(decoder, pasteboardImage.clientTypes, pasteboardImage.clientData))
index ab1bae6..ba778ac 100644 (file)
@@ -1,3 +1,25 @@
+2017-07-17  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [iOS DnD] Web process uses too much memory when beginning a drag on a very large image
+        https://bugs.webkit.org/show_bug.cgi?id=174585
+        <rdar://problem/33302541>
+
+        Reviewed by Tim Horton.
+
+        Adds a new test verifying that we don't try to allocate any image buffer equal to the true size of the image
+        being dragged when initiating a drag.
+
+        * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+        * TestWebKitAPI/Tests/WebKit2Cocoa/enormous.svg: Added.
+        * TestWebKitAPI/Tests/ios/DataInteractionTests.mm:
+        (TestWebKitAPI::TEST):
+        * TestWebKitAPI/cocoa/TestWKWebView.h:
+
+        Add a new -synchronouslyLoadHTMLString: helper that works like -synchronouslyLoadTestPage:, but takes markup.
+
+        * TestWebKitAPI/cocoa/TestWKWebView.mm:
+        (-[TestWKWebView synchronouslyLoadHTMLString:]):
+
 2017-07-17  Michael Catanzaro  <mcatanzaro@igalia.com>
 
         [CMake] Macros in WebKitMacros.cmake should be prefixed with WEBKIT_ namespace
index 2edc18c..98c9abf 100644 (file)
                E1220DCA155B28AA0013E2FC /* MemoryCacheDisableWithinResourceLoadDelegate.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = E1220DC9155B287D0013E2FC /* MemoryCacheDisableWithinResourceLoadDelegate.html */; };
                E194E1BD177E53C7009C4D4E /* StopLoadingFromDidReceiveResponse.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = E194E1BC177E534A009C4D4E /* StopLoadingFromDidReceiveResponse.html */; };
                ECA680CE1E68CC0900731D20 /* StringUtilities.mm in Sources */ = {isa = PBXBuildFile; fileRef = ECA680CD1E68CC0900731D20 /* StringUtilities.mm */; };
+               F407FE391F1D0DFC0017CF25 /* enormous.svg in Copy Resources */ = {isa = PBXBuildFile; fileRef = F407FE381F1D0DE60017CF25 /* enormous.svg */; };
                F415086D1DA040C50044BE9B /* play-audio-on-click.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F415086C1DA040C10044BE9B /* play-audio-on-click.html */; };
                F41AB99F1EF4696B0083FA08 /* autofocus-contenteditable.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F41AB9981EF4692C0083FA08 /* autofocus-contenteditable.html */; };
                F41AB9A01EF4696B0083FA08 /* background-image-link-and-input.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F41AB9971EF4692C0083FA08 /* background-image-link-and-input.html */; };
                        dstSubfolderSpec = 7;
                        files = (
                                F45B63FB1F197F4A009D38B9 /* image-map.html in Copy Resources */,
+                               F407FE391F1D0DFC0017CF25 /* enormous.svg in Copy Resources */,
                                F4D5E4E81F0C5D38008C1A49 /* dragstart-clear-selection.html in Copy Resources */,
                                F4A32EC41F05F3850047C544 /* dragstart-change-selection-offscreen.html in Copy Resources */,
                                F4A32ECB1F0643370047C544 /* contenteditable-in-iframe.html in Copy Resources */,
                E4C9ABC71B3DB1710040A987 /* RunLoop.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = RunLoop.cpp; sourceTree = "<group>"; };
                ECA680CD1E68CC0900731D20 /* StringUtilities.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = StringUtilities.mm; sourceTree = "<group>"; };
                F3FC3EE213678B7300126A65 /* libgtest.a */ = {isa = PBXFileReference; lastKnownFileType = archive.ar; path = libgtest.a; sourceTree = BUILT_PRODUCTS_DIR; };
+               F407FE381F1D0DE60017CF25 /* enormous.svg */ = {isa = PBXFileReference; lastKnownFileType = text; path = enormous.svg; sourceTree = "<group>"; };
                F415086C1DA040C10044BE9B /* play-audio-on-click.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "play-audio-on-click.html"; sourceTree = "<group>"; };
                F41AB9931EF4692C0083FA08 /* image-and-textarea.html */ = {isa = PBXFileReference; lastKnownFileType = text.html; path = "image-and-textarea.html"; sourceTree = "<group>"; };
                F41AB9941EF4692C0083FA08 /* prevent-operation.html */ = {isa = PBXFileReference; lastKnownFileType = text.html; path = "prevent-operation.html"; sourceTree = "<group>"; };
                                5714ECBA1CA8BFD100051AC8 /* DownloadRequestOriginalURLFrame.html */,
                                A155022B1E050BC500A24C57 /* duplicate-completion-handler-calls.html */,
                                9984FACD1CFFB038008D198C /* editable-body.html */,
+                               F407FE381F1D0DE60017CF25 /* enormous.svg */,
                                F4C2AB211DD6D94100E06D5B /* enormous-video-with-sound.html */,
                                93575C551D30366E000D604D /* focus-inputs.html */,
                                F4F405BA1D4C0CF8007A9707 /* full-size-autoplaying-video-with-audio.html */,
diff --git a/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/enormous.svg b/Tools/TestWebKitAPI/Tests/WebKit2Cocoa/enormous.svg
new file mode 100644 (file)
index 0000000..4725531
--- /dev/null
@@ -0,0 +1,3 @@
+<svg width="3276800" height="3276800" version="1.1" xmlns="http://www.w3.org/2000/svg">
+  <rect x="0" y="0" width="3276800" height="3276800" stroke="black" fill="red" stroke-width="10"/>
+</svg>
index b772baa..0684be6 100644 (file)
@@ -170,6 +170,18 @@ TEST(DataInteractionTests, ImageToContentEditable)
     checkSuggestedNameAndEstimatedSize(dataInteractionSimulator.get(), @"icon.png", { 215, 174 });
 }
 
+TEST(DataInteractionTests, CanStartDragOnEnormousImage)
+{
+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);
+    [webView synchronouslyLoadHTML:@"<img src='enormous.svg'></img>"];
+
+    auto dataInteractionSimulator = adoptNS([[DataInteractionSimulator alloc] initWithWebView:webView.get()]);
+    [dataInteractionSimulator runFrom:CGPointMake(100, 100) to:CGPointMake(100, 100)];
+
+    NSArray *registeredTypes = [[dataInteractionSimulator sourceItemProviders].firstObject registeredTypeIdentifiers];
+    EXPECT_WK_STREQ((NSString *)kUTTypeScalableVectorGraphics, [registeredTypes firstObject]);
+}
+
 TEST(DataInteractionTests, ImageToTextarea)
 {
     RetainPtr<TestWKWebView> webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);
index 0c3a31e..edb1fd5 100644 (file)
@@ -39,6 +39,7 @@
 - (void)clearMessageHandlers:(NSArray *)messageNames;
 - (void)performAfterReceivingMessage:(NSString *)message action:(dispatch_block_t)action;
 - (void)loadTestPageNamed:(NSString *)pageName;
+- (void)synchronouslyLoadHTMLString:(NSString *)html;
 - (void)synchronouslyLoadTestPageNamed:(NSString *)pageName;
 - (NSString *)stringByEvaluatingJavaScript:(NSString *)script;
 - (void)waitForMessage:(NSString *)message;
index 254729d..1f0f07e 100644 (file)
@@ -223,6 +223,13 @@ NSEventMask __simulated_forceClickAssociatedEventsMask(id self, SEL _cmd)
     [self loadRequest:request];
 }
 
+- (void)synchronouslyLoadHTMLString:(NSString *)html
+{
+    NSURL *testResourceURL = [[[NSBundle mainBundle] bundleURL] URLByAppendingPathComponent:@"TestWebKitAPI.resources"];
+    [self loadHTMLString:html baseURL:testResourceURL];
+    [self _test_waitForDidFinishNavigation];
+}
+
 - (void)synchronouslyLoadTestPageNamed:(NSString *)pageName
 {
     [self loadTestPageNamed:pageName];