QUIC - Minor clean up of QUIC Server Info memory cache.

QUIC - Minor clean up of QUIC Server Info memory cache.

R=wtc@chromium.org

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

[wtc, 2014-04-25 18:17:28]

Patch set 2 LGTM.

https://codereview.chromium.org/255783002/diff/20001/net/quic/crypto/quic_cry...
File net/quic/crypto/quic_crypto_client_config_test.cc (left):

https://codereview.chromium.org/255783002/diff/20001/net/quic/crypto/quic_cry...
net/quic/crypto/quic_crypto_client_config_test.cc:166: QuicServerId
canonical_server_id("www.google.com", 80, false,

Please undo the changes to this unit test. In this unit test,
canonical_server_id is used as the canonical server id. other_server_id contains
a different hostname (mail.google.com vs. www.google.com).

https://codereview.chromium.org/255783002/diff/20001/net/quic/crypto/quic_cry...
File net/quic/crypto/quic_crypto_client_config_test.cc (right):

https://codereview.chromium.org/255783002/diff/20001/net/quic/crypto/quic_cry...
net/quic/crypto/quic_crypto_client_config_test.cc:248: EXPECT_EQ(1u,
other->generation_counter());

Once we verify |other| and |state| point to the same object, lines 243-247
should all be deleted. Line 248 can be kept because it is related to the
EXPECT_EQ on line 262.

You can add
  EXPECT_EQ(1u, state->generation_counter());
after line 237.

[ramant (doing other things), 2014-04-25 18:55:30]

Thanks much Wan-Teh. Undid the canonical_server_id rename. Added check for 1u.

https://codereview.chromium.org/255783002/diff/20001/net/quic/crypto/quic_cry...
File net/quic/crypto/quic_crypto_client_config_test.cc (left):

https://codereview.chromium.org/255783002/diff/20001/net/quic/crypto/quic_cry...
net/quic/crypto/quic_crypto_client_config_test.cc:166: QuicServerId
canonical_server_id("www.google.com", 80, false,
On 2014/04/25 18:17:28, wtc wrote:
> 
> Please undo the changes to this unit test. In this unit test,
> canonical_server_id is used as the canonical server id. other_server_id
contains
> a different hostname (http://mail.google.com vs. http://www.google.com).

Done.

https://codereview.chromium.org/255783002/diff/20001/net/quic/crypto/quic_cry...
File net/quic/crypto/quic_crypto_client_config_test.cc (right):

https://codereview.chromium.org/255783002/diff/20001/net/quic/crypto/quic_cry...
net/quic/crypto/quic_crypto_client_config_test.cc:248: EXPECT_EQ(1u,
other->generation_counter());
On 2014/04/25 18:17:28, wtc wrote:
> 
> Once we verify |other| and |state| point to the same object, lines 243-247
> should all be deleted. Line 248 can be kept because it is related to the
> EXPECT_EQ on line 262.
> 
> You can add
>   EXPECT_EQ(1u, state->generation_counter());
> after line 237.

Done.

[ramant (doing other things), 2014-04-25 18:55:34]

The CQ bit was checked by rtenneti@chromium.org

[wtc, 2014-04-25 18:59:18]

Patch set 3 LGTM. Thanks.

[commit-bot: I haz the power, 2014-04-25 22:11:32]

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

[commit-bot: I haz the power, 2014-04-25 23:19:53]

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

[commit-bot: I haz the power, 2014-04-25 23:19:54]

Try jobs failed on following builders:
  tryserver.chromium on chromium_presubmit

[ramant (doing other things), 2014-04-25 23:51:04]

Hi michaeln@,
  Requesting rubber stamping (for
chrome/browser/browsing_data/browsing_data_remover.cc).

Sincerely appreciating it
raman

[ramant (doing other things), 2014-04-28 16:12:42]

On 2014/04/25 23:51:04, ramant wrote:
> Hi michaeln@,
>   Requesting rubber stamping (for
> chrome/browser/browsing_data/browsing_data_remover.cc).
> 
> Sincerely appreciating it
> raman

Hi michaeln@, Mike West, Bernhard Bauer,
  Would appreciate if we could owner's rubber stamp for
browsing_data_remover.cc.

thanks very much
raman

[michaeln, 2014-04-28 18:08:01]

rs lgtm

[ramant (doing other things), 2014-04-28 18:08:52]

On 2014/04/28 18:08:01, michaeln wrote:
> rs lgtm

Thanks much michaeln,
raman

[ramant (doing other things), 2014-04-28 18:08:56]

The CQ bit was checked by rtenneti@chromium.org

[commit-bot: I haz the power, 2014-04-28 18:09:48]

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

[commit-bot: I haz the power, 2014-04-28 23:23:19]

Change committed as 266684