Bug 882664 - Part 2: Perform sorting on TextTrackLists. r=rillian
authorRick Eyre <rick.eyre@hotmail.com>
Thu, 27 Feb 2014 12:47:23 -0500
changeset 191711 824d1ef99ba2a6ff441fe5cd655201b93f24acaa
parent 191710 43585af186ecc38b52e3fbcd611a5db91226d1f5
child 191712 1d6e179a67d7eb2df1b9b3a089fe1e88b988c617
push id474
push userasasaki@mozilla.com
push dateMon, 02 Jun 2014 21:01:02 +0000
treeherdermozilla-release@967f4cf1b31c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrillian
bugs882664
milestone30.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 882664 - Part 2: Perform sorting on TextTrackLists. r=rillian - This implements sorting on the MediaElement's list of TextTracks as described in: http://www.whatwg.org/specs/web-apps/current-work/multipage/the-video-element.html#text-track-model This is important as automatically positioned cues will have a line position that is relative to their TextTrack's position in the MediaElement's list of TextTracks. - Added a TextTrackSource enum which all TextTracks have. This helps distinguish where a TextTrack came from.
content/html/content/src/HTMLMediaElement.cpp
content/html/content/src/HTMLTrackElement.cpp
content/html/content/src/TextTrackManager.cpp
content/html/content/src/TextTrackManager.h
content/media/TextTrack.cpp
content/media/TextTrack.h
content/media/TextTrackList.cpp
content/media/TextTrackList.h
content/media/test/test_texttrack.html
--- a/content/html/content/src/HTMLMediaElement.cpp
+++ b/content/html/content/src/HTMLMediaElement.cpp
@@ -73,16 +73,18 @@
 
 #include "AudioChannelService.h"
 
 #include "nsCSSParser.h"
 #include "nsIMediaList.h"
 #include "mozilla/dom/power/PowerManagerService.h"
 #include "mozilla/dom/WakeLock.h"
 
+#include "mozilla/dom/TextTrack.h"
+
 #include "ImageContainer.h"
 #include "nsRange.h"
 #include <algorithm>
 
 #ifdef PR_LOGGING
 static PRLogModuleInfo* gMediaElementLog;
 static PRLogModuleInfo* gMediaElementEventsLog;
 #define LOG(type, msg) PR_LOG(gMediaElementLog, type, msg)
@@ -3942,19 +3944,21 @@ HTMLMediaElement::TextTracks() const
   return mTextTrackManager ? mTextTrackManager->TextTracks() : nullptr;
 }
 
 already_AddRefed<TextTrack>
 HTMLMediaElement::AddTextTrack(TextTrackKind aKind,
                                const nsAString& aLabel,
                                const nsAString& aLanguage)
 {
-  return mTextTrackManager ? mTextTrackManager->AddTextTrack(aKind, aLabel,
-                                                             aLanguage)
-                           : nullptr;
+  if (mTextTrackManager) {
+    return mTextTrackManager->AddTextTrack(aKind, aLabel, aLanguage,
+                                           TextTrackSource::AddTextTrack);
+  }
+  return nullptr;
 }
 
 void
 HTMLMediaElement::PopulatePendingTextTrackList()
 {
   if (mTextTrackManager) {
     mTextTrackManager->PopulatePendingList();
   }
--- a/content/html/content/src/HTMLTrackElement.cpp
+++ b/content/html/content/src/HTMLTrackElement.cpp
@@ -148,17 +148,18 @@ HTMLTrackElement::CreateTextTrack()
 
   TextTrackKind kind;
   if (const nsAttrValue* value = GetParsedAttr(nsGkAtoms::kind)) {
     kind = static_cast<TextTrackKind>(value->GetEnumValue());
   } else {
     kind = TextTrackKind::Subtitles;
   }
 
-  mTrack = new TextTrack(OwnerDoc()->GetParentObject(), kind, label, srcLang);
+  mTrack = new TextTrack(OwnerDoc()->GetParentObject(), kind, label, srcLang,
+                         TextTrackSource::Track);
   mTrack->SetTrackElement(this);
 
   if (mMediaParent) {
     mMediaParent->AddTextTrack(mTrack);
   }
 }
 
 bool
