Title: [279006] trunk
Revision
279006
Author
[email protected]
Date
2021-06-17 13:48:44 -0700 (Thu, 17 Jun 2021)

Log Message

Trying to take empty snapshots of the view should not cause crashes
https://bugs.webkit.org/show_bug.cgi?id=227133

Reviewed by Tim Horton.

Source/WebKit:

Trying to take empty snapshots of the view should not cause crashes. We would previously
see crashes because:
1. We would end up doing divisions by zero
2. We would hit the ASSERT(size) in SharedMemory::allocate().

* UIProcess/API/Cocoa/WKWebView.mm:
(-[WKWebView takeSnapshotWithConfiguration:completionHandler:]):

Tools:

Add API test coverage.

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

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (279005 => 279006)


--- trunk/Source/WebKit/ChangeLog	2021-06-17 20:07:14 UTC (rev 279005)
+++ trunk/Source/WebKit/ChangeLog	2021-06-17 20:48:44 UTC (rev 279006)
@@ -1,3 +1,18 @@
+2021-06-17  Chris Dumez  <[email protected]>
+
+        Trying to take empty snapshots of the view should not cause crashes
+        https://bugs.webkit.org/show_bug.cgi?id=227133
+
+        Reviewed by Tim Horton.
+
+        Trying to take empty snapshots of the view should not cause crashes. We would previously
+        see crashes because:
+        1. We would end up doing divisions by zero
+        2. We would hit the ASSERT(size) in SharedMemory::allocate().
+
+        * UIProcess/API/Cocoa/WKWebView.mm:
+        (-[WKWebView takeSnapshotWithConfiguration:completionHandler:]):
+
 2021-06-17  Kate Cheney  <[email protected]>
 
         Storage Access quirks should prompt up to twice if a user does not allow storage access

Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm (279005 => 279006)


--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm	2021-06-17 20:07:14 UTC (rev 279005)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm	2021-06-17 20:48:44 UTC (rev 279006)
@@ -1170,7 +1170,19 @@
 
     auto handler = makeBlockPtr(completionHandler);
 
+    if (CGRectIsEmpty(rectInViewCoordinates) || !snapshotWidth) {
+        RunLoop::main().dispatch([completionHandler = WTFMove(completionHandler)]() mutable {
 #if USE(APPKIT)
+            auto image = adoptNS([[NSImage alloc] initWithSize:NSMakeSize(0, 0)]);
+#else
+            auto image = adoptNS([[UIImage alloc] init]);
+#endif
+            completionHandler(image.get(), nil);
+        });
+        return;
+    }
+
+#if USE(APPKIT)
     CGFloat imageScale = snapshotWidth / rectInViewCoordinates.size.width;
     CGFloat imageHeight = imageScale * rectInViewCoordinates.size.height;
 

Modified: trunk/Tools/ChangeLog (279005 => 279006)


--- trunk/Tools/ChangeLog	2021-06-17 20:07:14 UTC (rev 279005)
+++ trunk/Tools/ChangeLog	2021-06-17 20:48:44 UTC (rev 279006)
@@ -1,3 +1,15 @@
+2021-06-17  Chris Dumez  <[email protected]>
+
+        Trying to take empty snapshots of the view should not cause crashes
+        https://bugs.webkit.org/show_bug.cgi?id=227133
+
+        Reviewed by Tim Horton.
+
+        Add API test coverage.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/WKWebViewSnapshot.mm:
+        (TEST):
+
 2021-06-17  W.D. Xiong  <[email protected]>
 
         [iOS 15] Add iOS 15 simulator/device to bot watchers' dashboard

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebViewSnapshot.mm (279005 => 279006)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebViewSnapshot.mm	2021-06-17 20:07:14 UTC (rev 279005)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebViewSnapshot.mm	2021-06-17 20:48:44 UTC (rev 279006)
@@ -82,6 +82,112 @@
     TestWebKitAPI::Util::run(&isDone);
 }
 
