Release assert in ScriptController::canExecuteScripts via WebCore::We…

…bSocket::didReceiveMessage https://bugs.webkit.org/show_bug.cgi?id=229301 Reviewed by Ryosuke Niwa. Source/WebCore: Test: http/tests/websocket/tests/hybi/inspector/send-and-recieve-debugger.html `WebCore::WebSocket::didReceiveMessage` and other functions that fire JS events may be called while the `WebSocketChannel` is being resumed, but it is not yet safe to evaluate JavaScript during resuming. This was already accounted for with some events like errors and closing, but it holds true for all events. We should delay firing these events as well. * Modules/websockets/WebSocket.cpp: (WebCore::WebSocket::didConnect): (WebCore::WebSocket::didReceiveMessage): (WebCore::WebSocket::didReceiveBinaryData): LayoutTests: * http/tests/websocket/tests/hybi/inspector/echo-delayed_wsh.py: Added. (web_socket_do_extra_handshake): (web_socket_transfer_data): - Similar to `echo_wsh.py`, respond with the provided message when received, but delay the response by 100ms. * http/tests/websocket/tests/hybi/inspector/send-and-recieve-debugger-expected.txt: Added. * http/tests/websocket/tests/hybi/inspector/send-and-recieve-debugger.html: Added. Canonical link: https://commits.webkit.org/240740@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@281323 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Aug 20, 2021 · 1b71292 · 1b71292
1 parent 6fd46ef
commit 1b71292
Show file tree

Hide file tree

Showing 6 changed files with 164 additions and 4 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,17 @@
+2021-08-20  Patrick Angle  <pangle@apple.com>
+
+        Release assert in ScriptController::canExecuteScripts via WebCore::WebSocket::didReceiveMessage
+        https://bugs.webkit.org/show_bug.cgi?id=229301
+
+        Reviewed by Ryosuke Niwa.
+
+        * http/tests/websocket/tests/hybi/inspector/echo-delayed_wsh.py: Added.
+        (web_socket_do_extra_handshake):
+        (web_socket_transfer_data):
+        - Similar to `echo_wsh.py`, respond with the provided message when received, but delay the response by 100ms.
+        * http/tests/websocket/tests/hybi/inspector/send-and-recieve-debugger-expected.txt: Added.
+        * http/tests/websocket/tests/hybi/inspector/send-and-recieve-debugger.html: Added.
+
 2021-08-20  Ayumi Kojima  <ayumi_kojima@apple.com>
 
         [ iOS Debug ] animations/unprefixed-events.html is flaky failing.

