QUIC - fix uninitialized reads in net::QuicCongestionManager::SetFromConfig

QUIC - fix uninitialized reads in
net::QuicCongestionManager::SetFromConfig

BUG=313738
R=rch@chromium.org

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

[Ryan Hamilton, 2013-10-31 19:43:43]

lgtm

Thanks for doing this.  How does it fix the uninitialized reads?

https://codereview.chromium.org/50603015/diff/1/net/quic/quic_connection_help...
File net/quic/quic_connection_helper_test.cc (right):

https://codereview.chromium.org/50603015/diff/1/net/quic/quic_connection_help...
net/quic/quic_connection_helper_test.cc:136: ON_CALL(*send_algorithm_,
SetFromConfig(_, _)).WillByDefault(Return());
SetFromConfig is a void method, right?  Can you do
EXPECT_CALL(...).Times(AnyNumber())?  The Return() for void is a bit funky.

https://codereview.chromium.org/50603015/diff/1/net/quic/quic_http_stream_tes...
File net/quic/quic_http_stream_test.cc (right):

https://codereview.chromium.org/50603015/diff/1/net/quic/quic_http_stream_tes...
net/quic/quic_http_stream_test.cc:201: ON_CALL(*send_algorithm_,
SetFromConfig(_, _)).WillByDefault(Return());
ditto

[ramant (doing other things), 2013-10-31 23:11:51]

Wasn't able to reproduce the issue with my local valgrind builds. Trying TSAN,
Vangrind on try bots.

We got the error when the following gets printed during tests. Adding the
EXPECT_CALL fixed it because it was not printing "Uninteresting mock...." didn't
get called.

QuicConfig's initial_round_trip_time_us_ is initialized to default values (or 0,
but we don't call set 

GMOCK WARNING:
Uninteresting mock function call - returning directly.
    Function call: SetFromConfig(@0x1ed3933c 184-byte object <43-47 53-54 01-00
00-00 00-DA FF-BF 4B-55 00-00 A0-36 29-3B A4-36 29-3B A4-36 29-3B 51-42 49-43
49-43 53-4C 01-00 00-00 00-9A 28-3B 58-02 00-00 78-00 00-00 04-00 00-00 4B-41
54-4F 00-00 00-00 ... 00-A6 09-00 64-00 00-00 0A-00 00-00 80-9A 28-3B 53-4D
53-53 00-00 00-00 00-00 00-00 C0-05 00-00 B0-04 00-00 02-11 00-00 49-52 54-54
00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 20-DC FF-BF>, false)
Stack trace:

https://codereview.chromium.org/50603015/diff/1/net/quic/quic_connection_help...
File net/quic/quic_connection_helper_test.cc (right):

https://codereview.chromium.org/50603015/diff/1/net/quic/quic_connection_help...
net/quic/quic_connection_helper_test.cc:136: ON_CALL(*send_algorithm_,
SetFromConfig(_, _)).WillByDefault(Return());
On 2013/10/31 19:43:44, Ryan Hamilton wrote:
> SetFromConfig is a void method, right?  Can you do
> EXPECT_CALL(...).Times(AnyNumber())?  The Return() for void is a bit funky.

Done.

https://codereview.chromium.org/50603015/diff/1/net/quic/quic_http_stream_tes...
File net/quic/quic_http_stream_test.cc (right):

https://codereview.chromium.org/50603015/diff/1/net/quic/quic_http_stream_tes...
net/quic/quic_http_stream_test.cc:201: ON_CALL(*send_algorithm_,
SetFromConfig(_, _)).WillByDefault(Return());
On 2013/10/31 19:43:44, Ryan Hamilton wrote:
> ditto

Done.

[ramant (doing other things), 2013-10-31 23:12:42]

On 2013/10/31 23:11:51, ramant wrote:
> Wasn't able to reproduce the issue with my local valgrind builds. Trying TSAN,
> Vangrind on try bots.
> 
> We got the error when the following gets printed during tests. Adding the
> EXPECT_CALL fixed it because it was not printing "Uninteresting mock...."
didn't
> get called.
> 
> QuicConfig's initial_round_trip_time_us_ is initialized to default values (or
0,
> but we don't call set 

I meant to say we didn't call set, but we don't need to call set because the
values are 0.


> 
> GMOCK WARNING:
> Uninteresting mock function call - returning directly.
>     Function call: SetFromConfig(@0x1ed3933c 184-byte object <43-47 53-54
01-00
> 00-00 00-DA FF-BF 4B-55 00-00 A0-36 29-3B A4-36 29-3B A4-36 29-3B 51-42 49-43
> 49-43 53-4C 01-00 00-00 00-9A 28-3B 58-02 00-00 78-00 00-00 04-00 00-00 4B-41
> 54-4F 00-00 00-00 ... 00-A6 09-00 64-00 00-00 0A-00 00-00 80-9A 28-3B 53-4D
> 53-53 00-00 00-00 00-00 00-00 C0-05 00-00 B0-04 00-00 02-11 00-00 49-52 54-54
> 00-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 20-DC FF-BF>, false)
> Stack trace:
> 
>
https://codereview.chromium.org/50603015/diff/1/net/quic/quic_connection_help...
> File net/quic/quic_connection_helper_test.cc (right):
> 
>
https://codereview.chromium.org/50603015/diff/1/net/quic/quic_connection_help...
> net/quic/quic_connection_helper_test.cc:136: ON_CALL(*send_algorithm_,
> SetFromConfig(_, _)).WillByDefault(Return());
> On 2013/10/31 19:43:44, Ryan Hamilton wrote:
> > SetFromConfig is a void method, right?  Can you do
> > EXPECT_CALL(...).Times(AnyNumber())?  The Return() for void is a bit funky.
> 
> Done.
> 
>
https://codereview.chromium.org/50603015/diff/1/net/quic/quic_http_stream_tes...
> File net/quic/quic_http_stream_test.cc (right):
> 
>
https://codereview.chromium.org/50603015/diff/1/net/quic/quic_http_stream_tes...
> net/quic/quic_http_stream_test.cc:201: ON_CALL(*send_algorithm_,
> SetFromConfig(_, _)).WillByDefault(Return());
> On 2013/10/31 19:43:44, Ryan Hamilton wrote:
> > ditto
> 
> Done.

[commit-bot: I haz the power, 2013-11-01 00:17:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rtenneti@chromium.org/50603015/280001

[commit-bot: I haz the power, 2013-11-01 13:07:14]

Change committed as 232344