[webkit-changes] [295534] trunk

Tue, 14 Jun 2022 13:53:45 -0700

Title: [295534] trunk
Revision
295534
Author
akeer...@apple.com
Date
2022-06-14 13:52:45 -0700 (Tue, 14 Jun 2022)

Log Message

CrashTracer: MobileSafari at UIKitCore: -[UITextSearchingFindSession foundRange:forSearchString:inDocument:]
https://bugs.webkit.org/show_bug.cgi?id=241602
rdar://94622715

Reviewed by Wenson Hsieh.

UIKit retains the search string passed in to
-[UITextSearchingFindSession foundRange:forSearchString:inDocument:]. However,
since WebKit returns search results asynchronously, the string may be released
by the time we inform the aggregator of the results.

To fix, retain the search string in the capture list of the search completion
handler.

* Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView performTextSearchWithQueryString:usingOptions:resultAggregator:]):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/FindInPage.mm:

Updated tests to use find interaction API rather than SPI.

Explicitly declare WKWebView's conformance to UITextSearching, since the
conformance to _UITextSearching in WKWebViewPrivate.h hides it.

_UITextSearching is still kept around for SPI clients, and will be removed
once they have migrated to the API.

(-[TestSearchAggregator foundRange:forSearchString:inDocument:]):
(-[TestSearchAggregator invalidateFoundRange:inDocument:]):
(testPerformTextSearchWithQueryStringInWebView):
(textRangesForQueryString):
(TEST):

Added an API test to verify the crash no longer occurs.

* Tools/TestWebKitAPI/ios/UIKitSPI.h:

Declared UITextSearchOptions properties as readwrite for use in testing.

Canonical link: https://commits.webkit.org/251539@main

Modified Paths

Diff

Modified: trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm (295533 => 295534)


--- trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm	2022-06-14 20:33:23 UTC (rev 295533)
+++ trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm	2022-06-14 20:52:45 UTC (rev 295534)
@@ -10434,10 +10434,10 @@
 
     // The limit matches the limit set on existing WKWebView find API.
     constexpr auto maxMatches = 1000;
