Title: [256297] trunk/Tools
Revision
256297
Author
wenson_hs...@apple.com
Date
2020-02-11 00:24:47 -0800 (Tue, 11 Feb 2020)

Log Message

[iOS] Non-internal API test runners frequently crash due to Objective-C exceptions
https://bugs.webkit.org/show_bug.cgi?id=207525
<rdar://problem/59110543>

Reviewed by Tim Horton.

After the fix for <rdar://problem/56301207>, some scroll view content offset changes will attempt to call into
CoreAnalytics API to try and report data about scrolling velocities. In the iOS 13.3 simulator, this involves
creating a dictionary, of which one of the keys is the bundle identifier of the application. The value is
unconditionally inserted into the dictionary. Since TestWebKitAPI does not run in the context of a
UIApplication, the bundle identifier (that is, `NSBundle.mainBundle.bundleIdentifier`) ends up being nil,
causing us to crash upon trying to create the dictionary.

While it would make things easier, we can't just swizzle -bundleIdentifier for the entirely of every test, since
some tests expect the bundle identifier to be nil (or call into system frameworks that expect the bundle
identifier to be nil). These tests fail or time out when -bundleIdentifier is unconditionally swizzled
throughout the test run. To work around this bug for the time being, simply pretend that we have a bundle
identifier when running API tests on iOS, by swizzling `-[NSBundle bundleIdentifier]` to return a string at the
beginning of each API test.

* TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* TestWebKitAPI/cocoa/TestWKWebView.mm:
(overrideBundleIdentifier):

Move this to the top of the file, so it can be shared.

(+[WKWebView initialize]):

At the start of each test, force UIKit to cache a fake value for `_UIMainBundleIdentifier()` by invoking an
internal class method that calls into the internal helper function, with no other side effects.

* TestWebKitAPI/ios/UIKitSPI.h:

Modified Paths

Diff

Modified: trunk/Tools/ChangeLog (256296 => 256297)


--- trunk/Tools/ChangeLog	2020-02-11 08:21:31 UTC (rev 256296)
+++ trunk/Tools/ChangeLog	2020-02-11 08:24:47 UTC (rev 256297)
@@ -1,3 +1,38 @@
+2020-02-11  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        [iOS] Non-internal API test runners frequently crash due to Objective-C exceptions
+        https://bugs.webkit.org/show_bug.cgi?id=207525
+        <rdar://problem/59110543>
+
+        Reviewed by Tim Horton.
+
+        After the fix for <rdar://problem/56301207>, some scroll view content offset changes will attempt to call into
+        CoreAnalytics API to try and report data about scrolling velocities. In the iOS 13.3 simulator, this involves
+        creating a dictionary, of which one of the keys is the bundle identifier of the application. The value is
+        unconditionally inserted into the dictionary. Since TestWebKitAPI does not run in the context of a
+        UIApplication, the bundle identifier (that is, `NSBundle.mainBundle.bundleIdentifier`) ends up being nil,
+        causing us to crash upon trying to create the dictionary.
+
+        While it would make things easier, we can't just swizzle -bundleIdentifier for the entirely of every test, since
+        some tests expect the bundle identifier to be nil (or call into system frameworks that expect the bundle
+        identifier to be nil). These tests fail or time out when -bundleIdentifier is unconditionally swizzled
+        throughout the test run. To work around this bug for the time being, simply pretend that we have a bundle
+        identifier when running API tests on iOS, by swizzling `-[NSBundle bundleIdentifier]` to return a string at the
+        beginning of each API test.
+
+        * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+        * TestWebKitAPI/cocoa/TestWKWebView.mm:
+        (overrideBundleIdentifier):
+
+        Move this to the top of the file, so it can be shared.
+
+        (+[WKWebView initialize]):
+
+        At the start of each test, force UIKit to cache a fake value for `_UIMainBundleIdentifier()` by invoking an
+        internal class method that calls into the internal helper function, with no other side effects.
+
+        * TestWebKitAPI/ios/UIKitSPI.h:
+
 2020-02-10  Jonathan Bedard  <jbed...@apple.com>
 
         TestWebKitAPI: Support ClipboardTests.ReadMultipleItems on Catalyst

Modified: trunk/Tools/TestWebKitAPI/cocoa/TestWKWebView.mm (256296 => 256297)


--- trunk/Tools/TestWebKitAPI/cocoa/TestWKWebView.mm	2020-02-11 08:21:31 UTC (rev 256296)
+++ trunk/Tools/TestWebKitAPI/cocoa/TestWKWebView.mm	2020-02-11 08:24:47 UTC (rev 256297)
@@ -51,6 +51,11 @@
 SOFT_LINK_FRAMEWORK(UIKit)
 SOFT_LINK_CLASS(UIKit, UIWindow)
 
+static NSString *overrideBundleIdentifier(id, SEL)
+{
+    return @"com.apple.TestWebKitAPI";
+}
+
 @implementation WKWebView (WKWebViewTestingQuirks)
 
 // TestWebKitAPI is currently not a UIApplication so we are unable to track if it is in
@@ -66,6 +71,22 @@
 
 @implementation WKWebView (TestWebKitAPI)
 
+#if PLATFORM(IOS_FAMILY)
+
++ (void)initialize
+{
+    // FIXME: This hack should no longer be necessary on builds that have the fix for <rdar://problem/56790195>.
+    // Calling +displayIdentifier will guarantee a call to an internal UIKit helper method that caches the fake
+    // bundle name "com.apple.TestWebKitAPI" for the rest of the process' lifetime. This allows us to avoid crashing
+    // under -[UIScrollView setContentOffset:animated:] due to telemetry code that requires a bundle identifier.
+    // Note that this swizzling is temporary, since unconditionally swizzling -[NSBundle bundleIdentifier] for the
+    // entirely of the test causes other tests to fail or time out.
+    InstanceMethodSwizzler bundleIdentifierSwizzler(NSBundle.class, @selector(bundleIdentifier), reinterpret_cast<IMP>(overrideBundleIdentifier));
+    [UIApplication displayIdentifier];
+}
+
+#endif // PLATFORM(IOS_FAMILY)
+
 - (void)loadTestPageNamed:(NSString *)pageName
 {
     NSURLRequest *request = [NSURLRequest requestWithURL:[[NSBundle mainBundle] URLForResource:pageName withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"]];
@@ -312,11 +333,6 @@
 static NeverDestroyed<std::unique_ptr<InstanceMethodSwizzler>> gApplicationKeyWindowSwizzler;
 static NeverDestroyed<std::unique_ptr<InstanceMethodSwizzler>> gSharedApplicationSwizzler;
 
-static NSString *overrideBundleIdentifier()
-{
-    return @"com.apple.TestWebKitAPI";
-}
-
 static void setOverriddenApplicationKeyWindow(UIWindow *window)
 {
     if (gOverriddenApplicationKeyWindow.get() == window)

Modified: trunk/Tools/TestWebKitAPI/ios/UIKitSPI.h (256296 => 256297)


--- trunk/Tools/TestWebKitAPI/ios/UIKitSPI.h	2020-02-11 08:21:31 UTC (rev 256296)
+++ trunk/Tools/TestWebKitAPI/ios/UIKitSPI.h	2020-02-11 08:24:47 UTC (rev 256297)
@@ -261,4 +261,8 @@
 - (void)_share:(id)sender;
 @end
 
+@interface UIApplication (Internal)
++ (NSString *)displayIdentifier;
+@end
+
 #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