Add helper method to delete and remove SourceBufferRange objects.

Consolidates logic for deleting a SourceBufferRange and removing
it from the ranges_ list. This change will make it easier to ensure
that cached references to ranges, like selected_range_, always
get cleared when a range is removed. No changes in functionality
are included in this patch.

BUG=303413
TEST=All existing unit tests still pass.
R=scherkus@chromium.org

Review URL: https://codereview.chromium.org/29453002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@229992 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 60 insertions, 55 deletions

diff --git a/media/filters/source_buffer_stream.cc b/media/filters/source_buffer_stream.cc
index 8a797de8..07ebf33 100644
--- a/media/filters/source_buffer_stream.cc
+++ b/media/filters/source_buffer_stream.cc
@@ -519,11 +519,7 @@ void SourceBufferStream::Remove(base::TimeDelta start, base::TimeDelta end,
     // If the current range now is completely covered by the removal
     // range then delete it and move on.
     if (start <= range->GetStartTimestamp()) {
-      if (selected_range_ == range)
-        SetSelectedRange(NULL);
-
-      delete range;
-      itr = ranges_.erase(itr);
+      DeleteAndRemoveRange(&itr);
       continue;
     }
 
@@ -937,10 +933,8 @@ void SourceBufferStream::ResolveCompleteOverlaps(
     if (*next_range_itr == selected_range_) {
       DCHECK(deleted_buffers->empty());
       selected_range_->DeleteAll(deleted_buffers);
-      SetSelectedRange(NULL);
     }
-    delete *next_range_itr;
-    next_range_itr = ranges_.erase(next_range_itr);
+    DeleteAndRemoveRange(&next_range_itr);
   }
 }
 
@@ -950,48 +944,43 @@ void SourceBufferStream::ResolveEndOverlap(
   DCHECK(deleted_buffers);
 
   SourceBufferRange* range_with_new_buffers = *range_with_new_buffers_itr;
-  RangeList::iterator next_range_itr = range_with_new_buffers_itr;
-  ++next_range_itr;
+  RangeList::iterator overlapped_range_itr = range_with_new_buffers_itr;
+  ++overlapped_range_itr;
 
-  if (next_range_itr == ranges_.end() ||
-      !range_with_new_buffers->EndOverlaps(**next_range_itr)) {
+  if (overlapped_range_itr == ranges_.end() ||
+      !range_with_new_buffers->EndOverlaps(**overlapped_range_itr)) {
     return;
   }
 
   // Split the overlapped range after |range_with_new_buffers|'s last buffer
-  // overlaps. Now |overlapped_range| contains only the buffers that do not
+  // overlaps. Now |*overlapped_range_itr| contains only the buffers that do not
   // belong in |ranges_| anymore, and |new_next_range| contains buffers that
   // go after |range_with_new_buffers| (without overlap).
-  scoped_ptr<SourceBufferRange> overlapped_range(*next_range_itr);
-  next_range_itr = ranges_.erase(next_range_itr);
-
   SourceBufferRange* new_next_range =
-      overlapped_range->SplitRange(
+      (*overlapped_range_itr)->SplitRange(
           range_with_new_buffers->GetEndTimestamp(), true);
 
+  if (selected_range_ == *overlapped_range_itr) {
+    SourceBufferRange* overlapped_range = *overlapped_range_itr;
+    // If the |overlapped_range| transfers its next buffer position to
+    // |new_next_range|, make |new_next_range| the |selected_range_|.
+    if (new_next_range && new_next_range->HasNextBufferPosition()) {
+      DCHECK(!overlapped_range->HasNextBufferPosition());
+      SetSelectedRange(new_next_range);
+    } else {
+      // The |overlapped_range| still has the current playback position.
+      // Move the buffers for the current playback position in
+      // |overlapped_range| into |deleted_buffers|.
+      DCHECK(overlapped_range->HasNextBufferPosition());
+      DCHECK(deleted_buffers->empty());
+      overlapped_range->DeleteAll(deleted_buffers);
+    }
+  }
+  DeleteAndRemoveRange(&overlapped_range_itr);
+
   // If there were non-overlapped buffers, add the new range to |ranges_|.
   if (new_next_range)
     AddToRanges(new_next_range);
-
-  // If we didn't overlap a selected range, return.
-  if (selected_range_ != overlapped_range)
-    return;
-
-  // If the |overlapped_range| transfers its next buffer position to
-  // |new_next_range|, make |new_next_range| the |selected_range_|.
-  if (new_next_range && new_next_range->HasNextBufferPosition()) {
-    DCHECK(!overlapped_range->HasNextBufferPosition());
-    SetSelectedRange(new_next_range);
-    return;
-  }
-
-  // Save the buffers in |overlapped_range|.
-  DCHECK(deleted_buffers->empty());
-  DCHECK_EQ(overlapped_range.get(), selected_range_);
-  overlapped_range->DeleteAll(deleted_buffers);
-
-  // |overlapped_range| will be deleted, so set |selected_range_| to NULL.
-  SetSelectedRange(NULL);
 }
 
 void SourceBufferStream::PruneTrackBuffer(const base::TimeDelta timestamp) {
@@ -1007,26 +996,29 @@ void SourceBufferStream::PruneTrackBuffer(const base::TimeDelta timestamp) {
 
 void SourceBufferStream::MergeWithAdjacentRangeIfNecessary(
     const RangeList::iterator& range_with_new_buffers_itr) {
+  DCHECK(range_with_new_buffers_itr != ranges_.end());
+
   SourceBufferRange* range_with_new_buffers = *range_with_new_buffers_itr;
   RangeList::iterator next_range_itr = range_with_new_buffers_itr;
   ++next_range_itr;
 
-  if (next_range_itr != ranges_.end() &&
-      range_with_new_buffers->CanAppendRangeToEnd(**next_range_itr)) {
-    bool transfer_current_position = selected_range_ == *next_range_itr;
-    range_with_new_buffers->AppendRangeToEnd(**next_range_itr,
-                                             transfer_current_position);
-    // Update |selected_range_| pointer if |range| has become selected after
-    // merges.
-    if (transfer_current_position)
-      SetSelectedRange(range_with_new_buffers);
+  if (next_range_itr == ranges_.end() ||
+      !range_with_new_buffers->CanAppendRangeToEnd(**next_range_itr)) {
+    return;
+  }
 
-    if (next_range_itr == range_for_next_append_)
-      range_for_next_append_ = range_with_new_buffers_itr;
+  bool transfer_current_position = selected_range_ == *next_range_itr;
+  range_with_new_buffers->AppendRangeToEnd(**next_range_itr,
+                                           transfer_current_position);
+  // Update |selected_range_| pointer if |range| has become selected after
+  // merges.
+  if (transfer_current_position)
+    SetSelectedRange(range_with_new_buffers);
 
-    delete *next_range_itr;
-    ranges_.erase(next_range_itr);
-  }
+  if (next_range_itr == range_for_next_append_)
+    range_for_next_append_ = range_with_new_buffers_itr;
+
+  DeleteAndRemoveRange(&next_range_itr);
 }
 
 void SourceBufferStream::Seek(base::TimeDelta timestamp) {
@@ -1079,10 +1071,9 @@ void SourceBufferStream::OnSetDuration(base::TimeDelta duration) {
   while (itr != ranges_.end()) {
     // If we're about to delete the selected range, also reset the seek state.
     DCHECK((*itr)->GetStartTimestamp() >= duration);
-    if (*itr== selected_range_)
+    if (*itr == selected_range_)
       ResetSeekState();
-    delete *itr;
-    itr = ranges_.erase(itr);
+    DeleteAndRemoveRange(&itr);
   }
 }
 
@@ -1462,6 +1453,15 @@ std::string SourceBufferStream::GetStreamTypeName() const {
   return "AUDIO";
 }
 
+void SourceBufferStream::DeleteAndRemoveRange(RangeList::iterator* itr) {
+  DCHECK(*itr != ranges_.end());
+  if (**itr == selected_range_)
+    SetSelectedRange(NULL);
+
+  delete **itr;
+  *itr = ranges_.erase(*itr);
+}
+
 SourceBufferRange::SourceBufferRange(
     const BufferQueue& new_buffers, base::TimeDelta media_segment_start_time,
     const InterbufferDistanceCB& interbuffer_distance_cb)
@@ -1827,7 +1827,6 @@ int SourceBufferRange::GetNextConfigId() const {
   return buffers_.at(next_buffer_index_)->GetConfigId();
 }
 
-
 base::TimeDelta SourceBufferRange::GetNextTimestamp() const {
   DCHECK(!buffers_.empty());
   DCHECK(HasNextBufferPosition());
diff --git a/media/filters/source_buffer_stream.h b/media/filters/source_buffer_stream.h
index b218b31..9594d9d 100644
--- a/media/filters/source_buffer_stream.h
+++ b/media/filters/source_buffer_stream.h
@@ -291,6 +291,12 @@ class MEDIA_EXPORT SourceBufferStream {
   // or there is a pending seek beyond any existing ranges.
   bool IsEndSelected() const;
 
+  // Deletes the range pointed to by |*itr| and removes it from |ranges_|.
+  // If |*itr| points to |selected_range_|, then |selected_range_| is set to
+  // NULL. After the range is removed, |*itr| is to the range after the one that
+  // was removed or to |ranges_.end()| if the last range was removed.
+  void DeleteAndRemoveRange(RangeList::iterator* itr);
+
   // Callback used to report error strings that can help the web developer
   // figure out what is wrong with the content.
   LogCB log_cb_;