<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[284472] trunk</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/284472">284472</a></dd>
<dt>Author</dt> <dd>youenn@apple.com</dd>
<dt>Date</dt> <dd>2021-10-19 11:41:13 -0700 (Tue, 19 Oct 2021)</dd>
</dl>

<h3>Log Message</h3>
<pre>Guarantee order of WebSocket events in case of being resumed
https://bugs.webkit.org/show_bug.cgi?id=231664

Reviewed by Chris Dumez.

LayoutTests/imported/w3c:

* web-platform-tests/websockets/interfaces/WebSocket/close/close-nested-expected.txt:
* web-platform-tests/websockets/interfaces/WebSocket/readyState/003-expected.txt:

Source/WebCore:

Introduce a WebSocket task source as per spec.
This aligns with https://html.spec.whatwg.org/multipage/web-sockets.html where the user agent is expected to a queue task
when closing handshake is started and so on.

By queuing an event loop task to fire WebSocket events, we ensure ordering of events even in case of resuming, while simplifying the implementation.
This makes it use the event loop which is extra nice if we resolve promises in event listeners.

A follow-up patch should refactor code so that WebSocket::didReceiveMessageError can directly fire the close event without having to wait for a didClose call.

Covered by existing tests.

* Modules/websockets/WebSocket.cpp:
(WebCore::WebSocket::WebSocket):
(WebCore::WebSocket::suspend):
(WebCore::WebSocket::resume):
(WebCore::WebSocket::stop):
(WebCore::WebSocket::dispatchOrQueueEvent):
(WebCore::WebSocket::resumeTimerFired): Deleted.
* Modules/websockets/WebSocket.h:
* Modules/websockets/WebSocketChannel.cpp:
(WebCore::WebSocketChannel::send):
* dom/TaskSource.h:

Source/WebKit:

We no longer need to handle resume/suspend in WebSocketChannel layer since WebSocket will deal with itself by enqueuing a task in the event loop.

* WebProcess/Network/WebSocketChannel.cpp:
(WebKit::WebSocketChannel::disconnect):
(WebKit::WebSocketChannel::didConnect):
(WebKit::WebSocketChannel::didReceiveText):
(WebKit::WebSocketChannel::didReceiveBinaryData):
(WebKit::WebSocketChannel::didClose):
(WebKit::WebSocketChannel::didReceiveMessageError):
(WebKit::WebSocketChannel::suspend):
(WebKit::WebSocketChannel::resume):
(WebKit::WebSocketChannel::didSendHandshakeRequest):
(WebKit::WebSocketChannel::didReceiveHandshakeResponse):
(WebKit::WebSocketChannel::enqueueTask): Deleted.
* WebProcess/Network/WebSocketChannel.h:

LayoutTests:

