Changeset 256297 in webkit

Timestamp:

Feb 11, 2020 12:24:47 AM (4 years ago)

Author:

Wenson Hsieh

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:

Location:

trunk/Tools

Files:

• ChangeLog (modified) (1 diff)
• TestWebKitAPI/cocoa/TestWKWebView.mm (modified) (3 diffs)
• TestWebKitAPI/ios/UIKitSPI.h (modified) (1 diff)

trunk/Tools/ChangeLog

@@ +1 @@
+2020-02-11  Wenson Hsieh  <wenson_hsieh@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  <jbedard@apple.com>
 

trunk/Tools/TestWebKitAPI/cocoa/TestWKWebView.mm

@@ -52 +52 @@
 SOFT_LINK_CLASS(UIKit, UIWindow)
 
+static NSString *overrideBundleIdentifier(id, SEL)
+{
+    return @"com.apple.TestWebKitAPI";
+}
+
 @implementation WKWebView (WKWebViewTestingQuirks)
 
@@ -66 +71 @@
 
 @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
@@ -312 +333 @@
 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)

trunk/Tools/TestWebKitAPI/ios/UIKitSPI.h

@@ -262 +262 @@
 @end
 
+@interface UIApplication (Internal)
++ (NSString *)displayIdentifier;
+@end
+
 #endif // PLATFORM(IOS_FAMILY)