Modified: trunk/Source/WebCore/ChangeLog (93772 => 93773)
--- trunk/Source/WebCore/ChangeLog 2011-08-25 10:30:47 UTC (rev 93772)
+++ trunk/Source/WebCore/ChangeLog 2011-08-25 11:31:46 UTC (rev 93773)
@@ -1,3 +1,33 @@
+2011-08-25 Yuta Kitamura <[email protected]>
+
+ WebSocket: Queue messages to be sent
+ https://bugs.webkit.org/show_bug.cgi?id=66298
+
+ Reviewed by Kent Tamura.
+
+ Blobs must be read asynchronously and thus cannot be sent immediately. Therefore, we need to
+ create a queue of messages to be sent in order to handle pending requests.
+
+ No new tests. Strictly speaking, there is a small change in behavior but it is hard to
+ reproduce in tests. See description of WebSocketChannel::send below.
+
+ * websockets/WebSocketChannel.cpp:
+ (WebCore::WebSocketChannel::WebSocketChannel):
+ (WebCore::WebSocketChannel::send):
+ Now, this function always returns true if hybi-10 protocol is selected. The impact of this change
+ should be minimum, because the current WebSocket API defines the return type of WebSocket.send()
+ is void (see also bug 65850).
+ It's hard to make SocketStreamHandle::send() fail deliberately, therefore it's difficult to write
+ a test to detect this behavior change.
+ (WebCore::WebSocketChannel::didCloseSocketStream):
+ (WebCore::WebSocketChannel::startClosingHandshake):
+ (WebCore::WebSocketChannel::processFrame):
+ (WebCore::WebSocketChannel::enqueueTextFrame):
+ (WebCore::WebSocketChannel::enqueueRawFrame):
+ (WebCore::WebSocketChannel::processOutgoingFrameQueue):
+ (WebCore::WebSocketChannel::abortOutgoingFrameQueue):
+ * websockets/WebSocketChannel.h:
+
2011-08-24 Alexander Pavlov <[email protected]>
Web Inspector: Directional arrow on element info box looks terrible
Modified: trunk/Source/WebCore/websockets/WebSocketChannel.cpp (93772 => 93773)
--- trunk/Source/WebCore/websockets/WebSocketChannel.cpp 2011-08-25 10:30:47 UTC (rev 93772)
+++ trunk/Source/WebCore/websockets/WebSocketChannel.cpp 2011-08-25 11:31:46 UTC (rev 93773)
@@ -99,6 +99,7 @@
, m_useHixie76Protocol(true)
, m_hasContinuousFrame(false)
, m_closeEventCode(CloseEventCodeAbnormalClosure)
+ , m_outgoingFrameQueueStatus(OutgoingFrameQueueOpen)
{
ASSERT(m_context->isDocument());
Document* document = static_cast<Document*>(m_context);
@@ -146,10 +147,18 @@
bool WebSocketChannel::send(const String& message)
{
LOG(Network, "WebSocketChannel %p send %s", this, message.utf8().data());
- CString utf8 = message.utf8();
- if (m_useHixie76Protocol)
+ if (m_useHixie76Protocol) {
+ CString utf8 = message.utf8();
return sendFrameHixie76(utf8.data(), utf8.length());
- return sendFrame(OpCodeText, utf8.data(), utf8.length());
+ }
+ enqueueTextFrame(message);
+ // According to WebSocket API specification, WebSocket.send() should return void instead
+ // of boolean. However, our implementation still returns boolean due to compatibility
+ // concern (see bug 65850).
+ // m_channel->send() may happen later, thus it's not always possible to know whether
+ // the message has been sent to the socket successfully. In this case, we have no choice
+ // but to return true.
+ return true;
}
unsigned long WebSocketChannel::bufferedAmount() const
@@ -238,6 +247,8 @@
m_closed = true;
if (m_closingTimer.isActive())
m_closingTimer.stop();
+ if (!m_useHixie76Protocol && m_outgoingFrameQueueStatus != OutgoingFrameQueueClosed)
+ abortOutgoingFrameQueue();
if (m_handle) {
m_unhandledBufferedAmount = m_handle->bufferedAmount();
if (m_suspended)
@@ -417,19 +428,17 @@
if (m_closing)
return;
ASSERT(m_handle);
- bool sentSuccessfully;
if (m_useHixie76Protocol) {
Vector<char> buf;
buf.append('\xff');
buf.append('\0');
- sentSuccessfully = m_handle->send(buf.data(), buf.size());
+ if (!m_handle->send(buf.data(), buf.size())) {
+ m_handle->disconnect();
+ return;
+ }
} else
- sentSuccessfully = sendFrame(OpCodeClose, "", 0); // FIXME: Send status code and reason message.
+ enqueueRawFrame(OpCodeClose, "", 0); // FIXME: Send status code and reason message.
- if (!sentSuccessfully) {
- m_handle->disconnect();
- return;
- }
m_closing = true;
if (m_client)
m_client->didStartClosingHandshake();
@@ -637,17 +646,16 @@
skipBuffer(frame.frameEnd - m_buffer);
m_receivedClosingHandshake = true;
startClosingHandshake();
- if (m_closing)
- m_handle->close(); // Close after sending a close frame.
+ if (m_closing) {
+ m_outgoingFrameQueueStatus = OutgoingFrameQueueClosing;
+ processOutgoingFrameQueue();
+ }
break;
- case OpCodePing: {
- bool result = sendFrame(OpCodePong, frame.payload, frame.payloadLength);
+ case OpCodePing:
+ enqueueRawFrame(OpCodePong, frame.payload, frame.payloadLength);
skipBuffer(frame.frameEnd - m_buffer);
- if (!result)
- fail("Failed to send a pong frame.");
break;
- }
case OpCodePong:
// A server may send a pong in response to our ping, or an unsolicited pong which is not associated with
@@ -748,6 +756,73 @@
return false;
}
+void WebSocketChannel::enqueueTextFrame(const String& string)
+{
+ ASSERT(!m_useHixie76Protocol);
+ ASSERT(m_outgoingFrameQueueStatus == OutgoingFrameQueueOpen);
+ OwnPtr<QueuedFrame> frame = adoptPtr(new QueuedFrame);
+ frame->opCode = OpCodeText;
+ frame->frameType = QueuedFrameTypeString;
+ frame->stringData = string;
+ m_outgoingFrameQueue.append(frame.release());
+ processOutgoingFrameQueue();
+}
+
+void WebSocketChannel::enqueueRawFrame(OpCode opCode, const char* data, size_t dataLength)
+{
+ ASSERT(!m_useHixie76Protocol);
+ ASSERT(m_outgoingFrameQueueStatus == OutgoingFrameQueueOpen);
+ OwnPtr<QueuedFrame> frame = adoptPtr(new QueuedFrame);
+ frame->opCode = opCode;
+ frame->frameType = QueuedFrameTypeVector;
+ frame->vectorData.resize(dataLength);
+ if (dataLength)
+ memcpy(frame->vectorData.data(), data, dataLength);
+ m_outgoingFrameQueue.append(frame.release());
+ processOutgoingFrameQueue();
+}
+
+void WebSocketChannel::processOutgoingFrameQueue()
+{
+ ASSERT(!m_useHixie76Protocol);
+ if (m_outgoingFrameQueueStatus == OutgoingFrameQueueClosed)
+ return;
+
+ while (!m_outgoingFrameQueue.isEmpty()) {
+ OwnPtr<QueuedFrame> frame = m_outgoingFrameQueue.takeFirst();
+ switch (frame->frameType) {
+ case QueuedFrameTypeString: {
+ CString utf8 = frame->stringData.utf8();
+ if (!sendFrame(frame->opCode, utf8.data(), utf8.length()))
+ fail("Failed to send WebSocket frame.");
+ break;
+ }
+
+ case QueuedFrameTypeVector:
+ if (!sendFrame(frame->opCode, frame->vectorData.data(), frame->vectorData.size()))
+ fail("Failed to send WebSocket frame.");
+ break;
+
+ default:
+ ASSERT_NOT_REACHED();
+ break;
+ }
+ }
+
+ ASSERT(m_outgoingFrameQueue.isEmpty());
+ if (m_outgoingFrameQueueStatus == OutgoingFrameQueueClosing) {
+ m_outgoingFrameQueueStatus = OutgoingFrameQueueClosed;
+ m_handle->close();
+ }
+}
+
+void WebSocketChannel::abortOutgoingFrameQueue()
+{
+ ASSERT(!m_useHixie76Protocol);
+ m_outgoingFrameQueue.clear();
+ m_outgoingFrameQueueStatus = OutgoingFrameQueueClosed;
+}
+
bool WebSocketChannel::sendFrame(OpCode opCode, const char* data, size_t dataLength)
{
ASSERT(m_handle);
Modified: trunk/Source/WebCore/websockets/WebSocketChannel.h (93772 => 93773)
--- trunk/Source/WebCore/websockets/WebSocketChannel.h 2011-08-25 10:30:47 UTC (rev 93772)
+++ trunk/Source/WebCore/websockets/WebSocketChannel.h 2011-08-25 11:31:46 UTC (rev 93773)
@@ -37,6 +37,7 @@
#include "ThreadableWebSocketChannel.h"
#include "Timer.h"
#include "WebSocketHandshake.h"
+#include <wtf/Deque.h>
#include <wtf/Forward.h>
#include <wtf/RefCounted.h>
#include <wtf/Vector.h>
@@ -138,6 +139,50 @@
bool processFrame();
bool processFrameHixie76();
+ // It is allowed to send a Blob as a binary frame if hybi-10 protocol is in use. Sending a Blob
+ // can be delayed because it must be read asynchronously. Other types of data (String or
+ // ArrayBuffer) may also be blocked by preceding sending request of a Blob.
+ //
+ // To address this situation, messages to be sent need to be stored in a queue. Whenever a new
+ // data frame is going to be sent, it first must go to the queue. Items in the queue are processed
+ // in the order they were put into the queue. Sending request of a Blob blocks further processing
+ // until the Blob is completely read and sent to the socket stream.
+ //
+ // When hixie-76 protocol is chosen, the queue is not used and messages are sent directly.
+ enum QueuedFrameType {
+ QueuedFrameTypeString,
+ QueuedFrameTypeVector
+ // FIXME: Add QueuedFrameTypeBlob.
+ };
+ struct QueuedFrame {
+ OpCode opCode;
+ QueuedFrameType frameType;
+ // Only one of the following items is used, according to the value of frameType.
+ String stringData;
+ Vector<char> vectorData;
+ // FIXME: Add blobData.
+ };
+ void enqueueTextFrame(const String&);
+ void enqueueRawFrame(OpCode, const char* data, size_t dataLength);
+ // FIXME: Add enqueueBlobFrame().
+
+ void processOutgoingFrameQueue();
+ void abortOutgoingFrameQueue();
+
+ enum OutgoingFrameQueueStatus {
+ // It is allowed to put a new item into the queue.
+ OutgoingFrameQueueOpen,
+ // Close frame has already been put into the queue but may not have been sent yet;
+ // m_handle->close() will be called as soon as the queue is cleared. It is not
+ // allowed to put a new item into the queue.
+ OutgoingFrameQueueClosing,
+ // Close frame has been sent or the queue was aborted. It is not allowed to put
+ // a new item to the queue.
+ OutgoingFrameQueueClosed
+ };
+
+ // If you are going to send a hybi-10 frame, you need to use the outgoing frame queue
+ // instead of call sendFrame() directly.
bool sendFrame(OpCode, const char* data, size_t dataLength);
bool sendFrameHixie76(const char* data, size_t dataLength);
@@ -167,6 +212,9 @@
Vector<char> m_continuousFrameData;
unsigned short m_closeEventCode;
String m_closeEventReason;
+
+ Deque<OwnPtr<QueuedFrame> > m_outgoingFrameQueue;
+ OutgoingFrameQueueStatus m_outgoingFrameQueueStatus;
};
} // namespace WebCore
