<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - No reliable way to get a snapshot of WKWebView (macOS)"
href="https://bugs.webkit.org/show_bug.cgi?id=161450#c45">Comment # 45</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - No reliable way to get a snapshot of WKWebView (macOS)"
href="https://bugs.webkit.org/show_bug.cgi?id=161450">bug 161450</a>
from <span class="vcard"><a class="email" href="mailto:thorton@apple.com" title="Tim Horton <thorton@apple.com>"> <span class="fn">Tim Horton</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=300288&action=diff" name="attach_300288" title="Patch">attachment 300288</a> <a href="attachment.cgi?id=300288&action=edit" title="Patch">[details]</a></span>
Patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=300288&action=review">https://bugs.webkit.org/attachment.cgi?id=300288&action=review</a>
<span class="quote">> Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKWebViewSnapshot.mm:28
> +#if WK_API_ENABLED</span >
Do these tests work with different display colorspaces? We should test with a few.
<span class="quote">> Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKWebViewSnapshot.mm:44
> + RetainPtr<CGImageSourceRef> source = adoptCF(CGImageSourceCreateWithData((CFDataRef)[image TIFFRepresentation], NULL));
> + return adoptCF(CGImageSourceCreateImageAtIndex(source.get(), 0, NULL));</span >
Is there any reason this doesn't just use CGImageForProposedRect?
<span class="quote">> Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKWebViewSnapshot.mm:54
> + CGImageRef cgImage = [image CGImage];
> + CFRetain(cgImage);
> + return adoptCF(cgImage);</span >
No need for this dance, just 'return image.CGImage;' will do the right thing (it is autoreleased, RetainPtr constructor will retain it).
<span class="quote">> Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKWebViewSnapshot.mm:73
> + WKSnapshotConfiguration *snapshotConfiguration = [[WKSnapshotConfiguration alloc] init];</span >
Pretty sure this leaks.
<span class="quote">> 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];</span >
Spaces after semicolons? Kind of inconsistent at the moment.
<span class="quote">> Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKWebViewSnapshot.mm:97
> + WKSnapshotConfiguration *snapshotConfiguration = [[WKSnapshotConfiguration alloc] init];</span >
Pretty sure this leaks.
<span class="quote">> Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKWebViewSnapshot.mm:99
> + snapshotConfiguration.snapshotWidth = viewWidth;</span >
Here, viewWidth is being used as points...
<span class="quote">> Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKWebViewSnapshot.mm:108
> + CGColorSpaceRef colorSpace = CGColorSpaceCreateDeviceRGB();</span >
Why not a RetainPtr?
<span class="quote">> Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKWebViewSnapshot.mm:111
> + CGContextRef context = CGBitmapContextCreate(rgba, viewWidth, viewHeight, 8, 4 * viewWidth, colorSpace, kCGImageAlphaPremultipliedLast | kCGBitmapByteOrder32Big);</span >
... and here as pixels. Does this matter? Does the test succeed on hardware of various deviceScaleFactors?
Also, why not a RetainPtr?
<span class="quote">> Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKWebViewSnapshot.mm:112
> + CGContextDrawImage(context, CGRectMake(0, 0, viewWidth, viewHeight), cgImage.get());</span >
Or maybe it's OK because it gets scaled into place here?
<span class="quote">> Tools/TestWebKitAPI/Tests/WebKit2Cocoa/WKWebViewSnapshot.mm:148
> + WKSnapshotConfiguration *snapshotConfiguration = [[WKSnapshotConfiguration alloc] init];</span >
Pretty sure this leaks.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>