Improve NSURLSession WebSocket message handling in case of error
authoryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 15 Feb 2020 02:15:50 +0000 (02:15 +0000)
committeryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 15 Feb 2020 02:15:50 +0000 (02:15 +0000)
https://bugs.webkit.org/show_bug.cgi?id=207799

Reviewed by Alex Christensen.

Tested by running layout tests with NSURLSession WebSocket code path enabled..

* NetworkProcess/cocoa/WebSocketTaskCocoa.mm:
(WebKit::WebSocketTask::readNextMessage):
readNextMessage completion handler may be called when the connection is closed or in case of real error.
We discriminate this case by checking closeCode and if not null, we do nothing since the connection is being closed.
Otherwise, we communicate the error to WebProcess and close the connection.
(WebKit::WebSocketTask::sendString):
Add workaround until this gets fixed underneath.
(WebKit::WebSocketTask::sendData):
Add workaround until this gets fixed underneath.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@256662 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit/ChangeLog
Source/WebKit/NetworkProcess/cocoa/WebSocketTaskCocoa.mm

index 53b8291..e6e048d 100644 (file)
@@ -1,3 +1,22 @@
+2020-02-14  Youenn Fablet  <youenn@apple.com>
+
+        Improve NSURLSession WebSocket message handling in case of error
+        https://bugs.webkit.org/show_bug.cgi?id=207799
+
+        Reviewed by Alex Christensen.
+
+        Tested by running layout tests with NSURLSession WebSocket code path enabled..
+
+        * NetworkProcess/cocoa/WebSocketTaskCocoa.mm:
+        (WebKit::WebSocketTask::readNextMessage):
+        readNextMessage completion handler may be called when the connection is closed or in case of real error.
+        We discriminate this case by checking closeCode and if not null, we do nothing since the connection is being closed.
+        Otherwise, we communicate the error to WebProcess and close the connection.
+        (WebKit::WebSocketTask::sendString):
+        Add workaround until this gets fixed underneath.
+        (WebKit::WebSocketTask::sendData):
+        Add workaround until this gets fixed underneath.
+
 2020-02-14  Jiewen Tan  <jiewen_tan@apple.com>
 
         [WebAuthn] Make Local Authenticator appear as an experimental feature
index e10451c..0b9c954 100644 (file)
@@ -54,8 +54,11 @@ void WebSocketTask::readNextMessage()
             return;
 
         if (error) {
-            // FIXME: the error code is probably not a correct WebSocket code.
-            didClose([error code], [error localizedDescription]);
+            // If closeCode is not zero, we are closing the connection and didClose will be called for us.
+            if ([m_task closeCode])
+                return;
+            m_channel.didReceiveMessageError([error localizedDescription]);
+            didClose(WebCore::WebSocketChannel::CloseEventCodeAbnormalClosure, emptyString());
             return;
         }
         if (!message) {
@@ -101,7 +104,8 @@ void WebSocketTask::sendString(const String& text , CompletionHandler<void()>&&
 {
     auto message = adoptNS([[NSURLSessionWebSocketMessage alloc] initWithString: text]);
     [m_task sendMessage: message.get() completionHandler: makeBlockPtr([callback = WTFMove(callback)](NSError * _Nullable) mutable {
-        callback();
+        // Workaround rdar://problem/55324926 until it gets fixed.
+        callOnMainRunLoop(WTFMove(callback));
     }).get()];
 }
 
@@ -110,7 +114,8 @@ void WebSocketTask::sendData(const IPC::DataReference& data, CompletionHandler<v
     auto nsData = adoptNS([[NSData alloc] initWithBytes:data.data() length:data.size()]);
     auto message = adoptNS([[NSURLSessionWebSocketMessage alloc] initWithData: nsData.get()]);
     [m_task sendMessage: message.get() completionHandler: makeBlockPtr([callback = WTFMove(callback)](NSError * _Nullable) mutable {
-        callback();
+        // Workaround rdar://problem/55324926 until it gets fixed.
+        callOnMainRunLoop(WTFMove(callback));
     }).get()];
 }