[webkit-reviews] review granted: [Bug 39340] WebSocket: resume should not process buffer if already processing. : [Attachment 56684] Patch

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has granted Fumitoshi Ukai
<ukai at chromium.org>'s request for review:
Bug 39340: WebSocket: resume should not process buffer if already processing.
https://bugs.webkit.org/show_bug.cgi?id=39340

Attachment 56684: Patch
https://bugs.webkit.org/attachment.cgi?id=56684&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
Nice catch!

+++ b/WebCore/websockets/ThreadableWebSocketChannelClientWrapper.h

Do we even need to suspend WebSocket message processing below an alert() in
workers?

+    if (!m_resumeTimer.isActive())
+	 m_resumeTimer.startOneShot(0);

So, a timer will be started for all WebSocketChannel objects, even those that
have no pending data? This doesn't look great.

+    RefPtr<WebSocketChannel> protect(this); // The client can close the
channel, potentially removing the last reference.

Is this covered by regression tests?

+++ b/LayoutTests/websocket/tests/script-tests/alert-in-event-handler.js

I still hate it that I can't see test content when I open .html, and have to
dig inside a subdirectory for matching .js.

r=me