Add a new QUIC platform API for text utilities.

Add a new QUIC platform API for text utilities.

Merge internal change: 143121845

Committed: https://crrev.com/5ca9da50e452c66316bb8b0b993a52bd27d07166
Cr-Commit-Position: refs/heads/master@{#440905}

[Ryan Hamilton, 2016-12-26 23:41:11]

The CQ bit was checked by rch@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-26 23:41:17]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-26 23:43:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-26 23:43:07]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[Ryan Hamilton, 2016-12-27 00:04:19]

The CQ bit was checked by rch@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-27 00:04:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-27 00:08:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-27 00:08:01]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Ryan Hamilton, 2016-12-27 00:19:44]

The CQ bit was checked by rch@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-27 00:19:50]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-27 00:37:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-27 00:37:05]

Dry run: Try jobs failed on following builders:
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Ryan Hamilton, 2016-12-27 01:58:36]

The CQ bit was checked by rch@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-27 01:58:43]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-27 02:21:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-27 02:21:04]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Ryan Hamilton, 2016-12-27 04:53:13]

The CQ bit was checked by rch@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-27 04:53:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Ryan Hamilton, 2016-12-27 23:14:18]

rch@chromium.org changed reviewers:
+ jri@chromium.org

[Jana, 2016-12-27 23:52:49]

A few nits

https://codereview.chromium.org/2603723002/diff/140001/net/quic/core/quic_cry...
File net/quic/core/quic_crypto_server_stream.cc (left):

https://codereview.chromium.org/2603723002/diff/140001/net/quic/core/quic_cry...
net/quic/core/quic_crypto_server_stream.cc:434: output->resize(len);
Why's this resize not required anymore?

https://codereview.chromium.org/2603723002/diff/140001/net/quic/platform/api/...
File net/quic/platform/api/quic_text_utils.h (right):

https://codereview.chromium.org/2603723002/diff/140001/net/quic/platform/api/...
net/quic/platform/api/quic_text_utils.h:49: // hexadecimal representation.
How does the conversion cause the output to be 2*|length| ?

https://codereview.chromium.org/2603723002/diff/140001/net/quic/platform/api/...
net/quic/platform/api/quic_text_utils.h:62: // Converts |data| from a
hexadecimal ASCII std::string to binary.
nit: "from a  hexadecimal number in ASCII std::string form to a binary number in
ASCII std::string form."

Also, I would suggest making the comments for HexEncode and HexDecode uniform.

https://codereview.chromium.org/2603723002/diff/140001/net/quic/platform/api/...
File net/quic/platform/api/quic_text_utils_test.cc (right):

https://codereview.chromium.org/2603723002/diff/140001/net/quic/platform/api/...
net/quic/platform/api/quic_text_utils_test.cc:71: EXPECT_EQ("48656c6c6f",
QuicTextUtils::HexEncode("Hello", 5));
nit: Make this call HexEncode("HelloWorld", 5) so that you check that the length
variable is being respected.

[Ryan Hamilton, 2016-12-28 01:48:38]

https://codereview.chromium.org/2603723002/diff/140001/net/quic/core/quic_cry...
File net/quic/core/quic_crypto_server_stream.cc (left):

https://codereview.chromium.org/2603723002/diff/140001/net/quic/core/quic_cry...
net/quic/core/quic_crypto_server_stream.cc:434: output->resize(len);
On 2016/12/27 23:52:48, Jana wrote:
> Why's this resize not required anymore?

All of this logic has moved into the new QuicTextUtils::Base64Encode method
(including the code to strip the padding)

https://codereview.chromium.org/2603723002/diff/140001/net/quic/platform/api/...
File net/quic/platform/api/quic_text_utils.h (right):

https://codereview.chromium.org/2603723002/diff/140001/net/quic/platform/api/...
net/quic/platform/api/quic_text_utils.h:49: // hexadecimal representation.
On 2016/12/27 23:52:48, Jana wrote:
> How does the conversion cause the output to be 2*|length| ?

'cause each byte turns into two ascii bytes:

H => 48
' ' => 20
etc.

https://codereview.chromium.org/2603723002/diff/140001/net/quic/platform/api/...
net/quic/platform/api/quic_text_utils.h:62: // Converts |data| from a
hexadecimal ASCII std::string to binary.
On 2016/12/27 23:52:48, Jana wrote:
> nit: "from a  hexadecimal number in ASCII std::string form to a binary number
in
> ASCII std::string form."

But it doesn't take a "number", does it? It just takes binary data.

(removed the extra std:: in the comments)

> Also, I would suggest making the comments for HexEncode and HexDecode uniform.

Done (and in the internal version of this CL).

https://codereview.chromium.org/2603723002/diff/140001/net/quic/platform/api/...
File net/quic/platform/api/quic_text_utils_test.cc (right):

https://codereview.chromium.org/2603723002/diff/140001/net/quic/platform/api/...
net/quic/platform/api/quic_text_utils_test.cc:71: EXPECT_EQ("48656c6c6f",
QuicTextUtils::HexEncode("Hello", 5));
On 2016/12/27 23:52:49, Jana wrote:
> nit: Make this call HexEncode("HelloWorld", 5) so that you check that the
length
> variable is being respected.

Done.

