Teach gtest how to print QuicConsumedData and QuicEncryptedPacket.

Teach gtest how to print QuicConsumedData and QuicEncryptedPacket.

R=rch@chromium.org,glider@chromium.org
BUG=170327
TEST=net_unittests has no unitialized memory errors.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=177869

[wtc, 2013-01-17 02:34:58]

https://codereview.chromium.org/11973021/diff/1/net/quic/quic_protocol.cc
File net/quic/quic_protocol.cc (left):

https://codereview.chromium.org/11973021/diff/1/net/quic/quic_protocol.cc#old...
net/quic/quic_protocol.cc:82: os << "least_waiting: " <<
sent_info.least_unacked;

I also fixed this typo.

https://codereview.chromium.org/11973021/diff/1/net/quic/quic_protocol.h
File net/quic/quic_protocol.h (right):

https://codereview.chromium.org/11973021/diff/1/net/quic/quic_protocol.h#newc...
net/quic/quic_protocol.h:49: bool fin_consumed;

There are padding bytes after this bool member.

https://codereview.chromium.org/11973021/diff/1/net/quic/quic_protocol.h#newc...
net/quic/quic_protocol.h:387: bool owns_buffer_;

This class (QuicData) is the base class of QuicEncryptedPacket.
There are padding bytes after this bool member.

[Ryan Hamilton, 2013-01-17 05:00:00]

Ugh.  I'd really prefer to not have << overloads, if possible since it's not
typical code style.  But I'm happy to add it if this is the only/best way to
avoid the valgrind problems.

[Alexander Potapenko, 2013-01-17 08:19:19]

On 2013/01/17 05:00:00, Ryan Hamilton wrote:
> Ugh.  I'd really prefer to not have << overloads, if possible since it's not
> typical code style.  But I'm happy to add it if this is the only/best way to
> avoid the valgrind problems.

"operator<<" is defined for several classes in Chromium:
https://code.google.com/p/chromium/source/search?q=%22operator%3C%3C%22&origq...

However gTest allows to define PrintTo() instead of operator<< for the same
purpose
(https://code.google.com/p/googletest/wiki/AdvancedGuide#Teaching_Google_Test_...)

[Alexander Potapenko, 2013-01-17 08:20:06]

https://codereview.chromium.org/11973021/diff/1/net/quic/quic_protocol.h
File net/quic/quic_protocol.h (right):

https://codereview.chromium.org/11973021/diff/1/net/quic/quic_protocol.h#newc...
net/quic/quic_protocol.h:49: bool fin_consumed;
On 2013/01/17 02:34:58, wtc wrote:
> 
> There are padding bytes after this bool member.
It's better to have this in the code comment rather than in Rietveld.

[Alexander Potapenko, 2013-01-17 08:26:37]

Also note the tests started failing under Valgrind.

[wtc, 2013-01-17 16:49:21]

On 2013/01/17 08:26:37, Alexander Potapenko wrote:
> Also note the tests started failing under Valgrind.

Thank you for letting me know. The QuicStreamFactoryTest.CancelCreate
failure on mac_valgrind is caused by a different bug, which I fixed
in r177297 (https://chromiumcodereview.appspot.com/11956029/).

That mac_valgrind run used r177252, which was before my fix.

The other test failures under Valgrind were unrelated to the net/quic
code.

[wtc, 2013-01-17 18:59:35]

Please review patch set 3. I added the comments that glider
suggested. Thanks.

glider: what do you think of simply suppressing the
Memcheck:Uninitialized error in the default gtest object
printer testing::internal::PrintBytesInObjectTo()? Any
struct/class that has a bool data member is likely to have
padding bytes and run into this problem.

https://codereview.chromium.org/11973021/diff/1/net/quic/quic_protocol.h
File net/quic/quic_protocol.h (right):

https://codereview.chromium.org/11973021/diff/1/net/quic/quic_protocol.h#newc...
net/quic/quic_protocol.h:49: bool fin_consumed;

On 2013/01/17 08:20:06, Alexander Potapenko wrote:
>
> It's better to have this in the code comment rather than in Rietveld.

Done.

I verified the bool member was the problem using two methods.
One method was to simply defining the boolean members as
size_t to eliminate padding. That fixed the valgrind memory
errors. (The other method was to use a memset call to
initialize the padding bytes.)

https://codereview.chromium.org/11973021/diff/13002/net/quic/quic_protocol.h
File net/quic/quic_protocol.h (right):

https://codereview.chromium.org/11973021/diff/13002/net/quic/quic_protocol.h#...
net/quic/quic_protocol.h:199: std::ostream& os, const ReceivedPacketInfo& s);

I copied the operator<< solution from several other classes
in the same file, such as this one.

But I just noticed that none of those classes have a bool
member. So the operator<< may have been added for a different
reason.

[Ryan Hamilton, 2013-01-18 01:39:47]

lgtm

[commit-bot: I haz the power, 2013-01-18 05:20:26]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wtc@chromium.org/11973021/1006

[commit-bot: I haz the power, 2013-01-18 21:12:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wtc@chromium.org/11973021/34002

[commit-bot: I haz the power, 2013-01-18 21:40:29]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_aura.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-01-19 02:21:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wtc@chromium.org/11973021/34002

[commit-bot: I haz the power, 2013-01-19 02:31:54]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_aura.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-01-20 01:25:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/wtc@chromium.org/11973021/34002

[commit-bot: I haz the power, 2013-01-20 02:49:11]

Change committed as 177869