[webkit-changes] [294413] trunk

Wed, 18 May 2022 11:41:00 -0700

Title: [294413] trunk
Revision
294413
Author
achristen...@apple.com
Date
2022-05-18 11:40:08 -0700 (Wed, 18 May 2022)

Log Message

REGRESSION (STP): WebSocket.bufferedAmount does not get updated in some cases
https://bugs.webkit.org/show_bug.cgi?id=240311

Reviewed by Youenn Fablet.

This reverts the functional part of r290995 for OSes what don't have an NSURLSession WebSocket implementation we can use.
The rest of r290995 was just moving UTF-8 encoding to a different location in the pipeline, which has no observable behavior change.

It is unfortunate, but our old implementation of WebSockets that uses CFWriteStreamWrite thinks it has written immediately,
so if I make SocketStreamHandleImpl::platformSend call m_client.didUpdateBufferedAmount even when bytesWritten >= length,
I don't have the bufferent amount updated synchronously like Chrome, Firefox, and the NSURLSession-based implementation have.
If I leave it as it is now, the buffered amount never goes back down to zero sometimes after writing to the stream.

It was broken before, it is differently broken now, and this restores the status quo on Big Sur.  This is why we can't have nice things.

* Modules/websockets/WebSocket.cpp:
(WebCore::WebSocket::send):

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

Modified Paths

Added Paths

Diff

Added: trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/Send-65K-data.any-expected.txt (0 => 294413)


--- trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/Send-65K-data.any-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/Send-65K-data.any-expected.txt	2022-05-18 18:40:08 UTC (rev 294413)
@@ -0,0 +1,3 @@
+
+FAIL Send 65K data on a WebSocket - Connection should be closed assert_equals: expected 0 but got 65000
+

Added: trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/Send-binary-65K-arraybuffer.any-expected.txt (0 => 294413)


--- trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/Send-binary-65K-arraybuffer.any-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/Send-binary-65K-arraybuffer.any-expected.txt	2022-05-18 18:40:08 UTC (rev 294413)
@@ -0,0 +1,3 @@
+
+FAIL Send 65K binary data on a WebSocket - ArrayBuffer - Connection should be closed assert_equals: expected 0 but got 65000
+

Added: trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/Send-binary-65K-arraybuffer.any.worker-expected.txt (0 => 294413)


--- trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/Send-binary-65K-arraybuffer.any.worker-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/Send-binary-65K-arraybuffer.any.worker-expected.txt	2022-05-18 18:40:08 UTC (rev 294413)
@@ -0,0 +1,3 @@
+
+FAIL Send 65K binary data on a WebSocket - ArrayBuffer - Connection should be closed assert_equals: expected 0 but got 65000
+

Added: trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/Send-binary-arraybuffer.any-expected.txt (0 => 294413)


--- trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/Send-binary-arraybuffer.any-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/Send-binary-arraybuffer.any-expected.txt	2022-05-18 18:40:08 UTC (rev 294413)
@@ -0,0 +1,3 @@
+
+FAIL Send binary data on a WebSocket - ArrayBuffer - Connection should be closed assert_equals: expected 0 but got 15
+

Added: trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/Send-binary-arraybuffer.any.worker-expected.txt (0 => 294413)


--- trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/Send-binary-arraybuffer.any.worker-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/Send-binary-arraybuffer.any.worker-expected.txt	2022-05-18 18:40:08 UTC (rev 294413)
@@ -0,0 +1,3 @@
+
+FAIL Send binary data on a WebSocket - ArrayBuffer - Connection should be closed assert_equals: expected 0 but got 15
+

Added: trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/Send-data.any-expected.txt (0 => 294413)


--- trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/Send-data.any-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/Send-data.any-expected.txt	2022-05-18 18:40:08 UTC (rev 294413)
@@ -0,0 +1,3 @@
+
+FAIL Send data on a WebSocket - Connection should be closed assert_equals: expected 0 but got 15
+

Added: trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/Send-data.any.worker-expected.txt (0 => 294413)


--- trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/Send-data.any.worker-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/Send-data.any.worker-expected.txt	2022-05-18 18:40:08 UTC (rev 294413)
@@ -0,0 +1,3 @@
+
+FAIL Send data on a WebSocket - Connection should be closed assert_equals: expected 0 but got 15
+

Added: trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/Send-paired-surrogates.any-expected.txt (0 => 294413)


--- trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/Send-paired-surrogates.any-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/Send-paired-surrogates.any-expected.txt	2022-05-18 18:40:08 UTC (rev 294413)
@@ -0,0 +1,3 @@
+
+FAIL Send paired surrogates data on a WebSocket - Connection should be closed assert_equals: expected 0 but got 4
+

Added: trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/Send-paired-surrogates.any.worker-expected.txt (0 => 294413)


--- trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/Send-paired-surrogates.any.worker-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/Send-paired-surrogates.any.worker-expected.txt	2022-05-18 18:40:08 UTC (rev 294413)
@@ -0,0 +1,3 @@
+
+FAIL Send paired surrogates data on a WebSocket - Connection should be closed assert_equals: expected 0 but got 4
+

Added: trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/Send-unicode-data.any.worker-expected.txt (0 => 294413)


--- trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/Send-unicode-data.any.worker-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/Send-unicode-data.any.worker-expected.txt	2022-05-18 18:40:08 UTC (rev 294413)
@@ -0,0 +1,3 @@
+
+FAIL Send unicode data on a WebSocket - Connection should be closed assert_equals: expected 0 but got 12
+

Added: trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/bufferedAmount-unchanged-by-sync-xhr.any.worker-expected.txt (0 => 294413)


--- trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/bufferedAmount-unchanged-by-sync-xhr.any.worker-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/bufferedAmount-unchanged-by-sync-xhr.any.worker-expected.txt	2022-05-18 18:40:08 UTC (rev 294413)
@@ -0,0 +1,3 @@
+
+FAIL bufferedAmount should not be updated during a sync XHR assert_equals: expected 5 but got 0
+

Added: trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/interfaces/WebSocket/bufferedAmount/bufferedAmount-arraybuffer-expected.txt (0 => 294413)


--- trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/interfaces/WebSocket/bufferedAmount/bufferedAmount-arraybuffer-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/interfaces/WebSocket/bufferedAmount/bufferedAmount-arraybuffer-expected.txt	2022-05-18 18:40:08 UTC (rev 294413)
@@ -0,0 +1,3 @@
+
+FAIL WebSockets: bufferedAmount for ArrayBuffer assert_equals: expected 10 but got 0
+

Added: trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/interfaces/WebSocket/bufferedAmount/bufferedAmount-blob-expected.txt (0 => 294413)


--- trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/interfaces/WebSocket/bufferedAmount/bufferedAmount-blob-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/interfaces/WebSocket/bufferedAmount/bufferedAmount-blob-expected.txt	2022-05-18 18:40:08 UTC (rev 294413)
@@ -0,0 +1,3 @@
+
+FAIL WebSockets: bufferedAmount for blob assert_equals: expected 10 but got 0
+

Modified: trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/interfaces/WebSocket/bufferedAmount/bufferedAmount-getting-expected.txt (294412 => 294413)


--- trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/interfaces/WebSocket/bufferedAmount/bufferedAmount-getting-expected.txt	2022-05-18 18:37:30 UTC (rev 294412)
+++ trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/interfaces/WebSocket/bufferedAmount/bufferedAmount-getting-expected.txt	2022-05-18 18:40:08 UTC (rev 294413)
@@ -1,3 +1,3 @@
 
-FAIL WebSockets: bufferedAmount after send()ing assert_true: bufferedAmount after received "x" expected true got false
+FAIL WebSockets: bufferedAmount after send()ing assert_equals: bufferedAmount after sent "x" expected 1 but got 0

Added: trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/interfaces/WebSocket/bufferedAmount/bufferedAmount-large-expected.txt (0 => 294413)


--- trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/interfaces/WebSocket/bufferedAmount/bufferedAmount-large-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/interfaces/WebSocket/bufferedAmount/bufferedAmount-large-expected.txt	2022-05-18 18:40:08 UTC (rev 294413)
@@ -0,0 +1,3 @@
+
+FAIL WebSockets: bufferedAmount for 65K data assert_equals: expected 0 but got 65000
+

Added: trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/interfaces/WebSocket/bufferedAmount/bufferedAmount-unicode-expected.txt (0 => 294413)


--- trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/interfaces/WebSocket/bufferedAmount/bufferedAmount-unicode-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/platform/mac-bigsur/imported/w3c/web-platform-tests/websockets/interfaces/WebSocket/bufferedAmount/bufferedAmount-unicode-expected.txt	2022-05-18 18:40:08 UTC (rev 294413)
@@ -0,0 +1,3 @@
+
+FAIL WebSockets: bufferedAmount for unicode data assert_equals: expected 0 but got 12
+

