Bug 718557: Clean up DOM Websocket close logic. r=smaug
authorJason Duell <jduell.mcbugs@gmail.com>
Tue, 31 Jan 2012 20:41:43 -0800
changeset 88823 77991fb798e53f46c4ad2881966d69dec68fbd9a
parent 88822 57cd18f07290b60f57c6adcb58ffb5214d215023
child 88824 88ac74cd45ad153d76a13510eefaf3eb3c2ab1b1
push id783
push userlsblakk@mozilla.com
push dateTue, 24 Apr 2012 17:33:42 +0000
treeherdermozilla-beta@11faed19f136 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs718557
milestone13.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 718557: Clean up DOM Websocket close logic. r=smaug
content/base/src/nsWebSocket.cpp
content/base/src/nsWebSocket.h
netwerk/protocol/websocket/nsIWebSocketChannel.idl
--- a/content/base/src/nsWebSocket.cpp
+++ b/content/base/src/nsWebSocket.cpp
@@ -86,28 +86,28 @@
 
 using namespace mozilla;
 
 #define UTF_8_REPLACEMENT_CHAR    static_cast<PRUnichar>(0xFFFD)
 
 #define TRUE_OR_FAIL_WEBSOCKET(x, ret)                                    \
   PR_BEGIN_MACRO                                                          \
     if (NS_UNLIKELY(!(x))) {                                              \
-       NS_WARNING("ENSURE_TRUE_AND_FAIL_IF_FAILED(" #x ") failed");       \
-       FailConnection();                                                  \
-       return ret;                                                        \
+      NS_WARNING("ENSURE_TRUE_AND_FAIL_IF_FAILED(" #x ") failed");        \
+      FailConnection(nsIWebSocketChannel::CLOSE_INTERNAL_ERROR);          \
+      return ret;                                                         \
     }                                                                     \
   PR_END_MACRO
 
 #define SUCCESS_OR_FAIL_WEBSOCKET(res, ret)                               \
   PR_BEGIN_MACRO                                                          \
     nsresult __rv = res;                                                  \
     if (NS_FAILED(__rv)) {                                                \
       NS_ENSURE_SUCCESS_BODY(res, ret)                                    \
-      FailConnection();                                                   \
+      FailConnection(nsIWebSocketChannel::CLOSE_INTERNAL_ERROR);          \
       return ret;                                                         \
     }                                                                     \
   PR_END_MACRO
 
 nsresult
 nsWebSocket::PrintErrorOnConsole(const char *aBundleURI,
                                  const PRUnichar *aError,
                                  const PRUnichar **aFormatStrings,
@@ -154,44 +154,45 @@ nsWebSocket::PrintErrorOnConsole(const c
   rv = console->LogMessage(errorObject);
   NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;
 }
 
 // when this is called the browser side wants no more part of it
 nsresult
-nsWebSocket::CloseConnection()
+nsWebSocket::CloseConnection(PRUint16 aReasonCode,
+                             const nsACString& aReasonString)
 {
   NS_ABORT_IF_FALSE(NS_IsMainThread(), "Not running on main thread");
   if (mDisconnected)
     return NS_OK;
 
   // Disconnect() can release this object, so we keep a
   // reference until the end of the method
   nsRefPtr<nsWebSocket> kungfuDeathGrip = this;
 
   if (mReadyState == nsIWebSocket::CONNECTING) {
     SetReadyState(nsIWebSocket::CLOSED);
     if (mChannel) {
-      mChannel->Close(mClientReasonCode, mClientReason);
+      mChannel->Close(aReasonCode, aReasonString);
     }
     Disconnect();
     return NS_OK;
   }
 
   SetReadyState(nsIWebSocket::CLOSING);
 
   if (mDisconnected) {
     SetReadyState(nsIWebSocket::CLOSED);
     Disconnect();
     return NS_OK;
   }
 
-  return mChannel->Close(mClientReasonCode, mClientReason);
+  return mChannel->Close(aReasonCode, aReasonString);
 }
 
 nsresult
 nsWebSocket::ConsoleError()
 {
   NS_ABORT_IF_FALSE(NS_IsMainThread(), "Not running on main thread");
   nsresult rv;
 
@@ -214,22 +215,23 @@ nsWebSocket::ConsoleError()
     }
   }
   /// todo some specific errors - like for message too large
   return rv;
 }
 
 
 nsresult
-nsWebSocket::FailConnection()
+nsWebSocket::FailConnection(PRUint16 aReasonCode,
+                            const nsACString& aReasonString)
 {
   NS_ABORT_IF_FALSE(NS_IsMainThread(), "Not running on main thread");
   ConsoleError();
 
-  nsresult rv = CloseConnection();
+  nsresult rv = CloseConnection(aReasonCode, aReasonString);
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = CreateAndDispatchSimpleEvent(NS_LITERAL_STRING("error"));
   if (NS_FAILED(rv))
     NS_WARNING("Failed to dispatch the error event");
 
   return NS_OK;
 }
@@ -317,17 +319,17 @@ nsWebSocket::OnStart(nsISupports *aConte
 
 NS_IMETHODIMP
 nsWebSocket::OnStop(nsISupports *aContext, nsresult aStatusCode)
 {
   NS_ABORT_IF_FALSE(NS_IsMainThread(), "Not running on main thread");
   if (mDisconnected)
     return NS_OK;
 
-  mClosedCleanly = NS_SUCCEEDED(aStatusCode);
+  mCloseEventWasClean = NS_SUCCEEDED(aStatusCode);
 
   if (aStatusCode == NS_BASE_STREAM_CLOSED &&
       mReadyState >= nsIWebSocket::CLOSING) {
     // don't generate an error event just because of an unclean close
     aStatusCode = NS_OK;
   }
 
   if (NS_FAILED(aStatusCode)) {
@@ -355,20 +357,26 @@ nsWebSocket::OnAcknowledge(nsISupports *
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsWebSocket::OnServerClose(nsISupports *aContext, PRUint16 aCode,
                            const nsACString &aReason)
 {
   NS_ABORT_IF_FALSE(NS_IsMainThread(), "Not running on main thread");
-  mServerReasonCode = aCode;
-  CopyUTF8toUTF16(aReason, mServerReason);
+
+  // store code/string for onclose DOM event
+  mCloseEventCode = aCode;
+  CopyUTF8toUTF16(aReason, mCloseEventReason);
 
-  CloseConnection();                              /* reciprocate! */
+  // Send reciprocal Close frame.
+  // 5.5.1: "When sending a Close frame in response, the endpoint typically
+  // echos the status code it received"
+  CloseConnection(aCode, aReason);
+
   return NS_OK;
 }
 
 //-----------------------------------------------------------------------------
 // nsWebSocket::nsIInterfaceRequestor
 //-----------------------------------------------------------------------------
 
 NS_IMETHODIMP
@@ -401,20 +409,19 @@ nsWebSocket::GetInterface(const nsIID &a
 
 ////////////////////////////////////////////////////////////////////////////////
 // nsWebSocket
 ////////////////////////////////////////////////////////////////////////////////
 
 nsWebSocket::nsWebSocket() : mKeepingAlive(false),
                              mCheckMustKeepAlive(true),
                              mTriggeredCloseEvent(false),
-                             mClosedCleanly(false),
                              mDisconnected(false),
-                             mClientReasonCode(0),
-                             mServerReasonCode(nsIWebSocketChannel::CLOSE_ABNORMAL),
+                             mCloseEventWasClean(false),
+                             mCloseEventCode(nsIWebSocketChannel::CLOSE_ABNORMAL),
                              mReadyState(nsIWebSocket::CONNECTING),
                              mOutgoingBufferedAmount(0),
                              mBinaryType(WS_BINARY_TYPE_BLOB),
                              mScriptLine(0),
                              mInnerWindowID(0)
 {
   NS_ABORT_IF_FALSE(NS_IsMainThread(), "Not running on main thread");
   nsLayoutStatics::AddRef();
@@ -618,17 +625,17 @@ class nsAutoCloseWS
 public:
   nsAutoCloseWS(nsWebSocket *aWebSocket)
     : mWebSocket(aWebSocket)
   {}
 
   ~nsAutoCloseWS()
   {
     if (!mWebSocket->mChannel) {
-      mWebSocket->CloseConnection();
+      mWebSocket->CloseConnection(nsIWebSocketChannel::CLOSE_INTERNAL_ERROR);
     }
   }
 private:
   nsRefPtr<nsWebSocket> mWebSocket;
 };
 
 nsresult
 nsWebSocket::EstablishConnection()
@@ -901,19 +908,19 @@ nsWebSocket::SetReadyState(PRUint16 aNew
     mReadyState = aNewReadyState;
     return;
   }
 
   if (aNewReadyState == nsIWebSocket::CLOSED) {
     mReadyState = aNewReadyState;
 
     // The close event must be dispatched asynchronously.
-    rv = NS_DispatchToMainThread(new nsWSCloseEvent(this, mClosedCleanly,
-                                                    mServerReasonCode,
-                                                    mServerReason),
+    rv = NS_DispatchToMainThread(new nsWSCloseEvent(this, mCloseEventWasClean,
+                                                    mCloseEventCode,
+                                                    mCloseEventReason),
                                  NS_DISPATCH_NORMAL);
     if (NS_FAILED(rv)) {
       NS_WARNING("Failed to dispatch the close event");
       mTriggeredCloseEvent = true;
       UpdateMustKeepAlive();
     }
   }
 }
@@ -1408,55 +1415,53 @@ nsWebSocket::ConvertTextToUTF8(const nsS
 }
 
 NS_IMETHODIMP
 nsWebSocket::Close(PRUint16 code, const nsAString & reason, PRUint8 argc)
 {
   NS_ABORT_IF_FALSE(NS_IsMainThread(), "Not running on main thread");
 
   // the reason code is optional, but if provided it must be in a specific range
+  PRUint16 closeCode = 0;
   if (argc >= 1) {
-    if (code != 1000 && (code < 3000 || code > 4999))
+    if (code != 1000 && (code < 3000 || code > 4999)) {
       return NS_ERROR_DOM_INVALID_ACCESS_ERR;
+    }
+    closeCode = code;
   }
 
-  nsCAutoString utf8Reason;
+  nsCAutoString closeReason;
   if (argc >= 2) {
-    if (ContainsUnpairedSurrogates(reason))
+    if (ContainsUnpairedSurrogates(reason)) {
       return NS_ERROR_DOM_SYNTAX_ERR;
-
-    CopyUTF16toUTF8(reason, utf8Reason);
+    }
+    CopyUTF16toUTF8(reason, closeReason);
 
     // The API requires the UTF-8 string to be 123 or less bytes
-    if (utf8Reason.Length() > 123)
+    if (closeReason.Length() > 123) {
       return NS_ERROR_DOM_SYNTAX_ERR;
+    }
   }
 
-  // Format checks for reason and code both passed, they can now be assigned.
-  if (argc >= 1)
-    mClientReasonCode = code;
-  if (argc >= 2)
-    mClientReason = utf8Reason;
-  
   if (mReadyState == nsIWebSocket::CLOSING ||
       mReadyState == nsIWebSocket::CLOSED) {
     return NS_OK;
   }
 
   if (mReadyState == nsIWebSocket::CONNECTING) {
     // FailConnection() can release the object, so we keep a reference
     // before calling it
     nsRefPtr<nsWebSocket> kungfuDeathGrip = this;
 
-    FailConnection();
+    FailConnection(closeCode, closeReason);
     return NS_OK;
   }
 
   // mReadyState == nsIWebSocket::OPEN
-  CloseConnection();
+  CloseConnection(closeCode, closeReason);
 
   return NS_OK;
 }
 
 /**
  * This Init method should only be called by C++ consumers.
  */
 NS_IMETHODIMP
@@ -1589,18 +1594,17 @@ NS_IMETHODIMP
 nsWebSocket::Cancel(nsresult aStatus)
 {
   NS_ABORT_IF_FALSE(NS_IsMainThread(), "Not running on main thread");
 
   if (mDisconnected)
     return NS_OK;
 
   ConsoleError();
-  mClientReasonCode = nsIWebSocketChannel::CLOSE_GOING_AWAY;
-  return CloseConnection();
+  return CloseConnection(nsIWebSocketChannel::CLOSE_GOING_AWAY);
 }
 
 NS_IMETHODIMP
 nsWebSocket::Suspend()
 {
   return NS_ERROR_NOT_IMPLEMENTED;
 }
 
--- a/content/base/src/nsWebSocket.h
+++ b/content/base/src/nsWebSocket.h
@@ -106,19 +106,21 @@ public:
 
   // Determine if preferences allow WebSocket
   static bool PrefEnabled();
 
 protected:
   nsresult ParseURL(const nsString& aURL);
   nsresult EstablishConnection();
 
-  // these three methods when called can release the WebSocket object
-  nsresult FailConnection();
-  nsresult CloseConnection();
+  // These methods when called can release the WebSocket object
+  nsresult FailConnection(PRUint16 reasonCode,
+                          const nsACString& aReasonString = EmptyCString());
+  nsresult CloseConnection(PRUint16 reasonCode,
+                           const nsACString& aReasonString = EmptyCString());
   nsresult Disconnect();
 
   nsresult ConsoleError();
   nsresult PrintErrorOnConsole(const char       *aBundleURI,
                                const PRUnichar  *aError,
                                const PRUnichar **aFormatStrings,
                                PRUint32          aFormatStringsLen);
 
@@ -161,23 +163,22 @@ protected:
   nsString mOriginalURL;
   nsString mEffectiveURL;   // after redirects
   bool mSecure; // if true it is using SSL and the wss scheme,
                         // otherwise it is using the ws scheme with no SSL
 
   bool mKeepingAlive;
   bool mCheckMustKeepAlive;
   bool mTriggeredCloseEvent;
-  bool mClosedCleanly;
   bool mDisconnected;
 
-  nsCString mClientReason;
-  nsString  mServerReason;
-  PRUint16  mClientReasonCode;
-  PRUint16  mServerReasonCode;
+  // Set attributes of DOM 'onclose' message
+  bool      mCloseEventWasClean;
+  nsString  mCloseEventReason;
+  PRUint16  mCloseEventCode;
 
   nsCString mAsciiHost;  // hostname
   PRUint32  mPort;
   nsCString mResource; // [filepath[?query]]
   nsString  mUTF16Origin;
 
   nsCOMPtr<nsIURI> mURI;
   nsCString mRequestedProtocolList;
--- a/netwerk/protocol/websocket/nsIWebSocketChannel.idl
+++ b/netwerk/protocol/websocket/nsIWebSocketChannel.idl
@@ -130,16 +130,20 @@ interface nsIWebSocketChannel : nsISuppo
     //  code 1004 is reserved
     const unsigned short CLOSE_NO_STATUS            = 1005;
     const unsigned short CLOSE_ABNORMAL             = 1006;
     const unsigned short CLOSE_INVALID_PAYLOAD      = 1007;
     const unsigned short CLOSE_POLICY_VIOLATION     = 1008;
     const unsigned short CLOSE_TOO_LARGE            = 1009;
     const unsigned short CLOSE_EXTENSION_MISSING    = 1010;
 
+    // Websocket spec doesn't have equivalent of HTTP 500 code for internal
+    // errors: just use CLOSE_GOING_AWAY for now
+    const unsigned short CLOSE_INTERNAL_ERROR      = CLOSE_GOING_AWAY;
+
     /**
      * Use to send text message down the connection to WebSocket peer.
      *
      * @param aMsg the utf8 string to send
      */
     void sendMsg(in AUTF8String aMsg);
 
     /**