-    _page->findTextRangesForStringMatches(string, findOptions, maxMatches, [string, aggregator = retainPtr(aggregator)](const Vector<WebKit::WebFoundTextRange> ranges) {
+    _page->findTextRangesForStringMatches(string, findOptions, maxMatches, [string = retainPtr(string), aggregator = retainPtr(aggregator)](const Vector<WebKit::WebFoundTextRange> ranges) {
         for (auto& range : ranges) {
             WKFoundTextRange *textRange = [WKFoundTextRange foundTextRangeWithWebFoundTextRange:range];
-            [aggregator foundRange:textRange forSearchString:string inDocument:nil];
+            [aggregator foundRange:textRange forSearchString:string.get() inDocument:nil];
         }
 
         [aggregator finishedSearching];

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/FindInPage.mm (295533 => 295534)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/FindInPage.mm	2022-06-14 20:33:23 UTC (rev 295533)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/FindInPage.mm	2022-06-14 20:52:45 UTC (rev 295534)
@@ -331,6 +331,10 @@
 
 #if HAVE(UIFINDINTERACTION)
 
+// FIXME: (rdar://95125552) Remove conformance to _UITextSearching.
+@interface WKWebView () <UITextSearching>
+@end
+
 @interface TestScrollViewDelegate : NSObject<UIScrollViewDelegate>  {
     @public bool _finishedScrolling;
 }
@@ -355,16 +359,8 @@
 
 @end
 
-@interface TestTextSearchOptions : NSObject
-@property (nonatomic) _UITextSearchMatchMethod wordMatchMethod;
-@property (nonatomic) NSStringCompareOptions stringCompareOptions;
-@end
+@interface TestSearchAggregator : NSObject <UITextSearchAggregator>
 
-@implementation TestTextSearchOptions
-@end
-
-@interface TestSearchAggregator : NSObject <_UITextSearchAggregator>
-
 @property (readonly) NSUInteger count;
 @property (nonatomic, readonly) NSOrderedSet<UITextRange *> *allFoundRanges;
 
@@ -388,8 +384,11 @@
     return self;
 }
 
-- (void)foundRange:(UITextRange *)range forSearchString:(NSString *)string inDocument:(_UITextSearchDocumentIdentifier)document
+- (void)foundRange:(UITextRange *)range forSearchString:(NSString *)string inDocument:(UITextSearchDocumentIdentifier)document
 {
+    if (!string.length)
+        return;
+
     [_foundRanges addObject:range];
 }
 
@@ -404,7 +403,7 @@
     return _foundRanges.get();
 }
 
-- (void)invalidateFoundRange:(UITextRange *)range inDocument:(_UITextSearchDocumentIdentifier)document
+- (void)invalidateFoundRange:(UITextRange *)range inDocument:(UITextSearchDocumentIdentifier)document
 {
     [_foundRanges removeObject:range];
 }
@@ -438,7 +437,7 @@
     return count;
 }
 
-static void testPerformTextSearchWithQueryStringInWebView(WKWebView *webView, NSString *query, TestTextSearchOptions *searchOptions, NSUInteger expectedMatches)
+static void testPerformTextSearchWithQueryStringInWebView(WKWebView *webView, NSString *query, UITextSearchOptions *searchOptions, NSUInteger expectedMatches)
 {
     __block bool finishedSearching = false;
     RetainPtr aggregator = adoptNS([[TestSearchAggregator alloc] initWithCompletionHandler:^{
@@ -445,8 +444,7 @@
         finishedSearching = true;
     }]);
 
-    // FIXME: (rdar://86140914) Use _UITextSearchOptions directly when the symbol is exported.
-    [webView performTextSearchWithQueryString:query usingOptions:(_UITextSearchOptions *)searchOptions resultAggregator:aggregator.get()];
+    [webView performTextSearchWithQueryString:query usingOptions:searchOptions resultAggregator:aggregator.get()];
 
     TestWebKitAPI::Util::run(&finishedSearching);
 
@@ -460,9 +458,8 @@
         finishedSearching = true;
     }]);
 
-    // FIXME: (rdar://86140914) Use _UITextSearchOptions directly when the symbol is exported.
-    auto options = adoptNS([[TestTextSearchOptions alloc] init]);
-    [webView performTextSearchWithQueryString:query usingOptions:(_UITextSearchOptions *)options.get() resultAggregator:aggregator.get()];
+    auto options = adoptNS([[UITextSearchOptions alloc] init]);
+    [webView performTextSearchWithQueryString:query usingOptions:options.get() resultAggregator:aggregator.get()];
 
     TestWebKitAPI::Util::run(&finishedSearching);
 
@@ -477,7 +474,7 @@
     [webView loadRequest:request];
     [webView _test_waitForDidFinishNavigation];
 
-    RetainPtr searchOptions = adoptNS([[TestTextSearchOptions alloc] init]);
+    RetainPtr searchOptions = adoptNS([[UITextSearchOptions alloc] init]);
     testPerformTextSearchWithQueryStringInWebView(webView.get(), @"Birthday", searchOptions.get(), 360UL);
 }
 
@@ -489,7 +486,7 @@
     [webView loadRequest:request];
     [webView _test_waitForDidFinishNavigation];
 
-    RetainPtr searchOptions = adoptNS([[TestTextSearchOptions alloc] init]);
+    RetainPtr searchOptions = adoptNS([[UITextSearchOptions alloc] init]);
     testPerformTextSearchWithQueryStringInWebView(webView.get(), @"birthday", searchOptions.get(), 0UL);
 
     [searchOptions setStringCompareOptions:NSCaseInsensitiveSearch];
@@ -504,12 +501,12 @@
     [webView loadRequest:request];
     [webView _test_waitForDidFinishNavigation];
 
