Title: [283599] trunk/Tools
Revision
283599
Author
[email protected]
Date
2021-10-05 20:47:16 -0700 (Tue, 05 Oct 2021)

Log Message

[ iOS15 ] TestWebKitAPI.ResourceLoadStatistics.DataTaskIdentifierCollision is a constant crash
https://bugs.webkit.org/show_bug.cgi?id=231246

Patch by Alex Christensen <[email protected]> on 2021-10-05
Reviewed by Chris Dumez.

For a reason that is mysterious to me, this test was timing out on iOS
in the call to synchronouslyLoadHTMLString unless I added "addToWindow:NO"
to the TestWKWebView initialization.

For a reason that is also mysterious to me, the test was crashing when closing
because of something in the autoreleasepool, but using Vector<String> instead of
RetainPtr<NSArray<NSString *>> in DataTaskIdentifierCollisionDelegate makes that
stop crashing.

I've looked quite closely and don't see why this fixes it, but I verified that it does.

While I was at it, I migrated from TCPServer to HTTPServer to be more robust against timeouts,
because the TCPServer destructor waits forever for threads to join, and if not everything is
perfect it will make the tests time out, which isn't great.  HTTPServer does everything on the
main thread with callbacks instead.

* TestWebKitAPI/Tests/WebKitCocoa/ResourceLoadStatistics.mm:
(-[DataTaskIdentifierCollisionDelegate webView:runJavaScriptAlertPanelWithMessage:initiatedByFrame:completionHandler:]):
(-[DataTaskIdentifierCollisionDelegate waitForMessages:]):
(waitUntilTwoServersConnected):
(TEST):

Modified Paths

Diff

Modified: trunk/Tools/ChangeLog (283598 => 283599)


--- trunk/Tools/ChangeLog	2021-10-06 02:06:08 UTC (rev 283598)
+++ trunk/Tools/ChangeLog	2021-10-06 03:47:16 UTC (rev 283599)
@@ -1,5 +1,34 @@
 2021-10-05  Alex Christensen  <[email protected]>
 
+        [ iOS15 ] TestWebKitAPI.ResourceLoadStatistics.DataTaskIdentifierCollision is a constant crash
+        https://bugs.webkit.org/show_bug.cgi?id=231246
+
+        Reviewed by Chris Dumez.
+
+        For a reason that is mysterious to me, this test was timing out on iOS
+        in the call to synchronouslyLoadHTMLString unless I added "addToWindow:NO"
+        to the TestWKWebView initialization.
+
+        For a reason that is also mysterious to me, the test was crashing when closing
+        because of something in the autoreleasepool, but using Vector<String> instead of
+        RetainPtr<NSArray<NSString *>> in DataTaskIdentifierCollisionDelegate makes that
+        stop crashing.
+
+        I've looked quite closely and don't see why this fixes it, but I verified that it does.
+
+        While I was at it, I migrated from TCPServer to HTTPServer to be more robust against timeouts,
+        because the TCPServer destructor waits forever for threads to join, and if not everything is
+        perfect it will make the tests time out, which isn't great.  HTTPServer does everything on the
+        main thread with callbacks instead.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/ResourceLoadStatistics.mm:
+        (-[DataTaskIdentifierCollisionDelegate webView:runJavaScriptAlertPanelWithMessage:initiatedByFrame:completionHandler:]):
+        (-[DataTaskIdentifierCollisionDelegate waitForMessages:]):
+        (waitUntilTwoServersConnected):
+        (TEST):
+
+2021-10-05  Alex Christensen  <[email protected]>
+
         Add an entitlement check to only allow AdAttributionDaemon to be connected to by the network process
         https://bugs.webkit.org/show_bug.cgi?id=231248
 

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ResourceLoadStatistics.mm (283598 => 283599)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ResourceLoadStatistics.mm	2021-10-06 02:06:08 UTC (rev 283598)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/ResourceLoadStatistics.mm	2021-10-06 03:47:16 UTC (rev 283599)
@@ -25,8 +25,8 @@
 
 #import "config.h"
 
+#import "HTTPServer.h"
 #import "PlatformUtilities.h"
-#import "TCPServer.h"
 #import "TestNavigationDelegate.h"
 #import "TestWKWebView.h"
 #import <WebKit/WKFoundation.h>
@@ -37,6 +37,7 @@
 #import <WebKit/WKWebsiteDataStorePrivate.h>
 #import <WebKit/_WKProcessPoolConfiguration.h>
 #import <WebKit/_WKWebsiteDataStoreConfiguration.h>
+#import <wtf/BlockPtr.h>
 #import <wtf/RetainPtr.h>
 #import <wtf/text/StringConcatenateNumbers.h>
 
@@ -481,25 +482,24 @@
 }
 
 @interface DataTaskIdentifierCollisionDelegate : NSObject <WKNavigationDelegate, WKUIDelegate>
-- (NSArray<NSString *> *)waitForMessages:(size_t)count;
+- (Vector<String>)waitForMessages:(size_t)count;
 @end
 
 @implementation DataTaskIdentifierCollisionDelegate {
-    RetainPtr<NSMutableArray<NSString *>> _messages;
+    Vector<String> _messages;
 }
 
 - (void)webView:(WKWebView *)webView runJavaScriptAlertPanelWithMessage:(NSString *)message initiatedByFrame:(WKFrameInfo *)frame completionHandler:(void (^)(void))completionHandler
 {
-    [_messages addObject:message];
+    _messages.append(message);
     completionHandler();
 }
 
