Bug 1271566 - VideoTrackList::RemoveTrack/AddTrack should also update selected index. r=jesup r=pehrsons
authorctai <ctai@mozilla.com>
Tue, 10 May 2016 16:28:24 +0800
changeset 337497 eb749a28eee0decab6c333ec283569f0e924ff7f
parent 337496 50c739a2cf3f55f3aad70d3dfe52a49d674356af
child 337498 5109a152a7715ccf704aa06224b6993f1285c4d8
push id6249
push userjlund@mozilla.com
push dateMon, 01 Aug 2016 13:59:36 +0000
treeherdermozilla-beta@bad9d4f5bf7e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjesup, pehrsons
bugs1271566
milestone49.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1271566 - VideoTrackList::RemoveTrack/AddTrack should also update selected index. r=jesup r=pehrsons MozReview-Commit-ID: Bb4iPGbb9KW
dom/media/MediaTrackList.h
dom/media/VideoTrackList.cpp
dom/media/VideoTrackList.h
--- a/dom/media/MediaTrackList.h
+++ b/dom/media/MediaTrackList.h
@@ -40,17 +40,19 @@ public:
   using DOMEventTargetHelper::DispatchTrustedEvent;
 
   // The return value is non-null, assert an error if aIndex is out of bound
   // for array mTracks.
   MediaTrack* operator[](uint32_t aIndex);
 
   void AddTrack(MediaTrack* aTrack);
 
-  void RemoveTrack(const RefPtr<MediaTrack>& aTrack);
+  // In remove track case, the VideoTrackList::mSelectedIndex should be updated
+  // due to mTracks changed. No need to take care this in add track case.
+  virtual void RemoveTrack(const RefPtr<MediaTrack>& aTrack);
 
   void RemoveTracks();
 
   static already_AddRefed<AudioTrack>
   CreateAudioTrack(const nsAString& aId,
                    const nsAString& aKind,
                    const nsAString& aLabel,
                    const nsAString& aLanguage,
--- a/dom/media/VideoTrackList.cpp
+++ b/dom/media/VideoTrackList.cpp
@@ -19,16 +19,51 @@ VideoTrackList::WrapObject(JSContext* aC
 VideoTrack*
 VideoTrackList::operator[](uint32_t aIndex)
 {
   MediaTrack* track = MediaTrackList::operator[](aIndex);
   return track->AsVideoTrack();
 }
 
 void
+VideoTrackList::RemoveTrack(const RefPtr<MediaTrack>& aTrack)
+{
+  // we need to find the video track before |MediaTrackList::RemoveTrack|. Or
+  // mSelectedIndex will not be valid. The check of mSelectedIndex == -1
+  // need to be done after RemoveTrack. Also the call of
+  // |MediaTrackList::RemoveTrack| is necessary even when mSelectedIndex = -1.
+  bool found;
+  VideoTrack* videoTrack = IndexedGetter(mSelectedIndex, found);
+  MediaTrackList::RemoveTrack(aTrack);
+  if (mSelectedIndex == -1) {
+    // There was no selected track and we don't select another track on removal.
+    return;
+  }
+  MOZ_ASSERT(found, "When mSelectedIndex is set it should point to a track");
+  MOZ_ASSERT(videoTrack, "The mSelectedIndex should be set to video track only");
+
+  // Let the caller of RemoveTrack deal with choosing the new selected track if
+  // it removes the currently-selected track.
+  if (aTrack == videoTrack) {
+    mSelectedIndex = -1;
+    return;
+  }
+
+  // The removed track was not the selected track and there is a
+  // currently-selected video track. We need to find the new location of the
+  // selected track.
+  for (size_t ix = 0; ix < mTracks.Length(); ix++) {
+    if (mTracks[ix] == videoTrack) {
+      mSelectedIndex = ix;
+      return;
+    }
+  }
+}
+
+void
 VideoTrackList::EmptyTracks()
 {
   mSelectedIndex = -1;
   MediaTrackList::EmptyTracks();
 }
 
 VideoTrack*
 VideoTrackList::IndexedGetter(uint32_t aIndex, bool& aFound)
--- a/dom/media/VideoTrackList.h
+++ b/dom/media/VideoTrackList.h
@@ -23,16 +23,18 @@ public:
     : MediaTrackList(aOwnerWindow, aMediaElement)
     , mSelectedIndex(-1)
   {}
 
   JSObject* WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto) override;
 
   VideoTrack* operator[](uint32_t aIndex);
 
+  void RemoveTrack(const RefPtr<MediaTrack>& aTrack) override;
+
   void EmptyTracks() override;
 
   // WebIDL
   int32_t SelectedIndex() const
   {
     return mSelectedIndex;
   }