Add afterScreenUpdates to WKSnapshotConfiguration
authorbdakin@apple.com <bdakin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 Feb 2019 18:20:12 +0000 (18:20 +0000)
committerbdakin@apple.com <bdakin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 Feb 2019 18:20:12 +0000 (18:20 +0000)
https://bugs.webkit.org/show_bug.cgi?id=194362
-and corresponding-
<rdar://problem/40655528> Please add an "after screen updates" property to
WKSnapshotConfiguration (to solve blank snapshots)

Reviewed by Tim Horton.

Source/WebKit:

This is the WebKit equivalent of - (UIView *)snapshotViewAfterScreenUpdates:(BOOL)afterUpdates;
This makes our snapshotting API more predictable and reliable on iOS devices,
which is why the new configuration property defaults to YES.

New property that defaults to YES.
* UIProcess/API/Cocoa/WKSnapshotConfiguration.h:
* UIProcess/API/Cocoa/WKSnapshotConfiguration.mm:
(-[WKSnapshotConfiguration init]):
(-[WKSnapshotConfiguration copyWithZone:]):

When afterScreenUpdates is set, invoke the snapshot via
callAfterNextPresentationUpdate.
* UIProcess/API/Cocoa/WKWebView.mm:
(-[WKWebView takeSnapshotWithConfiguration:completionHandler:]):
(-[WKWebView _callCompletionHandler:withSnapshotImage:atDeviceScale:]):

Tools:

* TestWebKitAPI/Tests/WebKitCocoa/WKWebViewSnapshot.mm:
(TEST):

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

Source/WebKit/ChangeLog
Source/WebKit/UIProcess/API/Cocoa/WKSnapshotConfiguration.h
Source/WebKit/UIProcess/API/Cocoa/WKSnapshotConfiguration.mm
Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebViewSnapshot.mm

index d5a70a1..e1c379b 100644 (file)
@@ -1,3 +1,29 @@
+2019-02-08  Beth Dakin  <bdakin@apple.com>
+
+        Add afterScreenUpdates to WKSnapshotConfiguration
+        https://bugs.webkit.org/show_bug.cgi?id=194362
+        -and corresponding-
+        <rdar://problem/40655528> Please add an "after screen updates" property to 
+        WKSnapshotConfiguration (to solve blank snapshots)
+
+        Reviewed by Tim Horton.
+
+        This is the WebKit equivalent of - (UIView *)snapshotViewAfterScreenUpdates:(BOOL)afterUpdates;
+        This makes our snapshotting API more predictable and reliable on iOS devices, 
+        which is why the new configuration property defaults to YES.
+
+        New property that defaults to YES.
+        * UIProcess/API/Cocoa/WKSnapshotConfiguration.h:
+        * UIProcess/API/Cocoa/WKSnapshotConfiguration.mm:
+        (-[WKSnapshotConfiguration init]):
+        (-[WKSnapshotConfiguration copyWithZone:]):
+
+        When afterScreenUpdates is set, invoke the snapshot via 
+        callAfterNextPresentationUpdate.
+        * UIProcess/API/Cocoa/WKWebView.mm:
+        (-[WKWebView takeSnapshotWithConfiguration:completionHandler:]):
+        (-[WKWebView _callCompletionHandler:withSnapshotImage:atDeviceScale:]):
+
 2019-02-08  Alex Christensen  <achristensen@webkit.org>
 
         Add SPI to use networking daemon instead of XPC service
index c9983a4..cdbe90e 100644 (file)
@@ -48,6 +48,8 @@ WK_CLASS_AVAILABLE(macosx(10.13), ios(11.0))
  */
 @property (nullable, nonatomic, copy) NSNumber *snapshotWidth;
 
+@property (nonatomic) BOOL afterScreenUpdates WK_API_AVAILABLE(macosx(WK_MAC_TBA), ios(WK_IOS_TBA));
+
 @end
 
 NS_ASSUME_NONNULL_END
index 5b50d25..9570543 100644 (file)
@@ -36,6 +36,7 @@
         return nil;
 
     self.rect = CGRectNull;
