Do not call SpdySession::GetPushedStream() from SpdyHttpStream::InitializeStream() if pushed_stream_id_ == kNoPushedStreamFound. |
||
Issue descriptionHttp2PushPromiseIndex::ClaimPushedStream() is called earlier and it looks for matching pushed streams. It seems silly to search in the index again from SpdyHttpStream::InitializeStream() if no pushed stream was found the first time. Sure enough this will miss pushed streams that have been created between the two calls (there is a PostTask there), but that race condition does not justify the extra complexity. http resources pushed over a trusted HTTP/2 proxy need to be sorted out though, because currently they are not handled by Http2PushPromiseIndex::ClaimPushedStream(). This is a follow-up to Helen's comment at https://chromium-review.googlesource.com/c/chromium/src/+/829993/2/net/spdy/chromium/spdy_http_stream.cc#85.
,
Jan 10 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7bf9436874ee25176216edebd0335484759c8e39 commit 7bf9436874ee25176216edebd0335484759c8e39 Author: Bence Béky <bnc@chromium.org> Date: Wed Jan 10 13:19:36 2018 Do not call GetPushedStream if there is no pushed stream. Do not call SpdySession::GetPushedStream() from SpdyHttpStream::InitializeStream() if |pushed_stream_id_ == kNoPushedStreamFound|. Pushed streams are claimed by Http2PushPromiseIndex::ClaimPushedStream(), there is no need to do an extra lookup in the index if a pushed stream was not found the first time. This is a follow-up to https://chromium-review.googlesource.com/c/chromium/src/+/829993/2/net/spdy/chromium/spdy_http_stream.cc#85. Note that it is possible that a stream was created after the Http2PushPromiseIndex::ClaimPushedStream() call but before SpdySession::GetPushedStream() is called (they do not happen synchronously). In two unittests, mock read data has to be reordered to make sure pushed stream is claimed. Http2PushPromiseIndex::ClaimPushedStream() calls are added to some other unittests to make sure the stream is not in the index by the time SpdySession::GetPushedStream() is called. Bug: 799937 Change-Id: If91fe95fd65362a9c650b502402796deb251e887 Reviewed-on: https://chromium-review.googlesource.com/857278 Reviewed-by: Helen Li <xunjieli@chromium.org> Commit-Queue: Bence Béky <bnc@chromium.org> Cr-Commit-Position: refs/heads/master@{#528290} [modify] https://crrev.com/7bf9436874ee25176216edebd0335484759c8e39/net/http/http_network_transaction_unittest.cc [modify] https://crrev.com/7bf9436874ee25176216edebd0335484759c8e39/net/spdy/chromium/spdy_http_stream.cc [modify] https://crrev.com/7bf9436874ee25176216edebd0335484759c8e39/net/spdy/chromium/spdy_http_stream_unittest.cc [modify] https://crrev.com/7bf9436874ee25176216edebd0335484759c8e39/net/spdy/chromium/spdy_network_transaction_unittest.cc [modify] https://crrev.com/7bf9436874ee25176216edebd0335484759c8e39/net/spdy/chromium/spdy_session.cc [modify] https://crrev.com/7bf9436874ee25176216edebd0335484759c8e39/net/spdy/chromium/spdy_session_unittest.cc [modify] https://crrev.com/7bf9436874ee25176216edebd0335484759c8e39/net/spdy/chromium/spdy_stream_unittest.cc
,
Jan 10 2018
|
||
►
Sign in to add a comment |
||
Comment 1 by bugdroid1@chromium.org
, Jan 8 2018