Introduce WebSocketDeflateParameters.

Introduce WebSocketDeflateParameters.

This CL introduces WebSocketDeflateParameters which represents WebSocket
"permessage-deflate" extension parameters. The class will also be used
by WebSocketEncoder sitting on net/server in the future.

This CL doesn't change the behavior.

BUG=523228
Committed: https://crrev.com/8387aee95b6206d01bbd988f87b0d082ad18ed4e
Cr-Commit-Position: refs/heads/master@{#348567}

[yhirano, 2015-09-02 06:08:42]

Patchset #1 (id:1) has been deleted

[yhirano, 2015-09-02 14:16:28]

Patchset #1 (id:20001) has been deleted

[yhirano, 2015-09-02 14:16:33]

Patchset #1 (id:40001) has been deleted

[yhirano, 2015-09-02 14:16:59]

Patchset #1 (id:60001) has been deleted

[yhirano, 2015-09-02 14:17:01]

Patchset #1 (id:80001) has been deleted

[yhirano, 2015-09-02 14:17:04]

Patchset #1 (id:100001) has been deleted

[yhirano, 2015-09-02 14:18:52]

yhirano@chromium.org changed reviewers:
+ tyoshino@chromium.org

[yhirano, 2015-09-10 06:04:35]

On 2015/09/02 14:18:52, yhirano wrote:

tyoshino: ping

[tyoshino (SeeGerritForStatus), 2015-09-10 09:06:20]

lgtm

https://codereview.chromium.org/1324113002/diff/140002/net/websockets/websock...
File net/websockets/websocket_deflate_parameters.cc (right):

https://codereview.chromium.org/1324113002/diff/140002/net/websockets/websock...
net/websockets/websocket_deflate_parameters.cc:174: }
L166-174 are unnecessary. The value part of the client_max_window_bits parameter
coming from client is just a hint

https://codereview.chromium.org/1324113002/diff/140002/net/websockets/websock...
File net/websockets/websocket_deflate_parameters.h (right):

https://codereview.chromium.org/1324113002/diff/140002/net/websockets/websock...
net/websockets/websocket_deflate_parameters.h:38: // Returns an
WebSocketExtension instance containing this parameters.
an -> a

https://codereview.chromium.org/1324113002/diff/140002/net/websockets/websock...
net/websockets/websocket_deflate_parameters.h:38: // Returns an
WebSocketExtension instance containing this parameters.
this -> these

or

containing the parameters stored in this object.

or

representing the parameters stored in this object.

https://codereview.chromium.org/1324113002/diff/140002/net/websockets/websock...
net/websockets/websocket_deflate_parameters.h:42: // Returns false and store the
failure message to |failure_message| otherwise.
store -> stores

https://codereview.chromium.org/1324113002/diff/140002/net/websockets/websock...
File net/websockets/websocket_deflate_parameters_test.cc (right):

https://codereview.chromium.org/1324113002/diff/140002/net/websockets/websock...
net/websockets/websocket_deflate_parameters_test.cc:8: 
base/macros.h

