Skip to content

Commit 172c431

Browse files
[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. 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
1 parent b309e16 commit 172c431

File tree

15 files changed

+143
-15
lines changed

15 files changed

+143
-15
lines changed

Source/WebCore/ChangeLog

+43
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,46 @@
1+
2017-07-17 Wenson Hsieh <[email protected]>
2+
3+
[iOS DnD] Web process uses too much memory when beginning a drag on a very large image
4+
https://bugs.webkit.org/show_bug.cgi?id=174585
5+
<rdar://problem/33302541>
6+
7+
Reviewed by Tim Horton.
8+
9+
Currently, attempting to drag a very large image fails, either due to us telling CoreGraphics to create an image
10+
buffer that is too large, or because the web process exceeds its memory limit and gets jetsamed. There are two
11+
places where we can optimize our memory use during the drag initialization sequence, and this patch improves
12+
both.
13+
14+
First, on iOS, we attempt to encode and send over a WebCore::Image in the PasteboardImage when writing to the
15+
item providers upon starting a drag. Currently, this Image is only used in the drag and drop codepath, in
16+
PlatformPasteboard::writeObjectRepresentations, to grab the size of the image being written for the purpose of
17+
specifying estimated display size. Serializing and deserializing an Image calls into Image::nativeImage, which
18+
attempts to draw the contents of the image into a buffer so that it can be shipped across to the UI process.
19+
Instead, we can simply compute the size in the web process while we already have the Image, and simply send that
20+
across. For copy/paste, this doesn't result in any behavior change, since we don't use the PasteboardImage's
21+
image in the first place.
22+
23+
Secondly, when starting a drag, we try to allocate create an image buffer the size of the WebCore::Image for the
24+
purpose of generating the drag preview. Instead, this patch establishes a limit on the size of this drag preview
25+
image, such that if the Image's size is larger, we'll scale down the drag preview image to be the maximum
26+
allowed size.
27+
28+
Test: DataInteractionTests.CanStartDragOnEnormousImage.
29+
30+
* editing/ios/EditorIOS.mm:
31+
(WebCore::Editor::writeImageToPasteboard):
32+
* platform/Pasteboard.h:
33+
* platform/graphics/GeometryUtilities.cpp:
34+
(WebCore::sizeWithAreaAndAspectRatio):
35+
36+
Introduce a new helper function to compute a size with the given aspect ratio and area.
37+
38+
* platform/graphics/GeometryUtilities.h:
39+
* platform/ios/DragImageIOS.mm:
40+
(WebCore::createDragImageFromImage):
41+
* platform/ios/PlatformPasteboardIOS.mm:
42+
(WebCore::PlatformPasteboard::writeObjectRepresentations):
43+
144
2017-07-17 Michael Catanzaro <[email protected]>
245

346
[CMake] Include most CMake modules from WebKitCommon.cmake

Source/WebCore/editing/ios/EditorIOS.mm

+4-2
Original file line numberDiff line numberDiff line change
@@ -199,16 +199,18 @@ static void getImage(Element& imageElement, RefPtr<Image>& image, CachedImage*&
199199
{
200200
PasteboardImage pasteboardImage;
201201

202+
RefPtr<Image> image;
202203
CachedImage* cachedImage;
203-
getImage(imageElement, pasteboardImage.image, cachedImage);
204-
if (!pasteboardImage.image)
204+
getImage(imageElement, image, cachedImage);
205+
if (!image)
205206
return;
206207
ASSERT(cachedImage);
207208

208209
auto imageSourceURL = imageElement.document().completeURL(stripLeadingAndTrailingHTMLSpaces(imageElement.imageSourceURL()));
209210
pasteboardImage.url.url = url.isEmpty() ? imageSourceURL : url;
210211
pasteboardImage.url.title = title;
211212
pasteboardImage.suggestedName = imageSourceURL.lastPathComponent();
213+
pasteboardImage.imageSize = image->size();
212214
pasteboardImage.resourceMIMEType = pasteboard.resourceMIMEType(cachedImage->response().mimeType());
213215
pasteboardImage.resourceData = cachedImage->resourceBuffer();
214216

Source/WebCore/platform/Pasteboard.h

+1
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ struct PasteboardImage {
117117
Vector<RefPtr<SharedBuffer>> clientData;
118118
#endif
119119
String suggestedName;
120+
FloatSize imageSize;
120121
};
121122

122123
// For reading from the pasteboard.

Source/WebCore/platform/graphics/GeometryUtilities.cpp

+6
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,12 @@ FloatRect smallestRectWithAspectRatioAroundRect(float aspectRatio, const FloatRe
146146
return destRect;
147147
}
148148

149+
FloatSize sizeWithAreaAndAspectRatio(float area, float aspectRatio)
150+
{
151+
auto scaledWidth = std::sqrt(area * aspectRatio);
152+
return { scaledWidth, scaledWidth / aspectRatio };
153+
}
154+
149155
bool ellipseContainsPoint(const FloatPoint& center, const FloatSize& radii, const FloatPoint& point)
150156
{
151157
FloatPoint transformedPoint(point);

Source/WebCore/platform/graphics/GeometryUtilities.h

+2
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ FloatRect mapRect(const FloatRect&, const FloatRect& srcRect, const FloatRect& d
4747
WEBCORE_EXPORT FloatRect largestRectWithAspectRatioInsideRect(float aspectRatio, const FloatRect&);
4848
WEBCORE_EXPORT FloatRect smallestRectWithAspectRatioAroundRect(float aspectRatio, const FloatRect&);
4949

50+
FloatSize sizeWithAreaAndAspectRatio(float area, float aspectRatio);
51+
5052
// Compute a rect that encloses all points covered by the given rect if it were rotated a full turn around (0,0).
5153
FloatRect boundsOfRotatingRect(const FloatRect&);
5254

Source/WebCore/platform/ios/DragImageIOS.mm

+12-3
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
#import "FontCascade.h"
3535
#import "FontPlatformData.h"
3636
#import "Frame.h"
37+
#import "GeometryUtilities.h"
3738
#import "GraphicsContext.h"
3839
#import "Image.h"
3940
#import "NotImplemented.h"
@@ -88,18 +89,26 @@ DragImageRef scaleDragImage(DragImageRef image, FloatSize scale)
8889
return imageCopy.CGImage;
8990
}
9091

91-
DragImageRef createDragImageFromImage(Image * _Nullable image, ImageOrientationDescription orientation)
92+
static float maximumAllowedDragImageArea = 600 * 1024;
93+
94+
DragImageRef createDragImageFromImage(Image* image, ImageOrientationDescription orientation)
9295
{
93-
if (!image)
96+
if (!image || !image->width() || !image->height())
9497
return nil;
9598

99+
float adjustedImageScale = 1;
96100
CGSize imageSize(image->size());
101+
if (imageSize.width * imageSize.height > maximumAllowedDragImageArea) {
102+
auto adjustedSize = roundedIntSize(sizeWithAreaAndAspectRatio(maximumAllowedDragImageArea, imageSize.width / imageSize.height));
103+
adjustedImageScale = adjustedSize.width() / imageSize.width;
104+
imageSize = adjustedSize;
105+
}
97106

98107
RetainPtr<UIGraphicsImageRenderer> render = adoptNS([allocUIGraphicsImageRendererInstance() initWithSize:imageSize]);
99108
UIImage *imageCopy = [render.get() imageWithActions:^(UIGraphicsImageRendererContext *rendererContext) {
100109
GraphicsContext context(rendererContext.CGContext);
101110
context.translate(0, imageSize.height);
102-
context.scale({ 1, -1 });
111+
context.scale({ adjustedImageScale, -adjustedImageScale });
103112
ImagePaintingOptions paintingOptions;
104113
paintingOptions.m_orientationDescription = orientation;
105114
context.drawImage(*image, FloatPoint(), paintingOptions);

Source/WebCore/platform/ios/PlatformPasteboardIOS.mm

+8-10
Original file line numberDiff line numberDiff line change
@@ -290,16 +290,14 @@ static void addRepresentationsForPlainText(WebItemProviderRegistrationInfoList *
290290
for (size_t i = 0, size = types.size(); i < size; ++i)
291291
[itemsToRegister addData:data[i]->createNSData().get() forType:types[i]];
292292

293-
if (auto image = pasteboardImage.image) {
294-
NSString *mimeType = pasteboardImage.resourceMIMEType;
295-
if (UTTypeIsDeclared((CFStringRef)mimeType)) {
296-
auto imageData = pasteboardImage.resourceData->createNSData();
297-
[itemsToRegister addData:imageData.get() forType:mimeType];
298-
} else if (auto nativeImage = image->nativeImage()) {
299-
if (auto uiImage = adoptNS([allocUIImageInstance() initWithCGImage:nativeImage.get()]))
300-
[itemsToRegister addRepresentingObject:uiImage.get()];
301-
}
302-
[itemsToRegister setEstimatedDisplayedSize:image->size()];
293+
if (pasteboardImage.resourceData && !pasteboardImage.resourceMIMEType.isEmpty()) {
294+
auto utiOrMIMEType = pasteboardImage.resourceMIMEType.createCFString();
295+
if (!UTTypeIsDeclared(utiOrMIMEType.get()))
296+
utiOrMIMEType = UTTypeCreatePreferredIdentifierForTag(kUTTagClassMIMEType, utiOrMIMEType.get(), nil);
297+
298+
auto imageData = pasteboardImage.resourceData->createNSData();
299+
[itemsToRegister addData:imageData.get() forType:(NSString *)utiOrMIMEType.get()];
300+
[itemsToRegister setEstimatedDisplayedSize:pasteboardImage.imageSize];
303301
[itemsToRegister setSuggestedName:pasteboardImage.suggestedName];
304302
}
305303

Source/WebKit/ChangeLog

+15
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,18 @@
1+
2017-07-17 Wenson Hsieh <[email protected]>
2+
3+
[iOS DnD] Web process uses too much memory when beginning a drag on a very large image
4+
https://bugs.webkit.org/show_bug.cgi?id=174585
5+
<rdar://problem/33302541>
6+
7+
Reviewed by Tim Horton.
8+
9+
Add IPC support for serializing/deserializing the size of an image written to the pasteboard. See WebCore
10+
ChangeLogs for more details.
11+
12+
* Shared/WebCoreArgumentCoders.cpp:
13+
(IPC::ArgumentCoder<PasteboardImage>::encode):
14+
(IPC::ArgumentCoder<PasteboardImage>::decode):
15+
116
2017-07-17 Konstantin Tokarev <[email protected]>
217

318
Unreviewed attempt to fix Mac cmake build

Source/WebKit/Shared/WebCoreArgumentCoders.cpp

+3
Original file line numberDiff line numberDiff line change
@@ -1491,6 +1491,7 @@ void ArgumentCoder<PasteboardImage>::encode(Encoder& encoder, const PasteboardIm
14911491
encoder << pasteboardImage.url.title;
14921492
encoder << pasteboardImage.resourceMIMEType;
14931493
encoder << pasteboardImage.suggestedName;
1494+
encoder << pasteboardImage.imageSize;
14941495
if (pasteboardImage.resourceData)
14951496
encodeSharedBuffer(encoder, pasteboardImage.resourceData.get());
14961497
encodeClientTypesAndData(encoder, pasteboardImage.clientTypes, pasteboardImage.clientData);
@@ -1508,6 +1509,8 @@ bool ArgumentCoder<PasteboardImage>::decode(Decoder& decoder, PasteboardImage& p
15081509
return false;
15091510
if (!decoder.decode(pasteboardImage.suggestedName))
15101511
return false;
1512+
if (!decoder.decode(pasteboardImage.imageSize))
1513+
return false;
15111514
if (!decodeSharedBuffer(decoder, pasteboardImage.resourceData))
15121515
return false;
15131516
if (!decodeClientTypesAndData(decoder, pasteboardImage.clientTypes, pasteboardImage.clientData))

Tools/ChangeLog

+22
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,25 @@
1+
2017-07-17 Wenson Hsieh <[email protected]>
2+
3+
[iOS DnD] Web process uses too much memory when beginning a drag on a very large image
4+
https://bugs.webkit.org/show_bug.cgi?id=174585
5+
<rdar://problem/33302541>
6+
7+
Reviewed by Tim Horton.
8+
9+
Adds a new test verifying that we don't try to allocate any image buffer equal to the true size of the image
10+
being dragged when initiating a drag.
11+
12+
* TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
13+
* TestWebKitAPI/Tests/WebKit2Cocoa/enormous.svg: Added.
14+
* TestWebKitAPI/Tests/ios/DataInteractionTests.mm:
15+
(TestWebKitAPI::TEST):
16+
* TestWebKitAPI/cocoa/TestWKWebView.h:
17+
18+
Add a new -synchronouslyLoadHTMLString: helper that works like -synchronouslyLoadTestPage:, but takes markup.
19+
20+
* TestWebKitAPI/cocoa/TestWKWebView.mm:
21+
(-[TestWKWebView synchronouslyLoadHTMLString:]):
22+
123
2017-07-17 Michael Catanzaro <[email protected]>
224

325
[CMake] Macros in WebKitMacros.cmake should be prefixed with WEBKIT_ namespace

Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj

+4
Original file line numberDiff line numberDiff line change
@@ -632,6 +632,7 @@
632632
E1220DCA155B28AA0013E2FC /* MemoryCacheDisableWithinResourceLoadDelegate.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = E1220DC9155B287D0013E2FC /* MemoryCacheDisableWithinResourceLoadDelegate.html */; };
633633
E194E1BD177E53C7009C4D4E /* StopLoadingFromDidReceiveResponse.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = E194E1BC177E534A009C4D4E /* StopLoadingFromDidReceiveResponse.html */; };
634634
ECA680CE1E68CC0900731D20 /* StringUtilities.mm in Sources */ = {isa = PBXBuildFile; fileRef = ECA680CD1E68CC0900731D20 /* StringUtilities.mm */; };
635+
F407FE391F1D0DFC0017CF25 /* enormous.svg in Copy Resources */ = {isa = PBXBuildFile; fileRef = F407FE381F1D0DE60017CF25 /* enormous.svg */; };
635636
F415086D1DA040C50044BE9B /* play-audio-on-click.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F415086C1DA040C10044BE9B /* play-audio-on-click.html */; };
636637
F41AB99F1EF4696B0083FA08 /* autofocus-contenteditable.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F41AB9981EF4692C0083FA08 /* autofocus-contenteditable.html */; };
637638
F41AB9A01EF4696B0083FA08 /* background-image-link-and-input.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = F41AB9971EF4692C0083FA08 /* background-image-link-and-input.html */; };
@@ -742,6 +743,7 @@
742743
dstSubfolderSpec = 7;
743744
files = (
744745
F45B63FB1F197F4A009D38B9 /* image-map.html in Copy Resources */,
746+
F407FE391F1D0DFC0017CF25 /* enormous.svg in Copy Resources */,
745747
F4D5E4E81F0C5D38008C1A49 /* dragstart-clear-selection.html in Copy Resources */,
746748
F4A32EC41F05F3850047C544 /* dragstart-change-selection-offscreen.html in Copy Resources */,
747749
F4A32ECB1F0643370047C544 /* contenteditable-in-iframe.html in Copy Resources */,
@@ -1618,6 +1620,7 @@
16181620
E4C9ABC71B3DB1710040A987 /* RunLoop.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = RunLoop.cpp; sourceTree = "<group>"; };
16191621
ECA680CD1E68CC0900731D20 /* StringUtilities.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = StringUtilities.mm; sourceTree = "<group>"; };
16201622
F3FC3EE213678B7300126A65 /* libgtest.a */ = {isa = PBXFileReference; lastKnownFileType = archive.ar; path = libgtest.a; sourceTree = BUILT_PRODUCTS_DIR; };
1623+
F407FE381F1D0DE60017CF25 /* enormous.svg */ = {isa = PBXFileReference; lastKnownFileType = text; path = enormous.svg; sourceTree = "<group>"; };
16211624
F415086C1DA040C10044BE9B /* play-audio-on-click.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "play-audio-on-click.html"; sourceTree = "<group>"; };
16221625
F41AB9931EF4692C0083FA08 /* image-and-textarea.html */ = {isa = PBXFileReference; lastKnownFileType = text.html; path = "image-and-textarea.html"; sourceTree = "<group>"; };
16231626
F41AB9941EF4692C0083FA08 /* prevent-operation.html */ = {isa = PBXFileReference; lastKnownFileType = text.html; path = "prevent-operation.html"; sourceTree = "<group>"; };
@@ -2082,6 +2085,7 @@
20822085
5714ECBA1CA8BFD100051AC8 /* DownloadRequestOriginalURLFrame.html */,
20832086
A155022B1E050BC500A24C57 /* duplicate-completion-handler-calls.html */,
20842087
9984FACD1CFFB038008D198C /* editable-body.html */,
2088+
F407FE381F1D0DE60017CF25 /* enormous.svg */,
20852089
F4C2AB211DD6D94100E06D5B /* enormous-video-with-sound.html */,
20862090
93575C551D30366E000D604D /* focus-inputs.html */,
20872091
F4F405BA1D4C0CF8007A9707 /* full-size-autoplaying-video-with-audio.html */,
Loading

Tools/TestWebKitAPI/Tests/ios/DataInteractionTests.mm

+12
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,18 @@ static void checkDragCaretRectIsContainedInRect(CGRect caretRect, CGRect contain
170170
checkSuggestedNameAndEstimatedSize(dataInteractionSimulator.get(), @"icon.png", { 215, 174 });
171171
}
172172

173+
TEST(DataInteractionTests, CanStartDragOnEnormousImage)
174+
{
175+
auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);
176+
[webView synchronouslyLoadHTML:@"<img src='enormous.svg'></img>"];
177+
178+
auto dataInteractionSimulator = adoptNS([[DataInteractionSimulator alloc] initWithWebView:webView.get()]);
179+
[dataInteractionSimulator runFrom:CGPointMake(100, 100) to:CGPointMake(100, 100)];
180+
181+
NSArray *registeredTypes = [[dataInteractionSimulator sourceItemProviders].firstObject registeredTypeIdentifiers];
182+
EXPECT_WK_STREQ((NSString *)kUTTypeScalableVectorGraphics, [registeredTypes firstObject]);
183+
}
184+
173185
TEST(DataInteractionTests, ImageToTextarea)
174186
{
175187
RetainPtr<TestWKWebView> webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);

Tools/TestWebKitAPI/cocoa/TestWKWebView.h

+1
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
- (void)clearMessageHandlers:(NSArray *)messageNames;
4040
- (void)performAfterReceivingMessage:(NSString *)message action:(dispatch_block_t)action;
4141
- (void)loadTestPageNamed:(NSString *)pageName;
42+
- (void)synchronouslyLoadHTMLString:(NSString *)html;
4243
- (void)synchronouslyLoadTestPageNamed:(NSString *)pageName;
4344
- (NSString *)stringByEvaluatingJavaScript:(NSString *)script;
4445
- (void)waitForMessage:(NSString *)message;

Tools/TestWebKitAPI/cocoa/TestWKWebView.mm

+7
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,13 @@ - (void)loadTestPageNamed:(NSString *)pageName
223223
[self loadRequest:request];
224224
}
225225

226+
- (void)synchronouslyLoadHTMLString:(NSString *)html
227+
{
228+
NSURL *testResourceURL = [[[NSBundle mainBundle] bundleURL] URLByAppendingPathComponent:@"TestWebKitAPI.resources"];
229+
[self loadHTMLString:html baseURL:testResourceURL];
230+
[self _test_waitForDidFinishNavigation];
231+
}
232+
226233
- (void)synchronouslyLoadTestPageNamed:(NSString *)pageName
227234
{
228235
[self loadTestPageNamed:pageName];

0 commit comments

Comments
 (0)