Index: net/quic/chromium/quic_chromium_client_stream.cc |
diff --git a/net/quic/chromium/quic_chromium_client_stream.cc b/net/quic/chromium/quic_chromium_client_stream.cc |
index 5eb505a382db4011e8ab0e6ed3bd9f794c365855..5f806595a430bfd589e1863e418b9dd5904525d2 100644 |
--- a/net/quic/chromium/quic_chromium_client_stream.cc |
+++ b/net/quic/chromium/quic_chromium_client_stream.cc |
@@ -21,9 +21,31 @@ |
namespace net { |
+namespace { |
+ |
+// Sets a boolean to a value, and restores it to the previous value once |
+// the saver goes out of scope. |
+class ScopedBoolSaver { |
+ public: |
+ ScopedBoolSaver(bool* var, bool new_val) : var_(var), old_val_(*var) { |
+ *var_ = new_val; |
+ } |
+ |
+ ~ScopedBoolSaver() { *var_ = old_val_; } |
+ |
+ private: |
+ bool* var_; |
+ bool old_val_; |
+}; |
+ |
+} // namespace |
+ |
QuicChromiumClientStream::Handle::Handle(QuicChromiumClientStream* stream, |
Delegate* delegate) |
- : stream_(stream), delegate_(delegate) { |
+ : stream_(stream), |
+ delegate_(delegate), |
+ can_invoke_callbacks_(true), |
+ read_headers_buffer_(nullptr) { |
SaveState(); |
} |
@@ -40,10 +62,15 @@ void QuicChromiumClientStream::Handle::ClearDelegate() { |
delegate_ = nullptr; |
} |
-void QuicChromiumClientStream::Handle::OnInitialHeadersAvailable( |
- const SpdyHeaderBlock& headers, |
- size_t frame_len) { |
- delegate_->OnInitialHeadersAvailable(headers, frame_len); |
+void QuicChromiumClientStream::Handle::OnInitialHeadersAvailable() { |
+ if (!read_headers_callback_ || !can_invoke_callbacks_) |
+ return; // Wait for ReadInitialHeaders to be called. |
+ |
+ int rv; |
xunjieli
2017/05/10 17:54:27
Looks like DeliverInitialiHeaders() can fail to in
Ryan Hamilton
2017/05/10 18:26:40
I think that should only happen when Deliver retur
|
+ if (!stream_->DeliverInitialHeaders(read_headers_buffer_, &rv)) |
xunjieli
2017/05/10 17:54:27
If BidiStream or QuicHttpStream is gone but Handle
Ryan Hamilton
2017/05/10 18:26:40
The Handle is by definition owned by "the caller"
xunjieli
2017/05/10 18:58:08
Acknowledged. The ownership model takes a while to
Ryan Hamilton
2017/05/10 19:33:46
*fingers crossed*!
|
+ rv = ERR_QUIC_PROTOCOL_ERROR; |
+ |
+ ResetAndReturn(&read_headers_callback_).Run(rv); |
} |
void QuicChromiumClientStream::Handle::OnTrailingHeadersAvailable( |
@@ -78,6 +105,22 @@ void QuicChromiumClientStream::Handle::OnError(int error) { |
} |
} |
+int QuicChromiumClientStream::Handle::ReadInitialHeaders( |
+ SpdyHeaderBlock* header_block, |
+ const CompletionCallback& callback) { |
+ ScopedBoolSaver saver(&can_invoke_callbacks_, false); |
xunjieli
2017/05/10 17:54:27
Why do we need to have this bool? I don't see any
Ryan Hamilton
2017/05/10 18:26:40
AH! Sorry, I meant to mention this in the CL descr
xunjieli
2017/05/10 18:58:08
I see. That makes sense. Let's omit this in this C
Ryan Hamilton
2017/05/10 19:33:46
SGTM. Done.
|
+ if (!stream_) |
+ return ERR_CONNECTION_CLOSED; |
+ |
+ int frame_len; |
xunjieli
2017/05/10 17:54:27
same here. |frame_len| might not be initialized.
Ryan Hamilton
2017/05/10 18:26:40
Done.
|
+ if (stream_->DeliverInitialHeaders(header_block, &frame_len)) |
+ return frame_len; |
+ |
+ read_headers_buffer_ = header_block; |
+ SetCallback(callback, &read_headers_callback_); |
+ return ERR_IO_PENDING; |
+} |
+ |
size_t QuicChromiumClientStream::Handle::WriteHeaders( |
SpdyHeaderBlock header_block, |
bool fin, |
@@ -234,6 +277,13 @@ void QuicChromiumClientStream::Handle::SaveState() { |
priority_ = stream_->priority(); |
} |
+void QuicChromiumClientStream::Handle::SetCallback( |
+ CompletionCallback new_callback, |
+ CompletionCallback* callback) { |
+ DCHECK(!can_invoke_callbacks_); |
+ *callback = new_callback; |
+} |
+ |
QuicChromiumClientStream::QuicChromiumClientStream( |
QuicStreamId id, |
QuicClientSessionBase* session, |
@@ -271,16 +321,14 @@ void QuicChromiumClientStream::OnInitialHeadersComplete( |
ConsumeHeaderList(); |
session_->OnInitialHeadersComplete(id(), header_block); |
- if (handle_) { |
- // The handle will receive the headers via a posted task. |
- NotifyHandleOfInitialHeadersAvailableLater(std::move(header_block), |
- frame_len); |
- return; |
- } |
- |
// Buffer the headers and deliver them when the handle arrives. |
initial_headers_ = std::move(header_block); |
initial_headers_frame_len_ = frame_len; |
+ |
+ if (handle_) { |
+ // The handle will be notified of the headers via a posted task. |
+ NotifyHandleOfInitialHeadersAvailableLater(); |
+ } |
} |
void QuicChromiumClientStream::OnTrailingHeadersComplete( |
@@ -414,10 +462,8 @@ QuicChromiumClientStream::CreateHandle( |
handle_ = handle.get(); |
// Should this perhaps be via PostTask to make reasoning simpler? |
- if (!initial_headers_.empty()) { |
- handle_->OnInitialHeadersAvailable(std::move(initial_headers_), |
- initial_headers_frame_len_); |
- } |
+ if (!initial_headers_.empty()) |
+ handle_->OnInitialHeadersAvailable(); |
return handle; |
} |
@@ -450,31 +496,21 @@ int QuicChromiumClientStream::Read(IOBuffer* buf, int buf_len) { |
return bytes_read; |
} |
-void QuicChromiumClientStream::NotifyHandleOfInitialHeadersAvailableLater( |
- SpdyHeaderBlock headers, |
- size_t frame_len) { |
+void QuicChromiumClientStream::NotifyHandleOfInitialHeadersAvailableLater() { |
DCHECK(handle_); |
base::ThreadTaskRunnerHandle::Get()->PostTask( |
FROM_HERE, |
base::Bind( |
&QuicChromiumClientStream::NotifyHandleOfInitialHeadersAvailable, |
- weak_factory_.GetWeakPtr(), base::Passed(std::move(headers)), |
- frame_len)); |
+ weak_factory_.GetWeakPtr())); |
} |
-void QuicChromiumClientStream::NotifyHandleOfInitialHeadersAvailable( |
- SpdyHeaderBlock headers, |
- size_t frame_len) { |
+void QuicChromiumClientStream::NotifyHandleOfInitialHeadersAvailable() { |
if (!handle_) |
return; |
- DCHECK(!headers_delivered_); |
- headers_delivered_ = true; |
- net_log_.AddEvent( |
- NetLogEventType::QUIC_CHROMIUM_CLIENT_STREAM_READ_RESPONSE_HEADERS, |
- base::Bind(&SpdyHeaderBlockNetLogCallback, &headers)); |
- |
- handle_->OnInitialHeadersAvailable(headers, frame_len); |
+ if (!headers_delivered_) |
+ handle_->OnInitialHeadersAvailable(); |
} |
void QuicChromiumClientStream::NotifyHandleOfTrailingHeadersAvailableLater( |
@@ -503,10 +539,24 @@ void QuicChromiumClientStream::NotifyHandleOfTrailingHeadersAvailable( |
net_log_.AddEvent( |
NetLogEventType::QUIC_CHROMIUM_CLIENT_STREAM_READ_RESPONSE_TRAILERS, |
base::Bind(&SpdyHeaderBlockNetLogCallback, &headers)); |
- |
handle_->OnTrailingHeadersAvailable(headers, frame_len); |
} |
+bool QuicChromiumClientStream::DeliverInitialHeaders(SpdyHeaderBlock* headers, |
+ int* frame_len) { |
+ if (initial_headers_.empty()) |
+ return false; |
+ |
+ headers_delivered_ = true; |
+ net_log_.AddEvent( |
+ NetLogEventType::QUIC_CHROMIUM_CLIENT_STREAM_READ_RESPONSE_HEADERS, |
+ base::Bind(&SpdyHeaderBlockNetLogCallback, &initial_headers_)); |
+ |
+ *headers = std::move(initial_headers_); |
+ *frame_len = initial_headers_frame_len_; |
+ return true; |
+} |
+ |
void QuicChromiumClientStream::NotifyHandleOfDataAvailableLater() { |
DCHECK(handle_); |
base::ThreadTaskRunnerHandle::Get()->PostTask( |