https://codereview.chromium.org/1324113002/diff/140002/net/websockets/websock...
net/websockets/websocket_deflate_parameters_test.cc:202: {";
client_max_window_bits=15; client_max_window_bits=15",
include the client_max_window_bits version of L191 and 192.

https://codereview.chromium.org/1324113002/diff/140002/net/websockets/websock...
net/websockets/websocket_deflate_parameters_test.cc:243: {";
client_max_window_bits=12", "; client_max_window_bits=13", false},
this response is fine

[yhirano, 2015-09-10 11:20:13]

https://codereview.chromium.org/1324113002/diff/140002/net/websockets/websock...
File net/websockets/websocket_deflate_parameters.cc (right):

https://codereview.chromium.org/1324113002/diff/140002/net/websockets/websock...
net/websockets/websocket_deflate_parameters.cc:174: }
On 2015/09/10 09:06:19, tyoshino wrote:
> L166-174 are unnecessary. The value part of the client_max_window_bits
parameter
> coming from client is just a hint

Done.

https://codereview.chromium.org/1324113002/diff/140002/net/websockets/websock...
File net/websockets/websocket_deflate_parameters.h (right):

https://codereview.chromium.org/1324113002/diff/140002/net/websockets/websock...
net/websockets/websocket_deflate_parameters.h:38: // Returns an
WebSocketExtension instance containing this parameters.
On 2015/09/10 09:06:19, tyoshino wrote:
> an -> a

Done.

https://codereview.chromium.org/1324113002/diff/140002/net/websockets/websock...
net/websockets/websocket_deflate_parameters.h:38: // Returns an
WebSocketExtension instance containing this parameters.
On 2015/09/10 09:06:19, tyoshino wrote:
> this -> these
> 
> or
> 
> containing the parameters stored in this object.
> 
> or
> 
> representing the parameters stored in this object.

Done.

https://codereview.chromium.org/1324113002/diff/140002/net/websockets/websock...
net/websockets/websocket_deflate_parameters.h:42: // Returns false and store the
failure message to |failure_message| otherwise.
On 2015/09/10 09:06:19, tyoshino wrote:
> store -> stores

Done.

https://codereview.chromium.org/1324113002/diff/140002/net/websockets/websock...
File net/websockets/websocket_deflate_parameters_test.cc (right):

https://codereview.chromium.org/1324113002/diff/140002/net/websockets/websock...
net/websockets/websocket_deflate_parameters_test.cc:8: 
On 2015/09/10 09:06:19, tyoshino wrote:
> base/macros.h

Done.

https://codereview.chromium.org/1324113002/diff/140002/net/websockets/websock...
net/websockets/websocket_deflate_parameters_test.cc:202: {";
client_max_window_bits=15; client_max_window_bits=15",
On 2015/09/10 09:06:20, tyoshino wrote:
> include the client_max_window_bits version of L191 and 192.

Done.

https://codereview.chromium.org/1324113002/diff/140002/net/websockets/websock...
net/websockets/websocket_deflate_parameters_test.cc:243: {";
client_max_window_bits=12", "; client_max_window_bits=13", false},
On 2015/09/10 09:06:19, tyoshino wrote:
> this response is fine

Done.

[tyoshino (SeeGerritForStatus), 2015-09-10 11:47:12]

lgtm

[yhirano, 2015-09-10 12:15:10]

yhirano@chromium.org changed reviewers:
+ davidben@chromium.org

[yhirano, 2015-09-10 12:15:11]

+davidben@ for net/net.gypi

[davidben, 2015-09-10 15:59:39]

I didn't look through this carefully and assume it's mostly moving code around.
Two things that did catch my eye though.

https://codereview.chromium.org/1324113002/diff/180001/net/websockets/websock...
File net/websockets/websocket_deflate_parameters.h (right):

https://codereview.chromium.org/1324113002/diff/180001/net/websockets/websock...
net/websockets/websocket_deflate_parameters.h:24:
WebSocketDeflater::DO_NOT_TAKE_OVER_CONTEXT;
This violates the One Definition Rule and is undefined behavior. See
https://groups.google.com/a/chromium.org/forum/#!searchin/chromium-dev/static...

https://codereview.chromium.org/1324113002/diff/180001/net/websockets/websock...
net/websockets/websocket_deflate_parameters.h:35: };
Style: private values should be at the bottom.

[yhirano, 2015-09-11 01:45:37]

Patchset #6 (id:200001) has been deleted

[yhirano, 2015-09-11 01:46:49]

Patchset #6 (id:220001) has been deleted

[yhirano, 2015-09-11 01:49:12]

https://codereview.chromium.org/1324113002/diff/180001/net/websockets/websock...
File net/websockets/websocket_deflate_parameters.h (right):

https://codereview.chromium.org/1324113002/diff/180001/net/websockets/websock...
net/websockets/websocket_deflate_parameters.h:24:
WebSocketDeflater::DO_NOT_TAKE_OVER_CONTEXT;
On 2015/09/10 15:59:39, David Benjamin wrote:
> This violates the One Definition Rule and is undefined behavior. See
>
https://groups.google.com/a/chromium.org/forum/#!searchin/chromium-dev/static...

Thanks, I didn't notice the thread. Fixed.

https://codereview.chromium.org/1324113002/diff/180001/net/websockets/websock...
net/websockets/websocket_deflate_parameters.h:35: };
On 2015/09/10 15:59:39, David Benjamin wrote:
> Style: private values should be at the bottom.

Done.

[davidben, 2015-09-11 15:40:50]

net/net.gypi lgtm. I didn't look closely at the websockets files apart from
skimming for style and am deferring to the main review there.

https://codereview.chromium.org/1324113002/diff/240001/net/websockets/websock...
File net/websockets/websocket_deflate_parameters.h (right):

https://codereview.chromium.org/1324113002/diff/240001/net/websockets/websock...
net/websockets/websocket_deflate_parameters.h:21: using ContextTakeOverMode =
WebSocketDeflater::ContextTakeOverMode;
Nit: I'd probably stick a newline here.

https://codereview.chromium.org/1324113002/diff/240001/net/websockets/websock...
net/websockets/websocket_deflate_parameters.h:102: WindowBits(int16 bits, bool
is_specified, bool has_value)
Nit: New code should use int16_t per
http://google-styleguide.googlecode.com/svn/trunk/cppguide.html?showone=Integ...

[yhirano, 2015-09-14 02:01:10]

https://codereview.chromium.org/1324113002/diff/240001/net/websockets/websock...
File net/websockets/websocket_deflate_parameters.h (right):

https://codereview.chromium.org/1324113002/diff/240001/net/websockets/websock...
net/websockets/websocket_deflate_parameters.h:21: using ContextTakeOverMode =
WebSocketDeflater::ContextTakeOverMode;
On 2015/09/11 15:40:49, David Benjamin wrote:
> Nit: I'd probably stick a newline here.

Done.

https://codereview.chromium.org/1324113002/diff/240001/net/websockets/websock...
net/websockets/websocket_deflate_parameters.h:102: WindowBits(int16 bits, bool
is_specified, bool has_value)
On 2015/09/11 15:40:49, David Benjamin wrote:
> Nit: New code should use int16_t per
>
http://google-styleguide.googlecode.com/svn/trunk/cppguide.html?showone=Integ...

Done.

[yhirano, 2015-09-14 02:01:15]

The CQ bit was checked by yhirano@chromium.org

[yhirano, 2015-09-14 02:01:16]

The patchset sent to the CQ was uploaded after l-g-t-m from
tyoshino@chromium.org, davidben@chromium.org
Link to the patchset: https://codereview.chromium.org/1324113002/#ps260001
(title: " ")

[commit-bot: I haz the power, 2015-09-14 02:02:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1324113002/260001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1324113002/260001

[yhirano, 2015-09-14 03:52:06]

Patchset #8 (id:280001) has been deleted

[yhirano, 2015-09-14 03:52:28]

The CQ bit was checked by yhirano@chromium.org

[yhirano, 2015-09-14 03:52:28]

The patchset sent to the CQ was uploaded after l-g-t-m from
tyoshino@chromium.org, davidben@chromium.org
Link to the patchset: https://codereview.chromium.org/1324113002/#ps260001
(title: " ")

[commit-bot: I haz the power, 2015-09-14 03:53:37]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1324113002/260001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1324113002/260001

[yhirano, 2015-09-14 04:32:40]

The CQ bit was unchecked by yhirano@chromium.org

[yhirano, 2015-09-14 04:33:48]

The CQ bit was checked by yhirano@chromium.org

[yhirano, 2015-09-14 04:33:49]

The patchset sent to the CQ was uploaded after l-g-t-m from
tyoshino@chromium.org, davidben@chromium.org
Link to the patchset: https://codereview.chromium.org/1324113002/#ps300001
(title: " ")

[commit-bot: I haz the power, 2015-09-14 04:34:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1324113002/300001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1324113002/300001

[commit-bot: I haz the power, 2015-09-14 05:47:00]

Committed patchset #8 (id:300001)

[commit-bot: I haz the power, 2015-09-14 05:49:08]

Patchset 8 (id:??) landed as
https://crrev.com/8387aee95b6206d01bbd988f87b0d082ad18ed4e
Cr-Commit-Position: refs/heads/master@{#348567}

[commit-bot: I haz the power, 2015-09-23 12:29:49]

Patchset 8 (id:??) landed as
https://crrev.com/8387aee95b6206d01bbd988f87b0d082ad18ed4e
Cr-Commit-Position: refs/heads/master@{#348567}