Modify PeerConnection for end-to-end QuicDataChannel usage

webrtc/base/bytebuffer.cc

Index: webrtc/base/bytebuffer.cc
diff --git a/webrtc/base/bytebuffer.cc b/webrtc/base/bytebuffer.cc
index 8bc1f23670e7917736ee1e79fa304b57ca1187ff..1e45394a36fe1ea1abc3573f0b2baef8332ddebf 100644
--- a/webrtc/base/bytebuffer.cc
+++ b/webrtc/base/bytebuffer.cc
@@ -235,4 +235,41 @@ void ByteBuffer::Clear() {
   ++version_;
 }
 

[pthatcher1, 2016/03/30 20:34:49]

Since this is basically what is described in
https://developers.google.com/protocol-buffers/docs/encoding#varints, we should
probably add a comment with a reference to that, with the caveat that we only go
to 64 bits, not 128 bits.

[mikescarlett, 2016/04/05 19:58:51]

I added a comment.

+bool ByteBuffer::ReadVarint(uint64_t* val) {

[pthatcher1, 2016/03/30 20:34:49]

These should be ReadUvarint and WriteUvarint, since they are unsigned.

[pthatcher1, 2016/03/30 20:34:49]

It might be a good idea to pull this varint stuff into its own CL and land it
separately.

[mikescarlett, 2016/04/05 19:58:51]

Done.

[mikescarlett, 2016/04/05 19:58:51]

Will make this a separate CL.

+  if (!val)

[pthatcher1, 2016/03/30 20:34:49]

{}s please

[mikescarlett, 2016/04/05 19:58:51]

Done.

+    return false;
+
+  uint64_t v = 0;
+  for (int i = 0; i < 64; i += 7) {
+    char byte;
+    if (!ReadBytes(&byte, 1)) {
+      return false;
+    }
+    uint8_t numeric_value = static_cast<uint8_t>(byte);
+    // Read first 7 bits of the byte, then offset by bits read so far.
+    v |= static_cast<uint64_t>(numeric_value & (0x80 - 1)) << i;

[pthatcher1, 2016/03/30 20:34:50]

This might be more clear as:

v |= (static_cast<uint64_t>(byte) & 0x7F) << i;
if (byte < 0x80) {
  *val = v;
  return true;
}

In other words, use 0x7F instead of 0x80-1 and remove "numeric_value" if it's
not needed.

[mikescarlett, 2016/04/05 19:58:51]

Done.

+    // True if the msb is not a continuation byte.
+    if (numeric_value < 0x80) {
+      *val = (byte_order_ == ORDER_NETWORK) ? NetworkToHost64(v) : v;

[pthatcher1, 2016/03/30 20:34:49]

I don't think this is correct.  Varints are always LSB first, regardless of the
byte_order_, so we shouldn't do this swap at the end.

[mikescarlett, 2016/04/05 19:58:51]

Correct. I removed this.

+      return true;
+    }
+  }
+  return false;
+}
+
+void ByteBuffer::WriteVarint(uint64_t val) {
+  // Unsigned integers are serialized 7 bits at a time, starting with the
+  // least significant bit. The most significant bit (msb) in each output byte
+  // indicates if there is a continuation byte (msb = 1). At most 10 bytes are
+  // needed for 64-bit integers.
+  while (val >= 0x80) {
+    uint32_t numeric_value = ((val & (0x80 - 1)) | 0x80);
+    char byte = static_cast<char>(numeric_value);

[pthatcher1, 2016/03/30 20:34:49]

Does this work?  It would be a bit shorter:

char byte = static_cast<char>(val) | 0x80;

Or this:

char byte = static_cast<char>(val | 0x80);


In other words, the mask with 0x7FF isn't necessary because you truncate right
after anyway.

[mikescarlett, 2016/04/05 19:58:51]

That works. I changed it.

+    WriteBytes(&byte, 1);
+    val >>= 7;
+  }
+  char byte = static_cast<char>(val);

[pthatcher1, 2016/03/30 20:34:50]

Might as well call this "last_byte".

[mikescarlett, 2016/04/05 19:58:51]

Done.

+  WriteBytes(&byte, 1);
+}
+
 }  // namespace rtc