-    RetainPtr searchOptions = adoptNS([[TestTextSearchOptions alloc] init]);
+    RetainPtr searchOptions = adoptNS([[UITextSearchOptions alloc] init]);
 
     testPerformTextSearchWithQueryStringInWebView(webView.get(), @"Birth", searchOptions.get(), 360UL);
     testPerformTextSearchWithQueryStringInWebView(webView.get(), @"day", searchOptions.get(), 360UL);
 
-    [searchOptions setWordMatchMethod:_UITextSearchMatchMethodStartsWith];
+    [searchOptions setWordMatchMethod:UITextSearchMatchMethodStartsWith];
 
     testPerformTextSearchWithQueryStringInWebView(webView.get(), @"Birth", searchOptions.get(), 360UL);
     testPerformTextSearchWithQueryStringInWebView(webView.get(), @"day", searchOptions.get(), 0UL);
@@ -523,16 +520,41 @@
     [webView loadRequest:request];
     [webView _test_waitForDidFinishNavigation];
 
-    RetainPtr searchOptions = adoptNS([[TestTextSearchOptions alloc] init]);
+    RetainPtr searchOptions = adoptNS([[UITextSearchOptions alloc] init]);
 
     testPerformTextSearchWithQueryStringInWebView(webView.get(), @"Birth", searchOptions.get(), 360UL);
 
-    [searchOptions setWordMatchMethod:_UITextSearchMatchMethodFullWord];
+    [searchOptions setWordMatchMethod:UITextSearchMatchMethodFullWord];
 
     testPerformTextSearchWithQueryStringInWebView(webView.get(), @"Birthday", searchOptions.get(), 360UL);
     testPerformTextSearchWithQueryStringInWebView(webView.get(), @"Birth", searchOptions.get(), 0UL);
 }
 
+TEST(WebKit, FindInPageDoNotCrashWhenUsingMutableString)
+{
+    auto webView = adoptNS([[WKWebView alloc] initWithFrame:CGRectMake(0, 0, 200, 200)]);
+
+    NSURLRequest *request = [NSURLRequest requestWithURL:[[NSBundle mainBundle] URLForResource:@"lots-of-text" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"]];
+    [webView loadRequest:request];
+    [webView _test_waitForDidFinishNavigation];
+
+    __block bool finishedSearching = false;
+    RetainPtr aggregator = adoptNS([[TestSearchAggregator alloc] initWithCompletionHandler:^{
+        finishedSearching = true;
+    }]);
+
+    {
+        RetainPtr searchString = adoptNS([[NSMutableString alloc] initWithString:@"Birthday"]);
+        RetainPtr searchOptions = adoptNS([[UITextSearchOptions alloc] init]);
+
+        [webView performTextSearchWithQueryString:searchString.get() usingOptions:searchOptions.get() resultAggregator:aggregator.get()];
+    }
+
+    TestWebKitAPI::Util::run(&finishedSearching);
+
+    EXPECT_EQ([aggregator count], 360UL);
+}
+
 TEST(WebKit, FindAndReplace)
 {
     NSString *originalContent = @"Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.";

Modified: trunk/Tools/TestWebKitAPI/ios/UIKitSPI.h (295533 => 295534)


--- trunk/Tools/TestWebKitAPI/ios/UIKitSPI.h	2022-06-14 20:33:23 UTC (rev 295533)
+++ trunk/Tools/TestWebKitAPI/ios/UIKitSPI.h	2022-06-14 20:52:45 UTC (rev 295534)
@@ -357,4 +357,11 @@
 - (void)didInsertFinalDictationResult;
 @end
 
+#if HAVE(UIFINDINTERACTION)
+@interface UITextSearchOptions ()
+@property (nonatomic, readwrite) UITextSearchMatchMethod wordMatchMethod;
+@property (nonatomic, readwrite) NSStringCompareOptions stringCompareOptions;
+@end
+#endif
+
 #endif // PLATFORM(IOS_FAMILY)

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to