Title: [286522] trunk
Revision
286522
Author
wenson_hs...@apple.com
Date
2021-12-03 17:35:53 -0800 (Fri, 03 Dec 2021)

Log Message

[iOS] Web content process sometimes crashes under WebPage::positionInformation()
https://bugs.webkit.org/show_bug.cgi?id=233841
rdar://85917212

Reviewed by Geoffrey Garen.

Source/WebKit:

Add a missing null check in the case where the hit-tested node is null in `WebPage::positionInformation`. This
can happen in a number of ways (one of which is exercised by the new API test).

Test: ImageAnalysisTests.DoNotCrashWhenHitTestingOutsideOfWebView

* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::positionInformation):

Tools:

Add a test that exercises hit-testing for Live Text, but induces this crash by hit-testing outside of the bounds
of the web view (such that the hit-tested node ends up as null).

* TestWebKitAPI/Tests/WebKitCocoa/ImageAnalysisTests.mm:
(TestWebKitAPI::swizzledLocationInView):
(TestWebKitAPI::TEST):

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (286521 => 286522)


--- trunk/Source/WebKit/ChangeLog	2021-12-04 01:14:53 UTC (rev 286521)
+++ trunk/Source/WebKit/ChangeLog	2021-12-04 01:35:53 UTC (rev 286522)
@@ -1,3 +1,19 @@
+2021-12-03  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        [iOS] Web content process sometimes crashes under WebPage::positionInformation()
+        https://bugs.webkit.org/show_bug.cgi?id=233841
+        rdar://85917212
+
+        Reviewed by Geoffrey Garen.
+
+        Add a missing null check in the case where the hit-tested node is null in `WebPage::positionInformation`. This
+        can happen in a number of ways (one of which is exercised by the new API test).
+
+        Test: ImageAnalysisTests.DoNotCrashWhenHitTestingOutsideOfWebView
+
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::WebPage::positionInformation):
+
 2021-12-03  John Wilander  <wilan...@apple.com>
 
         PCM: Unlinkable tokens for triggering event to prevent fraud

Modified: trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm (286521 => 286522)


--- trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm	2021-12-04 01:14:53 UTC (rev 286521)
+++ trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm	2021-12-04 01:35:53 UTC (rev 286522)
@@ -3141,7 +3141,7 @@
             info.image = shareableBitmapSnapshotForNode(element);
     }
 
-    if (!info.isImage && request.includeImageData) {
+    if (!info.isImage && request.includeImageData && hitTestNode) {
         if (auto video = hostVideoElementIgnoringImageOverlay(*hitTestNode))
             videoPositionInformation(*this, *video, request, info);
         else if (is<HTMLImageElement>(hitTestNode))

Modified: trunk/Tools/ChangeLog (286521 => 286522)


--- trunk/Tools/ChangeLog	2021-12-04 01:14:53 UTC (rev 286521)
+++ trunk/Tools/ChangeLog	2021-12-04 01:35:53 UTC (rev 286522)
@@ -1,3 +1,18 @@
+2021-12-03  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        [iOS] Web content process sometimes crashes under WebPage::positionInformation()
+        https://bugs.webkit.org/show_bug.cgi?id=233841
+        rdar://85917212
+
+        Reviewed by Geoffrey Garen.
+
+        Add a test that exercises hit-testing for Live Text, but induces this crash by hit-testing outside of the bounds
+        of the web view (such that the hit-tested node ends up as null).
+
+        * TestWebKitAPI/Tests/WebKitCocoa/ImageAnalysisTests.mm:
+        (TestWebKitAPI::swizzledLocationInView):
+        (TestWebKitAPI::TEST):
+
 2021-12-03  John Wilander  <wilan...@apple.com>
 
         PCM: Unlinkable tokens for triggering event to prevent fraud

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ImageAnalysisTests.mm (286521 => 286522)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ImageAnalysisTests.mm	2021-12-04 01:14:53 UTC (rev 286521)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ImageAnalysisTests.mm	2021-12-04 01:35:53 UTC (rev 286522)
@@ -80,9 +80,10 @@
 
 #if PLATFORM(IOS_FAMILY)
 
+static CGPoint gSwizzledLocationInView = CGPoint { 100, 100 };
 static CGPoint swizzledLocationInView(id, SEL, UIView *)
 {
-    return CGPointMake(100, 100);
+    return gSwizzledLocationInView;
 }
 
 static void swizzledProcessRequestWithError(id, SEL, VKImageAnalyzerRequest *, void (^)(double progress), void (^completion)(VKImageAnalysis *analysis, NSError *error))
@@ -93,7 +94,7 @@
 
 TEST(ImageAnalysisTests, DoNotAnalyzeImagesInEditableContent)
 {
-    InstanceMethodSwizzler gestureLocationSwizzler { UIGestureRecognizer.class, @selector(locationInView:), reinterpret_cast<IMP>(swizzledLocationInView) };
+    InstanceMethodSwizzler gestureLocationSwizzler { UILongPressGestureRecognizer.class, @selector(locationInView:), reinterpret_cast<IMP>(swizzledLocationInView) };
     InstanceMethodSwizzler imageAnalysisRequestSwizzler { PAL::getVKImageAnalyzerClass(), @selector(processRequest:progressHandler:completionHandler:), reinterpret_cast<IMP>(swizzledProcessRequestWithError) };
 
     auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 400, 400)]);
@@ -107,7 +108,7 @@
 
 TEST(ImageAnalysisTests, HandleImageAnalyzerError)
 {
-    InstanceMethodSwizzler gestureLocationSwizzler { UIGestureRecognizer.class, @selector(locationInView:), reinterpret_cast<IMP>(swizzledLocationInView) };
+    InstanceMethodSwizzler gestureLocationSwizzler { UILongPressGestureRecognizer.class, @selector(locationInView:), reinterpret_cast<IMP>(swizzledLocationInView) };
     InstanceMethodSwizzler imageAnalysisRequestSwizzler { PAL::getVKImageAnalyzerClass(), @selector(processRequest:progressHandler:completionHandler:), reinterpret_cast<IMP>(swizzledProcessRequestWithError) };
 
     auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 400, 400)]);
@@ -118,6 +119,21 @@
     EXPECT_EQ(gDidProcessRequestCount, 1U);
 }
 
+TEST(ImageAnalysisTests, DoNotCrashWhenHitTestingOutsideOfWebView)
+{
+    InstanceMethodSwizzler gestureLocationSwizzler { UILongPressGestureRecognizer.class, @selector(locationInView:), reinterpret_cast<IMP>(swizzledLocationInView) };
+    InstanceMethodSwizzler imageAnalysisRequestSwizzler { PAL::getVKImageAnalyzerClass(), @selector(processRequest:progressHandler:completionHandler:), reinterpret_cast<IMP>(swizzledProcessRequestWithError) };
+
+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 400, 400)]);
+    [webView synchronouslyLoadTestPageNamed:@"image"];
+
+    gSwizzledLocationInView = CGPointMake(500, 500);
+    [webView _imageAnalysisGestureRecognizer].state = UIGestureRecognizerStateBegan;
+    [webView waitForNextPresentationUpdate];
+    [webView expectElementCount:1 querySelector:@"img"];
+    EXPECT_EQ(gDidProcessRequestCount, 0U);
+}
+
 #endif // PLATFORM(IOS_FAMILY)
 
 TEST(ImageAnalysisTests, StartImageAnalysisWithoutIdentifier)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to