+    self.afterScreenUpdates = YES;
     return self;
 }
 
@@ -52,6 +53,7 @@
 
     snapshotConfiguration.rect = self.rect;
     snapshotConfiguration.snapshotWidth = self.snapshotWidth;
+    snapshotConfiguration.afterScreenUpdates = self.afterScreenUpdates;
 
     return snapshotConfiguration;
 }
index 171fcde..26d8ca0 100644 (file)
@@ -1141,6 +1141,9 @@ static WKErrorCode callbackErrorCode(WebKit::CallbackBase::Error error)
     bitmapSize.scale(deviceScale, deviceScale);
 
     // Software snapshot will not capture elements rendered with hardware acceleration (WebGL, video, etc).
+    // This code doesn't consider snapshotConfiguration.afterScreenUpdates since the software snapshot always
+    // contains recent updates. If we ever have a UI-side snapshot mechanism on macOS, we will need to factor
+    // in snapshotConfiguration.afterScreenUpdates at that time.
     _page->takeSnapshot(WebCore::enclosingIntRect(rectInViewCoordinates), bitmapSize, WebKit::SnapshotOptionsInViewCoordinates, [handler, snapshotWidth, imageHeight](const WebKit::ShareableBitmap::Handle& imageHandle, WebKit::CallbackBase::Error errorCode) {
         if (errorCode != WebKit::ScriptValueCallback::Error::None) {
             auto error = createNSError(callbackErrorCode(errorCode));
@@ -1167,18 +1170,33 @@ static WKErrorCode callbackErrorCode(WebKit::CallbackBase::Error error)
 
     auto handler = makeBlockPtr(completionHandler);
     CGFloat deviceScale = _page->deviceScaleFactor();
+    RetainPtr<WKWebView> strongSelf = self;
+    auto callSnapshotRect = [strongSelf, rectInViewCoordinates, snapshotWidth, deviceScale, handler] {
+        [strongSelf _snapshotRect:rectInViewCoordinates intoImageOfWidth:(snapshotWidth * deviceScale) completionHandler:[strongSelf, handler, deviceScale](CGImageRef snapshotImage) {
+            RetainPtr<NSError> error;
+            RetainPtr<UIImage> uiImage;
+            
+            if (!snapshotImage)
+                error = createNSError(WKErrorUnknown);
+            else
+                uiImage = adoptNS([[UIImage alloc] initWithCGImage:snapshotImage scale:deviceScale orientation:UIImageOrientationUp]);
+            
+            handler(uiImage.get(), error.get());
+        }];
+    };
 
-    [self _snapshotRect:rectInViewCoordinates intoImageOfWidth:(snapshotWidth * deviceScale) completionHandler:^(CGImageRef snapshotImage) {
-        RetainPtr<NSError> error;
-        RetainPtr<UIImage> uiImage;
-
-        if (!snapshotImage)
-            error = createNSError(WKErrorUnknown);
-        else
-            uiImage = adoptNS([[UIImage alloc] initWithCGImage:snapshotImage scale:deviceScale orientation:UIImageOrientationUp]);
+    if (snapshotConfiguration && !snapshotConfiguration.afterScreenUpdates) {
+        callSnapshotRect();
+        return;
+    }
 
-        handler(uiImage.get(), error.get());
-    }];
+    _page->callAfterNextPresentationUpdate([callSnapshotRect = WTFMove(callSnapshotRect), handler](WebKit::CallbackBase::Error error) {
+        if (error != WebKit::CallbackBase::Error::None) {
+            handler(nil, createNSError(WKErrorUnknown).get());
+            return;
+        }
+        callSnapshotRect();
+    });
 }
 #endif
 
index 7afbe31..65f74df 100644 (file)
@@ -1,3 +1,16 @@
+2019-02-08  Beth Dakin  <bdakin@apple.com>
+
+        Add afterScreenUpdates to WKSnapshotConfiguration
+        https://bugs.webkit.org/show_bug.cgi?id=194362
+        -and corresponding-
+        <rdar://problem/40655528> Please add an "after screen updates" property to 
+        WKSnapshotConfiguration (to solve blank snapshots)
+
+        Reviewed by Tim Horton.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/WKWebViewSnapshot.mm:
+        (TEST):
+
 2019-02-08  Benjamin Poulain  <benjamin@webkit.org>
 
         clampTo(): do not convert the input to double when dealing with integers
index f6fb958..f22ef0e 100644 (file)
@@ -294,4 +294,125 @@ TEST(WKWebView, SnapshotImageLargeAsyncDecoding)
     TestWebKitAPI::Util::run(&isDone);
 }
 
+TEST(WKWebView, SnapshotAfterScreenUpdates)
+{
+    // The API tests currently cannot truly test SnapshotConfiguration.afterScreenUpdates since it is only needed
+    // on iOS devices, and we do not currently run API tests on iOS devices. So we expect this test to pass with
+    // afterScreenUpdates set to YES or NO on the configuration. On device, afterScreenUpdates must be YES in order
+    // pass this test.
+    NSInteger viewWidth = 800;
+    NSInteger viewHeight = 600;
+    RetainPtr<WKWebView> webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, viewWidth, viewHeight)]);
+    
+    RetainPtr<PlatformWindow> window;
+    CGFloat backingScaleFactor;
+    
+#if PLATFORM(MAC)
+    window = adoptNS([[NSWindow alloc] initWithContentRect:[webView frame] styleMask:NSBorderlessWindowMask backing:NSBackingStoreBuffered defer:NO]);
+    [[window contentView] addSubview:webView.get()];
+    backingScaleFactor = [window backingScaleFactor];
+#elif PLATFORM(IOS_FAMILY)
+    window = adoptNS([[UIWindow alloc] initWithFrame:[webView frame]]);
+    [window addSubview:webView.get()];
+    backingScaleFactor = [[window screen] scale];
+#endif
+    
+    [webView loadHTMLString:@"<body style='margin:0'><div id='change-me' style='background-color:red; position:fixed; width:100%; height:100%'></div></body>" baseURL:nil];
+    [webView _test_waitForDidFinishNavigation];
+    
+    RetainPtr<WKSnapshotConfiguration> snapshotConfiguration = adoptNS([[WKSnapshotConfiguration alloc] init]);
+    [snapshotConfiguration setRect:NSMakeRect(0, 0, viewWidth, viewHeight)];
+    [snapshotConfiguration setSnapshotWidth:@(viewWidth)];
+    [snapshotConfiguration setAfterScreenUpdates:YES];
+
+    [webView evaluateJavaScript:@"var div = document.getElementById('change-me');div.style.backgroundColor = 'blue';" completionHandler:nil];
+
+    isDone = false;
+    [webView takeSnapshotWithConfiguration:snapshotConfiguration.get() completionHandler:^(PlatformImage snapshotImage, NSError *error) {
+        EXPECT_NULL(error);
+        
+        EXPECT_EQ(viewWidth, snapshotImage.size.width);
+        
+        RetainPtr<CGImageRef> cgImage = convertToCGImage(snapshotImage);
+        RetainPtr<CGColorSpaceRef> colorSpace = adoptCF(CGColorSpaceCreateDeviceRGB());
+        
+        NSInteger viewWidthInPixels = viewWidth * backingScaleFactor;
+        NSInteger viewHeightInPixels = viewHeight * backingScaleFactor;
+        
+        unsigned char rgba[viewWidthInPixels * viewHeightInPixels * 4];
+        RetainPtr<CGContextRef> context = CGBitmapContextCreate(rgba, viewWidthInPixels, viewHeightInPixels, 8, 4 * viewWidthInPixels, colorSpace.get(), kCGImageAlphaPremultipliedLast | kCGBitmapByteOrder32Big);
+        CGContextDrawImage(context.get(), CGRectMake(0, 0, viewWidthInPixels, viewHeightInPixels), cgImage.get());
+        
+        NSInteger pixelIndex = getPixelIndex(0, 0, viewWidthInPixels);
+        EXPECT_EQ(0, rgba[pixelIndex]);
+        EXPECT_EQ(0, rgba[pixelIndex + 1]);
+        EXPECT_EQ(255, rgba[pixelIndex + 2]);
+        
+        isDone = true;
+    }];
+    
+    TestWebKitAPI::Util::run(&isDone);
+}
+
+TEST(WKWebView, SnapshotWithoutAfterScreenUpdates)
+{
+    // SnapshotConfiguration.afterScreenUpdates tests currently cannot truly test this API since it is only needed
+    // on iOS devices, and we do not currently run API tests on iOS devices. The expectations below are based on
+    // what we expect in the simulator and on macOS, which is that setting afterScreenUpdates to NO will still
+    // result in a snapshot that includes the recent screen updates. If we get these tests running on iOS device,
+    // then we would expect the pixels to be red instead of blue.
+    NSInteger viewWidth = 800;
+    NSInteger viewHeight = 600;
+    RetainPtr<WKWebView> webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, viewWidth, viewHeight)]);
+    
+    RetainPtr<PlatformWindow> window;
+    CGFloat backingScaleFactor;
+    
+#if PLATFORM(MAC)
+    window = adoptNS([[NSWindow alloc] initWithContentRect:[webView frame] styleMask:NSBorderlessWindowMask backing:NSBackingStoreBuffered defer:NO]);
+    [[window contentView] addSubview:webView.get()];
+    backingScaleFactor = [window backingScaleFactor];
+#elif PLATFORM(IOS_FAMILY)
+    window = adoptNS([[UIWindow alloc] initWithFrame:[webView frame]]);
+    [window addSubview:webView.get()];
+    backingScaleFactor = [[window screen] scale];
+#endif
+    
+    [webView loadHTMLString:@"<body style='margin:0'><div id='change-me' style='background-color:red; position:fixed; width:100%; height:100%'></div></body>" baseURL:nil];
+    [webView _test_waitForDidFinishNavigation];
+    
+    RetainPtr<WKSnapshotConfiguration> snapshotConfiguration = adoptNS([[WKSnapshotConfiguration alloc] init]);
+    [snapshotConfiguration setRect:NSMakeRect(0, 0, viewWidth, viewHeight)];
+    [snapshotConfiguration setSnapshotWidth:@(viewWidth)];
+    [snapshotConfiguration setAfterScreenUpdates:NO];
+    
+    [webView evaluateJavaScript:@"var div = document.getElementById('change-me');div.style.backgroundColor = 'blue';" completionHandler:nil];
+    
+    isDone = false;
+    [webView takeSnapshotWithConfiguration:snapshotConfiguration.get() completionHandler:^(PlatformImage snapshotImage, NSError *error) {
+        EXPECT_NULL(error);
+        
+        EXPECT_EQ(viewWidth, snapshotImage.size.width);
+        
+        RetainPtr<CGImageRef> cgImage = convertToCGImage(snapshotImage);
+        RetainPtr<CGColorSpaceRef> colorSpace = adoptCF(CGColorSpaceCreateDeviceRGB());
+        
+        NSInteger viewWidthInPixels = viewWidth * backingScaleFactor;
+        NSInteger viewHeightInPixels = viewHeight * backingScaleFactor;
+        
+        unsigned char rgba[viewWidthInPixels * viewHeightInPixels * 4];
+        RetainPtr<CGContextRef> context = CGBitmapContextCreate(rgba, viewWidthInPixels, viewHeightInPixels, 8, 4 * viewWidthInPixels, colorSpace.get(), kCGImageAlphaPremultipliedLast | kCGBitmapByteOrder32Big);
+        CGContextDrawImage(context.get(), CGRectMake(0, 0, viewWidthInPixels, viewHeightInPixels), cgImage.get());
+        
+        NSInteger pixelIndex = getPixelIndex(0, 0, viewWidthInPixels);
+        EXPECT_EQ(0, rgba[pixelIndex]);
+        EXPECT_EQ(0, rgba[pixelIndex + 1]);
+        EXPECT_EQ(255, rgba[pixelIndex + 2]);
+        
+        isDone = true;
+    }];
+    
+    TestWebKitAPI::Util::run(&isDone);
+}
+
 #endif