[Webkit-unassigned] [Bug 161450] No reliable way to get a snapshot of WKWebView (macOS)

Tue Jan 31 17:53:01 PST 2017

https://bugs.webkit.org/show_bug.cgi?id=161450

--- Comment #45 from Tim Horton <thorton at apple.com> ---
Comment on attachment 300288
  --> https://bugs.webkit.org/attachment.cgi?id=300288
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=300288&action=review

> Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKWebViewSnapshot.mm:28
> +#if WK_API_ENABLED

Do these tests work with different display colorspaces? We should test with a few.

> Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKWebViewSnapshot.mm:44
> +    RetainPtr<CGImageSourceRef> source = adoptCF(CGImageSourceCreateWithData((CFDataRef)[image TIFFRepresentation], NULL));
> +    return adoptCF(CGImageSourceCreateImageAtIndex(source.get(), 0, NULL));

Is there any reason this doesn't just use CGImageForProposedRect?

> Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKWebViewSnapshot.mm:54
> +    CGImageRef cgImage = [image CGImage];
> +    CFRetain(cgImage);
> +    return adoptCF(cgImage);

No need for this dance, just 'return image.CGImage;' will do the right thing (it is autoreleased, RetainPtr constructor will retain it).

> Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKWebViewSnapshot.mm:73
> +    WKSnapshotConfiguration *snapshotConfiguration = [[WKSnapshotConfiguration alloc] init];

Pretty sure this leaks.

> Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKWebViewSnapshot.mm:94
> +    [webView loadHTMLString:@"<body style='background-color: red;'><div style='background-color: blue;position: absolute;width: 100px; height: 100px; top: 50px; left: 50px'></div></body>" baseURL:nil];

Spaces after semicolons? Kind of inconsistent at the moment.

> Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKWebViewSnapshot.mm:97
> +    WKSnapshotConfiguration *snapshotConfiguration = [[WKSnapshotConfiguration alloc] init];

Pretty sure this leaks.

> Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKWebViewSnapshot.mm:99
> +    snapshotConfiguration.snapshotWidth = viewWidth;

Here, viewWidth is being used as points...

> Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKWebViewSnapshot.mm:108
> +        CGColorSpaceRef colorSpace = CGColorSpaceCreateDeviceRGB();

Why not a RetainPtr?

> Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKWebViewSnapshot.mm:111
> +        CGContextRef context = CGBitmapContextCreate(rgba, viewWidth, viewHeight, 8, 4 * viewWidth, colorSpace, kCGImageAlphaPremultipliedLast | kCGBitmapByteOrder32Big);

... and here as pixels. Does this matter? Does the test succeed on hardware of various deviceScaleFactors?

Also, why not a RetainPtr?

> Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKWebViewSnapshot.mm:112
> +        CGContextDrawImage(context, CGRectMake(0, 0, viewWidth, viewHeight), cgImage.get());

Or maybe it's OK because it gets scaled into place here?

> Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKWebViewSnapshot.mm:148
> +    WKSnapshotConfiguration *snapshotConfiguration = [[WKSnapshotConfiguration alloc] init];

Pretty sure this leaks.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20170201/884c6025/attachment.html>