Title: [280151] trunk
- Revision
- 280151
- Author
- [email protected]
- Date
- 2021-07-21 11:11:25 -0700 (Wed, 21 Jul 2021)
Log Message
Crash in -[WKWebView takeSnapshotWithConfiguration:completionHandler:] when taking empty snapshots
https://bugs.webkit.org/show_bug.cgi?id=228134
rdar://80146087
Reviewed by Chris Dumez.
Source/WebKit:
r279006 made it so that taking empty snapshots would no longer crash due
to division by zero, or assertions on the size of the returned image.
However, the change introduced a new crash, as a result of a deallocated
completionHandler. r279006 move-captured the completion handler. However,
since completionHandler is an Objective-C block, and WebKit is built
without ARC, the move does not retain the block. Consequently, the
method can call a deallocated completion handler.
To fix, capture the local handler variable already available in the
method, which retains the completion handler using makeBlockPtr.
* UIProcess/API/Cocoa/WKWebView.mm:
(-[WKWebView takeSnapshotWithConfiguration:completionHandler:]):
Tools:
Added a test to verify the crash no longer occurs.
* TestWebKitAPI/Tests/WebKitCocoa/WKWebViewSnapshot.mm:
(-[TestSnapshotWrapper takeSnapshotWithWebView:configuration:completionHandler:]):
(TEST):
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (280150 => 280151)
--- trunk/Source/WebKit/ChangeLog 2021-07-21 18:09:17 UTC (rev 280150)
+++ trunk/Source/WebKit/ChangeLog 2021-07-21 18:11:25 UTC (rev 280151)
@@ -1,3 +1,26 @@
+2021-07-21 Aditya Keerthi <[email protected]>
+
+ Crash in -[WKWebView takeSnapshotWithConfiguration:completionHandler:] when taking empty snapshots
+ https://bugs.webkit.org/show_bug.cgi?id=228134
+ rdar://80146087
+
+ Reviewed by Chris Dumez.
+
+ r279006 made it so that taking empty snapshots would no longer crash due
+ to division by zero, or assertions on the size of the returned image.
+
+ However, the change introduced a new crash, as a result of a deallocated
+ completionHandler. r279006 move-captured the completion handler. However,
+ since completionHandler is an Objective-C block, and WebKit is built
+ without ARC, the move does not retain the block. Consequently, the
+ method can call a deallocated completion handler.
+
+ To fix, capture the local handler variable already available in the
+ method, which retains the completion handler using makeBlockPtr.
+
+ * UIProcess/API/Cocoa/WKWebView.mm:
+ (-[WKWebView takeSnapshotWithConfiguration:completionHandler:]):
+
2021-07-21 Alexander Mikhaylenko <[email protected]>
[GTK] Theme scrollbar css is no longer respected
Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm (280150 => 280151)
--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm 2021-07-21 18:09:17 UTC (rev 280150)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm 2021-07-21 18:11:25 UTC (rev 280151)
@@ -1176,13 +1176,13 @@
auto handler = makeBlockPtr(completionHandler);
if (CGRectIsEmpty(rectInViewCoordinates) || !snapshotWidth) {
- RunLoop::main().dispatch([completionHandler = WTFMove(completionHandler)]() mutable {
+ RunLoop::main().dispatch([handler = WTFMove(handler)] {
#if USE(APPKIT)
auto image = adoptNS([[NSImage alloc] initWithSize:NSMakeSize(0, 0)]);
#else
auto image = adoptNS([[UIImage alloc] init]);
#endif
- completionHandler(image.get(), nil);
+ handler(image.get(), nil);
});
return;
}
Modified: trunk/Tools/ChangeLog (280150 => 280151)
--- trunk/Tools/ChangeLog 2021-07-21 18:09:17 UTC (rev 280150)
+++ trunk/Tools/ChangeLog 2021-07-21 18:11:25 UTC (rev 280151)
@@ -1,3 +1,17 @@
+2021-07-21 Aditya Keerthi <[email protected]>
+
+ Crash in -[WKWebView takeSnapshotWithConfiguration:completionHandler:] when taking empty snapshots
+ https://bugs.webkit.org/show_bug.cgi?id=228134
+ rdar://80146087
+
+ Reviewed by Chris Dumez.
+
+ Added a test to verify the crash no longer occurs.
+
+ * TestWebKitAPI/Tests/WebKitCocoa/WKWebViewSnapshot.mm:
+ (-[TestSnapshotWrapper takeSnapshotWithWebView:configuration:completionHandler:]):
+ (TEST):
+
2021-07-20 Saam Barati <[email protected]>
Don't run ftl-eager-no-cjit on debug builds
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebViewSnapshot.mm (280150 => 280151)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebViewSnapshot.mm 2021-07-21 18:09:17 UTC (rev 280150)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebViewSnapshot.mm 2021-07-21 18:11:25 UTC (rev 280151)
@@ -29,6 +29,7 @@
#import "Test.h"
#import "TestNavigationDelegate.h"
#import <WebKit/WKSnapshotConfiguration.h>
+#import <wtf/BlockPtr.h>
#import <wtf/RetainPtr.h>
static bool isDone;
@@ -57,6 +58,25 @@
return (y * width + x) * 4;
}
+@interface TestSnapshotWrapper : NSObject
+@property (nonatomic, readonly) RetainPtr<CGImageRef> image;
+@property (nonatomic, readonly) RetainPtr<NSError> error;
+@end
+
+@implementation TestSnapshotWrapper
+
+- (void)takeSnapshotWithWebView:(WKWebView *)webView configuration:(WKSnapshotConfiguration *)configuration completionHandler:(void (^)(void))completionHandler
+{
+ auto completionBlock = makeBlockPtr(completionHandler);
+ [webView takeSnapshotWithConfiguration:configuration completionHandler:^(PlatformImage snapshotImage, NSError *error) {
+ _image = convertToCGImage(snapshotImage);
+ _error = error;
+ completionBlock();
+ }];
+}
+
+@end
+
TEST(WKWebView, SnapshotImageError)
{
CGFloat viewWidth = 800;
@@ -188,6 +208,39 @@
EXPECT_EQ(pid, [webView _webProcessIdentifier]); // Make sure the WebProcess did not crash.
}
+TEST(WKWebView, SnapshotImageEmptyWithOutOfScopeCompletionHandler)
+{
+ 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, 0, 0)];
+ [snapshotConfiguration setSnapshotWidth:@(viewWidth)];
+
+ isDone = false;
+
+ auto snapshotWrapper = adoptNS([[TestSnapshotWrapper alloc] init]);
+ [snapshotWrapper takeSnapshotWithWebView:webView.get() configuration:snapshotConfiguration.get() completionHandler:^{
+ isDone = true;
+ }];
+
+ TestWebKitAPI::Util::run(&isDone);
+
+ EXPECT_NULL([snapshotWrapper error]);
+
+ auto image = [snapshotWrapper image].get();
+ EXPECT_EQ(0UL, CGImageGetWidth(image));
+ EXPECT_EQ(0UL, CGImageGetHeight(image));
+
+ 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
