Add websocket.Upgrader option.

[wavded]

This pull request adds a new option WebsocketCompression that when used will allow supported clients to receive and send compressed messages. This is a minimal approach that should be backwards compatible with the current implementation.

It takes cues from some prior art:

#56 (Switches to use websocket.Updater but made several other changes)
#53 (Enabled compression always rather than made it configurable)

---

This change is 

[coveralls]

Coverage increased (+0.1%) to 94.972% when pulling 826659b on wavded:compression into c8a8c64 on igm:master.

[wavded]

After further review of #56, I think that may be a better overall solution to allow complete customization of the WebSocket Upgrader. But am happy to go through a review of this if it seems like a better incremental solution to just get compression working. Thoughts @igm ?

[igm]

thanks for the PR @wavded . The PR looks good and I see the upgrader has more knobs to configure websocket, so it looks like we could extract entire upgrader into options. The DefaultOptions could just use Upgrader with default buffer values just to be sure we keep defaults untouched.
Does it make sense or am I missing something in bigger picture?

[wavded]

That seems to be the most flexible option, I went ahead and changed it. Does that match what you had in mind @igm ?

[igm]

Yes, this is what I had in mind. My only concern is the buffer size variables (WebSocketReadBufSize, WebSocketWriteBufSize) have no effect if changed. I think it is safer to preserve previous behaviour.

I suggest small changes to keep those vars in action and at the same time provide upgrader as option flag. What do you think?

---

Review status: all files reviewed at latest revision, all discussions resolved.

---

sockjs/handler.go, line 24 at r2 (raw file):

	handlerFunc func(Session)
	mappings    []*mapping
	upgrader    websocket.Upgrader

upgrader is accessible via options, we can omit this field

---

sockjs/handler.go, line 42 at r2 (raw file):

		handlerFunc: handlerFunc,
		sessions:    make(map[string]*session),
		upgrader:    opts.WebsocketUpgrader,

same as above

---

sockjs/options.go, line 45 at r2 (raw file):

WebsocketUpgrader websocket.Upgrader

WebsocketUpgrader *websocket.Upgrader

---

sockjs/options.go, line 69 at r2 (raw file):

WebsocketUpgrader: nil,

---

sockjs/rawwebsocket.go, line 11 at r2 (raw file):

func (h *handler) rawWebsocket(rw http.ResponseWriter, req *http.Request) {
	conn, err := h.upgrader.Upgrade(rw, req, nil)

I also suggest something like this:

var conn *websocket.Conn
var err error
if h.opts.WebsocketUpgrader!=nil {
   conn, err =  h.opts.WebsocketUpgrader.Upgrade(rw, req, nil)
} else {
   // use default as before, so that those 2 buffer size variables are used as before
   conn, err = websocket.Upgrade(rw, req, nil, WebSocketReadBufSize, WebSocketWriteBufSize)
}

---

sockjs/websocket.go, line 17 at r2 (raw file):

var WebSocketReadBufSize = 4096
var WebSocketWriteBufSize = 4096

Even though I am not happy with these public vars, we should keep them to preserve previous functionality.

---

sockjs/websocket.go, line 20 at r2 (raw file):

func (h *handler) sockjsWebsocket(rw http.ResponseWriter, req *http.Request) {
	conn, err := h.upgrader.Upgrade(rw, req, nil)

same if as above in rawwebsocket.go

---

Comments from Reviewable

[wavded]

Thanks for the feedback @igm. I have that integrated those suggestions and added some additional coverage.

[igm]

---

Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable