Make QuicControlFrameManager talk to QuicSession via a delegate.
This makes the ownership clear and prevents the control frame manager from unnecessarily accessing the session.
Refactor only. not protected.
PiperOrigin-RevId: 321460931
Change-Id: Iff7f35f9626a46f6afe37f9a02c8ff653aa9437e
diff --git a/quic/core/quic_control_frame_manager.cc b/quic/core/quic_control_frame_manager.cc
index ba00b88..ba00015 100644
--- a/quic/core/quic_control_frame_manager.cc
+++ b/quic/core/quic_control_frame_manager.cc
@@ -24,14 +24,11 @@
} // namespace
-#define ENDPOINT \
- (session_->perspective() == Perspective::IS_SERVER ? "Server: " : "Client: ")
-
QuicControlFrameManager::QuicControlFrameManager(QuicSession* session)
: last_control_frame_id_(kInvalidControlFrameId),
least_unacked_(1),
least_unsent_(1),
- session_(session) {}
+ delegate_(session) {}
QuicControlFrameManager::~QuicControlFrameManager() {
while (!control_frames_.empty()) {
@@ -44,13 +41,12 @@
const bool had_buffered_frames = HasBufferedFrames();
control_frames_.emplace_back(frame);
if (control_frames_.size() > kMaxNumControlFrames) {
- session_->connection()->CloseConnection(
+ delegate_->OnControlFrameManagerError(
QUIC_TOO_MANY_BUFFERED_CONTROL_FRAMES,
quiche::QuicheStrCat(
"More than ", kMaxNumControlFrames,
"buffered control frames, least_unacked: ", least_unacked_,
- ", least_unsent_: ", least_unsent_),
- ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
+ ", least_unsent_: ", least_unsent_));
return;
}
if (had_buffered_frames) {
@@ -131,13 +127,12 @@
control_frames_.emplace_back(
QuicFrame(QuicPingFrame(++last_control_frame_id_)));
if (control_frames_.size() > kMaxNumControlFrames) {
- session_->connection()->CloseConnection(
+ delegate_->OnControlFrameManagerError(
QUIC_TOO_MANY_BUFFERED_CONTROL_FRAMES,
quiche::QuicheStrCat(
"More than ", kMaxNumControlFrames,
"buffered control frames, least_unacked: ", least_unacked_,
- ", least_unsent_: ", least_unsent_),
- ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
+ ", least_unsent_: ", least_unsent_));
return;
}
WriteBufferedFrames();
@@ -167,9 +162,8 @@
if (id > least_unsent_) {
QUIC_BUG << "Try to send control frames out of order, id: " << id
<< " least_unsent: " << least_unsent_;
- session_->connection()->CloseConnection(
- QUIC_INTERNAL_ERROR, "Try to send control frames out of order",
- ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
+ delegate_->OnControlFrameManagerError(
+ QUIC_INTERNAL_ERROR, "Try to send control frames out of order");
return;
}
++least_unsent_;
@@ -198,9 +192,8 @@
}
if (id >= least_unsent_) {
QUIC_BUG << "Try to mark unsent control frame as lost";
- session_->connection()->CloseConnection(
- QUIC_INTERNAL_ERROR, "Try to mark unsent control frame as lost",
- ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
+ delegate_->OnControlFrameManagerError(
+ QUIC_INTERNAL_ERROR, "Try to mark unsent control frame as lost");
return;
}
if (id < least_unacked_ ||
@@ -267,9 +260,8 @@
}
if (id >= least_unsent_) {
QUIC_BUG << "Try to retransmit unsent control frame";
- session_->connection()->CloseConnection(
- QUIC_INTERNAL_ERROR, "Try to retransmit unsent control frame",
- ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
+ delegate_->OnControlFrameManagerError(
+ QUIC_INTERNAL_ERROR, "Try to retransmit unsent control frame");
return false;
}
if (id < least_unacked_ ||
@@ -281,7 +273,7 @@
QuicFrame copy = CopyRetransmittableControlFrame(frame);
QUIC_DVLOG(1) << "control frame manager is forced to retransmit frame: "
<< frame;
- if (session_->WriteControlFrame(copy, type)) {
+ if (delegate_->WriteControlFrame(copy, type)) {
return true;
}
DeleteFrame(©);
@@ -289,13 +281,11 @@
}
void QuicControlFrameManager::WriteBufferedFrames() {
- DCHECK(session_->connection()->connected())
- << ENDPOINT << "Try to write control frames when connection is closed.";
while (HasBufferedFrames()) {
QuicFrame frame_to_send =
control_frames_.at(least_unsent_ - least_unacked_);
QuicFrame copy = CopyRetransmittableControlFrame(frame_to_send);
- if (!session_->WriteControlFrame(copy, NOT_RETRANSMISSION)) {
+ if (!delegate_->WriteControlFrame(copy, NOT_RETRANSMISSION)) {
// Connection is write blocked.
DeleteFrame(©);
break;
@@ -308,7 +298,7 @@
while (HasPendingRetransmission()) {
QuicFrame pending = NextPendingRetransmission();
QuicFrame copy = CopyRetransmittableControlFrame(pending);
- if (!session_->WriteControlFrame(copy, LOSS_RETRANSMISSION)) {
+ if (!delegate_->WriteControlFrame(copy, LOSS_RETRANSMISSION)) {
// Connection is write blocked.
DeleteFrame(©);
break;
@@ -324,9 +314,8 @@
}
if (id >= least_unsent_) {
QUIC_BUG << "Try to ack unsent control frame";
- session_->connection()->CloseConnection(
- QUIC_INTERNAL_ERROR, "Try to ack unsent control frame",
- ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
+ delegate_->OnControlFrameManagerError(QUIC_INTERNAL_ERROR,
+ "Try to ack unsent control frame");
return false;
}
if (id < least_unacked_ ||
diff --git a/quic/core/quic_control_frame_manager.h b/quic/core/quic_control_frame_manager.h
index ffd378c..7615754 100644
--- a/quic/core/quic_control_frame_manager.h
+++ b/quic/core/quic_control_frame_manager.h
@@ -30,6 +30,18 @@
// which need to be retransmitted.
class QUIC_EXPORT_PRIVATE QuicControlFrameManager {
public:
+ class QUIC_EXPORT_PRIVATE DelegateInterface {
+ public:
+ virtual ~DelegateInterface() = default;
+
+ // Notifies the delegate of errors.
+ virtual void OnControlFrameManagerError(QuicErrorCode error_code,
+ std::string error_details) = 0;
+
+ virtual bool WriteControlFrame(const QuicFrame& frame,
+ TransmissionType type) = 0;
+ };
+
explicit QuicControlFrameManager(QuicSession* session);
QuicControlFrameManager(const QuicControlFrameManager& other) = delete;
QuicControlFrameManager(QuicControlFrameManager&& other) = delete;
@@ -146,8 +158,7 @@
// Lost control frames waiting to be retransmitted.
QuicLinkedHashMap<QuicControlFrameId, bool> pending_retransmissions_;
- // Pointer to the owning QuicSession object.
- QuicSession* session_;
+ DelegateInterface* delegate_;
// Last sent window update frame for each stream.
QuicSmallMap<QuicStreamId, QuicControlFrameId, 10> window_update_frames_;
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc
index e09decf..d63006c 100644
--- a/quic/core/quic_session.cc
+++ b/quic/core/quic_session.cc
@@ -739,8 +739,17 @@
return bytes_consumed;
}
+void QuicSession::OnControlFrameManagerError(QuicErrorCode error_code,
+ std::string error_details) {
+ connection_->CloseConnection(
+ error_code, error_details,
+ ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
+}
+
bool QuicSession::WriteControlFrame(const QuicFrame& frame,
TransmissionType type) {
+ DCHECK(connection()->connected())
+ << ENDPOINT << "Try to write control frames when connection is closed.";
SetTransmissionType(type);
return connection_->SendControlFrame(frame);
}
diff --git a/quic/core/quic_session.h b/quic/core/quic_session.h
index 54a1426..88e4412 100644
--- a/quic/core/quic_session.h
+++ b/quic/core/quic_session.h
@@ -53,7 +53,8 @@
public QuicStreamFrameDataProducer,
public QuicStreamIdManager::DelegateInterface,
public HandshakerDelegateInterface,
- public StreamDelegateInterface {
+ public StreamDelegateInterface,
+ public QuicControlFrameManager::DelegateInterface {
public:
// An interface from the session to the entity owning the session.
// This lets the session notify its owner (the Dispatcher) when the connection
@@ -199,10 +200,15 @@
// Called when message with |message_id| is considered as lost.
virtual void OnMessageLost(QuicMessageId message_id);
+ // QuicControlFrameManager::DelegateInterface
+ // Close the connection on error.
+ void OnControlFrameManagerError(QuicErrorCode error_code,
+ std::string error_details) override;
// Called by control frame manager when it wants to write control frames to
// the peer. Returns true if |frame| is consumed, false otherwise. The frame
// will be sent in specified transmission |type|.
- bool WriteControlFrame(const QuicFrame& frame, TransmissionType type);
+ bool WriteControlFrame(const QuicFrame& frame,
+ TransmissionType type) override;
// Called by stream to send RST_STREAM (and STOP_SENDING).
virtual void SendRstStream(QuicStreamId id,
diff --git a/quic/core/stream_delegate_interface.h b/quic/core/stream_delegate_interface.h
index de77bad..9f27147 100644
--- a/quic/core/stream_delegate_interface.h
+++ b/quic/core/stream_delegate_interface.h
@@ -14,6 +14,8 @@
class QuicStream;
+// Pure virtual class to get notified when particular QuicStream events
+// occurred.
class QUIC_EXPORT_PRIVATE StreamDelegateInterface {
public:
virtual ~StreamDelegateInterface() {}
