In QUIC, add WriteStatus WRITE_STATUS_BLOCKED_DATA_BUFFERED and use it to represent "A WritePacket caused socket to block and the packet is buffered in writer". Not protected, because this status code is not used in GFE.
This is intended to be used by batch packet writers, for them, whether the
packet that caused write block is buffered can vary from one WritePacket call
to another. The per-class IsWriteBlockedDataBuffered() method is not enough to
express that.
Merge internal change: 228872041
R=rch@chromium.org
Change-Id: I39bdd8f6d1b9e7442f7d51c54231002227790809
Reviewed-on: https://chromium-review.googlesource.com/c/1407030
Reviewed-by: Ryan Hamilton <rch@chromium.org>
Commit-Queue: Bin Wu <wub@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622070}diff --git a/net/quic/quic_chromium_packet_writer.cc b/net/quic/quic_chromium_packet_writer.cc
index a460e5f0..526c48e9 100644
--- a/net/quic/quic_chromium_packet_writer.cc
+++ b/net/quic/quic_chromium_packet_writer.cc
@@ -151,7 +151,8 @@
kTrafficAnnotation);
if (MaybeRetryAfterWriteError(rv))
- return quic::WriteResult(quic::WRITE_STATUS_BLOCKED, ERR_IO_PENDING);
+ return quic::WriteResult(quic::WRITE_STATUS_BLOCKED_DATA_BUFFERED,
+ ERR_IO_PENDING);
if (rv < 0 && rv != ERR_IO_PENDING && delegate_ != nullptr) {
// If write error, then call delegate's HandleWriteError, which
@@ -166,7 +167,7 @@
if (rv != ERR_IO_PENDING) {
status = quic::WRITE_STATUS_ERROR;
} else {
- status = quic::WRITE_STATUS_BLOCKED;
+ status = quic::WRITE_STATUS_BLOCKED_DATA_BUFFERED;
write_in_progress_ = true;
}
}
@@ -174,7 +175,7 @@
base::TimeDelta delta = base::TimeTicks::Now() - now;
if (status == quic::WRITE_STATUS_OK) {
UMA_HISTOGRAM_TIMES("Net.QuicSession.PacketWriteTime.Synchronous", delta);
- } else if (status == quic::WRITE_STATUS_BLOCKED) {
+ } else if (quic::IsWriteBlockedStatus(status)) {
UMA_HISTOGRAM_TIMES("Net.QuicSession.PacketWriteTime.Asynchronous", delta);
}
diff --git a/net/third_party/quic/core/quic_connection.cc b/net/third_party/quic/core/quic_connection.cc
index 2bee1c1..1292bf6 100644
--- a/net/third_party/quic/core/quic_connection.cc
+++ b/net/third_party/quic/core/quic_connection.cc
@@ -1602,9 +1602,9 @@
OnWriteError(result.error_code);
return;
}
- if (result.status == WRITE_STATUS_BLOCKED) {
+ if (IsWriteBlockedStatus(result.status)) {
visitor_->OnWriteBlocked();
- if (writer_->IsWriteBlockedDataBuffered()) {
+ if (result.status == WRITE_STATUS_BLOCKED_DATA_BUFFERED) {
pending_version_negotiation_packet_ = false;
}
return;
@@ -2197,7 +2197,7 @@
packet->encrypted_buffer, encrypted_length, self_address().host(),
peer_address(), per_packet_options_);
if (result.error_code == net::ERR_IO_PENDING) {
- DCHECK_EQ(WRITE_STATUS_BLOCKED, result.status);
+ DCHECK_EQ(WRITE_STATUS_BLOCKED_DATA_BUFFERED, result.status);
}
QUIC_HISTOGRAM_ENUM(
@@ -2205,7 +2205,7 @@
WRITE_STATUS_NUM_VALUES,
"Status code returned by writer_->WritePacket() in QuicConnection.");
- if (result.status == WRITE_STATUS_BLOCKED) {
+ if (IsWriteBlockedStatus(result.status)) {
// Ensure the writer is still write blocked, otherwise QUIC may continue
// trying to write when it will not be able to.
DCHECK(writer_->IsWriteBlocked());
@@ -2214,7 +2214,7 @@
// be queued and sent again, which would result in an unnecessary
// duplicate packet being sent. The helper must call OnCanWrite
// when the write completes, and OnWriteError if an error occurs.
- if (!writer_->IsWriteBlockedDataBuffered()) {
+ if (result.status != WRITE_STATUS_BLOCKED_DATA_BUFFERED) {
return false;
}
}
@@ -3196,13 +3196,13 @@
packet_send_time, probing_packet->transmission_type,
NO_RETRANSMITTABLE_DATA);
- if (result.status == WRITE_STATUS_BLOCKED) {
+ if (IsWriteBlockedStatus(result.status)) {
if (probing_writer == writer_) {
// Visitor should not be write blocked if the probing writer is not the
// default packet writer.
visitor_->OnWriteBlocked();
}
- if (probing_writer->IsWriteBlockedDataBuffered()) {
+ if (result.status == WRITE_STATUS_BLOCKED_DATA_BUFFERED) {
QUIC_DLOG(INFO) << ENDPOINT << "Write probing packet blocked";
}
}
diff --git a/net/third_party/quic/core/quic_connection_test.cc b/net/third_party/quic/core/quic_connection_test.cc
index b6143f30..caba6484 100644
--- a/net/third_party/quic/core/quic_connection_test.cc
+++ b/net/third_party/quic/core/quic_connection_test.cc
@@ -342,7 +342,10 @@
return WriteResult(WRITE_STATUS_ERROR, net::ERR_MSG_TOO_BIG);
}
if (IsWriteBlocked()) {
- return WriteResult(WRITE_STATUS_BLOCKED, -1);
+ return WriteResult(is_write_blocked_data_buffered_
+ ? WRITE_STATUS_BLOCKED_DATA_BUFFERED
+ : WRITE_STATUS_BLOCKED,
+ 0);
}
if (ShouldWriteFail()) {
diff --git a/net/third_party/quic/core/quic_default_packet_writer.cc b/net/third_party/quic/core/quic_default_packet_writer.cc
index 9d464b1..cfd37f7a 100644
--- a/net/third_party/quic/core/quic_default_packet_writer.cc
+++ b/net/third_party/quic/core/quic_default_packet_writer.cc
@@ -24,7 +24,7 @@
<< "QuicDefaultPacketWriter does not accept any options.";
WriteResult result = QuicSocketUtils::WritePacket(fd_, buffer, buf_len,
self_address, peer_address);
- if (result.status == WRITE_STATUS_BLOCKED) {
+ if (IsWriteBlockedStatus(result.status)) {
write_blocked_ = true;
}
return result;
diff --git a/net/third_party/quic/core/quic_time_wait_list_manager.cc b/net/third_party/quic/core/quic_time_wait_list_manager.cc
index a237f59..ca7a79b 100644
--- a/net/third_party/quic/core/quic_time_wait_list_manager.cc
+++ b/net/third_party/quic/core/quic_time_wait_list_manager.cc
@@ -246,11 +246,11 @@
result = writer_->Flush();
}
- if (result.status == WRITE_STATUS_BLOCKED) {
+ if (IsWriteBlockedStatus(result.status)) {
// If blocked and unbuffered, return false to retry sending.
DCHECK(writer_->IsWriteBlocked());
visitor_->OnWriteBlocked(this);
- return writer_->IsWriteBlockedDataBuffered();
+ return result.status == WRITE_STATUS_BLOCKED_DATA_BUFFERED;
} else if (IsWriteError(result.status)) {
QUIC_LOG_FIRST_N(WARNING, 1)
<< "Received unknown error while sending termination packet to "
diff --git a/net/third_party/quic/core/quic_types.h b/net/third_party/quic/core/quic_types.h
index 4cc8255d..5f3e02cc 100644
--- a/net/third_party/quic/core/quic_types.h
+++ b/net/third_party/quic/core/quic_types.h
@@ -80,7 +80,10 @@
// TODO(wtc): see if WriteStatus can be replaced by QuicAsyncStatus.
enum WriteStatus {
WRITE_STATUS_OK,
+ // Write is blocked, caller needs to retry.
WRITE_STATUS_BLOCKED,
+ // Write is blocked but the packet data is buffered, caller should not retry.
+ WRITE_STATUS_BLOCKED_DATA_BUFFERED,
// To make the IsWriteError(WriteStatus) function work properly:
// - Non-errors MUST be added before WRITE_STATUS_ERROR.
// - Errors MUST be added after WRITE_STATUS_ERROR.
@@ -89,6 +92,11 @@
WRITE_STATUS_NUM_VALUES,
};
+inline bool IsWriteBlockedStatus(WriteStatus status) {
+ return status == WRITE_STATUS_BLOCKED ||
+ status == WRITE_STATUS_BLOCKED_DATA_BUFFERED;
+}
+
inline bool IsWriteError(WriteStatus status) {
return status >= WRITE_STATUS_ERROR;
}
@@ -107,6 +115,7 @@
case WRITE_STATUS_OK:
return bytes_written == other.bytes_written;
case WRITE_STATUS_BLOCKED:
+ case WRITE_STATUS_BLOCKED_DATA_BUFFERED:
return true;
default:
return error_code == other.error_code;
diff --git a/net/tools/quic/quic_simple_server_packet_writer.cc b/net/tools/quic/quic_simple_server_packet_writer.cc
index 629f4f4..c8cfb44 100644
--- a/net/tools/quic/quic_simple_server_packet_writer.cc
+++ b/net/tools/quic/quic_simple_server_packet_writer.cc
@@ -36,7 +36,7 @@
callback_ = callback;
quic::WriteResult result =
WritePacket(buffer, buf_len, self_address, peer_address, options);
- if (result.status != quic::WRITE_STATUS_BLOCKED) {
+ if (!quic::IsWriteBlockedStatus(result.status)) {
callback_.Reset();
}
return result;
@@ -91,7 +91,7 @@
base::UmaHistogramSparse("Net.quic::QuicSession.WriteError", -rv);
status = quic::WRITE_STATUS_ERROR;
} else {
- status = quic::WRITE_STATUS_BLOCKED;
+ status = quic::WRITE_STATUS_BLOCKED_DATA_BUFFERED;
write_blocked_ = true;
}
}
