718557 - Clean up DOM Websocket close logic.

Status: RESOLVED FIXED

(Core :: Networking: WebSockets, defect)

Description

[Jason Duell]

Attachment: v1

This patch does a bunch of related things:

1) It cleans up nsWebSocket's CloseConnection and FailConnection so that they
   explictly take close code/reason arguments, rather than rely on member
   variables.  It's always clear at call time what error code should be passed,
   so there's no need to use member variables.  Makes code cleaner & safer IMO.

2) Adds a new CLOSE_INTERNAL_ERROR code that we return whenever we close the WS
   connection due to some internal problem (malloc fails, can't get
   charConverter, etc).  The WS spec suprisingly doesn't have any equivalent of
   the HTTP 500 (server internal error) code.  On consultation with Patrick
   McManus we're going to go with 1001 ("endpoint is going away").  Previously
   we were responding with whatever mClientReasonCode was set to (by default it
   was 0 and we sent no close code at all).

3) Changes OnServerClose so that when we receive a server close, we reply with a
   close message that has the same code/reason as the server provide, as the
   spec recommends (previously we were sending whatever mClientReasonCode was
   set to--usually 0, so no close code at all).  

4) Gets rid of m{Client|Server}Reason{Code}, which are no longer needed.
   Centralize naming of mCloseEventXXX variables, and give them names that match
   the spec for code clarity.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 589100 [details] [diff] [review]
v1

>+nsWebSocket::FailConnection(PRUint16 reasonCode) {
>+    return FailConnection(reasonCode, NS_LITERAL_CSTRING(""));
>+}
>+
>+inline nsresult
>+nsWebSocket::CloseConnection(PRUint16 reasonCode) {
>+    return CloseConnection(reasonCode, NS_LITERAL_CSTRING(""));
>+}


EmptyCString() please. Could you actually have
the methods like
CloseConnection(PRUint16 reasonCode, const nsAString& aReason = EmptyCString())
Not sure if that compiles.
Also, aReasonCode. Parameters start with 'a'

Sorry for the delay.

Comment 2

[Jason Duell]

Attachment: v2

all fixes made.

Comment 3

[Jason Duell]

Attachment: v2 (for realz)

forgot to refresh mq patch

Comment 4

[Jason Duell]

https://hg.mozilla.org/integration/mozilla-inbound/rev/77991fb798e5

Comment 5

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/77991fb798e5