--- a/content/html/content/src/TextTrackManager.cpp
+++ b/content/html/content/src/TextTrackManager.cpp
@@ -15,16 +15,69 @@
 #include "nsVideoFrame.h"
 #include "nsIFrame.h"
 #include "nsTArrayHelpers.h"
 #include "nsIWebVTTParserWrapper.h"
 
 namespace mozilla {
 namespace dom {
 
+CompareTextTracks::CompareTextTracks(HTMLMediaElement* aMediaElement)
+{
+  mMediaElement = aMediaElement;
+}
+
+int32_t
+CompareTextTracks::TrackChildPosition(TextTrack* aTextTrack) const {
+  HTMLTrackElement* trackElement = aTextTrack->GetTrackElement();
+  if (!trackElement) {
+    return -1;
+  }
+  return mMediaElement->IndexOf(trackElement);
+}
+
+bool
+CompareTextTracks::Equals(TextTrack* aOne, TextTrack* aTwo) const {
+  // Two tracks can never be equal. If they have corresponding TrackElements
+  // they would need to occupy the same tree position (impossible) and in the
+  // case of tracks coming from AddTextTrack source we put the newest at the
+  // last position, so they won't be equal as well.
+  return false;
+}
+
+bool
+CompareTextTracks::LessThan(TextTrack* aOne, TextTrack* aTwo) const
+{
+  TextTrackSource sourceOne = aOne->GetTextTrackSource();
+  TextTrackSource sourceTwo = aTwo->GetTextTrackSource();
+  if (sourceOne != sourceTwo) {
+    return sourceOne == Track ||
+           (sourceOne == AddTextTrack && sourceTwo == MediaResourceSpecific);
+  }
+  switch (sourceOne) {
+    case Track: {
+      int32_t positionOne = TrackChildPosition(aOne);
+      int32_t positionTwo = TrackChildPosition(aTwo);
+      // If either position one or positiontwo are -1 then something has gone
+      // wrong. In this case we should just put them at the back of the list.
+      return positionOne != -1 && positionTwo != -1 &&
+             positionOne < positionTwo;
+    }
+    case AddTextTrack:
+      // For AddTextTrack sources the tracks will already be in the correct relative
+      // order in the source array. Assume we're called in iteration order and can
+      // therefore always report aOne < aTwo to maintain the original temporal ordering.
+      return true;
+    case MediaResourceSpecific:
+      // No rules for Media Resource Specific tracks yet.
+      break;
+  }
+  return true;
+}
+
 NS_IMPL_CYCLE_COLLECTION_3(TextTrackManager, mTextTracks,
                            mPendingTextTracks, mNewCues)
 NS_IMPL_CYCLE_COLLECTION_ROOT_NATIVE(TextTrackManager, AddRef)
 NS_IMPL_CYCLE_COLLECTION_UNROOT_NATIVE(TextTrackManager, Release)
 
 StaticRefPtr<nsIWebVTTParserWrapper> TextTrackManager::sParserWrapper;
 
 TextTrackManager::TextTrackManager(HTMLMediaElement *aMediaElement)
@@ -53,29 +106,37 @@ TextTrackManager::~TextTrackManager()
 TextTrackList*
 TextTrackManager::TextTracks() const
 {
   return mTextTracks;
 }
 
 already_AddRefed<TextTrack>
 TextTrackManager::AddTextTrack(TextTrackKind aKind, const nsAString& aLabel,
-                               const nsAString& aLanguage)
+                               const nsAString& aLanguage,
+                               TextTrackSource aTextTrackSource)
 {
+  if (!mMediaElement) {
+    return nullptr;
+  }
   nsRefPtr<TextTrack> ttrack =
-    mTextTracks->AddTextTrack(aKind, aLabel, aLanguage);
+    mTextTracks->AddTextTrack(aKind, aLabel, aLanguage, aTextTrackSource,
+                              CompareTextTracks(mMediaElement));
   ttrack->SetReadyState(HTMLTrackElement::READY_STATE_LOADED);
   AddCues(ttrack);
   return ttrack.forget();
 }
 
 void
 TextTrackManager::AddTextTrack(TextTrack* aTextTrack)
 {
-  mTextTracks->AddTextTrack(aTextTrack);
+  if (!mMediaElement) {
+    return;
+  }
+  mTextTracks->AddTextTrack(aTextTrack, CompareTextTracks(mMediaElement));
   AddCues(aTextTrack);
 }
 
 void
 TextTrackManager::AddCues(TextTrack* aTextTrack)
 {
   TextTrackCueList* cueList = aTextTrack->GetCues();
   if (cueList) {
@@ -152,15 +213,16 @@ void
 TextTrackManager::PopulatePendingList()
 {
   uint32_t len = mTextTracks->Length();
   bool dummy;
   for (uint32_t index = 0; index < len; ++index) {
     TextTrack* ttrack = mTextTracks->IndexedGetter(index, dummy);
     if (ttrack && ttrack->Mode() != TextTrackMode::Disabled &&
         ttrack->ReadyState() == HTMLTrackElement::READY_STATE_LOADING) {
-      mPendingTextTracks->AddTextTrack(ttrack);
+      mPendingTextTracks->AddTextTrack(ttrack,
+                                       CompareTextTracks(mMediaElement));
     }
   }
 }
 
 } // namespace dom
 } // namespace mozilla
--- a/content/html/content/src/TextTrackManager.h
+++ b/content/html/content/src/TextTrackManager.h
@@ -14,32 +14,44 @@
 #include "mozilla/StaticPtr.h"
 
 class nsIWebVTTParserWrapper;
 
 namespace mozilla {
 namespace dom {
 
 class HTMLMediaElement;
+
+class CompareTextTracks {
+private:
+  HTMLMediaElement* mMediaElement;
+public:
+  CompareTextTracks(HTMLMediaElement* aMediaElement);
+  int32_t TrackChildPosition(TextTrack* aTrack) const;
+  bool Equals(TextTrack* aOne, TextTrack* aTwo) const;
+  bool LessThan(TextTrack* aOne, TextTrack* aTwo) const;
+};
+
 class TextTrack;
 class TextTrackCue;
 
 class TextTrackManager
 {
 public:
   NS_DECL_CYCLE_COLLECTION_NATIVE_CLASS(TextTrackManager)
   NS_INLINE_DECL_CYCLE_COLLECTING_NATIVE_REFCOUNTING(TextTrackManager);
 
   TextTrackManager(HTMLMediaElement *aMediaElement);
   ~TextTrackManager();
 
   TextTrackList* TextTracks() const;
   already_AddRefed<TextTrack> AddTextTrack(TextTrackKind aKind,
                                            const nsAString& aLabel,
-                                           const nsAString& aLanguage);
+                                           const nsAString& aLanguage,
+                                           TextTrackSource aTextTrackSource);
   void AddTextTrack(TextTrack* aTextTrack);
   void RemoveTextTrack(TextTrack* aTextTrack, bool aPendingListOnly);
   void DidSeek();
 
   void AddCue(TextTrackCue& aCue);
   void AddCues(TextTrack* aTextTrack);
 
   /**
--- a/content/media/TextTrack.cpp
+++ b/content/media/TextTrack.cpp
@@ -24,43 +24,48 @@ NS_IMPL_CYCLE_COLLECTION_INHERITED_5(Tex
                                      mTextTrackList,
                                      mTrackElement)
 
 NS_IMPL_ADDREF_INHERITED(TextTrack, nsDOMEventTargetHelper)
 NS_IMPL_RELEASE_INHERITED(TextTrack, nsDOMEventTargetHelper)
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION_INHERITED(TextTrack)
 NS_INTERFACE_MAP_END_INHERITING(nsDOMEventTargetHelper)
 
-TextTrack::TextTrack(nsISupports* aParent)
+TextTrack::TextTrack(nsISupports* aParent, TextTrackSource aTextTrackSource)
   : mParent(aParent)
+  , mTextTrackSource(aTextTrackSource)
 {
   SetDefaultSettings();
   SetIsDOMBinding();
 }
 
 TextTrack::TextTrack(nsISupports* aParent,
                      TextTrackKind aKind,
                      const nsAString& aLabel,
-                     const nsAString& aLanguage)
+                     const nsAString& aLanguage,
+                     TextTrackSource aTextTrackSource)
   : mParent(aParent)
+  , mTextTrackSource(aTextTrackSource)
 {
   SetDefaultSettings();
   mKind = aKind;
   mLabel = aLabel;
   mLanguage = aLanguage;
   SetIsDOMBinding();
 }
 
 TextTrack::TextTrack(nsISupports* aParent,
                      TextTrackList* aTextTrackList,
                      TextTrackKind aKind,
                      const nsAString& aLabel,
-                     const nsAString& aLanguage)
+                     const nsAString& aLanguage,
+                     TextTrackSource aTextTrackSource)
   : mParent(aParent)
   , mTextTrackList(aTextTrackList)
+  , mTextTrackSource(aTextTrackSource)
 {
   SetDefaultSettings();
   mKind = aKind;
   mLabel = aLabel;
   mLanguage = aLanguage;
   SetIsDOMBinding();
 }
 
--- a/content/media/TextTrack.h
+++ b/content/media/TextTrack.h
@@ -17,32 +17,41 @@ namespace mozilla {
 namespace dom {
 
 class TextTrackList;
 class TextTrackCue;
 class TextTrackCueList;
 class TextTrackRegion;
 class HTMLTrackElement;
 
+enum TextTrackSource {
+  Track,
+  AddTextTrack,
+  MediaResourceSpecific
+};
+
 class TextTrack MOZ_FINAL : public nsDOMEventTargetHelper
 {
 public:
   NS_DECL_ISUPPORTS_INHERITED
   NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(TextTrack, nsDOMEventTargetHelper)
 
-  TextTrack(nsISupports* aParent);
+  TextTrack(nsISupports* aParent,
+            TextTrackSource aTextTrackSource);
   TextTrack(nsISupports* aParent,
             TextTrackKind aKind,
             const nsAString& aLabel,
-            const nsAString& aLanguage);
+            const nsAString& aLanguage,
+            TextTrackSource aTextTrackSource);
   TextTrack(nsISupports* aParent,
             TextTrackList* aTextTrackList,
             TextTrackKind aKind,
             const nsAString& aLabel,
-            const nsAString& aLanguage);
+            const nsAString& aLanguage,
+            TextTrackSource aTextTrackSource);
 
   void SetDefaultSettings();
 
   virtual JSObject* WrapObject(JSContext* aCx,
                                JS::Handle<JSObject*> aScope) MOZ_OVERRIDE;
 
   nsISupports* GetParentObject() const
   {
@@ -98,16 +107,20 @@ public:
   TextTrackList* GetTextTrackList();
   void SetTextTrackList(TextTrackList* aTextTrackList);
 
   IMPL_EVENT_HANDLER(cuechange)
 
   HTMLTrackElement* GetTrackElement();
   void SetTrackElement(HTMLTrackElement* aTrackElement);
 
+  TextTrackSource GetTextTrackSource() {
+    return mTextTrackSource;
+  }
+
 private:
   void UpdateActiveCueList();
 
   nsCOMPtr<nsISupports> mParent;
   nsRefPtr<TextTrackList> mTextTrackList;
 
   TextTrackKind mKind;
   nsString mLabel;
@@ -118,14 +131,17 @@ private:
 
   nsRefPtr<TextTrackCueList> mCueList;
   nsRefPtr<TextTrackCueList> mActiveCueList;
   nsRefPtr<HTMLTrackElement> mTrackElement;
 
   uint32_t mCuePos;
   uint16_t mReadyState;
   bool mDirty;
+
+  // An enum that represents where the track was sourced from.
+  TextTrackSource mTextTrackSource;
 };
 
 } // namespace dom
 } // namespace mozilla
 
 #endif // mozilla_dom_TextTrack_h
--- a/content/media/TextTrackList.cpp
+++ b/content/media/TextTrackList.cpp
@@ -59,27 +59,31 @@ TextTrackList::IndexedGetter(uint32_t aI
 {
   aFound = aIndex < mTextTracks.Length();
   return aFound ? mTextTracks[aIndex] : nullptr;
 }
 
 already_AddRefed<TextTrack>
 TextTrackList::AddTextTrack(TextTrackKind aKind,
                             const nsAString& aLabel,
-                            const nsAString& aLanguage)
+                            const nsAString& aLanguage,
+                            TextTrackSource aTextTrackSource,
+                            const CompareTextTracks& aCompareTT)
 {
-  nsRefPtr<TextTrack> track = new TextTrack(mGlobal, this, aKind, aLabel, aLanguage);
-  AddTextTrack(track);
+  nsRefPtr<TextTrack> track = new TextTrack(mGlobal, this, aKind, aLabel, aLanguage,
+                                            aTextTrackSource);
+  AddTextTrack(track, aCompareTT);
   return track.forget();
 }
 
 void
-TextTrackList::AddTextTrack(TextTrack* aTextTrack)
+TextTrackList::AddTextTrack(TextTrack* aTextTrack,
+                            const CompareTextTracks& aCompareTT)
 {
-  if (mTextTracks.AppendElement(aTextTrack)) {
+  if (mTextTracks.InsertElementSorted(aTextTrack, aCompareTT)) {
     aTextTrack->SetTextTrackList(this);
     CreateAndDispatchTrackEventRunner(aTextTrack, NS_LITERAL_STRING("addtrack"));
   }
 }
 
 TextTrack*
 TextTrackList::GetTrackById(const nsAString& aId)
 {
--- a/content/media/TextTrackList.h
+++ b/content/media/TextTrackList.h
@@ -11,16 +11,17 @@
 #include "nsCycleCollectionParticipant.h"
 #include "nsDOMEventTargetHelper.h"
 
 namespace mozilla {
 namespace dom {
 
 class HTMLMediaElement;
 class TextTrackManager;
+class CompareTextTracks;
 class TrackEvent;
 class TrackEventRunner;
 
 class TextTrackList MOZ_FINAL : public nsDOMEventTargetHelper
 {
 public:
   NS_DECL_ISUPPORTS_INHERITED
   NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(TextTrackList, nsDOMEventTargetHelper)
@@ -43,20 +44,22 @@ public:
 
   // Get all the current active cues.
   void GetAllActiveCues(nsTArray<nsRefPtr<TextTrackCue> >& aCues);
 
   TextTrack* IndexedGetter(uint32_t aIndex, bool& aFound);
 
   already_AddRefed<TextTrack> AddTextTrack(TextTrackKind aKind,
                                            const nsAString& aLabel,
-                                           const nsAString& aLanguage);
+                                           const nsAString& aLanguage,
+                                           TextTrackSource aTextTrackSource,
+                                           const CompareTextTracks& aCompareTT);
   TextTrack* GetTrackById(const nsAString& aId);
 
-  void AddTextTrack(TextTrack* aTextTrack);
+  void AddTextTrack(TextTrack* aTextTrack, const CompareTextTracks& aCompareTT);
 
   void RemoveTextTrack(TextTrack* aTrack);
   void DidSeek();
 
   HTMLMediaElement* GetMediaElement();
   void SetTextTrackManager(TextTrackManager* aTextTrackManager);
 
   nsresult DispatchTrackEvent(nsIDOMEvent* aEvent);
--- a/content/media/test/test_texttrack.html
+++ b/content/media/test/test_texttrack.html
@@ -22,48 +22,89 @@ SpecialPowers.pushPrefEnv({"set": [["med
     var video = document.createElement("video");
 
     isnot(video.textTracks, undefined, "HTMLMediaElement::TextTrack() property should be available.")
 
     var trackList = video.textTracks;
     is(trackList.length, 0, "Length should be 0.");
 
     ok(typeof video.addTextTrack == "function", "HTMLMediaElement::AddTextTrack() function should be available.")
-    video.addTextTrack("subtitles", "label", "en-CA");
+    video.addTextTrack("subtitles", "third", "en-CA");
     is(trackList.length, 1, "Length should be 1.");
 
     var textTrack = video.textTracks[0];
-    is(textTrack.label, "label", "Label should be set to label.");
+    is(textTrack.label, "third", "Label should be set to third.");
     is(textTrack.language, "en-CA", "Language should be en-CA.");
     is(textTrack.kind, "subtitles", "Default kind should be subtitles.");
     is(textTrack.mode, "hidden", "Default mode should be hidden.");
 
     // Mode should not allow a bogus value.
     textTrack.mode = 'bogus';
     is(textTrack.mode, 'hidden', "Mode should be not allow a bogus value.");
 
     // Should allow all these values for mode.
     checkMode("showing", "Mode should allow \"showing\"");
     checkMode("disabled", "Mode should allow \"disabled\"");
     checkMode("hidden", "Mode should allow \"hidden\"");
 
     // All below are read-only properties and so should not allow setting.
     textTrack.label = "French subtitles";
-    is(textTrack.label, "label", "Label is read-only so should still be \"label\".");
+    is(textTrack.label, "third", "Label is read-only so should still be \"label\".");
 
     textTrack.language = "en";
     is(textTrack.language, "en-CA", "Language is read-only so should still be \"en-CA\".");
 
     textTrack.kind = "captions";
     is(textTrack.kind, "subtitles", "Kind is read-only so should still be \"subtitles\"");
 
     function checkMode(value, message) {
       textTrack.mode = value;
       is(textTrack.mode, value, message);
     }
 
-    SimpleTest.finish();
-  }
-);
+    // Test that text tracks are sorted correctly when being inserted on the
+    // MediaElements list of text tracks. For this test we add four tracks, the
+    // first one was at the start of the test, the next three are below.
+    var trackOne = document.createElement("track");
+    trackOne.label = "first";
+    trackOne.src = "basic.vtt";
+    video.appendChild(trackOne);
+
+    video.addTextTrack("subtitles", "fourth", "en-CA");
+
+    var trackTwo = document.createElement("track");
+    trackTwo.label = "second";
+    trackTwo.src = "basic.vtt";
+    video.appendChild(trackTwo);
+
+    video.src = "seek.webm";
+    video.preload = "auto";
+
+    document.getElementById("content").appendChild(video);
+
+    video.addEventListener("loadedmetadata", function run_tests() {
+      // Re-que run_tests() at the end of the event loop until the track
+      // element has loaded its data.
+      if (trackOne.readyState == 1 || trackTwo.readyState == 1) {
+        setTimeout(run_tests, 0);
+        return;
+      }
+      is(trackOne.readyState, 2, "First Track::ReadyState should be set to LOADED.");
+      is(trackTwo.readyState, 2, "Second Track::ReadyState should be set to LOADED.");
+
+      // For the tracks to be sorted the first two tracks, added through a
+      // TrackElement, must occupy the first two indexes in their TrackElement
+      // tree order. The second two tracks, added through the 'addTextTrack'
+      // method, will occupy the last two indexes in the order that they were
+      // added in.
+      var labels = [ "first", "second", "third", "fourth" ];
+      is(video.textTracks.length, labels.length, "TextTracks length should be " + labels.length);
+      for (var i = 0; i < video.textTracks.length; i++) {
+        isnot(video.textTracks[i], null, "Video should have a text track at " + i + " index.");
+        is(video.textTracks[i].label, labels[i], "Text track at " + i + " should be " + labels[i]);
+      }
+      SimpleTest.finish();
+    });
+});
 </script>
 </pre>
 </body>
 </html>