* http/tests/websocket/tests/hybi/inspector/send-and-receive.html:
The WebSocket server was racing to close the connection with the User Agent.
The patch queueing a task to fire events is further amplifying this race.
To make the test non flaky, we use an echo server that waits for User Agent to close the connection.</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkLayoutTestshttptestswebsockettestshybiinspectorsendandreceivehtml">trunk/LayoutTests/http/tests/websocket/tests/hybi/inspector/send-and-receive.html</a></li>
<li><a href="#trunkLayoutTestsimportedw3cChangeLog">trunk/LayoutTests/imported/w3c/ChangeLog</a></li>
<li><a href="#trunkLayoutTestsimportedw3cwebplatformtestswebsocketsinterfacesWebSocketcloseclosenestedexpectedtxt">trunk/LayoutTests/imported/w3c/web-platform-tests/websockets/interfaces/WebSocket/close/close-nested-expected.txt</a></li>
<li><a href="#trunkLayoutTestsimportedw3cwebplatformtestswebsocketsinterfacesWebSocketreadyState003expectedtxt">trunk/LayoutTests/imported/w3c/web-platform-tests/websockets/interfaces/WebSocket/readyState/003-expected.txt</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreModuleswebsocketsWebSocketcpp">trunk/Source/WebCore/Modules/websockets/WebSocket.cpp</a></li>
<li><a href="#trunkSourceWebCoreModuleswebsocketsWebSocketh">trunk/Source/WebCore/Modules/websockets/WebSocket.h</a></li>
<li><a href="#trunkSourceWebCoreModuleswebsocketsWebSocketChannelcpp">trunk/Source/WebCore/Modules/websockets/WebSocketChannel.cpp</a></li>
<li><a href="#trunkSourceWebCoredomTaskSourceh">trunk/Source/WebCore/dom/TaskSource.h</a></li>
<li><a href="#trunkSourceWebKitChangeLog">trunk/Source/WebKit/ChangeLog</a></li>
<li><a href="#trunkSourceWebKitWebProcessNetworkWebSocketChannelcpp">trunk/Source/WebKit/WebProcess/Network/WebSocketChannel.cpp</a></li>
<li><a href="#trunkSourceWebKitWebProcessNetworkWebSocketChannelh">trunk/Source/WebKit/WebProcess/Network/WebSocketChannel.h</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (284471 => 284472)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog      2021-10-19 18:40:20 UTC (rev 284471)
+++ trunk/LayoutTests/ChangeLog 2021-10-19 18:41:13 UTC (rev 284472)
</span><span class="lines">@@ -1,3 +1,15 @@
</span><ins>+2021-10-19  Youenn Fablet  <youenn@apple.com>
+
+        Guarantee order of WebSocket events in case of being resumed
+        https://bugs.webkit.org/show_bug.cgi?id=231664
+
+        Reviewed by Chris Dumez.
+
+        * http/tests/websocket/tests/hybi/inspector/send-and-receive.html:
+        The WebSocket server was racing to close the connection with the User Agent.
+        The patch queueing a task to fire events is further amplifying this race.
+        To make the test non flaky, we use an echo server that waits for User Agent to close the connection.
+
</ins><span class="cx"> 2021-10-19  Ayumi Kojima  <ayumi_kojima@apple.com>
</span><span class="cx"> 
</span><span class="cx">         [ iOS ] http/tests/cache/disk-cache/redirect-chain-limits.html is a flaky timeout.
</span></span></pre></div>
<a id="trunkLayoutTestshttptestswebsockettestshybiinspectorsendandreceivehtml"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/http/tests/websocket/tests/hybi/inspector/send-and-receive.html (284471 => 284472)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/http/tests/websocket/tests/hybi/inspector/send-and-receive.html        2021-10-19 18:40:20 UTC (rev 284471)
+++ trunk/LayoutTests/http/tests/websocket/tests/hybi/inspector/send-and-receive.html   2021-10-19 18:41:13 UTC (rev 284472)
</span><span class="lines">@@ -9,7 +9,7 @@
</span><span class="cx"> 
</span><span class="cx"> function createWebSocketConnection()
</span><span class="cx"> {
</span><del>-    webSocket = new WebSocket("ws://127.0.0.1:8880/websocket/tests/hybi/inspector/echo");
</del><ins>+    webSocket = new WebSocket("ws://127.0.0.1:8880/websocket/tests/hybi/echo");
</ins><span class="cx"> 
</span><span class="cx">     webSocket.onopen = function()
</span><span class="cx">     {
</span></span></pre></div>
<a id="trunkLayoutTestsimportedw3cChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/imported/w3c/ChangeLog (284471 => 284472)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/imported/w3c/ChangeLog 2021-10-19 18:40:20 UTC (rev 284471)
+++ trunk/LayoutTests/imported/w3c/ChangeLog    2021-10-19 18:41:13 UTC (rev 284472)
</span><span class="lines">@@ -1,3 +1,13 @@
</span><ins>+2021-10-19  Youenn Fablet  <youenn@apple.com>
+
+        Guarantee order of WebSocket events in case of being resumed
+        https://bugs.webkit.org/show_bug.cgi?id=231664
+
+        Reviewed by Chris Dumez.
+
+        * web-platform-tests/websockets/interfaces/WebSocket/close/close-nested-expected.txt:
+        * web-platform-tests/websockets/interfaces/WebSocket/readyState/003-expected.txt:
+
</ins><span class="cx"> 2021-10-19  Antoine Quint  <graouts@webkit.org>
</span><span class="cx"> 
</span><span class="cx">         REGRESSION(r284313): ::marker accelerated animations are broken
</span></span></pre></div>
<a id="trunkLayoutTestsimportedw3cwebplatformtestswebsocketsinterfacesWebSocketcloseclosenestedexpectedtxt"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/websockets/interfaces/WebSocket/close/close-nested-expected.txt (284471 => 284472)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/imported/w3c/web-platform-tests/websockets/interfaces/WebSocket/close/close-nested-expected.txt        2021-10-19 18:40:20 UTC (rev 284471)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/websockets/interfaces/WebSocket/close/close-nested-expected.txt   2021-10-19 18:41:13 UTC (rev 284472)
</span><span class="lines">@@ -1,3 +1,3 @@
</span><span class="cx"> 
</span><del>-FAIL WebSockets: close() in close event handler assert_equals: expected 2 but got 3
</del><ins>+PASS WebSockets: close() in close event handler
</ins><span class="cx"> 
</span></span></pre></div>
<a id="trunkLayoutTestsimportedw3cwebplatformtestswebsocketsinterfacesWebSocketreadyState003expectedtxt"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/websockets/interfaces/WebSocket/readyState/003-expected.txt (284471 => 284472)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/imported/w3c/web-platform-tests/websockets/interfaces/WebSocket/readyState/003-expected.txt    2021-10-19 18:40:20 UTC (rev 284471)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/websockets/interfaces/WebSocket/readyState/003-expected.txt       2021-10-19 18:41:13 UTC (rev 284472)
</span><span class="lines">@@ -1,3 +1,3 @@
</span><span class="cx"> 
</span><del>-FAIL WebSockets: delete readyState assert_equals: delete ws.readyState expected 2 but got 3
</del><ins>+PASS WebSockets: delete readyState
</ins><span class="cx"> 
</span></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (284471 => 284472)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog   2021-10-19 18:40:20 UTC (rev 284471)
+++ trunk/Source/WebCore/ChangeLog      2021-10-19 18:41:13 UTC (rev 284472)
</span><span class="lines">@@ -1,3 +1,33 @@
</span><ins>+2021-10-19  Youenn Fablet  <youenn@apple.com>
+
+        Guarantee order of WebSocket events in case of being resumed
+        https://bugs.webkit.org/show_bug.cgi?id=231664
+
+        Reviewed by Chris Dumez.
+
+        Introduce a WebSocket task source as per spec.
+        This aligns with https://html.spec.whatwg.org/multipage/web-sockets.html where the user agent is expected to a queue task
+        when closing handshake is started and so on.
+
+        By queuing an event loop task to fire WebSocket events, we ensure ordering of events even in case of resuming, while simplifying the implementation.
+        This makes it use the event loop which is extra nice if we resolve promises in event listeners.
+
+        A follow-up patch should refactor code so that WebSocket::didReceiveMessageError can directly fire the close event without having to wait for a didClose call.
+
+        Covered by existing tests.
+
+        * Modules/websockets/WebSocket.cpp:
+        (WebCore::WebSocket::WebSocket):
+        (WebCore::WebSocket::suspend):
+        (WebCore::WebSocket::resume):
+        (WebCore::WebSocket::stop):
+        (WebCore::WebSocket::dispatchOrQueueEvent):
+        (WebCore::WebSocket::resumeTimerFired): Deleted.
+        * Modules/websockets/WebSocket.h:
+        * Modules/websockets/WebSocketChannel.cpp:
+        (WebCore::WebSocketChannel::send):
+        * dom/TaskSource.h:
+
</ins><span class="cx"> 2021-10-19  Antoine Quint  <graouts@webkit.org>
</span><span class="cx"> 
</span><span class="cx">         REGRESSION(r284313): ::marker accelerated animations are broken
</span></span></pre></div>
<a id="trunkSourceWebCoreModuleswebsocketsWebSocketcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/Modules/websockets/WebSocket.cpp (284471 => 284472)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/Modules/websockets/WebSocket.cpp    2021-10-19 18:40:20 UTC (rev 284471)
+++ trunk/Source/WebCore/Modules/websockets/WebSocket.cpp       2021-10-19 18:41:13 UTC (rev 284472)
</span><span class="lines">@@ -145,7 +145,6 @@
</span><span class="cx">     : ActiveDOMObject(&context)
</span><span class="cx">     , m_subprotocol(emptyString())
</span><span class="cx">     , m_extensions(emptyString())
</span><del>-    , m_resumeTimer(*this, &WebSocket::resumeTimerFired)
</del><span class="cx"> {
</span><span class="cx">     Locker locker { allActiveWebSocketsLock() };
</span><span class="cx">     allActiveWebSockets().add(this);
</span><span class="lines">@@ -210,15 +209,13 @@
</span><span class="cx"> 
</span><span class="cx"> void WebSocket::failAsynchronously()
</span><span class="cx"> {
</span><del>-    m_pendingActivity = makePendingActivity(*this);
</del><ins>+    queueTaskKeepingObjectAlive(*this, TaskSource::WebSocket, [this] {
+        // We must block this connection. Instead of throwing an exception, we indicate this
+        // using the error event. But since this code executes as part of the WebSocket's
+        // constructor, we have to wait until the constructor has completed before firing the
+        // event; otherwise, users can't connect to the event.
</ins><span class="cx"> 
</span><del>-    // We must block this connection. Instead of throwing an exception, we indicate this
-    // using the error event. But since this code executes as part of the WebSocket's
-    // constructor, we have to wait until the constructor has completed before firing the
-    // event; otherwise, users can't connect to the event.
-
-    scriptExecutionContext()->postTask([this, protectedThis = Ref { *this }](auto&) {
-        this->dispatchOrQueueErrorEvent();
</del><ins>+        this->dispatchErrorEventIfNeeded();
</ins><span class="cx">         this->stop();
</span><span class="cx">     });
</span><span class="cx"> }
</span><span class="lines">@@ -511,18 +508,16 @@
</span><span class="cx"> 
</span><span class="cx"> void WebSocket::suspend(ReasonForSuspension reason)
</span><span class="cx"> {
</span><del>-    if (m_resumeTimer.isActive())
-        m_resumeTimer.stop();
</del><ins>+    if (!m_channel)
+        return;
</ins><span class="cx"> 
</span><del>-    m_shouldDelayEventFiring = true;
</del><ins>+    if (reason == ReasonForSuspension::BackForwardCache) {
+        // This will cause didClose() to be called.
+        m_channel->fail("WebSocket is closed due to suspension.");
+        return;
+    }
</ins><span class="cx"> 
</span><del>-    if (m_channel) {
-        if (reason == ReasonForSuspension::BackForwardCache) {
-            // This will cause didClose() to be called.
-            m_channel->fail("WebSocket is closed due to suspension.");
-        } else
-            m_channel->suspend();
-    }
</del><ins>+    m_channel->suspend();
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> void WebSocket::resume()
</span><span class="lines">@@ -529,27 +524,8 @@
</span><span class="cx"> {
</span><span class="cx">     if (m_channel)
</span><span class="cx">         m_channel->resume();
</span><del>-
-    if (!m_pendingEvents.isEmpty() && !m_resumeTimer.isActive()) {
-        // Fire the pending events in a timer as we are not allowed to execute arbitrary JS from resume().
-        m_resumeTimer.startOneShot(0_s);
-    }
-
-    m_shouldDelayEventFiring = false;
</del><span class="cx"> }
</span><span class="cx"> 
</span><del>-void WebSocket::resumeTimerFired()
-{
-    Ref<WebSocket> protectedThis(*this);
-
-    ASSERT(!m_pendingEvents.isEmpty());
-
-    // Check m_shouldDelayEventFiring when iterating in case firing an event causes
-    // suspend() to be called.
-    while (!m_pendingEvents.isEmpty() && !m_shouldDelayEventFiring)
-        dispatchEvent(m_pendingEvents.takeFirst());
-}
-
</del><span class="cx"> void WebSocket::stop()
</span><span class="cx"> {
</span><span class="cx">     if (m_channel)
</span><span class="lines">@@ -556,7 +532,6 @@
</span><span class="cx">         m_channel->disconnect();
</span><span class="cx">     m_channel = nullptr;
</span><span class="cx">     m_state = CLOSED;
</span><del>-    m_pendingEvents.clear();
</del><span class="cx">     ActiveDOMObject::stop();
</span><span class="cx">     m_pendingActivity = nullptr;
</span><span class="cx"> }
</span><span class="lines">@@ -569,46 +544,61 @@
</span><span class="cx"> void WebSocket::didConnect()
</span><span class="cx"> {
</span><span class="cx">     LOG(Network, "WebSocket %p didConnect()", this);
</span><del>-    if (m_state != CONNECTING) {
-        didClose(0, ClosingHandshakeIncomplete, WebSocketChannel::CloseEventCodeAbnormalClosure, emptyString());
-        return;
-    }
-    ASSERT(scriptExecutionContext());
-    m_state = OPEN;
-    m_subprotocol = m_channel->subprotocol();
-    m_extensions = m_channel->extensions();
-    dispatchOrQueueEvent(Event::create(eventNames().openEvent, Event::CanBubble::No, Event::IsCancelable::No));
</del><ins>+    queueTaskKeepingObjectAlive(*this, TaskSource::WebSocket, [this] {
+        if (m_state == CLOSED)
+            return;
+        if (m_state != CONNECTING) {
+            didClose(0, ClosingHandshakeIncomplete, WebSocketChannel::CloseEventCodeAbnormalClosure, emptyString());
+            return;
+        }
+        ASSERT(scriptExecutionContext());
+        m_state = OPEN;
+        m_subprotocol = m_channel->subprotocol();
+        m_extensions = m_channel->extensions();
+        dispatchEvent(Event::create(eventNames().openEvent, Event::CanBubble::No, Event::IsCancelable::No));
+    });
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> void WebSocket::didReceiveMessage(const String& msg)
</span><span class="cx"> {
</span><span class="cx">     LOG(Network, "WebSocket %p didReceiveMessage() Text message '%s'", this, msg.utf8().data());
</span><del>-    if (m_state != OPEN)
-        return;
-    ASSERT(scriptExecutionContext());
-    dispatchOrQueueEvent(MessageEvent::create(msg, SecurityOrigin::create(m_url)->toString()));
</del><ins>+    queueTaskKeepingObjectAlive(*this, TaskSource::WebSocket, [this, msg] {
+        if (m_state != OPEN)
+            return;
+        ASSERT(scriptExecutionContext());
+        dispatchEvent(MessageEvent::create(msg, SecurityOrigin::create(m_url)->toString()));
+    });
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> void WebSocket::didReceiveBinaryData(Vector<uint8_t>&& binaryData)
</span><span class="cx"> {
</span><span class="cx">     LOG(Network, "WebSocket %p didReceiveBinaryData() %u byte binary message", this, static_cast<unsigned>(binaryData.size()));
</span><del>-    switch (m_binaryType) {
-    case BinaryType::Blob:
-        // FIXME: We just received the data from NetworkProcess, and are sending it back. This is inefficient.
-        dispatchOrQueueEvent(MessageEvent::create(Blob::create(scriptExecutionContext(), WTFMove(binaryData), emptyString()), SecurityOrigin::create(m_url)->toString()));
-        break;
-    case BinaryType::ArrayBuffer:
-        dispatchOrQueueEvent(MessageEvent::create(ArrayBuffer::create(binaryData.data(), binaryData.size()), SecurityOrigin::create(m_url)->toString()));
-        break;
-    }
</del><ins>+    queueTaskKeepingObjectAlive(*this, TaskSource::WebSocket, [this, binaryData = WTFMove(binaryData)]() mutable {
+        if (m_state != OPEN)
+            return;
+        switch (m_binaryType) {
+        case BinaryType::Blob:
+            // FIXME: We just received the data from NetworkProcess, and are sending it back. This is inefficient.
+            dispatchEvent(MessageEvent::create(Blob::create(scriptExecutionContext(), WTFMove(binaryData), emptyString()), SecurityOrigin::create(m_url)->toString()));
+            break;
+        case BinaryType::ArrayBuffer:
+            dispatchEvent(MessageEvent::create(ArrayBuffer::create(binaryData.data(), binaryData.size()), SecurityOrigin::create(m_url)->toString()));
+            break;
+        }
+    });
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> void WebSocket::didReceiveMessageError()
</span><span class="cx"> {
</span><span class="cx">     LOG(Network, "WebSocket %p didReceiveErrorMessage()", this);
</span><del>-    m_state = CLOSED;
-    ASSERT(scriptExecutionContext());
-    dispatchOrQueueErrorEvent();
</del><ins>+    queueTaskKeepingObjectAlive(*this, TaskSource::WebSocket, [this] {
+        if (m_state == CLOSED)
+            return;
+        m_state = CLOSED;
+        ASSERT(scriptExecutionContext());
+        // FIXME: As per https://html.spec.whatwg.org/multipage/web-sockets.html#feedback-from-the-protocol:concept-websocket-closed, we should synchronously fire a close event.
+        dispatchErrorEventIfNeeded();
+    });
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> void WebSocket::didUpdateBufferedAmount(unsigned bufferedAmount)
</span><span class="lines">@@ -622,26 +612,33 @@
</span><span class="cx"> void WebSocket::didStartClosingHandshake()
</span><span class="cx"> {
</span><span class="cx">     LOG(Network, "WebSocket %p didStartClosingHandshake()", this);
</span><del>-    m_state = CLOSING;
</del><ins>+    queueTaskKeepingObjectAlive(*this, TaskSource::WebSocket, [this] {
+        if (m_state == CLOSED)
+            return;
+        m_state = CLOSING;
+    });
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> void WebSocket::didClose(unsigned unhandledBufferedAmount, ClosingHandshakeCompletionStatus closingHandshakeCompletion, unsigned short code, const String& reason)
</span><span class="cx"> {
</span><span class="cx">     LOG(Network, "WebSocket %p didClose()", this);
</span><del>-    if (!m_channel)
-        return;
-    bool wasClean = m_state == CLOSING && !unhandledBufferedAmount && closingHandshakeCompletion == ClosingHandshakeComplete && code != WebSocketChannel::CloseEventCodeAbnormalClosure;
-    m_state = CLOSED;
-    m_bufferedAmount = unhandledBufferedAmount;
-    ASSERT(scriptExecutionContext());
</del><ins>+    queueTaskKeepingObjectAlive(*this, TaskSource::WebSocket, [this, unhandledBufferedAmount, closingHandshakeCompletion, code, reason] {
+        if (!m_channel)
+            return;
</ins><span class="cx"> 
</span><del>-    dispatchOrQueueEvent(CloseEvent::create(wasClean, code, reason));
</del><ins>+        bool wasClean = m_state == CLOSING && !unhandledBufferedAmount && closingHandshakeCompletion == ClosingHandshakeComplete && code != WebSocketChannel::CloseEventCodeAbnormalClosure;
+        m_state = CLOSED;
+        m_bufferedAmount = unhandledBufferedAmount;
+        ASSERT(scriptExecutionContext());
</ins><span class="cx"> 
</span><del>-    if (m_channel) {
-        m_channel->disconnect();
-        m_channel = nullptr;
-    }
-    m_pendingActivity = nullptr;
</del><ins>+        dispatchEvent(CloseEvent::create(wasClean, code, reason));
+
+        if (m_channel) {
+            m_channel->disconnect();
+            m_channel = nullptr;
+        }
+        m_pendingActivity = nullptr;
+    });
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> void WebSocket::didUpgradeURL()
</span><span class="lines">@@ -664,21 +661,13 @@
</span><span class="cx">     return overhead;
</span><span class="cx"> }
</span><span class="cx"> 
</span><del>-void WebSocket::dispatchOrQueueErrorEvent()
</del><ins>+void WebSocket::dispatchErrorEventIfNeeded()
</ins><span class="cx"> {
</span><span class="cx">     if (m_dispatchedErrorEvent)
</span><span class="cx">         return;
</span><span class="cx"> 
</span><span class="cx">     m_dispatchedErrorEvent = true;
</span><del>-    dispatchOrQueueEvent(Event::create(eventNames().errorEvent, Event::CanBubble::No, Event::IsCancelable::No));
</del><ins>+    dispatchEvent(Event::create(eventNames().errorEvent, Event::CanBubble::No, Event::IsCancelable::No));
</ins><span class="cx"> }
</span><span class="cx"> 
</span><del>-void WebSocket::dispatchOrQueueEvent(Ref<Event>&& event)
-{
-    if (m_shouldDelayEventFiring)
-        m_pendingEvents.append(WTFMove(event));
-    else
-        dispatchEvent(event);
-}
-
</del><span class="cx"> } // namespace WebCore
</span></span></pre></div>
<a id="trunkSourceWebCoreModuleswebsocketsWebSocketh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/Modules/websockets/WebSocket.h (284471 => 284472)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/Modules/websockets/WebSocket.h      2021-10-19 18:40:20 UTC (rev 284471)
+++ trunk/Source/WebCore/Modules/websockets/WebSocket.h 2021-10-19 18:41:13 UTC (rev 284472)
</span><span class="lines">@@ -33,10 +33,8 @@
</span><span class="cx"> #include "ActiveDOMObject.h"
</span><span class="cx"> #include "EventTarget.h"
</span><span class="cx"> #include "ExceptionOr.h"
</span><del>-#include "Timer.h"
</del><span class="cx"> #include <wtf/URL.h>
</span><span class="cx"> #include "WebSocketChannelClient.h"
</span><del>-#include <wtf/Deque.h>
</del><span class="cx"> #include <wtf/HashSet.h>
</span><span class="cx"> #include <wtf/Lock.h>
</span><span class="cx"> 
</span><span class="lines">@@ -101,9 +99,7 @@
</span><span class="cx"> private:
</span><span class="cx">     explicit WebSocket(ScriptExecutionContext&);
</span><span class="cx"> 
</span><del>-    void resumeTimerFired();
-    void dispatchOrQueueErrorEvent();
-    void dispatchOrQueueEvent(Ref<Event>&&);
</del><ins>+    void dispatchErrorEventIfNeeded();
</ins><span class="cx"> 
</span><span class="cx">     void contextDestroyed() final;
</span><span class="cx">     void suspend(ReasonForSuspension) final;
</span><span class="lines">@@ -142,9 +138,6 @@
</span><span class="cx">     String m_subprotocol;
</span><span class="cx">     String m_extensions;
</span><span class="cx"> 
</span><del>-    Timer m_resumeTimer;
-    bool m_shouldDelayEventFiring { false };
-    Deque<Ref<Event>> m_pendingEvents;
</del><span class="cx">     bool m_dispatchedErrorEvent { false };
</span><span class="cx">     RefPtr<PendingActivity<WebSocket>> m_pendingActivity;
</span><span class="cx"> };
</span></span></pre></div>
<a id="trunkSourceWebCoreModuleswebsocketsWebSocketChannelcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/Modules/websockets/WebSocketChannel.cpp (284471 => 284472)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/Modules/websockets/WebSocketChannel.cpp     2021-10-19 18:40:20 UTC (rev 284471)
+++ trunk/Source/WebCore/Modules/websockets/WebSocketChannel.cpp        2021-10-19 18:41:13 UTC (rev 284472)
</span><span class="lines">@@ -139,6 +139,9 @@
</span><span class="cx"> 
</span><span class="cx"> ThreadableWebSocketChannel::SendResult WebSocketChannel::send(const String& message)
</span><span class="cx"> {
</span><ins>+    if (m_outgoingFrameQueueStatus != OutgoingFrameQueueOpen)
+        return ThreadableWebSocketChannel::SendSuccess;
+
</ins><span class="cx">     LOG(Network, "WebSocketChannel %p send() Sending String '%s'", this, message.utf8().data());
</span><span class="cx">     CString utf8 = message.utf8(StrictConversionReplacingUnpairedSurrogatesWithFFFD);
</span><span class="cx">     enqueueTextFrame(utf8);
</span><span class="lines">@@ -154,6 +157,9 @@
</span><span class="cx"> 
</span><span class="cx"> ThreadableWebSocketChannel::SendResult WebSocketChannel::send(const ArrayBuffer& binaryData, unsigned byteOffset, unsigned byteLength)
</span><span class="cx"> {
</span><ins>+    if (m_outgoingFrameQueueStatus != OutgoingFrameQueueOpen)
+        return ThreadableWebSocketChannel::SendSuccess;
+
</ins><span class="cx">     LOG(Network, "WebSocketChannel %p send() Sending ArrayBuffer %p byteOffset=%u byteLength=%u", this, &binaryData, byteOffset, byteLength);
</span><span class="cx">     enqueueRawFrame(WebSocketFrame::OpCodeBinary, static_cast<const uint8_t*>(binaryData.data()) + byteOffset, byteLength);
</span><span class="cx">     processOutgoingFrameQueue();
</span><span class="lines">@@ -162,6 +168,9 @@
</span><span class="cx"> 
</span><span class="cx"> ThreadableWebSocketChannel::SendResult WebSocketChannel::send(Blob& binaryData)
</span><span class="cx"> {
</span><ins>+    if (m_outgoingFrameQueueStatus != OutgoingFrameQueueOpen)
+        return ThreadableWebSocketChannel::SendSuccess;
+
</ins><span class="cx">     LOG(Network, "WebSocketChannel %p send() Sending Blob '%s'", this, binaryData.url().string().utf8().data());
</span><span class="cx">     enqueueBlobFrame(WebSocketFrame::OpCodeBinary, binaryData);
</span><span class="cx">     processOutgoingFrameQueue();
</span><span class="lines">@@ -170,6 +179,9 @@
</span><span class="cx"> 
</span><span class="cx"> bool WebSocketChannel::send(const uint8_t* data, int length)
</span><span class="cx"> {
</span><ins>+    if (m_outgoingFrameQueueStatus != OutgoingFrameQueueOpen)
+        return ThreadableWebSocketChannel::SendSuccess;
+
</ins><span class="cx">     LOG(Network, "WebSocketChannel %p send() Sending uint8_t* data=%p length=%d", this, data, length);
</span><span class="cx">     enqueueRawFrame(WebSocketFrame::OpCodeBinary, data, length);
</span><span class="cx">     processOutgoingFrameQueue();
</span></span></pre></div>
<a id="trunkSourceWebCoredomTaskSourceh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/dom/TaskSource.h (284471 => 284472)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/dom/TaskSource.h    2021-10-19 18:40:20 UTC (rev 284471)
+++ trunk/Source/WebCore/dom/TaskSource.h       2021-10-19 18:41:13 UTC (rev 284472)
</span><span class="lines">@@ -45,6 +45,7 @@
</span><span class="cx">     UserInteraction,
</span><span class="cx">     WebGL,
</span><span class="cx">     WebXR,
</span><ins>+    WebSocket,
</ins><span class="cx"> 
</span><span class="cx">     // Internal to WebCore
</span><span class="cx">     InternalAsyncTask, // Safe to re-order or delay.
</span></span></pre></div>
<a id="trunkSourceWebKitChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit/ChangeLog (284471 => 284472)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit/ChangeLog    2021-10-19 18:40:20 UTC (rev 284471)
+++ trunk/Source/WebKit/ChangeLog       2021-10-19 18:41:13 UTC (rev 284472)
</span><span class="lines">@@ -1,3 +1,26 @@
</span><ins>+2021-10-19  Youenn Fablet  <youenn@apple.com>
+
+        Guarantee order of WebSocket events in case of being resumed
+        https://bugs.webkit.org/show_bug.cgi?id=231664
+
+        Reviewed by Chris Dumez.
+
+        We no longer need to handle resume/suspend in WebSocketChannel layer since WebSocket will deal with itself by enqueuing a task in the event loop.
+
+        * WebProcess/Network/WebSocketChannel.cpp:
+        (WebKit::WebSocketChannel::disconnect):
+        (WebKit::WebSocketChannel::didConnect):
+        (WebKit::WebSocketChannel::didReceiveText):
+        (WebKit::WebSocketChannel::didReceiveBinaryData):
+        (WebKit::WebSocketChannel::didClose):
+        (WebKit::WebSocketChannel::didReceiveMessageError):
+        (WebKit::WebSocketChannel::suspend):
+        (WebKit::WebSocketChannel::resume):
+        (WebKit::WebSocketChannel::didSendHandshakeRequest):
+        (WebKit::WebSocketChannel::didReceiveHandshakeResponse):
+        (WebKit::WebSocketChannel::enqueueTask): Deleted.
+        * WebProcess/Network/WebSocketChannel.h:
+
</ins><span class="cx"> 2021-10-19  David Kilzer  <ddkilzer@apple.com>
</span><span class="cx"> 
</span><span class="cx">         Follow-up #2: WebKit::LocalConnection::createCredentialPrivateKey leaks an NSMutableDictionary
</span></span></pre></div>
<a id="trunkSourceWebKitWebProcessNetworkWebSocketChannelcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit/WebProcess/Network/WebSocketChannel.cpp (284471 => 284472)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit/WebProcess/Network/WebSocketChannel.cpp      2021-10-19 18:40:20 UTC (rev 284471)
+++ trunk/Source/WebKit/WebProcess/Network/WebSocketChannel.cpp 2021-10-19 18:41:13 UTC (rev 284472)
</span><span class="lines">@@ -232,7 +232,6 @@
</span><span class="cx"> {
</span><span class="cx">     m_client = nullptr;
</span><span class="cx">     m_document = nullptr;
</span><del>-    m_pendingTasks.clear();
</del><span class="cx">     m_messageQueue.clear();
</span><span class="cx"> 
</span><span class="cx">     m_inspector.didCloseWebSocket(m_document.get());
</span><span class="lines">@@ -248,13 +247,6 @@
</span><span class="cx">     if (!m_client)
</span><span class="cx">         return;
</span><span class="cx"> 
</span><del>-    if (m_isSuspended) {
-        enqueueTask([this, subprotocol = WTFMove(subprotocol), extensions = WTFMove(extensions)] () mutable {
-            didConnect(WTFMove(subprotocol), WTFMove(extensions));
-        });
-        return;
-    }
-
</del><span class="cx">     m_subprotocol = WTFMove(subprotocol);
</span><span class="cx">     m_extensions = WTFMove(extensions);
</span><span class="cx">     m_client->didConnect();
</span><span class="lines">@@ -286,13 +278,6 @@
</span><span class="cx">     if (!m_client)
</span><span class="cx">         return;
</span><span class="cx"> 
</span><del>-    if (m_isSuspended) {
-        enqueueTask([this, message = WTFMove(message)] () mutable {
-            didReceiveText(WTFMove(message));
-        });
-        return;
-    }
-
</del><span class="cx">     auto utf8Message = message.utf8();
</span><span class="cx">     m_inspector.didReceiveWebSocketFrame(m_document.get(), createWebSocketFrameForWebInspector(utf8Message.dataAsUInt8Ptr(), utf8Message.length(), WebSocketFrame::OpCode::OpCodeText));
</span><span class="cx"> 
</span><span class="lines">@@ -307,14 +292,6 @@
</span><span class="cx">     if (!m_client)
</span><span class="cx">         return;
</span><span class="cx"> 
</span><del>-    if (m_isSuspended) {
-        enqueueTask([this, data = data.vector()] () mutable {
-            if (!m_isClosing && m_client)
-                m_client->didReceiveBinaryData(WTFMove(data));
-        });
-        return;
-    }
-
</del><span class="cx">     m_inspector.didReceiveWebSocketFrame(m_document.get(), createWebSocketFrameForWebInspector(data.data(), data.size(), WebSocketFrame::OpCode::OpCodeBinary));
</span><span class="cx"> 
</span><span class="cx">     m_client->didReceiveBinaryData(data.vector());
</span><span class="lines">@@ -325,13 +302,6 @@
</span><span class="cx">     if (!m_client)
</span><span class="cx">         return;
</span><span class="cx"> 
</span><del>-    if (m_isSuspended) {
-        enqueueTask([this, code, reason = WTFMove(reason)] () mutable {
-            didClose(code, WTFMove(reason));
-        });
-        return;
-    }
-
</del><span class="cx">     WebSocketFrame closingFrame(WebSocketFrame::OpCodeClose, true, false, false);
</span><span class="cx">     m_inspector.didReceiveWebSocketFrame(m_document.get(), closingFrame);
</span><span class="cx">     m_inspector.didCloseWebSocket(m_document.get());
</span><span class="lines">@@ -364,13 +334,6 @@
</span><span class="cx">     if (!m_client)
</span><span class="cx">         return;
</span><span class="cx"> 
</span><del>-    if (m_isSuspended) {
-        enqueueTask([this, errorMessage = WTFMove(errorMessage)] () mutable {
-            didReceiveMessageError(WTFMove(errorMessage));
-        });
-        return;
-    }
-
</del><span class="cx">     logErrorMessage(errorMessage);
</span><span class="cx">     m_client->didReceiveMessageError();
</span><span class="cx"> }
</span><span class="lines">@@ -382,31 +345,14 @@
</span><span class="cx"> 
</span><span class="cx"> void WebSocketChannel::suspend()
</span><span class="cx"> {
</span><del>-    m_isSuspended = true;
</del><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> void WebSocketChannel::resume()
</span><span class="cx"> {
</span><del>-    Ref protectedThis { *this };
-    m_isSuspended = false;
-    while (!m_isSuspended && !m_pendingTasks.isEmpty())
-        m_pendingTasks.takeFirst()();
</del><span class="cx"> }
</span><span class="cx"> 
</span><del>-void WebSocketChannel::enqueueTask(Function<void()>&& task)
-{
-    m_pendingTasks.append(WTFMove(task));
-}
-
</del><span class="cx"> void WebSocketChannel::didSendHandshakeRequest(ResourceRequest&& request)
</span><span class="cx"> {
</span><del>-    if (m_isSuspended) {
-        enqueueTask([this, request = WTFMove(request)]() mutable {
-            didSendHandshakeRequest(WTFMove(request));
-        });
-        return;
-    }
-
</del><span class="cx">     m_inspector.willSendWebSocketHandshakeRequest(m_document.get(), request);
</span><span class="cx">     m_handshakeRequest = WTFMove(request);
</span><span class="cx"> }
</span><span class="lines">@@ -413,13 +359,6 @@
</span><span class="cx"> 
</span><span class="cx"> void WebSocketChannel::didReceiveHandshakeResponse(ResourceResponse&& response)
</span><span class="cx"> {
</span><del>-    if (m_isSuspended) {
-        enqueueTask([this, response = WTFMove(response)]() mutable {
-            didReceiveHandshakeResponse(WTFMove(response));
-        });
-        return;
-    }
-
</del><span class="cx">     m_inspector.didReceiveWebSocketHandshakeResponse(m_document.get(), response);
</span><span class="cx">     m_handshakeResponse = WTFMove(response);
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceWebKitWebProcessNetworkWebSocketChannelh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit/WebProcess/Network/WebSocketChannel.h (284471 => 284472)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit/WebProcess/Network/WebSocketChannel.h        2021-10-19 18:40:20 UTC (rev 284471)
+++ trunk/Source/WebKit/WebProcess/Network/WebSocketChannel.h   2021-10-19 18:41:13 UTC (rev 284472)
</span><span class="lines">@@ -96,7 +96,6 @@
</span><span class="cx">     bool increaseBufferedAmount(size_t);
</span><span class="cx">     void decreaseBufferedAmount(size_t);
</span><span class="cx">     template<typename T> void sendMessage(T&&, size_t byteLength);
</span><del>-    void enqueueTask(Function<void()>&&);
</del><span class="cx"> 
</span><span class="cx">     WebCore::WebSocketChannelIdentifier progressIdentifier() const final { return m_inspector.progressIdentifier(); }
</span><span class="cx">     bool hasCreatedHandshake() const final { return !m_url.isNull(); }
</span><span class="lines">@@ -111,8 +110,6 @@
</span><span class="cx">     String m_extensions;
</span><span class="cx">     size_t m_bufferedAmount { 0 };
</span><span class="cx">     bool m_isClosing { false };
</span><del>-    bool m_isSuspended { false };
-    Deque<Function<void()>> m_pendingTasks;
</del><span class="cx">     WebCore::NetworkSendQueue m_messageQueue;
</span><span class="cx">     WebCore::WebSocketChannelInspector m_inspector;
</span><span class="cx">     WebCore::ResourceRequest m_handshakeRequest;
</span></span></pre>
</div>
</div>

</body>
</html>