-- (NSArray<NSString *> *)waitForMessages:(size_t)messageCount
+- (Vector<String>)waitForMessages:(size_t)messageCount
 {
-    _messages = adoptNS([NSMutableArray arrayWithCapacity:messageCount]);
-    while ([_messages count] < messageCount)
+    while (_messages.size() < messageCount)
         TestWebKitAPI::Util::spinRunLoop();
-    return _messages.autorelease();
+    return std::exchange(_messages, { });
 }
 
 - (void)webView:(WKWebView *)webView didReceiveAuthenticationChallenge:(NSURLAuthenticationChallenge *)challenge completionHandler:(void (^)(NSURLSessionAuthChallengeDisposition disposition, NSURLCredential * credential))completionHandler
@@ -509,32 +509,40 @@
 
 @end
 
-#if HAVE(SSL)
+void waitUntilTwoServersConnected(const unsigned& serversConnected, CompletionHandler<void()>&& completionHandler)
+{
+    if (serversConnected >= 2) {
+        completionHandler();
+        return;
+    }
+    dispatch_async(dispatch_get_main_queue(), makeBlockPtr([&serversConnected, completionHandler = WTFMove(completionHandler)] () mutable {
+        waitUntilTwoServersConnected(serversConnected, WTFMove(completionHandler));
+    }).get());
+}
+
 TEST(ResourceLoadStatistics, DataTaskIdentifierCollision)
 {
     using namespace TestWebKitAPI;
 
-    std::atomic<unsigned> serversConnected { 0 };
+    unsigned serversConnected { 0 };
     const char* header = "HTTP/1.1 200 OK\r\nContent-Length: 27\r\n\r\n";
 
-    TCPServer httpsServer(TCPServer::Protocol::HTTPSProxy, [&] (SSL* ssl) {
+    HTTPServer httpsServer([&] (const Connection& connection) {
         serversConnected++;
-        while (serversConnected < 2)
-            usleep(100000);
-        TCPServer::read(ssl);
-        TCPServer::write(ssl, header, strlen(header));
-        const char* body = "<script>alert('1')</script>";
-        TCPServer::write(ssl, body, strlen(body));
-    });
+        waitUntilTwoServersConnected(serversConnected, [=] {
+            connection.receiveHTTPRequest([=](Vector<char>&&) {
+                connection.send(makeString(header, "<script>alert('1')</script>"));
+            });
+        });
+    }, HTTPServer::Protocol::HttpsProxy);
 
-    TCPServer httpServer([&] (int socket) {
+    HTTPServer httpServer([&] (const Connection& connection) {
         serversConnected++;
-        while (serversConnected < 2)
-            usleep(100000);
-        TCPServer::read(socket);
-        TCPServer::write(socket, header, strlen(header));
-        const char* body = "<script>alert('2')</script>";
-        TCPServer::write(socket, body, strlen(body));
+        waitUntilTwoServersConnected(serversConnected, [=] {
+            connection.receiveHTTPRequest([=](Vector<char>&&) {
+                connection.send(makeString(header, "<script>alert('2')</script>"));
+            });
+        });
     });
 
     auto storeConfiguration = adoptNS([_WKWebsiteDataStoreConfiguration new]);
@@ -544,10 +552,10 @@
     auto viewConfiguration = adoptNS([WKWebViewConfiguration new]);
     [viewConfiguration setWebsiteDataStore:dataStore.get()];
 
-    auto webView1 = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 100, 100) configuration:viewConfiguration.get()]);
+    auto webView1 = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 100, 100) configuration:viewConfiguration.get() addToWindow:NO]);
     [webView1 synchronouslyLoadHTMLString:@"start network process and initialize data store"];
     auto delegate = adoptNS([DataTaskIdentifierCollisionDelegate new]);
-    auto webView2 = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 100, 100) configuration:viewConfiguration.get()]);
+    auto webView2 = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 100, 100) configuration:viewConfiguration.get() addToWindow:NO]);
     [webView1 setUIDelegate:delegate.get()];
     [webView1 setNavigationDelegate:delegate.get()];
     [webView2 setUIDelegate:delegate.get()];
@@ -569,18 +577,17 @@
     [webView1 loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:[NSString stringWithFormat:@"http://127.0.0.1:%d/", httpServer.port()]]]];
     [webView2 loadRequest:[NSURLRequest requestWithURL:[NSURL URLWithString:@"https://prevalent-example.com/"]]];
 
-    NSArray<NSString *> *messages = [delegate waitForMessages:2];
-    auto contains = [] (NSArray<NSString *> *array, NSString *expected) {
-        for (NSString *s in array) {
-            if ([s isEqualToString:expected])
+    auto messages = [delegate waitForMessages:2];
+    auto contains = [] (const Vector<String>& array, const char* expected) {
+        for (auto& s : array) {
+            if (s == expected)
                 return true;
         }
         return false;
     };
-    EXPECT_TRUE(contains(messages, @"1"));
-    EXPECT_TRUE(contains(messages, @"2"));
+    EXPECT_TRUE(contains(messages, "1"));
+    EXPECT_TRUE(contains(messages, "2"));
 }
-#endif // HAVE(SSL)
 
 TEST(ResourceLoadStatistics, NoMessagesWhenNotTesting)
 {
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to