Bug 730441 - Fix null-pointer crash in nsTreeColumns::RestoreNaturalOrder(). r=tnikkel
authorMats Palmgren <matspal@gmail.com>
Tue, 23 Apr 2013 00:52:20 -0500
changeset 140504 352ceffb0d9ebe96eb8d976789e45a4647a6b50f
parent 140503 b17e0eb827c0d866e31875c9823768b1866cf8cb
child 140505 a6639d6743db4bfbb88e25214e466243bd8ad64a
push id2579
push userakeybl@mozilla.com
push dateMon, 24 Jun 2013 18:52:47 +0000
treeherdermozilla-beta@b69b7de8a05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstnikkel
bugs730441
milestone23.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 730441 - Fix null-pointer crash in nsTreeColumns::RestoreNaturalOrder(). r=tnikkel
layout/xul/tree/crashtests/730441-1.xul
layout/xul/tree/crashtests/730441-2.xul
layout/xul/tree/crashtests/crashtests.list
layout/xul/tree/nsTreeColumns.cpp
new file mode 100644
--- /dev/null
+++ b/layout/xul/tree/crashtests/730441-1.xul
@@ -0,0 +1,54 @@
+<?xml version="1.0"?>
+<!--
+Program received signal SIGSEGV, Segmentation fault.
+0xb6457185 in nsIContent::SetAttr (this=0x0, aNameSpaceID=0, aName=0xb0cb064c, aValue=..., aNotify=1) at ../../dist/include/nsIContent.h:285
+285	    return SetAttr(aNameSpaceID, aName, nsnull, aValue, aNotify);
+(gdb) p this
+$6 = (nsIContent * const) 0x0
+(gdb) bt 3
+#0  0xb6457185 in nsIContent::SetAttr (this=0x0, aNameSpaceID=0, aName=0xb0cb064c, aValue=..., aNotify=1) at ../../dist/include/nsIContent.h:285
+#1  0xb6b72072 in nsTreeColumns::RestoreNaturalOrder (this=0xaaf83cc0) at layout/xul/base/src/tree/src/nsTreeColumns.cpp:605
+#2  0xb736c76f in NS_InvokeByIndex_P () at xpcom/reflect/xptcall/src/md/unix/xptcinvoke_gcc_x86_unix.cpp:69
+(More stack frames follow...)
+(gdb) frame 1
+#1  0xb6b72072 in nsTreeColumns::RestoreNaturalOrder (this=0xaaf83cc0) at layout/xul/base/src/tree/src/nsTreeColumns.cpp:605
+605	    child->SetAttr(kNameSpaceID_None, nsGkAtoms::ordinal, ordinal, PR_TRUE);
+(gdb) list
+600	  PRUint32 numChildren = colsContent->GetChildCount();
+601	  for (PRUint32 i = 0; i < numChildren; ++i) {
+602	    nsIContent *child = colsContent->GetChildAt(i);
+603	    nsAutoString ordinal;
+604	    ordinal.AppendInt(i);
+605	    child->SetAttr(kNameSpaceID_None, nsGkAtoms::ordinal, ordinal, PR_TRUE);
+606	  }
+(gdb) p child
+$7 = (nsIContent *) 0x0
+
+First loop iteration: |child->SetAttr()| dispatches "DOMAttrModified" event.
+Event listener removes next column. Second loop iteration: |colsContent->GetChildAt(i)|
+returns null. Then we have |null->SetAttr()|.
+-->
+<window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
+        onload="run();">
+<tree id="tree">
+  <treecols>
+    <treecol id="col1"/>
+    <treecol id="col2"/>
+  </treecols>
+  <treechildren/>
+</tree>
+<script type="text/javascript"><![CDATA[
+function listener() {
+  var col2 = document.getElementById("col2");
+  col2.parentNode.removeChild(col2);
+}
+
+function run() {
+  var col1 = document.getElementById("col1");
+  col1.addEventListener("DOMAttrModified", listener, true);
+  var tree = document.getElementById("tree");
+  tree.columns.restoreNaturalOrder();
+}
+]]></script>
+</window>
+
new file mode 100644
--- /dev/null
+++ b/layout/xul/tree/crashtests/730441-2.xul
@@ -0,0 +1,52 @@
+<?xml version="1.0"?>
+<!--
+Program received signal SIGSEGV, Segmentation fault.
+0xb6b720a6 in nsTreeColumns::RestoreNaturalOrder (this=0xa947a580) at layout/xul/base/src/tree/src/nsTreeColumns.cpp:610
+610	  mTree->Invalidate();
+(gdb) bt 3
+#0  0xb6b720a6 in nsTreeColumns::RestoreNaturalOrder (this=0xa947a580) at layout/xul/base/src/tree/src/nsTreeColumns.cpp:610
+#1  0xb736c76f in NS_InvokeByIndex_P () at xpcom/reflect/xptcall/src/md/unix/xptcinvoke_gcc_x86_unix.cpp:69
+#2  0xb6171901 in XPCWrappedNative::CallMethod (ccx=..., mode=XPCWrappedNative::CALL_METHOD)
+    at js/src/xpconnect/src/xpcwrappednative.cpp:2722
+(More stack frames follow...)
+(gdb) list
+605	    child->SetAttr(kNameSpaceID_None, nsGkAtoms::ordinal, ordinal, PR_TRUE);
+606	  }
+607	
+608	  nsTreeColumns::InvalidateColumns();
+609	
+610	  mTree->Invalidate();
+611	
+612	  return NS_OK;
+613	}
+614	
+(gdb) p mTree
+$9 = (nsITreeBoxObject *) 0x0
+
+|child->SetAttr()| dispatches "DOMAttrModified" event. Event listener removes
+whole tree, |mTree| is being set to null. Then we have |null->Invalidate()|.
+-->
+<window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
+        onload="run();">
+<tree id="tree">
+  <treecols>
+    <treecol id="col"/>
+  </treecols>
+  <treechildren/>
+</tree>
+<script type="text/javascript"><![CDATA[
+var tree = null;
+
+function listener() {
+  tree.parentNode.removeChild(tree);
+}
+
+function run() {
+  col = document.getElementById("col");
+  col.addEventListener("DOMAttrModified", listener, true);
+  tree = document.getElementById("tree");
+  tree.columns.restoreNaturalOrder();
+}
+]]></script>
+</window>
+
--- a/layout/xul/tree/crashtests/crashtests.list
+++ b/layout/xul/tree/crashtests/crashtests.list
@@ -13,8 +13,10 @@ load 399692-1.xhtml
 load 399715-1.xhtml
 load 409807-1.xul
 load 414170-1.xul
 load 430394-1.xul
 load 454186-1.xul
 load 479931-1.xhtml
 load 509602-1.xul
 load 601427.html