Modified: trunk/Source/WebCore/Modules/websockets/WebSocket.cpp (294412 => 294413)


--- trunk/Source/WebCore/Modules/websockets/WebSocket.cpp	2022-05-18 18:37:30 UTC (rev 294412)
+++ trunk/Source/WebCore/Modules/websockets/WebSocket.cpp	2022-05-18 18:40:08 UTC (rev 294413)
@@ -353,7 +353,15 @@
         return { };
     }
     // FIXME: WebSocketChannel also has a m_bufferedAmount. Remove that one. This one is the correct one accessed by JS.
-    m_bufferedAmount = saturateAdd(m_bufferedAmount, utf8.length());
+#if HAVE(NSURLSESSION_WEBSOCKET)
+    bool shouldSynchronouslyUpdateBufferedAmount = RuntimeEnabledFeatures::sharedFeatures().isNSURLSessionWebSocketEnabled();
+#elif PLATFORM(MAC)
+    bool shouldSynchronouslyUpdateBufferedAmount = false;
+#else
+    bool shouldSynchronouslyUpdateBufferedAmount = true;
+#endif
+    if (shouldSynchronouslyUpdateBufferedAmount)
+        m_bufferedAmount = saturateAdd(m_bufferedAmount, utf8.length());
     ASSERT(m_channel);
     m_channel->send(WTFMove(utf8));
     return { };
@@ -370,7 +378,15 @@
         m_bufferedAmountAfterClose = saturateAdd(m_bufferedAmountAfterClose, getFramingOverhead(payloadSize));
         return { };
     }
-    m_bufferedAmount = saturateAdd(m_bufferedAmount, binaryData.byteLength());
+#if HAVE(NSURLSESSION_WEBSOCKET)
+    bool shouldSynchronouslyUpdateBufferedAmount = RuntimeEnabledFeatures::sharedFeatures().isNSURLSessionWebSocketEnabled();
+#elif PLATFORM(MAC)
+    bool shouldSynchronouslyUpdateBufferedAmount = false;
+#else
+    bool shouldSynchronouslyUpdateBufferedAmount = true;
+#endif
+    if (shouldSynchronouslyUpdateBufferedAmount)
+        m_bufferedAmount = saturateAdd(m_bufferedAmount, binaryData.byteLength());
     ASSERT(m_channel);
     m_channel->send(binaryData, 0, binaryData.byteLength());
     return { };
@@ -388,7 +404,15 @@
         m_bufferedAmountAfterClose = saturateAdd(m_bufferedAmountAfterClose, getFramingOverhead(payloadSize));
         return { };
     }
-    m_bufferedAmount = saturateAdd(m_bufferedAmount, arrayBufferView.byteLength());
+#if HAVE(NSURLSESSION_WEBSOCKET)
+    bool shouldSynchronouslyUpdateBufferedAmount = RuntimeEnabledFeatures::sharedFeatures().isNSURLSessionWebSocketEnabled();
+#elif PLATFORM(MAC)
+    bool shouldSynchronouslyUpdateBufferedAmount = false;
+#else
+    bool shouldSynchronouslyUpdateBufferedAmount = true;
+#endif
+    if (shouldSynchronouslyUpdateBufferedAmount)
+        m_bufferedAmount = saturateAdd(m_bufferedAmount, arrayBufferView.byteLength());
     ASSERT(m_channel);
     m_channel->send(*arrayBufferView.unsharedBuffer(), arrayBufferView.byteOffset(), arrayBufferView.byteLength());
     return { };
@@ -405,7 +429,15 @@
         m_bufferedAmountAfterClose = saturateAdd(m_bufferedAmountAfterClose, getFramingOverhead(payloadSize));
         return { };
     }
-    m_bufferedAmount = saturateAdd(m_bufferedAmount, binaryData.size());
+#if HAVE(NSURLSESSION_WEBSOCKET)
+    bool shouldSynchronouslyUpdateBufferedAmount = RuntimeEnabledFeatures::sharedFeatures().isNSURLSessionWebSocketEnabled();
+#elif PLATFORM(MAC)
+    bool shouldSynchronouslyUpdateBufferedAmount = false;
+#else
+    bool shouldSynchronouslyUpdateBufferedAmount = true;
+#endif
+    if (shouldSynchronouslyUpdateBufferedAmount)
+        m_bufferedAmount = saturateAdd(m_bufferedAmount, binaryData.size());
     ASSERT(m_channel);
     m_channel->send(binaryData);
     return { };

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

Reply via email to