repair QUIC server push support which was broken by a recent refactor |
|||||
Issue descriptionAs part of weekly merge of internal changes (https://codereview.chromium.org/1877703002), merge of internal change 119198371 broke QUIC's server push support. The new method QuicSpdyStream::OnPromiseHeaderList() was not overridden in QuicChromiumClient as its predecessor OnPromiseHeadersComplete() was. As a result, on receipt of a PUSH_PROMISE QUIC in chromium closes the connection with a error QUIC_INVALID_HEADERS_STREAM_DATA. Unfortunately none of the existing push related unit tests caught this. There should be a server push test(s) in quic_network_transaction_unittest.cc to prevent this from re-occurring.
,
Jun 30 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/769733cda1cf53c42ded5b8610b80e107a72771f commit 769733cda1cf53c42ded5b8610b80e107a72771f Author: ckrasic <ckrasic@chromium.org> Date: Thu Jun 30 00:42:13 2016 QUIC - expand server push test coverage and fix recent regression. Fix regression after https://codereview.chromium.org/1877703002. Add server push test to quic_network_transaction_unittest.cc that without the above fix catches the regression. R=rch@chromium.org BUG= 622043 Review-Url: https://codereview.chromium.org/2107353003 Cr-Commit-Position: refs/heads/master@{#403040} [modify] https://crrev.com/769733cda1cf53c42ded5b8610b80e107a72771f/net/quic/quic_chromium_client_stream.cc [modify] https://crrev.com/769733cda1cf53c42ded5b8610b80e107a72771f/net/quic/quic_chromium_client_stream.h [modify] https://crrev.com/769733cda1cf53c42ded5b8610b80e107a72771f/net/quic/quic_network_transaction_unittest.cc [modify] https://crrev.com/769733cda1cf53c42ded5b8610b80e107a72771f/net/quic/test_tools/quic_test_packet_maker.cc [modify] https://crrev.com/769733cda1cf53c42ded5b8610b80e107a72771f/net/quic/test_tools/quic_test_packet_maker.h
,
Aug 15 2016
,
Aug 15 2016
[Automated comment] Commit may have occurred before M53 branch point (6/30/2016), needs manual review.
,
Aug 15 2016
M53 was branched at Chromium revision: 403382 so this change {#403040} is already in branched. No M53 merge is needed. Hence, removing "Merge-Review-53" label. Please let me know if there is any concern here. Thank you.
,
Feb 14 2017
Any update on this?
,
Feb 14 2017
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by birenroy@chromium.org
, Jun 21 2016