+load 730441-1.xul
+load 730441-2.xul
--- a/layout/xul/tree/nsTreeColumns.cpp
+++ b/layout/xul/tree/nsTreeColumns.cpp
@@ -628,28 +628,28 @@ nsTreeColumns::RestoreNaturalOrder()
   nsIContent* content = mTree->GetBaseElement();
 
   // Strong ref, since we'll be setting attributes
   nsCOMPtr<nsIContent> colsContent =
     nsTreeUtils::GetImmediateChild(content, nsGkAtoms::treecols);
   if (!colsContent)
     return NS_OK;
 
-  uint32_t numChildren = colsContent->GetChildCount();
-  for (uint32_t i = 0; i < numChildren; ++i) {
-    nsIContent *child = colsContent->GetChildAt(i);
+  for (uint32_t i = 0; i < colsContent->GetChildCount(); ++i) {
+    nsCOMPtr<nsIContent> child = colsContent->GetChildAt(i);
     nsAutoString ordinal;
     ordinal.AppendInt(i);
     child->SetAttr(kNameSpaceID_None, nsGkAtoms::ordinal, ordinal, true);
   }
 
   nsTreeColumns::InvalidateColumns();
 
-  mTree->Invalidate();
-
+  if (mTree) {
+    mTree->Invalidate();
+  }
   return NS_OK;
 }
 
 nsTreeColumn*
 nsTreeColumns::GetPrimaryColumn()
 {
   EnsureColumns();
   for (nsTreeColumn* currCol = mFirstColumn; currCol; currCol = currCol->GetNext()) {