Fix Websocket compression tests for Client side.

Fix Websocket compression tests for Client side.

Client must use new encoding buffer when receiving clientNoContextTakeover
directive from server.

BUG=25317
Patch by Matthew Butler <butler.matthew@gmail.com>;.

R=sgjesse@google.com

Committed: https://github.com/dart-lang/sdk/commit/82aeb1997ed6f9d84c117fb3f1636b5966ec2f62

[butlermatt, 2016-01-14 17:00:00]

This patch resolves the few outstanding issues that existed in the client side
Websocket Compression tests.

All websocket compression tests for server and client are now green with this
patch.

[Søren Gjesse, 2016-01-15 08:08:38]

sgjesse@google.com changed reviewers:
+ sgjesse@google.com

[Søren Gjesse, 2016-01-15 08:08:39]

lgtm

lgtm

https://codereview.chromium.org/1583933007/diff/1/sdk/lib/io/websocket_impl.dart
File sdk/lib/io/websocket_impl.dart (right):

https://codereview.chromium.org/1583933007/diff/1/sdk/lib/io/websocket_impl.d...
sdk/lib/io/websocket_impl.dart:609: encoder = null;
Shouldn't there be similar logic in the other direction, or am I missing
something here?

From the way we create the headers it seems like we always honor both
client_no_context_takeover and server_no_context_takeover. I that case the
server side should also end its decoder when server_no_context_takeover is in
effect.

Likewise the decoder can also be "discarded" when no context takeover is on
effect.

Besides the Autobahn tests it would be great to have some local tests of the
different context take over combinations to ensure that the compression engines
are restarted/reused depending on these flags.

[Søren Gjesse, 2016-01-15 08:09:23]

Description was changed from

==========
Fix Websocket compression tests for Client side.

Client must use new encoding buffer when receiving clientNoContextTakeover
directive from server.

BUG=25317
==========

to

==========
Fix Websocket compression tests for Client side.

Client must use new encoding buffer when receiving clientNoContextTakeover
directive from server.

BUG=25317

Patch by Matthew Butler <butler.matthew@gmail.com>.
==========

[Søren Gjesse, 2016-01-15 08:09:41]

Description was changed from

==========
Fix Websocket compression tests for Client side.

Client must use new encoding buffer when receiving clientNoContextTakeover
directive from server.

BUG=25317

Patch by Matthew Butler <butler.matthew@gmail.com>.
==========

to

==========
Fix Websocket compression tests for Client side.

Client must use new encoding buffer when receiving clientNoContextTakeover
directive from server.

BUG=25317

Patch by Matthew Butler <butler.matthew@gmail.com>.

R=sgjesse@google.com

Committed:
https://github.com/dart-lang/sdk/commit/82aeb1997ed6f9d84c117fb3f1636b5966ec2f62
==========

[Søren Gjesse, 2016-01-15 08:09:42]

Committed patchset #1 (id:1) manually as
82aeb1997ed6f9d84c117fb3f1636b5966ec2f62 (presubmit successful).

[butlermatt, 2016-01-15 14:18:48]

On 2016/01/15 08:08:39, Søren Gjesse wrote:
> lgtm
> 
> lgtm
> 
>
https://codereview.chromium.org/1583933007/diff/1/sdk/lib/io/websocket_impl.dart
> File sdk/lib/io/websocket_impl.dart (right):
> 
>
https://codereview.chromium.org/1583933007/diff/1/sdk/lib/io/websocket_impl.d...
> sdk/lib/io/websocket_impl.dart:609: encoder = null;
> Shouldn't there be similar logic in the other direction, or am I missing
> something here?
> 
> From the way we create the headers it seems like we always honor both
> client_no_context_takeover and server_no_context_takeover. 

Not Entirely. We actually always honor it on the client side. On the server side
we only honor it when it's enabled in the compression options websocket
transformer.
By default the server side disables client and server no context takeovers. To
enable them you must pass the options in the CompressionOptions constructor. 

>I that case the
> server side should also end its decoder when server_no_context_takeover is in
> effect.
> 
> Likewise the decoder can also be "discarded" when no context takeover is on
> effect.

I'll setup a different patch for this (to enable discard on server no context
takeover and to also discard the decoders as well as the encoders)

> 
> Besides the Autobahn tests it would be great to have some local tests of the
> different context take over combinations to ensure that the compression
engines
> are restarted/reused depending on these flags.

I'm not entirely show how we could test that being part of the private classes,
but happy to hear suggestions.

Thanks,
Matt