From 055b8ce56f2e737e68730a9d3178ff4cb93fa7fb Mon Sep 17 00:00:00 2001
From: Dale Curtis <[email protected]>
Date: Wed, 08 Apr 2026 10:58:16 -0700
Subject: [PATCH] [MSE] Correct range_for_next_append_ during SourceBuffer GC

The range may become invalid during garbage collection.

R=tmathmeyer

Fixed: 498285711, 500387779
Change-Id: I2bc9150f69cf8daa92168d04022c6efd69f7ae6c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7737855
Auto-Submit: Dale Curtis <[email protected]>
Reviewed-by: Ted (Chromium) Meyer <[email protected]>
Commit-Queue: Ted (Chromium) Meyer <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1611660}
---

diff --git a/media/filters/source_buffer_stream.cc b/media/filters/source_buffer_stream.cc
index 21486d00..e970821 100644
--- a/media/filters/source_buffer_stream.cc
+++ b/media/filters/source_buffer_stream.cc
@@ -1030,11 +1030,10 @@
 
     if (current_range->GetMemoryUsage() == 0) {
       CHECK_NE(current_range, selected_range_);
-      CHECK(range_for_next_append_ == ranges_.end() ||
-            range_for_next_append_->get() != current_range);
-
-      // Delete |current_range| by popping it out of |ranges_|.
-      reverse_direction ? ranges_.pop_back() : ranges_.pop_front();
+      auto range_to_delete =
+          reverse_direction ? std::prev(ranges_.end()) : ranges_.begin();
+      current_range = nullptr;
+      DeleteAndRemoveRange(&range_to_delete);
     }
 
     if (reverse_direction && new_range_for_append) {
diff --git a/media/filters/source_buffer_stream_unittest.cc b/media/filters/source_buffer_stream_unittest.cc
index b154eeb..7cccec0 100644
--- a/media/filters/source_buffer_stream_unittest.cc
+++ b/media/filters/source_buffer_stream_unittest.cc
@@ -5747,4 +5747,38 @@
   EXPECT_TRUE(IsRangeListSorted());
 }
 
+TEST_F(SourceBufferStreamTest, GarbageCollectionUpdatesRangeForNextAppend) {
+  // Set memory limit to 10 buffers.
+  SetMemoryLimit(10);
+
+  // 1. Append 10 buffers to create Range A [0, 90ms].
+  NewCodedFrameGroupAppend("0K 10K 20K 30K 40K 50K 60K 70K 80K 90K");
+
+  // 2. Append 10 buffers to create Range B [1000ms, 1090ms].
+  // This exceeds the memory limit and triggers GC, but Range A is kept because
+  // it was recently appended.
+  NewCodedFrameGroupAppend(
+      "1000K 1010K 1020K 1030K 1040K 1050K 1060K 1070K 1080K 1090K");
+
+  // 3. Start a new coded frame group that overlaps Range A.
+  // This sets range_for_next_append_ to Range A and
+  // last_appended_buffer_timestamp_ to kNoTimestamp.
+  stream_->OnStartOfCodedFrameGroup(base::Milliseconds(0));
+
+  // 4. Trigger Garbage Collection.
+  // We want to free enough data that Range A is deleted.
+  // Set memory limit very low so GC must evict something.
+  SetMemoryLimit(5);
+
+  // Garbage collect with media time at Range B (1000ms).
+  // This should evict Range A from the front since it is far behind media time.
+  EXPECT_TRUE(GarbageCollect(base::Milliseconds(1000), 0));
+
+  // 5. Append data. If the bug exists, range_for_next_append_ is dangling and
+  // dereferencing it will cause a UAF or hit a CHECK.
+  // With the fix, range_for_next_append_ is reset to ranges_.end() when
+  // Range A is deleted.
+  AppendBuffers("0K 10K");
+}
+
 }  // namespace media

diff --git a/media/filters/source_buffer_stream_unittest.cc b/media/filters/source_buffer_stream_unittest.cc
index b154eeb..7cccec0 100644
--- a/media/filters/source_buffer_stream_unittest.cc
+++ b/media/filters/source_buffer_stream_unittest.cc
@@ -5747,4 +5747,38 @@
   EXPECT_TRUE(IsRangeListSorted());
 }
 
+TEST_F(SourceBufferStreamTest, GarbageCollectionUpdatesRangeForNextAppend) {
+  // Set memory limit to 10 buffers.
+  SetMemoryLimit(10);
+
+  // 1. Append 10 buffers to create Range A [0, 90ms].
+  NewCodedFrameGroupAppend("0K 10K 20K 30K 40K 50K 60K 70K 80K 90K");
+
+  // 2. Append 10 buffers to create Range B [1000ms, 1090ms].
+  // This exceeds the memory limit and triggers GC, but Range A is kept because
+  // it was recently appended.
+  NewCodedFrameGroupAppend(
+      "1000K 1010K 1020K 1030K 1040K 1050K 1060K 1070K 1080K 1090K");
+
+  // 3. Start a new coded frame group that overlaps Range A.
+  // This sets range_for_next_append_ to Range A and
+  // last_appended_buffer_timestamp_ to kNoTimestamp.
+  stream_->OnStartOfCodedFrameGroup(base::Milliseconds(0));
+
+  // 4. Trigger Garbage Collection.
+  // We want to free enough data that Range A is deleted.
+  // Set memory limit very low so GC must evict something.
+  SetMemoryLimit(5);
+
+  // Garbage collect with media time at Range B (1000ms).
+  // This should evict Range A from the front since it is far behind media time.
+  EXPECT_TRUE(GarbageCollect(base::Milliseconds(1000), 0));
+
+  // 5. Append data. If the bug exists, range_for_next_append_ is dangling and
+  // dereferencing it will cause a UAF or hit a CHECK.
+  // With the fix, range_for_next_append_ is reset to ranges_.end() when
+  // Range A is deleted.
+  AppendBuffers("0K 10K");
+}
+
 }  // namespace media