[Jana, 2016-12-28 03:11:51]

lgtm

LGTM, modulo one nit.

https://codereview.chromium.org/2603723002/diff/140001/net/quic/platform/api/...
File net/quic/platform/api/quic_text_utils.h (right):

https://codereview.chromium.org/2603723002/diff/140001/net/quic/platform/api/...
net/quic/platform/api/quic_text_utils.h:49: // hexadecimal representation.
On 2016/12/28 01:48:38, Ryan Hamilton wrote:
> On 2016/12/27 23:52:48, Jana wrote:
> > How does the conversion cause the output to be 2*|length| ?
> 
> 'cause each byte turns into two ascii bytes:
> 
> H => 48
> ' ' => 20
> etc.

Ah, right. I think the comment should probably say "ASCII" instead of "binary"
then for the input... the input is an ASCII string, right? Your call.

[Ryan Hamilton, 2016-12-28 19:15:04]

Submitting.

https://codereview.chromium.org/2603723002/diff/140001/net/quic/platform/api/...
File net/quic/platform/api/quic_text_utils.h (right):

https://codereview.chromium.org/2603723002/diff/140001/net/quic/platform/api/...
net/quic/platform/api/quic_text_utils.h:49: // hexadecimal representation.
On 2016/12/28 03:11:51, Jana wrote:
> On 2016/12/28 01:48:38, Ryan Hamilton wrote:
> > On 2016/12/27 23:52:48, Jana wrote:
> > > How does the conversion cause the output to be 2*|length| ?
> > 
> > 'cause each byte turns into two ascii bytes:
> > 
> > H => 48
> > ' ' => 20
> > etc.
> 
> Ah, right. I think the comment should probably say "ASCII" instead of "binary"
> then for the input... the input is an ASCII string, right? Your call.

No, the input is a sequence of bytes, each of which can have any value in
[0,255]. An ASCII string is a sequence of bytes in which each characters is
[0,127] or more likely the printable range.

[Ryan Hamilton, 2016-12-28 19:15:10]

The CQ bit was checked by rch@chromium.org

[Ryan Hamilton, 2016-12-28 19:15:10]

The patchset sent to the CQ was uploaded after l-g-t-m from jri@chromium.org
Link to the patchset: https://codereview.chromium.org/2603723002/#ps180001
(title: "Rebase")

[commit-bot: I haz the power, 2016-12-28 19:15:27]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-28 19:41:05]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-28 19:41:07]

Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Ryan Hamilton, 2016-12-28 20:27:17]

The CQ bit was checked by rch@chromium.org

[Ryan Hamilton, 2016-12-28 20:27:17]

The patchset sent to the CQ was uploaded after l-g-t-m from jri@chromium.org
Link to the patchset: https://codereview.chromium.org/2603723002/#ps200001
(title: "fix test")

[commit-bot: I haz the power, 2016-12-28 20:27:29]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-28 21:51:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-28 21:51:27]

Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[Ryan Hamilton, 2016-12-28 22:45:08]

The CQ bit was checked by rch@chromium.org

[Ryan Hamilton, 2016-12-28 22:45:09]

The patchset sent to the CQ was uploaded after l-g-t-m from jri@chromium.org
Link to the patchset: https://codereview.chromium.org/2603723002/#ps220001
(title: "net/tools/quic/quic_packet_printer_bin.cc")

[commit-bot: I haz the power, 2016-12-28 22:45:19]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Ryan Hamilton, 2016-12-28 23:27:35]

Description was changed from

==========
Add a new QUIC platform API for text utilities.

Merge internal change: 142984448
==========

to

==========
Add a new QUIC platform API for text utilities.

Merge internal change: 143121845
==========

[commit-bot: I haz the power, 2016-12-28 23:46:28]

CQ is committing da patch.

Bot data: {"patchset_id": 220001, "attempt_start_ts": 1482965108765540,
"parent_rev": "400096cf3d34583575561ed47a85fea475ccb1c3", "commit_rev":
"6cc47f4dae9bafda4f25873e76bcd447c0ccc19c"}

[commit-bot: I haz the power, 2016-12-28 23:46:59]

Description was changed from

==========
Add a new QUIC platform API for text utilities.

Merge internal change: 143121845
==========

to

==========
Add a new QUIC platform API for text utilities.

Merge internal change: 143121845

Review-Url: https://codereview.chromium.org/2603723002
==========

[commit-bot: I haz the power, 2016-12-28 23:47:01]

Committed patchset #12 (id:220001)

[commit-bot: I haz the power, 2017-01-02 15:50:44]

Description was changed from

==========
Add a new QUIC platform API for text utilities.

Merge internal change: 143121845

Review-Url: https://codereview.chromium.org/2603723002
==========

to

==========
Add a new QUIC platform API for text utilities.

Merge internal change: 143121845

Committed: https://crrev.com/5ca9da50e452c66316bb8b0b993a52bd27d07166
Cr-Commit-Position: refs/heads/master@{#440905}
==========

[commit-bot: I haz the power, 2017-01-02 15:50:46]

Patchset 12 (id:??) landed as
https://crrev.com/5ca9da50e452c66316bb8b0b993a52bd27d07166
Cr-Commit-Position: refs/heads/master@{#440905}