Sync the WebSocket interface with the spec

Source/modules/websockets/DOMWebSocket.cpp

 /*
  * Copyright (C) 2011 Google Inc.  All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 15 matching lines @@
  * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 #include "config.h"
 #include "modules/websockets/DOMWebSocket.h"
 
 #include "bindings/core/v8/ExceptionState.h"
 #include "bindings/core/v8/ScriptController.h"
+#include "bindings/modules/v8/UnionTypesModules.h"
 #include "core/dom/DOMArrayBuffer.h"
 #include "core/dom/DOMArrayBufferView.h"
 #include "core/dom/Document.h"
 #include "core/dom/ExceptionCode.h"
 #include "core/dom/ExecutionContext.h"
 #include "core/dom/SecurityContext.h"
 #include "core/events/MessageEvent.h"
 #include "core/fileapi/Blob.h"
 #include "core/frame/ConsoleTypes.h"
 #include "core/frame/LocalDOMWindow.h"
@@ skipping 189 matching lines @@
     ASSERT(!m_channel);
 }
 
 void DOMWebSocket::logError(const String& message)
 {
     executionContext()->addConsoleMessage(ConsoleMessage::create(JSMessageSource, ErrorMessageLevel, message));
 }
 
 DOMWebSocket* DOMWebSocket::create(ExecutionContext* context, const String& url, ExceptionState& exceptionState)
 {
-    Vector<String> protocols;
+    StringOrStringSequence protocols;
     return create(context, url, protocols, exceptionState);
 }
 
-DOMWebSocket* DOMWebSocket::create(ExecutionContext* context, const String& url, const Vector<String>& protocols, ExceptionState& exceptionState)
+DOMWebSocket* DOMWebSocket::create(ExecutionContext* context, const String& url, const StringOrStringSequence& protocols, ExceptionState& exceptionState)
 {
     if (url.isNull()) {
         exceptionState.throwDOMException(SyntaxError, "Failed to create a WebSocket: the provided URL is invalid.");
         return nullptr;
     }
 
     DOMWebSocket* webSocket = new DOMWebSocket(context);
     webSocket->suspendIfNeeded();
 
-    webSocket->connect(url, protocols, exceptionState);
+    if (protocols.isNull()) {
+        Vector<String> protocolsVector;
+        webSocket->connect(url, protocolsVector, exceptionState);
+    } else if (protocols.isString()) {
+        Vector<String> protocolsVector;
+        protocolsVector.append(protocols.getAsString());
+        webSocket->connect(url, protocolsVector, exceptionState);
+    } else {
+        ASSERT(protocols.isStringSequence());
+        webSocket->connect(url, protocols.getAsStringSequence(), exceptionState);
+    }
+
     if (exceptionState.hadException())
         return nullptr;
 
     return webSocket;
 }
 
-DOMWebSocket* DOMWebSocket::create(ExecutionContext* context, const String& url, const String& protocol, ExceptionState& exceptionState)
-{
-    Vector<String> protocols;
-    protocols.append(protocol);
-    return create(context, url, protocols, exceptionState);
-}
-
 void DOMWebSocket::connect(const String& url, const Vector<String>& protocols, ExceptionState& exceptionState)
 {
 
     WTF_LOG(Network, "WebSocket %p connect() url='%s'", this, url.utf8().data());
     m_url = KURL(KURL(), url);
 
     if (executionContext()->securityContext().insecureContentPolicy() == SecurityContext::InsecureContentUpgrade && m_url.protocol() == "ws") {
         m_url.setProtocol("wss");
         if (m_url.port() == 80)
             m_url.setPort(443);
@@ skipping 177 matching lines @@
 void DOMWebSocket::closeInternal(int code, const String& reason, ExceptionState& exceptionState)
 {
     if (code == WebSocketChannel::CloseEventCodeNotSpecified) {
         WTF_LOG(Network, "WebSocket %p close() without code and reason", this);
     } else {
         WTF_LOG(Network, "WebSocket %p close() code=%d reason='%s'", this, code, reason.utf8().data());
         if (!(code == WebSocketChannel::CloseEventCodeNormalClosure || (WebSocketChannel::CloseEventCodeMinimumUserDefined <= code && code <= WebSocketChannel::CloseEventCodeMaximumUserDefined))) {
             exceptionState.throwDOMException(InvalidAccessError, "The code must be either 1000, or between 3000 and 4999. " + String::number(code) + " is neither.");
             return;
         }
-        CString utf8 = reason.utf8(StrictUTF8ConversionReplacingUnpairedSurrogatesWithFFFD);
+        // Bindings specify USVString, so unpaired surrogates are already replaced with U+FFFD.

[jsbell, 2015/02/25 18:51:11]

Is this comment helpful here, or just let the bindings (and existing tests)
document this?

[philipj_slow, 2015/02/26 03:02:53]

Someone very familiar with bindings and the distinction between DOMString and
USVString may find this redundant, but it looks good to me. Is there a way to
assert it instead?

Also, have you looked at the generated code to see if it now does
StrictUTF8ConversionReplacingUnpairedSurrogatesWithFFFD? If it's not exactly
equivalent, some test exposing the change in behavior might be nice.

[jsbell, 2015/02/26 05:45:18]

That'd be me (I implemented our USVString handling), fortunately. Or
unfortunately, for getting a second opinion.
I verified that it's covered in:
http/tests/websocket/unpaired-surrogates-in-close-reason.html

.. and the other one in:
http/tests/websocket/unpaired-surrogates-in-message.html

+        CString utf8 = reason.utf8();
         if (utf8.length() > maxReasonSizeInBytes) {
             exceptionState.throwDOMException(SyntaxError, "The message must not be greater than " + String::number(maxReasonSizeInBytes) + " bytes.");
             return;
         }
     }
 
     if (m_state == CLOSING || m_state == CLOSED)
         return;
     if (m_state == CONNECTING) {
         m_state = CLOSING;
@@ skipping 185 matching lines @@
 DEFINE_TRACE(DOMWebSocket)
 {
     visitor->trace(m_channel);
     visitor->trace(m_eventQueue);
     WebSocketChannelClient::trace(visitor);
     RefCountedGarbageCollectedEventTargetWithInlineData<DOMWebSocket>::trace(visitor);
     ActiveDOMObject::trace(visitor);
 }
 
 } // namespace blink