Return the transport rollback error from the batch task A batch task's finalizer now also runs when the task returns an error, so MakeRollbackTask() no longer needs an RTCError out-parameter. Bug: webrtc:42222804 Change-Id: I2db35220a1e6a43fdd87b936af6098647d1fc236 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/509663 Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org> Reviewed-by: Henrik Boström <hbos@webrtc.org> Cr-Commit-Position: refs/heads/main@{#48954}

diff --git a/pc/jsep_transport_controller.cc b/pc/jsep_transport_controller.cc
index 5d0f9e1..cc2b857 100644
--- a/pc/jsep_transport_controller.cc
+++ b/pc/jsep_transport_controller.cc

@@ -564,26 +564,25 @@
 }
 
 ScopedOperationsBatcher::BatchTaskWithFinalizer
-JsepTransportController::MakeRollbackTask(RTCError* absl_nonnull error) {
+JsepTransportController::MakeRollbackTask() {
   RTC_DCHECK_RUN_ON(signaling_thread_);
   scoped_refptr<PendingTaskSafetyFlag> new_flag =
       PendingTaskSafetyFlag::CreateAttachedToTaskQueue(true, signaling_thread_);
-  return [this, error, new_flag = std::move(new_flag)]() mutable
+  return [this, new_flag = std::move(new_flag)]() mutable
              -> ScopedOperationsBatcher::BatchTaskResult {
     RTC_DCHECK_RUN_ON(network_thread_);
     RTCError rollback_error = RollbackTransports_n();
     TransportStateSnapshot new_states = GetTransportStates_n();
     role_update_safety_flag_n_ = new_flag;
-    return ScopedOperationsBatcher::FinalizerTask(
-        [this, error, rollback_error = std::move(rollback_error),
-         new_states = std::move(new_states),
+    ScopedOperationsBatcher::FinalizerTask finalizer(
+        [this, new_states = std::move(new_states),
          new_flag = std::move(new_flag)]() mutable {
           RTC_DCHECK_RUN_ON(signaling_thread_);
           SetTransportStates(std::move(new_states));
           role_update_safety_flag_s_->SetNotAlive();
           role_update_safety_flag_s_ = std::move(new_flag);
-          *error = std::move(rollback_error);
         });
+    return {std::move(rollback_error), std::move(finalizer)};
   };
 }
 

diff --git a/pc/jsep_transport_controller.h b/pc/jsep_transport_controller.h
index 158c206..38a7c2a 100644
--- a/pc/jsep_transport_controller.h
+++ b/pc/jsep_transport_controller.h

@@ -312,15 +312,12 @@
   bool GetStats(absl::string_view transport_name, TransportStats* stats) const;
 
   // Returns a task for a network thread ScopedOperationsBatcher that rolls
-  // back the transports to the last stable state. Its finalizer applies the
-  // resulting transport states on the signaling thread and stores the result
-  // of the rollback in `error`, which must outlive the batch. The task doesn't
-  // fail the batch, so the transport states are applied even if the rollback
-  // fails.
+  // back the transports to the last stable state. The task returns the result
+  // of the rollback. Its finalizer applies the resulting transport states on
+  // the signaling thread, also if the rollback fails.
   //
   // Must be called on the signaling thread.
-  ScopedOperationsBatcher::BatchTaskWithFinalizer MakeRollbackTask(
-      RTCError* absl_nonnull error);
+  ScopedOperationsBatcher::BatchTaskWithFinalizer MakeRollbackTask();
 
   // Must be called on the signaling thread.
   absl::AnyInvocable<void() &&> MakeCloseTask();

diff --git a/pc/jsep_transport_controller_unittest.cc b/pc/jsep_transport_controller_unittest.cc
index 0b314b6..187db00 100644
--- a/pc/jsep_transport_controller_unittest.cc
+++ b/pc/jsep_transport_controller_unittest.cc

@@ -256,12 +256,9 @@
   // Rolls back the transports the same way as SdpOfferAnswerHandler::Rollback()
   // does: with a task in a batch for the network thread.
   RTCError RollbackTransports() {
-    RTCError error;
     ScopedOperationsBatcher network_tasks(network_thread_.get());
-    network_tasks.AddWithFinalizer(
-        transport_controller_->MakeRollbackTask(&error));
-    EXPECT_TRUE(network_tasks.Run().ok());
-    return error;
+    network_tasks.AddWithFinalizer(transport_controller_->MakeRollbackTask());
+    return network_tasks.Run();
   }
 
   std::unique_ptr<SessionDescription> CreateSessionDescriptionWithoutBundle() {

diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc
index 181fb2f..34ee65b 100644
--- a/pc/sdp_offer_answer.cc
+++ b/pc/sdp_offer_answer.cc

@@ -3959,10 +3959,6 @@
       transceivers()->Remove(transceiver);
     }
   };
-  // The rollback task in `network_tasks` writes to this. Declared before the
-  // batchers so that it outlives them, since their destructors run pending
-  // tasks.
-  RTCError rollback_error;
   ScopedOperationsBatcher worker_tasks(context_->worker_thread());
   ScopedOperationsBatcher network_tasks(context_->network_thread());
 
@@ -4060,12 +4056,10 @@
   // those channels are attached to. GetDeleteChannelWorkerTask() has already
   // made the channels unreachable from their transceivers, so the rollback
   // can't attach them to a transport again.
-  network_tasks.AddWithFinalizer(
-      transport_controller_s()->MakeRollbackTask(&rollback_error));
+  network_tasks.AddWithFinalizer(transport_controller_s()->MakeRollbackTask());
 
-  RTCError e = network_tasks.Run();
-  RTC_DCHECK(e.ok());  // Rollback errors are reported in `rollback_error`.
-  e = worker_tasks.Run();
+  RTCError rollback_error = network_tasks.Run();
+  RTCError e = worker_tasks.Run();
   RTC_DCHECK(e.ok());  // only void tasks queued.
 
   for (const auto& [sender, encodings] : senders_to_restore) {