Update PUBLISH_DONE to draft-18. request_id is removed. Also on a failed REQUEST_UPDATE, PUBLISH_DONE is sent before REQUEST_ERROR. PiperOrigin-RevId: 996639950
diff --git a/quiche/quic/moqt/moqt_error.h b/quiche/quic/moqt/moqt_error.h index 57d7121..e226b04 100644 --- a/quiche/quic/moqt/moqt_error.h +++ b/quiche/quic/moqt/moqt_error.h
@@ -90,9 +90,10 @@ kTrackEnded = 0x2, kSubscriptionEnded = 0x3, kGoingAway = 0x4, - kExpired = 0x5, - kTooFarBehind = 0x6, + kTooFarBehind = 0x5, + kExpired = 0x6, kUpdateFailed = 0x8, + kExcessiveLoad = 0x9, kMalformedTrack = 0x12, };
diff --git a/quiche/quic/moqt/moqt_framer.cc b/quiche/quic/moqt/moqt_framer.cc index 64aa2cf..235b8e5 100644 --- a/quiche/quic/moqt/moqt_framer.cc +++ b/quiche/quic/moqt/moqt_framer.cc
@@ -804,8 +804,8 @@ quiche::QuicheBuffer MoqtFramer::SerializePublishDone( const MoqtPublishDone& message) { return SerializeControlMessage( - MoqtMessageType::kPublishDone, WireMoqVarInt(message.request_id), - WireMoqVarInt(message.status_code), WireMoqVarInt(message.stream_count), + MoqtMessageType::kPublishDone, WireMoqVarInt(message.status_code), + WireMoqVarInt(message.stream_count), WireStringWithMoqVarIntLength(message.error_reason)); }
diff --git a/quiche/quic/moqt/moqt_live_publisher.cc b/quiche/quic/moqt/moqt_live_publisher.cc index ce52a21..ea76f28 100644 --- a/quiche/quic/moqt/moqt_live_publisher.cc +++ b/quiche/quic/moqt/moqt_live_publisher.cc
@@ -82,6 +82,7 @@ track_publisher_->UpdateObjectListener(this, parameters); if (!pub_status.ok()) { if (!from_request_ok) { + PublishIsDone(PublishDoneCode::kUpdateFailed, "Update failed"); bidi_stream_->CheckStatus( bidi_stream_->SendRequestError(StatusToMoqtRequestError(pub_status))); } else { @@ -434,7 +435,6 @@ void LivePublisher::PublishIsDone(PublishDoneCode code, absl::string_view error_reason) { MoqtPublishDone publish_done; - publish_done.request_id = request_id_; publish_done.status_code = code; publish_done.stream_count = streams_opened_; publish_done.error_reason = error_reason; @@ -444,7 +444,9 @@ QUICHE_DLOG(INFO) << "Sending PUBLISH_DONE message for " << track_publisher_->GetTrackName(); bidi_stream_->SendOrBufferMessageOrFatal( - framer_.SerializePublishDone(publish_done), /*fin=*/true); + framer_.SerializePublishDone(publish_done), + // Don't send FIN if the UPDATE failed; REQUEST_ERROR will follow. + /*fin=*/code != PublishDoneCode::kUpdateFailed); // sending FIN will delete the class. }
diff --git a/quiche/quic/moqt/moqt_live_publisher_test.cc b/quiche/quic/moqt/moqt_live_publisher_test.cc index 67883cf..ac43e0b 100644 --- a/quiche/quic/moqt/moqt_live_publisher_test.cc +++ b/quiche/quic/moqt/moqt_live_publisher_test.cc
@@ -430,12 +430,32 @@ new_params.object_delivery_timeout = quic::QuicTimeDelta::FromSeconds(5); EXPECT_CALL(mock_bidi_stream_, CanWrite()).WillRepeatedly(Return(true)); - // When from_request_ok is false, sends REQUEST_ERROR. + // When from_request_ok is false, sends PUBLISH_DONE (no FIN) followed by + // REQUEST_ERROR (with FIN). EXPECT_CALL(*track_publisher_, UpdateObjectListener(publisher_.get(), _)) .WillOnce(Return(absl::InvalidArgumentError("invalid update"))); - EXPECT_CALL(mock_bidi_stream_, - Writev(ControlMessageOfType(MoqtMessageType::kRequestError), _)) - .WillOnce(Return(absl::OkStatus())); + { + testing::InSequence seq; + MoqtPublishDone expected_publish_done = { + PublishDoneCode::kUpdateFailed, + /*stream_count=*/0, + /*error_reason=*/"Update failed", + }; + EXPECT_CALL(mock_bidi_stream_, + Writev(SerializedControlMessage(expected_publish_done), _)) + .WillOnce([](absl::Span<quiche::QuicheMemSlice> data, + const webtransport::StreamWriteOptions& options) { + EXPECT_FALSE(options.send_fin()); + return absl::OkStatus(); + }); + EXPECT_CALL(mock_bidi_stream_, + Writev(ControlMessageOfType(MoqtMessageType::kRequestError), _)) + .WillOnce([](absl::Span<quiche::QuicheMemSlice> data, + const webtransport::StreamWriteOptions& options) { + EXPECT_TRUE(options.send_fin()); + return absl::OkStatus(); + }); + } publisher_->Update(new_params, /*from_request_ok=*/false); EXPECT_EQ(publisher_->parameters().object_delivery_timeout, quic::QuicTimeDelta::FromSeconds(1)); @@ -608,7 +628,6 @@ publisher_->Update(parameters_, true); CreateStream(Location(5, 0), 0, 128); struct MoqtPublishDone expected_publish_done = { - /*request_id=*/kRequestId, PublishDoneCode::kTooFarBehind, /*stream_count=*/1, /*error_reason=*/"",
diff --git a/quiche/quic/moqt/moqt_messages.h b/quiche/quic/moqt/moqt_messages.h index 05575b6..872eaae 100644 --- a/quiche/quic/moqt/moqt_messages.h +++ b/quiche/quic/moqt/moqt_messages.h
@@ -381,7 +381,6 @@ }; struct QUICHE_EXPORT MoqtPublishDone { - uint64_t request_id; PublishDoneCode status_code; uint64_t stream_count; std::string error_reason;
diff --git a/quiche/quic/moqt/moqt_parser.cc b/quiche/quic/moqt/moqt_parser.cc index 0a89924..93955e2 100644 --- a/quiche/quic/moqt/moqt_parser.cc +++ b/quiche/quic/moqt/moqt_parser.cc
@@ -779,8 +779,7 @@ quic::QuicDataReader reader(data); MoqtPublishDone publish_done; uint64_t value; - if (!reader.ReadMoqVarInt(&publish_done.request_id) || - !reader.ReadMoqVarInt(&value) || + if (!reader.ReadMoqVarInt(&value) || !reader.ReadMoqVarInt(&publish_done.stream_count) || !reader.ReadStringMoqVarInt(publish_done.error_reason)) { return absl::InvalidArgumentError("Message missing fields");
diff --git a/quiche/quic/moqt/moqt_publish_stream_test.cc b/quiche/quic/moqt/moqt_publish_stream_test.cc index 29b3928..ff1cbe7 100644 --- a/quiche/quic/moqt/moqt_publish_stream_test.cc +++ b/quiche/quic/moqt/moqt_publish_stream_test.cc
@@ -499,7 +499,6 @@ // Now receive PUBLISH_DONE. MoqtPublishDone publish_done; - publish_done.request_id = kRequestId; publish_done.status_code = PublishDoneCode::kTrackEnded; publish_done.stream_count = 0; // Trigger immediate Destroy EXPECT_CALL(mock_stream_, Writev) @@ -556,7 +555,6 @@ // Now receive PUBLISH_DONE. MoqtPublishDone publish_done; - publish_done.request_id = kRequestId; publish_done.status_code = PublishDoneCode::kTrackEnded; publish_done.stream_count = 0; QUICHE_EXPECT_OK(stream_->OnControlMessage(publish_done));
diff --git a/quiche/quic/moqt/moqt_subscribe_stream_test.cc b/quiche/quic/moqt/moqt_subscribe_stream_test.cc index 6a2d85c..575c10b 100644 --- a/quiche/quic/moqt/moqt_subscribe_stream_test.cc +++ b/quiche/quic/moqt/moqt_subscribe_stream_test.cc
@@ -233,7 +233,6 @@ TEST_F(MoqtSubscribeRequestStreamTest, ReceivePublishDone) { MoqtPublishDone publish_done; - publish_done.request_id = kRequestId; publish_done.stream_count = 5; EXPECT_CALL(mock_subscribe_visitor_, OnPublishDone(track_name_)); QUICHE_EXPECT_OK(stream_->OnControlMessage(publish_done));
diff --git a/quiche/quic/moqt/test_tools/moqt_test_message.h b/quiche/quic/moqt/test_tools/moqt_test_message.h index e2f6e69..6c6703a 100644 --- a/quiche/quic/moqt/test_tools/moqt_test_message.h +++ b/quiche/quic/moqt/test_tools/moqt_test_message.h
@@ -869,10 +869,6 @@ bool EqualFieldValues(const MessageStructuredData& values) const override { auto cast = std::get<MoqtPublishDone>(values); - if (cast.request_id != publish_done_.request_id) { - QUIC_LOG(INFO) << "PUBLISH_DONE request ID mismatch"; - return false; - } if (cast.status_code != publish_done_.status_code) { QUIC_LOG(INFO) << "PUBLISH_DONE status code mismatch"; return false; @@ -889,22 +885,21 @@ return true; } - void ExpandVarints() override { ExpandVarintsImpl("vvvv--"); } + void ExpandVarints() override { ExpandVarintsImpl("vvv--"); } MessageStructuredData structured_data() const override { return TestMessageBase::MessageStructuredData(publish_done_); } private: - uint8_t raw_packet_[9] = { - 0x0b, 0x00, 0x06, 0x02, 0x02, // request_id = 2, error_code = 2, - 0x05, // stream_count = 5 - 0x02, 0x68, 0x69, // error_reason = "hi" + uint8_t raw_packet_[8] = { + 0x0b, 0x00, 0x05, 0x02, // status_code = 2 + 0x05, // stream_count = 5 + 0x02, 0x68, 0x69, // error_reason = "hi" }; MoqtPublishDone publish_done_ = { - /*request_id=*/2, - /*error_code=*/PublishDoneCode::kTrackEnded, + /*status_code=*/PublishDoneCode::kTrackEnded, /*stream_count=*/5, /*error_reason=*/"hi", };