diff --git a/LayoutTests/http/tests/websocket/tests/hybi/inspector/echo-delayed_wsh.py b/LayoutTests/http/tests/websocket/tests/hybi/inspector/echo-delayed_wsh.py
@@ -0,0 +1,13 @@
+from mod_pywebsocket import msgutil
+from time import sleep
+
+
+def web_socket_do_extra_handshake(request):
+    pass  # Always accept.
+
+
+def web_socket_transfer_data(request):
+    # Echo message back
+    message = msgutil.receive_message(request)
+    sleep(0.1)
+    msgutil.send_message(request, message)
diff --git a/LayoutTests/http/tests/websocket/tests/hybi/inspector/send-and-recieve-debugger-expected.txt b/LayoutTests/http/tests/websocket/tests/hybi/inspector/send-and-recieve-debugger-expected.txt
@@ -0,0 +1,18 @@
+Tests sending and receiving WebSocket messages.
+
+
+== Running test suite: WebSocket.SendAndReceiveDebugger
+-- Running test case: WebSocket.SendAndReceiveDebugger.DefferedReceiveWhilePaused
+PASS: Resource size should be 33 bytes.
+PASS: Frame data should be 'Hello World! Привет Мир!'
+PASS: Frame should be text.
+PASS: Frame should be outgoing.
+PASS: Message is walltime.
+Pausing script execution with `debugger` statement.
+Resuming script execution.
+PASS: Resource size should double.
+PASS: Frame data should be 'Hello World! Привет Мир!'
+PASS: Frame should be text.
+PASS: Frame should be incoming.
+PASS: Frame walltime should be greater than the previous one.
+
diff --git a/LayoutTests/http/tests/websocket/tests/hybi/inspector/send-and-recieve-debugger.html b/LayoutTests/http/tests/websocket/tests/hybi/inspector/send-and-recieve-debugger.html
@@ -0,0 +1,96 @@
+<!doctype html>
+<html>
+<head>
+<meta charset="utf-8">
+<script src="../../../../inspector/resources/inspector-test.js"></script>
+<script>
+// Global variable to keep it alive.
+let webSocket = null;
+
+function createWebSocketConnection()
+{
+    webSocket = new WebSocket("ws://127.0.0.1:8880/websocket/tests/hybi/inspector/echo-delayed");
+
+    webSocket.onopen = function()
+    {
+        webSocket.send("Hello World! Привет Мир!");
+    };
+
+    webSocket.onmessage = function()
+    {
+        webSocket.close();
+    };
+}
+
+function test()
+{
+    let suite = InspectorTest.createAsyncSuite("WebSocket.SendAndReceiveDebugger");
+
+    suite.addTestCase({
+        name: "WebSocket.SendAndReceiveDebugger.DefferedReceiveWhilePaused",
+        description: "A WebSocket should fire events for received messages after the debugger resumes script execution.",
+        test(resolve, reject) {
+            WI.Frame.singleFireEventListener(WI.Frame.Event.ResourceWasAdded, function(event) {
+
+                let frameAddedCount = 0;
+                let lastMessageWalltime;
+                let resource = event.data.resource;
+
+                resource.addEventListener(WI.WebSocketResource.Event.FrameAdded, function(event) {
+                    let frame = event.data;
+                    frameAddedCount++;
+
+                    if (frameAddedCount === 1) {
+
+                        InspectorTest.expectEqual(frame.data, "Hello World! Привет Мир!", "Frame data should be 'Hello World! Привет Мир!'");
+                        InspectorTest.expectEqual(frame.opcode, WI.WebSocketResource.OpCodes.TextFrame, "Frame should be text.");
+                        InspectorTest.expectThat(frame.isOutgoing, "Frame should be outgoing.");
+                        InspectorTest.expectThat(typeof frame.walltime === "number", "Message is walltime.");
+
+                        InspectorTest.log("Pausing script execution with `debugger` statement.");
+                        InspectorTest.evaluateInPage(`debugger`);
+
+                        // The WebSocket echos with a 100ms delay, so wait a bit longer than that before resuming to
+                        // ensure we receive the message while still paused.
+                        setTimeout(() => {
+                        	InspectorTest.log("Resuming script execution.")
+                        	WI.debuggerManager.resume().catch(reject);
+                        }, 200);
+                    } else if (frameAddedCount === 2) {
+
+                        InspectorTest.expectEqual(frame.data, "Hello World! Привет Мир!", "Frame data should be 'Hello World! Привет Мир!'");
+                        InspectorTest.expectEqual(frame.opcode, WI.WebSocketResource.OpCodes.TextFrame, "Frame should be text.");
+                        InspectorTest.expectThat(!frame.isOutgoing, "Frame should be incoming.");
+                        InspectorTest.expectThat(frame.walltime > lastMessageWalltime, "Frame walltime should be greater than the previous one.");
+
+                        resolve();
+                    }
+
+                    lastMessageWalltime = frame.walltime;
+                });
+
+                let sizeDidChangeCount = 0;
+
+                resource.addEventListener(WI.Resource.Event.SizeDidChange, function(event) {
+                    sizeDidChangeCount++;
+
+                    if (sizeDidChangeCount === 1)
+                        InspectorTest.expectEqual(this.size, 33, "Resource size should be 33 bytes.");
+                    else if (sizeDidChangeCount === 2)
+                        InspectorTest.expectEqual(this.size, 33 * 2, "Resource size should double.");
+
+                }, resource);
+            });
+
+            InspectorTest.evaluateInPage(`createWebSocketConnection()`);
+        }
+    });
+
+    suite.runTestCasesAndFinish();
+}
+</script>
+</head>
+<body onload="runTest()">
+<p>Tests sending and receiving WebSocket messages.</p>
+</body>
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,22 @@
+2021-08-20  Patrick Angle  <pangle@apple.com>
+
+        Release assert in ScriptController::canExecuteScripts via WebCore::WebSocket::didReceiveMessage
+        https://bugs.webkit.org/show_bug.cgi?id=229301
+
+        Reviewed by Ryosuke Niwa.
+
+        Test: http/tests/websocket/tests/hybi/inspector/send-and-recieve-debugger.html
+
+        `WebCore::WebSocket::didReceiveMessage` and other functions that fire JS events may be called while the
+        `WebSocketChannel` is being resumed, but it is not yet safe to evaluate JavaScript during resuming. This was
+        already accounted for with some events like errors and closing, but it holds true for all events. We should
+        delay firing these events as well.
+
+        * Modules/websockets/WebSocket.cpp:
+        (WebCore::WebSocket::didConnect):
+        (WebCore::WebSocket::didReceiveMessage):
+        (WebCore::WebSocket::didReceiveBinaryData):
+
 2021-08-20  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, reverting r281307.

diff --git a/Source/WebCore/Modules/websockets/WebSocket.cpp b/Source/WebCore/Modules/websockets/WebSocket.cpp
@@ -577,7 +577,7 @@ void WebSocket::didConnect()
     m_state = OPEN;
     m_subprotocol = m_channel->subprotocol();
     m_extensions = m_channel->extensions();
-    dispatchEvent(Event::create(eventNames().openEvent, Event::CanBubble::No, Event::IsCancelable::No));
+    dispatchOrQueueEvent(Event::create(eventNames().openEvent, Event::CanBubble::No, Event::IsCancelable::No));
 }
 
 void WebSocket::didReceiveMessage(const String& msg)
@@ -586,7 +586,7 @@ void WebSocket::didReceiveMessage(const String& msg)
     if (m_state != OPEN)
         return;
     ASSERT(scriptExecutionContext());
-    dispatchEvent(MessageEvent::create(msg, SecurityOrigin::create(m_url)->toString()));
+    dispatchOrQueueEvent(MessageEvent::create(msg, SecurityOrigin::create(m_url)->toString()));
 }
 
 void WebSocket::didReceiveBinaryData(Vector<uint8_t>&& binaryData)
@@ -595,10 +595,10 @@ void WebSocket::didReceiveBinaryData(Vector<uint8_t>&& binaryData)
     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()));
+        dispatchOrQueueEvent(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()));
+        dispatchOrQueueEvent(MessageEvent::create(ArrayBuffer::create(binaryData.data(), binaryData.size()), SecurityOrigin::create(m_url)->toString()));
         break;
     }
 }