Changeset 290995 in webkit


Ignore:
Timestamp:
Mar 8, 2022 9:03:09 AM (4 months ago)
Author:
achristensen@apple.com
Message:

WebSocket.send() should synchronously update bufferedAmount
​https://bugs.webkit.org/show_bug.cgi?id=235707

Reviewed by Chris Dumez.

LayoutTests/imported/w3c:

  • web-platform-tests/websockets/Send-binary-65K-arraybuffer.any.worker-expected.txt:
  • web-platform-tests/websockets/Send-binary-arraybuffer.any.worker-expected.txt:
  • web-platform-tests/websockets/Send-data.any.worker-expected.txt:
  • web-platform-tests/websockets/Send-paired-surrogates.any.worker-expected.txt:
  • web-platform-tests/websockets/Send-unicode-data.any.worker-expected.txt:
  • web-platform-tests/websockets/bufferedAmount-unchanged-by-sync-xhr.any.worker-expected.txt:
  • web-platform-tests/websockets/interfaces/WebSocket/close/close-basic-expected.txt:
  • web-platform-tests/websockets/interfaces/WebSocket/close/close-connecting-expected.txt:

Source/WebCore:

This matches the behavior of Chrome and Firefox and is covered by web platform tests.

  • Modules/websockets/ThreadableWebSocketChannel.h:
  • Modules/websockets/WebSocket.cpp:

(WebCore::WebSocket::send):

  • Modules/websockets/WebSocketChannel.cpp:

(WebCore::WebSocketChannel::send):
(WebCore::WebSocketChannel::enqueueTextFrame):

  • Modules/websockets/WebSocketChannel.h:
  • Modules/websockets/WorkerThreadableWebSocketChannel.cpp:

(WebCore::WorkerThreadableWebSocketChannel::send):
(WebCore::WorkerThreadableWebSocketChannel::Peer::send):
(WebCore::WorkerThreadableWebSocketChannel::Bridge::send):

  • Modules/websockets/WorkerThreadableWebSocketChannel.h:

Source/WebKit:

  • WebProcess/Network/WebSocketChannel.cpp:

(WebKit::WebSocketChannel::send):

  • WebProcess/Network/WebSocketChannel.h:
Location:
trunk
Files:
15 added
32 deleted
29 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r290977 r290995  
     12022-03-08  Alex Christensen  <achristensen@webkit.org>
     2
     3        WebSocket.send() should synchronously update bufferedAmount
     4        https://bugs.webkit.org/show_bug.cgi?id=235707
     5
     6        Reviewed by Chris Dumez.
     7
     8        * web-platform-tests/websockets/Send-binary-65K-arraybuffer.any.worker-expected.txt:
     9        * web-platform-tests/websockets/Send-binary-arraybuffer.any.worker-expected.txt:
     10        * web-platform-tests/websockets/Send-data.any.worker-expected.txt:
     11        * web-platform-tests/websockets/Send-paired-surrogates.any.worker-expected.txt:
     12        * web-platform-tests/websockets/Send-unicode-data.any.worker-expected.txt:
     13        * web-platform-tests/websockets/bufferedAmount-unchanged-by-sync-xhr.any.worker-expected.txt:
     14        * web-platform-tests/websockets/interfaces/WebSocket/close/close-basic-expected.txt:
     15        * web-platform-tests/websockets/interfaces/WebSocket/close/close-connecting-expected.txt:
     16
    1172022-03-08  Chris Dumez  <cdumez@apple.com>
    218

  • trunk/LayoutTests/imported/w3c/web-platform-tests/websockets/Send-65K-data.any-expected.txt

    r279228 r290995  
    11
    2 FAIL Send 65K data on a WebSocket - Connection should be closed assert_equals: expected 0 but got 65000
     2PASS Send 65K data on a WebSocket - Connection should be closed
    33

  • trunk/LayoutTests/imported/w3c/web-platform-tests/websockets/Send-binary-65K-arraybuffer.any-expected.txt

    r279228 r290995  
    11
    2 FAIL Send 65K binary data on a WebSocket - ArrayBuffer - Connection should be closed assert_equals: expected 0 but got 65000
     2PASS Send 65K binary data on a WebSocket - ArrayBuffer - Connection should be closed
    33

  • trunk/LayoutTests/imported/w3c/web-platform-tests/websockets/Send-binary-65K-arraybuffer.any.worker-expected.txt

    r279228 r290995  
    11
    2 FAIL Send 65K binary data on a WebSocket - ArrayBuffer - Connection should be closed assert_equals: expected 0 but got 65000
     2PASS Send 65K binary data on a WebSocket - ArrayBuffer - Connection should be closed
    33

  • trunk/LayoutTests/imported/w3c/web-platform-tests/websockets/Send-binary-arraybuffer.any-expected.txt

    r279228 r290995  
    11
    2 FAIL Send binary data on a WebSocket - ArrayBuffer - Connection should be closed assert_equals: expected 0 but got 15
     2PASS Send binary data on a WebSocket - ArrayBuffer - Connection should be closed
    33

  • trunk/LayoutTests/imported/w3c/web-platform-tests/websockets/Send-binary-arraybuffer.any.worker-expected.txt

    r279228 r290995  
    11
    2 FAIL Send binary data on a WebSocket - ArrayBuffer - Connection should be closed assert_equals: expected 0 but got 15
     2PASS Send binary data on a WebSocket - ArrayBuffer - Connection should be closed
    33

  • trunk/LayoutTests/imported/w3c/web-platform-tests/websockets/Send-data.any-expected.txt

    r279228 r290995  
    11
    2 FAIL Send data on a WebSocket - Connection should be closed assert_equals: expected 0 but got 15
     2PASS Send data on a WebSocket - Connection should be closed
    33

  • trunk/LayoutTests/imported/w3c/web-platform-tests/websockets/Send-data.any.worker-expected.txt

    r279228 r290995  
    11
    2 FAIL Send data on a WebSocket - Connection should be closed assert_equals: expected 0 but got 15
     2PASS Send data on a WebSocket - Connection should be closed
    33

  • trunk/LayoutTests/imported/w3c/web-platform-tests/websockets/Send-paired-surrogates.any-expected.txt

    r279228 r290995  
    11
    2 FAIL Send paired surrogates data on a WebSocket - Connection should be closed assert_equals: expected 0 but got 4
     2PASS Send paired surrogates data on a WebSocket - Connection should be closed
    33

  • trunk/LayoutTests/imported/w3c/web-platform-tests/websockets/Send-paired-surrogates.any.worker-expected.txt

    r279228 r290995  
    11
    2 FAIL Send paired surrogates data on a WebSocket - Connection should be closed assert_equals: expected 0 but got 4
     2PASS Send paired surrogates data on a WebSocket - Connection should be closed
    33

  • trunk/LayoutTests/imported/w3c/web-platform-tests/websockets/Send-unicode-data.any.worker-expected.txt

    r279228 r290995  
    11
    2 FAIL Send unicode data on a WebSocket - Connection should be closed assert_equals: expected 0 but got 12
     2PASS Send unicode data on a WebSocket - Connection should be closed
    33

  • trunk/LayoutTests/imported/w3c/web-platform-tests/websockets/bufferedAmount-unchanged-by-sync-xhr.any.worker-expected.txt

    r252633 r290995  
    11
    2 FAIL bufferedAmount should not be updated during a sync XHR assert_equals: expected 5 but got 0
     2PASS bufferedAmount should not be updated during a sync XHR
    33

  • trunk/LayoutTests/imported/w3c/web-platform-tests/websockets/interfaces/WebSocket/bufferedAmount/bufferedAmount-arraybuffer-expected.txt

    r246406 r290995  
    11
    2 FAIL WebSockets: bufferedAmount for ArrayBuffer assert_equals: expected 10 but got 0
     2PASS WebSockets: bufferedAmount for ArrayBuffer
    33

  • trunk/LayoutTests/imported/w3c/web-platform-tests/websockets/interfaces/WebSocket/bufferedAmount/bufferedAmount-blob-expected.txt

    r246406 r290995  
    11
    2 FAIL WebSockets: bufferedAmount for blob assert_equals: expected 10 but got 0
     2PASS WebSockets: bufferedAmount for blob
    33

  • trunk/LayoutTests/imported/w3c/web-platform-tests/websockets/interfaces/WebSocket/bufferedAmount/bufferedAmount-getting-expected.txt

    r246406 r290995  
    11
    2 FAIL WebSockets: bufferedAmount after send()ing assert_equals: bufferedAmount after sent "x" expected 1 but got 0
     2PASS WebSockets: bufferedAmount after send()ing
    33

  • trunk/LayoutTests/imported/w3c/web-platform-tests/websockets/interfaces/WebSocket/bufferedAmount/bufferedAmount-large-expected.txt

    r246406 r290995  
    11
    2 FAIL WebSockets: bufferedAmount for 65K data assert_equals: expected 0 but got 65000
     2PASS WebSockets: bufferedAmount for 65K data
    33

  • trunk/LayoutTests/imported/w3c/web-platform-tests/websockets/interfaces/WebSocket/bufferedAmount/bufferedAmount-unicode-expected.txt

    r246406 r290995  
    11
    2 FAIL WebSockets: bufferedAmount for unicode data assert_equals: expected 0 but got 12
     2PASS WebSockets: bufferedAmount for unicode data
    33

  • trunk/LayoutTests/imported/w3c/web-platform-tests/websockets/interfaces/WebSocket/close/close-basic-expected.txt

    r253331 r290995  
    11
    2 FAIL WebSockets: close() assert_equals: expected 2 but got 3
     2PASS WebSockets: close()
    33

  • trunk/LayoutTests/imported/w3c/web-platform-tests/websockets/interfaces/WebSocket/close/close-connecting-expected.txt

    r253331 r290995  
    11
    2 FAIL WebSockets: close() when connecting assert_equals: expected 2 but got 3
     2PASS WebSockets: close() when connecting
    33

  • trunk/Source/WebCore/ChangeLog

    r290994 r290995  
     12022-03-08  Alex Christensen  <achristensen@webkit.org>
     2
     3        WebSocket.send() should synchronously update bufferedAmount
     4        https://bugs.webkit.org/show_bug.cgi?id=235707
     5
     6        Reviewed by Chris Dumez.
     7
     8        This matches the behavior of Chrome and Firefox and is covered by web platform tests.
     9
     10        * Modules/websockets/ThreadableWebSocketChannel.h:
     11        * Modules/websockets/WebSocket.cpp:
     12        (WebCore::WebSocket::send):
     13        * Modules/websockets/WebSocketChannel.cpp:
     14        (WebCore::WebSocketChannel::send):
     15        (WebCore::WebSocketChannel::enqueueTextFrame):
     16        * Modules/websockets/WebSocketChannel.h:
     17        * Modules/websockets/WorkerThreadableWebSocketChannel.cpp:
     18        (WebCore::WorkerThreadableWebSocketChannel::send):
     19        (WebCore::WorkerThreadableWebSocketChannel::Peer::send):
     20        (WebCore::WorkerThreadableWebSocketChannel::Bridge::send):
     21        * Modules/websockets/WorkerThreadableWebSocketChannel.h:
     22
    1232022-03-08  Kate Cheney  <katherine_cheney@apple.com>
    224

  • trunk/Source/WebCore/Modules/websockets/ThreadableWebSocketChannel.h

    r290856 r290995  
    7373
    7474    enum SendResult { SendSuccess, SendFail };
    75     virtual SendResult send(const String& message) = 0;
     75    virtual SendResult send(CString&&) = 0;
    7676    virtual SendResult send(const JSC::ArrayBuffer&, unsigned byteOffset, unsigned byteLength) = 0;
    7777    virtual SendResult send(Blob&) = 0;

  • trunk/Source/WebCore/Modules/websockets/WebSocket.cpp

    r290856 r290995  
    344344    if (m_state == CONNECTING)
    345345        return Exception { InvalidStateError };
     346    auto utf8 = message.utf8(StrictConversionReplacingUnpairedSurrogatesWithFFFD);
    346347    // No exception is raised if the connection was once established but has subsequently been closed.
    347348    if (m_state == CLOSING || m_state == CLOSED) {
    348         size_t payloadSize = message.utf8().length();
     349        size_t payloadSize = utf8.length();
    349350        m_bufferedAmountAfterClose = saturateAdd(m_bufferedAmountAfterClose, payloadSize);
    350351        m_bufferedAmountAfterClose = saturateAdd(m_bufferedAmountAfterClose, getFramingOverhead(payloadSize));
    351352        return { };
    352353    }
     354    // FIXME: WebSocketChannel also has a m_bufferedAmount. Remove that one. This one is the correct one accessed by JS.
     355    m_bufferedAmount = saturateAdd(m_bufferedAmount, utf8.length());
    353356    ASSERT(m_channel);
    354     m_channel->send(message);
     357    m_channel->send(WTFMove(utf8));
    355358    return { };
    356359}
    … …  
    367370        return { };
    368371    }
     372    m_bufferedAmount = saturateAdd(m_bufferedAmount, binaryData.byteLength());
    369373    ASSERT(m_channel);
    370374    m_channel->send(binaryData, 0, binaryData.byteLength());
    … …  
    384388        return { };
    385389    }
     390    m_bufferedAmount = saturateAdd(m_bufferedAmount, arrayBufferView.byteLength());
    386391    ASSERT(m_channel);
    387392    m_channel->send(*arrayBufferView.unsharedBuffer(), arrayBufferView.byteOffset(), arrayBufferView.byteLength());
    … …  
    400405        return { };
    401406    }
     407    m_bufferedAmount = saturateAdd(m_bufferedAmount, binaryData.size());
    402408    ASSERT(m_channel);
    403409    m_channel->send(binaryData);

  • trunk/Source/WebCore/Modules/websockets/WebSocketChannel.cpp

    r290856 r290995  
    143143}
    144144
    145 ThreadableWebSocketChannel::SendResult WebSocketChannel::send(const String& message)
     145ThreadableWebSocketChannel::SendResult WebSocketChannel::send(CString&& message)
    146146{
    147147    if (m_outgoingFrameQueueStatus != OutgoingFrameQueueOpen)
    148148        return ThreadableWebSocketChannel::SendSuccess;
    149149
    150     LOG(Network, "WebSocketChannel %p send() Sending String '%s'", this, message.utf8().data());
    151     CString utf8 = message.utf8(StrictConversionReplacingUnpairedSurrogatesWithFFFD);
    152     enqueueTextFrame(utf8);
     150    LOG(Network, "WebSocketChannel %p send() Sending String '%s'", this, message.data());
     151    enqueueTextFrame(WTFMove(message));
    153152    processOutgoingFrameQueue();
    154153    // According to WebSocket API specification, WebSocket.send() should return void instead
    … …  
    720719}
    721720
    722 void WebSocketChannel::enqueueTextFrame(const CString& string)
     721void WebSocketChannel::enqueueTextFrame(CString&& string)
    723722{
    724723    ASSERT(m_outgoingFrameQueueStatus == OutgoingFrameQueueOpen);
    … …  
    726725    frame->opCode = WebSocketFrame::OpCodeText;
    727726    frame->frameType = QueuedFrameTypeString;
    728     frame->stringData = string;
     727    frame->stringData = WTFMove(string);
    729728    m_outgoingFrameQueue.append(WTFMove(frame));
    730729}

  • trunk/Source/WebCore/Modules/websockets/WebSocketChannel.h

    r290856 r290995  
    7575    String subprotocol() final;
    7676    String extensions() final;
    77     ThreadableWebSocketChannel::SendResult send(const String& message) final;
     77    ThreadableWebSocketChannel::SendResult send(CString&&) final;
    7878    ThreadableWebSocketChannel::SendResult send(const JSC::ArrayBuffer&, unsigned byteOffset, unsigned byteLength) final;
    7979    ThreadableWebSocketChannel::SendResult send(Blob&) final;
    … …  
    168168        RefPtr<Blob> blobData;
    169169    };
    170     void enqueueTextFrame(const CString&);
     170    void enqueueTextFrame(CString&&);
    171171    void enqueueRawFrame(WebSocketFrame::OpCode, const uint8_t* data, size_t dataLength);
    172172    void enqueueBlobFrame(WebSocketFrame::OpCode, Blob&);

  • trunk/Source/WebCore/Modules/websockets/WorkerThreadableWebSocketChannel.cpp

    r290901 r290995  
    8585}
    8686
    87 ThreadableWebSocketChannel::SendResult WorkerThreadableWebSocketChannel::send(const String& message)
     87ThreadableWebSocketChannel::SendResult WorkerThreadableWebSocketChannel::send(CString&& message)
    8888{
    8989    if (!m_bridge)
    9090        return ThreadableWebSocketChannel::SendFail;
    91     return m_bridge->send(message);
     91    return m_bridge->send(WTFMove(message));
    9292}
    9393
    … …  
    169169}
    170170
    171 void WorkerThreadableWebSocketChannel::Peer::send(const String& message)
    172 {
    173     ASSERT(isMainThread());
    174     if (!m_mainWebSocketChannel)
    175         return;
    176 
    177     ThreadableWebSocketChannel::SendResult sendRequestResult = m_mainWebSocketChannel->send(message);
     171void WorkerThreadableWebSocketChannel::Peer::send(CString&& message)
     172{
     173    ASSERT(isMainThread());
     174    if (!m_mainWebSocketChannel)
     175        return;
     176
     177    ThreadableWebSocketChannel::SendResult sendRequestResult = m_mainWebSocketChannel->send(WTFMove(message));
    178178    m_loaderProxy.postTaskForModeToWorkerOrWorkletGlobalScope([workerClientWrapper = m_workerClientWrapper, sendRequestResult](ScriptExecutionContext&) mutable {
    179179        workerClientWrapper->setSendRequestResult(sendRequestResult);
    … …  
    425425}
    426426
    427 ThreadableWebSocketChannel::SendResult WorkerThreadableWebSocketChannel::Bridge::send(const String& message)
     427ThreadableWebSocketChannel::SendResult WorkerThreadableWebSocketChannel::Bridge::send(CString&& message)
    428428{
    429429    if (!m_peer)
    … …  
    431431    setMethodNotCompleted();
    432432
    433     m_loaderProxy.postTaskToLoader([peer = m_peer, message = message.isolatedCopy()](ScriptExecutionContext& context) {
    434         ASSERT(isMainThread());
    435         ASSERT_UNUSED(context, context.isDocument());
    436         ASSERT(peer);
    437 
    438         peer->send(message);
     433    m_loaderProxy.postTaskToLoader([peer = m_peer, message = WTFMove(message)](ScriptExecutionContext& context) mutable {
     434        ASSERT(isMainThread());
     435        ASSERT_UNUSED(context, context.isDocument());
     436        ASSERT(peer);
     437
     438        peer->send(WTFMove(message));
    439439    });
    440440

  • trunk/Source/WebCore/Modules/websockets/WorkerThreadableWebSocketChannel.h

    r290856 r290995  
    5959    String subprotocol() final;
    6060    String extensions() final;
    61     ThreadableWebSocketChannel::SendResult send(const String& message) final;
     61    ThreadableWebSocketChannel::SendResult send(CString&&) final;
    6262    ThreadableWebSocketChannel::SendResult send(const JSC::ArrayBuffer&, unsigned byteOffset, unsigned byteLength) final;
    6363    ThreadableWebSocketChannel::SendResult send(Blob&) final;
    … …  
    7878
    7979        ConnectStatus connect(const URL&, const String& protocol);
    80         void send(const String& message);
     80        void send(CString&&);
    8181        void send(const JSC::ArrayBuffer&);
    8282        void send(Blob&);
    … …  
    122122        void initialize();
    123123        void connect(const URL&, const String& protocol);
    124         ThreadableWebSocketChannel::SendResult send(const String& message);
     124        ThreadableWebSocketChannel::SendResult send(CString&&);
    125125        ThreadableWebSocketChannel::SendResult send(const JSC::ArrayBuffer&, unsigned byteOffset, unsigned byteLength);
    126126        ThreadableWebSocketChannel::SendResult send(Blob&);

  • trunk/Source/WebKit/ChangeLog

    r290989 r290995  
     12022-03-08  Alex Christensen  <achristensen@webkit.org>
     2
     3        WebSocket.send() should synchronously update bufferedAmount
     4        https://bugs.webkit.org/show_bug.cgi?id=235707
     5
     6        Reviewed by Chris Dumez.
     7
     8        * WebProcess/Network/WebSocketChannel.cpp:
     9        (WebKit::WebSocketChannel::send):
     10        * WebProcess/Network/WebSocketChannel.h:
     11
    1122022-03-08  Alex Christensen  <achristensen@webkit.org>
    213

  • trunk/Source/WebKit/WebProcess/Network/WebSocketChannel.cpp

    r290856 r290995  
    160160}
    161161
    162 WebSocketChannel::SendResult WebSocketChannel::send(const String& message)
    163 {
    164     auto utf8 = message.utf8(StrictConversionReplacingUnpairedSurrogatesWithFFFD);
    165     if (!increaseBufferedAmount(utf8.length()))
     162WebSocketChannel::SendResult WebSocketChannel::send(CString&& message)
     163{
     164    if (!increaseBufferedAmount(message.length()))
    166165        return SendFail;
    167166
    168     m_messageQueue.enqueue(WTFMove(utf8));
     167    m_messageQueue.enqueue(WTFMove(message));
    169168    return SendSuccess;
    170169}

  • trunk/Source/WebKit/WebProcess/Network/WebSocketChannel.h

    r290856 r290995  
    6666    String subprotocol() final;
    6767    String extensions() final;
    68     SendResult send(const String& message) final;
     68    SendResult send(CString&&) final;
    6969    SendResult send(const JSC::ArrayBuffer&, unsigned byteOffset, unsigned byteLength) final;
    7070    SendResult send(WebCore::Blob&) final;
Note: See TracChangeset for help on using the changeset viewer.