Attachment 361450 Details for Bug 194362 – Patch

[patch] Patch

for-review-2.txt (text/plain), 13.25 KB, created by Beth Dakin on 2019-02-07 14:48:39 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Beth Dakin

Created: 2019-02-07 14:48:39 PST

Size: 13.25 KB

patch

obsolete

>Index: Source/WebKit/ChangeLog
>===================================================================
>--- Source/WebKit/ChangeLog	(revision 241144)
>+++ Source/WebKit/ChangeLog	(working copy)
>@@ -1,3 +1,29 @@
>+2019-02-07  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 NOBODY (OOPS!).
>+
>+        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-07  Brent Fulgham  <bfulgham@apple.com>
> 
>         Permit additional IOKit Properties needed by Media PlugIns on older Mac Hardware
>Index: Source/WebKit/UIProcess/API/Cocoa/WKSnapshotConfiguration.h
>===================================================================
>--- Source/WebKit/UIProcess/API/Cocoa/WKSnapshotConfiguration.h	(revision 241144)
>+++ Source/WebKit/UIProcess/API/Cocoa/WKSnapshotConfiguration.h	(working copy)
>@@ -48,6 +48,8 @@ WK_CLASS_AVAILABLE(macosx(10.13), ios(11
>  */
> @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: Source/WebKit/UIProcess/API/Cocoa/WKSnapshotConfiguration.mm
>===================================================================
>--- Source/WebKit/UIProcess/API/Cocoa/WKSnapshotConfiguration.mm	(revision 241144)
>+++ Source/WebKit/UIProcess/API/Cocoa/WKSnapshotConfiguration.mm	(working copy)
>@@ -36,6 +36,7 @@ - (instancetype)init
>         return nil;
> 
>     self.rect = CGRectNull;
>+    self.afterScreenUpdates = YES;
>     return self;
> }
> 
>@@ -52,6 +53,7 @@ - (id)copyWithZone:(NSZone *)zone
> 
>     snapshotConfiguration.rect = self.rect;
>     snapshotConfiguration.snapshotWidth = self.snapshotWidth;
>+    snapshotConfiguration.afterScreenUpdates = self.afterScreenUpdates;
> 
>     return snapshotConfiguration;
> }
>Index: Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>===================================================================
>--- Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm	(revision 241144)
>+++ Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm	(working copy)
>@@ -1141,6 +1141,9 @@ - (void)takeSnapshotWithConfiguration:(W
>     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,31 @@ - (void)takeSnapshotWithConfiguration:(W
> 
>     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]);
>-
>-        handler(uiImage.get(), error.get());
>-    }];
>+    if (snapshotConfiguration && snapshotConfiguration.afterScreenUpdates) {
>+        _page->callAfterNextPresentationUpdate([callSnapshotRect = WTFMove(callSnapshotRect), handler](WebKit::CallbackBase::Error error) {
>+            if (error != WebKit::CallbackBase::Error::None) {
>+                handler(nil, createNSError(WKErrorUnknown).get());
>+                return;
>+            }
>+            callSnapshotRect();
>+        });
>+    } else
>+        callSnapshotRect();
> }
> #endif
> 
>Index: Tools/ChangeLog
>===================================================================
>--- Tools/ChangeLog	(revision 241144)
>+++ Tools/ChangeLog	(working copy)
>@@ -1,3 +1,16 @@
>+2019-02-07  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 NOBODY (OOPS!).
>+
>+        * TestWebKitAPI/Tests/WebKitCocoa/WKWebViewSnapshot.mm:
>+        (TEST):
>+
> 2019-02-07  Keith Rollin  <krollin@apple.com>
> 
>         Remove extra copy of 100x100-red.tga
>Index: Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebViewSnapshot.mm
>===================================================================
>--- Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebViewSnapshot.mm	(revision 241144)
>+++ Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebViewSnapshot.mm	(working copy)
>@@ -294,4 +294,125 @@ TEST(WKWebView, SnapshotImageLargeAsyncD
>     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

Flags:

thorton: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 194362: 361333 | 361450