+TEST(WKWebView, SnapshotImageEmptyRect)
+{
+    CGFloat viewWidth = 800;
+    CGFloat viewHeight = 600;
+    RetainPtr<WKWebView> webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, viewWidth, viewHeight)]);
+
+    [webView loadHTMLString:@"<body style='background-color: red;'></body>" baseURL:nil];
+    [webView _test_waitForDidFinishNavigation];
+
+    auto pid = [webView _webProcessIdentifier];
+
+    RetainPtr<WKSnapshotConfiguration> snapshotConfiguration = adoptNS([[WKSnapshotConfiguration alloc] init]);
+    [snapshotConfiguration setRect:NSMakeRect(0, 0, 0, 0)];
+    [snapshotConfiguration setSnapshotWidth:@(viewWidth)];
+
+    isDone = false;
+    [webView takeSnapshotWithConfiguration:snapshotConfiguration.get() completionHandler:^(PlatformImage snapshotImage, NSError *error) {
+        EXPECT_NULL(error);
+        EXPECT_EQ(0, snapshotImage.size.width);
+        EXPECT_EQ(0, snapshotImage.size.height);
+
+        isDone = true;
+    }];
+
+    TestWebKitAPI::Util::run(&isDone);
+    EXPECT_EQ(pid, [webView _webProcessIdentifier]); // Make sure the WebProcess did not crash.
+}
+
+TEST(WKWebView, SnapshotImageZeroWidth)
+{
+    CGFloat viewWidth = 800;
+    CGFloat viewHeight = 600;
+    RetainPtr<WKWebView> webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, viewWidth, viewHeight)]);
+
+    [webView loadHTMLString:@"<body style='background-color: red;'></body>" baseURL:nil];
+    [webView _test_waitForDidFinishNavigation];
+
+    auto pid = [webView _webProcessIdentifier];
+
+    RetainPtr<WKSnapshotConfiguration> snapshotConfiguration = adoptNS([[WKSnapshotConfiguration alloc] init]);
+    [snapshotConfiguration setRect:NSMakeRect(0, 0, viewWidth, viewHeight)];
+    [snapshotConfiguration setSnapshotWidth:@(0.0)]; // Will fall back to the view width.
+
+    isDone = false;
+    [webView takeSnapshotWithConfiguration:snapshotConfiguration.get() completionHandler:^(PlatformImage snapshotImage, NSError *error) {
+        EXPECT_NULL(error);
+        EXPECT_EQ(viewWidth, snapshotImage.size.width);
+        isDone = true;
+    }];
+
+    TestWebKitAPI::Util::run(&isDone);
+    EXPECT_EQ(pid, [webView _webProcessIdentifier]); // Make sure the WebProcess did not crash.
+}
+
+TEST(WKWebView, SnapshotImageZeroSizeView)
+{
+    CGFloat viewWidth = 0;
+    CGFloat viewHeight = 0;
+    RetainPtr<WKWebView> webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, viewWidth, viewHeight)]);
+
+    [webView loadHTMLString:@"<body style='background-color: red;'></body>" baseURL:nil];
+    [webView _test_waitForDidFinishNavigation];
+
+    auto pid = [webView _webProcessIdentifier];
+
+    RetainPtr<WKSnapshotConfiguration> snapshotConfiguration = adoptNS([[WKSnapshotConfiguration alloc] init]);
+    [snapshotConfiguration setRect:NSMakeRect(0, 0, viewWidth, viewHeight)];
+    [snapshotConfiguration setSnapshotWidth:@(100)];
+
+    isDone = false;
+    [webView takeSnapshotWithConfiguration:snapshotConfiguration.get() completionHandler:^(PlatformImage snapshotImage, NSError *error) {
+        EXPECT_NULL(error);
+        EXPECT_EQ(0, snapshotImage.size.width);
+        EXPECT_EQ(0, snapshotImage.size.height);
+
+        isDone = true;
+    }];
+
+    TestWebKitAPI::Util::run(&isDone);
+    EXPECT_EQ(pid, [webView _webProcessIdentifier]); // Make sure the WebProcess did not crash.
+}
+
+TEST(WKWebView, SnapshotImageZeroSizeViewNoConfiguration)
+{
+    CGFloat viewWidth = 0;
+    CGFloat viewHeight = 0;
+    RetainPtr<WKWebView> webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, viewWidth, viewHeight)]);
+
+    [webView loadHTMLString:@"<body style='background-color: red;'></body>" baseURL:nil];
+    [webView _test_waitForDidFinishNavigation];
+
+    auto pid = [webView _webProcessIdentifier];
+
+    isDone = false;
+    [webView takeSnapshotWithConfiguration:nil completionHandler:^(PlatformImage snapshotImage, NSError *error) {
+        EXPECT_NULL(error);
+        EXPECT_EQ(0, snapshotImage.size.width);
+        EXPECT_EQ(0, snapshotImage.size.height);
+
+        isDone = true;
+    }];
+
+    TestWebKitAPI::Util::run(&isDone);
+    EXPECT_EQ(pid, [webView _webProcessIdentifier]); // Make sure the WebProcess did not crash.
+}
+
 TEST(WKWebView, SnapshotImageBaseCase)
 {
     NSInteger viewWidth = 800;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to