Bug 882549 - Don't rely on mElement to find nsIDocument in TextTrackCue. r=rillian, r=smaug
authorCaitlin Potter <snowball@defpixel.com>
Wed, 19 Jun 2013 14:02:04 -0400
changeset 147330 14967188583ef1f76b31ecae435acda5bcd56462
parent 147329 e09d80620b6994cd1b0f75d093ef45fbfc4f037e
child 147331 880c8be7f35f00c6d54edabc83e8c837bd876b55
push id2697
push userbbajaj@mozilla.com
push dateMon, 05 Aug 2013 18:49:53 +0000
treeherdermozilla-beta@dfec938c7b63 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrillian, smaug
bugs882549
milestone24.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 882549 - Don't rely on mElement to find nsIDocument in TextTrackCue. r=rillian, r=smaug Find nsIDocument via mGlobal (as nsPIDOMWindow) rather than via pointer to HTMLTrackElement. (bz from #content has explained how we can access nsPIDOMWindow and nsIDocument without having to ask the HTMLTrackElement) For the time being, there are many null-ness checks for both 'window' and 'document', and no mechanism to report errors other than returning nullptr where possible. This will be addressed in a followup bug. Also adds a test for mHead nullness in TextTrackCue::ConvertNodeTreeToDOMTree to prevent SIGSEV.
content/media/TextTrackCue.cpp
content/media/test/crashtests/882549.html
content/media/test/crashtests/crashtests.list
--- a/content/media/TextTrackCue.cpp
+++ b/content/media/TextTrackCue.cpp
@@ -80,19 +80,27 @@ TextTrackCue::~TextTrackCue()
     // Release our reference and null mHead.
     webvtt_release_node(&mHead);
   }
 }
 
 void
 TextTrackCue::CreateCueOverlay()
 {
-  mTrackElement->OwnerDoc()->CreateElem(NS_LITERAL_STRING("div"), nullptr,
-                                        kNameSpaceID_XHTML,
-                                        getter_AddRefs(mCueDiv));
+  nsCOMPtr<nsPIDOMWindow> window(do_QueryInterface(mGlobal));
+  if(!window) {
+    return;
+  }
+  nsIDocument* document = window->GetDoc();
+  if(!document) {
+    return;
+  }
+  document->CreateElem(NS_LITERAL_STRING("div"), nullptr,
+                       kNameSpaceID_XHTML,
+                       getter_AddRefs(mCueDiv));
   nsGenericHTMLElement* cueDiv =
     static_cast<nsGenericHTMLElement*>(mCueDiv.get());
   cueDiv->SetClassName(NS_LITERAL_STRING("caption-text"));
 }
 
 void
 TextTrackCue::RenderCue()
 {
@@ -131,18 +139,26 @@ TextTrackCue::RenderCue()
 
   mCueDiv->AppendChild(*frag, rv);
   overlay->AppendChild(*mCueDiv, rv);
 }
 
 already_AddRefed<DocumentFragment>
 TextTrackCue::GetCueAsHTML()
 {
+  nsCOMPtr<nsPIDOMWindow> window(do_QueryInterface(mGlobal));
+  if(!window) {
+    return nullptr;
+  }
+  nsIDocument* document = window->GetDoc();
+  if(!document){
+    return nullptr;
+  }
   nsRefPtr<DocumentFragment> frag =
-    mTrackElement->OwnerDoc()->CreateDocumentFragment();
+    document->CreateDocumentFragment();
 
   ConvertNodeTreeToDOMTree(frag);
 
   return frag.forget();
 }
 
 struct WebVTTNodeParentPair
 {
@@ -178,17 +194,17 @@ PopChild(nsTArray<WebVTTNodeParentPair> 
 }
 
 void
 TextTrackCue::ConvertNodeTreeToDOMTree(nsIContent* aParentContent)
 {
   nsTArray<WebVTTNodeParentPair> nodeParentPairStack;
 
   // mHead should actually be the head of a node tree.
-  if (mHead->kind != WEBVTT_HEAD_NODE) {
+  if (!mHead || mHead->kind != WEBVTT_HEAD_NODE) {
     return;
   }
   // Seed the stack for traversal.
   PushChildren(nodeParentPairStack, mHead, aParentContent);
 
   while (!nodeParentPairStack.IsEmpty()) {
     WebVTTNodeParentPair nodeParentPair = PopChild(nodeParentPairStack);
     nsCOMPtr<nsIContent> content;
@@ -234,19 +250,27 @@ TextTrackCue::ConvertInternalNodeToConte
       atom = nsGkAtoms::span;
       break;
     default:
       return nullptr;
       break;
   }
 
   nsCOMPtr<nsIContent> cueTextContent;
-  mTrackElement->OwnerDoc()->CreateElem(nsDependentAtomString(atom), nullptr,
-                                        kNameSpaceID_XHTML,
-                                        getter_AddRefs(cueTextContent));
+  nsCOMPtr<nsPIDOMWindow> window(do_QueryInterface(mGlobal));
+  if(!window) {
+    return nullptr;
+  }
+  nsIDocument* document = window->GetDoc();
+  if(!document){
+    return nullptr;
+  }
+  document->CreateElem(nsDependentAtomString(atom), nullptr,
+                       kNameSpaceID_XHTML,
+                       getter_AddRefs(cueTextContent));
 
   if (aWebVTTNode->kind == WEBVTT_VOICE) {
     const char* text =
       webvtt_string_text(&aWebVTTNode->data.internal_data->annotation);
     if (text) {
       nsGenericHTMLElement* genericHtmlElement =
         static_cast<nsGenericHTMLElement*>(cueTextContent.get());
       genericHtmlElement->SetTitle(NS_ConvertUTF8toUTF16(text));
@@ -275,17 +299,25 @@ TextTrackCue::ConvertInternalNodeToConte
   }
   return cueTextContent.forget();
 }
 
 already_AddRefed<nsIContent>
 TextTrackCue::ConvertLeafNodeToContent(const webvtt_node* aWebVTTNode)
 {
   nsCOMPtr<nsIContent> cueTextContent;
-  nsNodeInfoManager* nimgr = mTrackElement->NodeInfo()->NodeInfoManager();
+  nsCOMPtr<nsPIDOMWindow> window(do_QueryInterface(mGlobal));
+  if(!window) {
+    return nullptr;
+  }
+  nsIDocument* document = window->GetDoc();
+  if(!document) {
+    return nullptr;
+  }
+  nsNodeInfoManager* nimgr = document->NodeInfoManager();
   switch (aWebVTTNode->kind) {
     case WEBVTT_TEXT:
     {
       cueTextContent = new nsTextNode(nimgr);
       const char* text = webvtt_string_text(&aWebVTTNode->data.text);
       if (text) {
         cueTextContent->SetText(NS_ConvertUTF8toUTF16(text), false);
       }
new file mode 100644
--- /dev/null
+++ b/content/media/test/crashtests/882549.html
@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<html>
+    <head>
+        <script>
+            var o0 = new TextTrackCue(0.000, 1.000, 'Bug882549');
+            var o1 = o0.getCueAsHTML(0);
+        </script>
+    </head>
+    <body>
+    </body>
+</html>
+<script>
+</script>
--- a/content/media/test/crashtests/crashtests.list
+++ b/content/media/test/crashtests/crashtests.list
@@ -42,8 +42,9 @@ load 880129.html
 skip-if(B2G) load 880202.html # load failed, bug 833371 for B2G
 load 880342-1.html
 load 880342-2.html
 load 880384.html
 load 880404.html
 load 880724.html
 load 881775.html
 load 882956.html
+test-pref(media.webvtt.enabled,true) load 882549.html