Cleanup of QuicInMemoryCache to sync with internal version.

Cleanup of QuicInMemoryCache to sync with internal version.

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

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

[Ryan Hamilton, 2013-09-26 04:40:40]

Merges pending internal change 53300985

[ramant (doing other things), 2013-09-26 15:32:50]

lgtm

These changes look great.

https://codereview.chromium.org/24596008/diff/4001/net/tools/quic/quic_in_mem...
File net/tools/quic/quic_in_memory_cache.cc (left):

https://codereview.chromium.org/24596008/diff/4001/net/tools/quic/quic_in_mem...
net/tools/quic/quic_in_memory_cache.cc:47: virtual void
HandleTrailerWarning(BalsaFrame* framer) { UnhandledError(); }
nit: internal code seems to have these two methods. Should we consider adding
them to NoOpBalsaVisitor to match internal code. Your call.

https://codereview.chromium.org/24596008/diff/4001/net/tools/quic/quic_in_mem...
File net/tools/quic/quic_in_memory_cache.cc (right):

https://codereview.chromium.org/24596008/diff/4001/net/tools/quic/quic_in_mem...
net/tools/quic/quic_in_memory_cache.cc:49: }
nit: indentation at line# 49.

https://codereview.chromium.org/24596008/diff/4001/net/tools/quic/quic_in_mem...
net/tools/quic/quic_in_memory_cache.cc:112: void
QuicInMemoryCache::ResetForTests() {
nit: would be great if QuicInMemoryCache::ResetForTests came after
QuicInMemoryCache::QuicInMemoryCache to match internal code (I can do this
during the next merge).

[Ryan Hamilton, 2013-09-26 17:58:08]

https://codereview.chromium.org/24596008/diff/4001/net/tools/quic/quic_in_mem...
File net/tools/quic/quic_in_memory_cache.cc (left):

https://codereview.chromium.org/24596008/diff/4001/net/tools/quic/quic_in_mem...
net/tools/quic/quic_in_memory_cache.cc:47: virtual void
HandleTrailerWarning(BalsaFrame* framer) { UnhandledError(); }
On 2013/09/26 15:32:50, ramant wrote:
> nit: internal code seems to have these two methods. Should we consider adding
> them to NoOpBalsaVisitor to match internal code. Your call.

Yes, they're in the internal version... but they're also in basla_visitor.h, and
the balsa_framer.  I think we might want to refresh our balsa code at some
point, but perhaps not here.

https://codereview.chromium.org/24596008/diff/4001/net/tools/quic/quic_in_mem...
File net/tools/quic/quic_in_memory_cache.cc (right):

https://codereview.chromium.org/24596008/diff/4001/net/tools/quic/quic_in_mem...
net/tools/quic/quic_in_memory_cache.cc:49: }
On 2013/09/26 15:32:50, ramant wrote:
> nit: indentation at line# 49.

Done.

https://codereview.chromium.org/24596008/diff/4001/net/tools/quic/quic_in_mem...
net/tools/quic/quic_in_memory_cache.cc:112: void
QuicInMemoryCache::ResetForTests() {
On 2013/09/26 15:32:50, ramant wrote:
> nit: would be great if QuicInMemoryCache::ResetForTests came after
> QuicInMemoryCache::QuicInMemoryCache to match internal code (I can do this
> during the next merge).

Done.

[commit-bot: I haz the power, 2013-09-26 18:01:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rch@chromium.org/24596008/8001

[commit-bot: I haz the power, 2013-09-26 22:27:47]

Change committed as 225588

[commit-bot: I haz the power, 2013-09-28 00:04:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rch@chromium.org/24596008/47001

[commit-bot: I haz the power, 2013-09-28 04:01:09]

Retried try job too often on linux_chromeos for step(s) unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...

[commit-bot: I haz the power, 2013-09-28 04:47:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rch@chromium.org/24596008/47001

[commit-bot: I haz the power, 2013-09-28 06:00:01]

Retried try job too often on linux_aura for step(s) unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...

[commit-bot: I haz the power, 2013-09-28 06:10:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rch@chromium.org/24596008/47001

[commit-bot: I haz the power, 2013-09-28 07:49:12]

Retried try job too often on linux_aura for step(s) unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...

[commit-bot: I haz the power, 2013-09-28 13:52:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rch@chromium.org/24596008/47001

[commit-bot: I haz the power, 2013-09-28 15:25:38]

Retried try job too often on linux_aura for step(s) unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...

[commit-bot: I haz the power, 2013-09-28 15:55:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rch@chromium.org/24596008/47001

[commit-bot: I haz the power, 2013-09-28 16:48:55]

Change committed as 225856