Bug 798849 - fix uninitialized values in WebGL element array cache... and reenable accidentally disabled canvas mochitests (!!!) - r=jgilbert
authorBenoit Jacob <bjacob@mozilla.com>
Thu, 11 Oct 2012 18:27:09 -0400
changeset 110121 4476b1668b33cb301cd3db8684998ca05a632c56
parent 110120 1c858930c3be5aafdc48db53c2e5d669c2d21bd1
child 110122 05567a9159c5ef3abea6dc57915bf7b296c5965a
push id93
push usernmatsakis@mozilla.com
push dateWed, 31 Oct 2012 21:26:57 +0000
reviewersjgilbert
bugs798849, 732660
milestone19.0a1
Bug 798849 - fix uninitialized values in WebGL element array cache... and reenable accidentally disabled canvas mochitests (!!!) - r=jgilbert This fixes Valgrind errors while running TestWebGLElementArrayCache. This also fixes the way we do content/canvas/test in the Makefiles, which had been broken by my attempt to enable compiled-code tests there (bug 732660). Since I still don't know how to do compiled-code tests there, I've disabled them. At least we have the rest of content/canvas/test back. Let's hope that no regression happened.
content/canvas/Makefile.in
content/canvas/src/WebGLElementArrayCache.cpp
content/canvas/src/WebGLElementArrayCache.h
content/canvas/test/Makefile.in
content/canvas/test/compiled/TestWebGLElementArrayCache.cpp
--- a/content/canvas/Makefile.in
+++ b/content/canvas/Makefile.in
@@ -7,12 +7,12 @@ DEPTH		= @DEPTH@
 topsrcdir	= @top_srcdir@
 srcdir		= @srcdir@
 VPATH		= @srcdir@
 
 include $(DEPTH)/config/autoconf.mk
 
 PARALLEL_DIRS	= public src
 
-TOOLS_DIRS += test
+TEST_DIRS += test
 
 include $(topsrcdir)/config/rules.mk
 
--- a/content/canvas/src/WebGLElementArrayCache.cpp
+++ b/content/canvas/src/WebGLElementArrayCache.cpp
@@ -141,63 +141,62 @@ public:
   WebGLElementArrayCacheTree(WebGLElementArrayCache& p)
     : mParent(p)
     , mNumLeaves(0)
     , mInvalidated(false)
     , mFirstInvalidatedLeaf(0)
     , mLastInvalidatedLeaf(0)
   {
     ResizeToParentSize();
-    Invalidate(0, mParent.ByteSize() - 1);
   }
 
   T GlobalMaximum() const {
     MOZ_ASSERT(!mInvalidated);
     return mTreeData[1];
   }
 
   // returns the index of the parent node; if treeIndex=1 (the root node),
   // the return value is 0.
   static size_t ParentNode(size_t treeIndex) {
-    MOZ_ASSERT(treeIndex);
+    MOZ_ASSERT(treeIndex > 1);
     return treeIndex >> 1;
   }
 
   static bool IsRightNode(size_t treeIndex) {
-    MOZ_ASSERT(treeIndex);
+    MOZ_ASSERT(treeIndex > 1);
     return treeIndex & 1;
   }
 
   static bool IsLeftNode(size_t treeIndex) {
-    MOZ_ASSERT(treeIndex);
+    MOZ_ASSERT(treeIndex > 1);
     return !IsRightNode(treeIndex);
   }
 
   static size_t SiblingNode(size_t treeIndex) {
-    MOZ_ASSERT(treeIndex);
+    MOZ_ASSERT(treeIndex > 1);
     return treeIndex ^ 1;
   }
 
   static size_t LeftChildNode(size_t treeIndex) {
     MOZ_ASSERT(treeIndex);
     return treeIndex << 1;
   }
 
   static size_t RightChildNode(size_t treeIndex) {
     MOZ_ASSERT(treeIndex);
     return SiblingNode(LeftChildNode(treeIndex));
   }
 
   static size_t LeftNeighborNode(size_t treeIndex, size_t distance = 1) {
-    MOZ_ASSERT(treeIndex);
+    MOZ_ASSERT(treeIndex > 1);
     return treeIndex - distance;
   }
 
   static size_t RightNeighborNode(size_t treeIndex, size_t distance = 1) {
-    MOZ_ASSERT(treeIndex);
+    MOZ_ASSERT(treeIndex > 1);
     return treeIndex + distance;
   }
 
   size_t LeafForElement(size_t element) {
     size_t leaf = element / sElementsPerLeaf;
     MOZ_ASSERT(leaf < mNumLeaves);
     return leaf;
   }
@@ -282,20 +281,28 @@ public:
     return result;
   }
 
   bool ResizeToParentSize()
   {
     size_t numberOfElements = mParent.ByteSize() / sizeof(T);
     size_t requiredNumLeaves = (numberOfElements + sElementsPerLeaf - 1) / sElementsPerLeaf;
 
+    size_t oldNumLeaves = mNumLeaves;
     mNumLeaves = NextPowerOfTwo(requiredNumLeaves);
+    Invalidate(0, mParent.ByteSize() - 1);
 
     // see class comment for why we the tree storage size is 2 * mNumLeaves
-    return mTreeData.SetLength(2 * mNumLeaves);
+    if (!mTreeData.SetLength(2 * mNumLeaves)) {
+      return false;
+    }
+    if (mNumLeaves != oldNumLeaves) {
+      memset(mTreeData.Elements(), 0, mTreeData.Length() * sizeof(mTreeData[0]));
+    }
+    return true;
   }
 
   void Invalidate(size_t firstByte, size_t lastByte);
 
   void Update();
 
   size_t SizeOfIncludingThis(nsMallocSizeOfFun aMallocSizeOf) const
   {
@@ -375,33 +382,28 @@ void WebGLElementArrayCacheTree<T>::Upda
       }
       mTreeData[treeIndex] = m;
       treeIndex++;
     }
   }
 
   // Step #2: propagate the values up the tree. This is simply a matter of walking up
   // the tree and setting each node to the max of its two children.
-  while (true) {
-
-    // fast-exit case where only one node is invalidated at the current level
-    if (firstTreeIndex == lastTreeIndex) {
-      size_t firstTreeIndexParent = ParentNode(firstTreeIndex);
-      while (firstTreeIndexParent) {
-        mTreeData[firstTreeIndexParent] = NS_MAX(mTreeData[firstTreeIndex], mTreeData[SiblingNode(firstTreeIndex)]);
-        firstTreeIndex = firstTreeIndexParent;
-        firstTreeIndexParent = ParentNode(firstTreeIndex);
-      }
-      break;
-    }
+  while (firstTreeIndex > 1) {
 
     // move up 1 level
     firstTreeIndex = ParentNode(firstTreeIndex);
     lastTreeIndex = ParentNode(lastTreeIndex);
 
+    // fast-exit case where only one node is invalidated at the current level
+    if (firstTreeIndex == lastTreeIndex) {
+      mTreeData[firstTreeIndex] = NS_MAX(mTreeData[LeftChildNode(firstTreeIndex)], mTreeData[RightChildNode(firstTreeIndex)]);
+      continue;
+    }
+
     // initialize local iteration variables: child and parent.
     size_t child = LeftChildNode(firstTreeIndex);
     size_t parent = firstTreeIndex;
 
     // the unrolling makes this look more complicated than it is; the plain non-unrolled
     // version is in the second while loop below
     const int unrollSize = 8;
     while (RightNeighborNode(parent, unrollSize - 1) <= lastTreeIndex)
--- a/content/canvas/src/WebGLElementArrayCache.h
+++ b/content/canvas/src/WebGLElementArrayCache.h
@@ -60,17 +60,17 @@ private:
   template<typename T>
   const T* Elements() const { return static_cast<const T*>(mUntypedData); }
   template<typename T>
   T* Elements() { return static_cast<T*>(mUntypedData); }
 
   void InvalidateTrees(size_t firstByte, size_t lastByte);
 
   template<typename T>
-  friend class WebGLElementArrayCacheTree;
+  friend struct WebGLElementArrayCacheTree;
   template<typename T>
   friend struct TreeForType;
 
   void* mUntypedData;
   size_t mByteSize;
   WebGLElementArrayCacheTree<uint8_t>* mUint8Tree;
   WebGLElementArrayCacheTree<uint16_t>* mUint16Tree;
 };
--- a/content/canvas/test/Makefile.in
+++ b/content/canvas/test/Makefile.in
@@ -5,17 +5,17 @@
 
 DEPTH		= @DEPTH@
 topsrcdir	= @top_srcdir@
 srcdir		= @srcdir@
 VPATH		= @srcdir@
 relativesrcdir  = @relativesrcdir@
 DIRS		+= webgl crossorigin
 
-TOOLS_DIRS += compiled
+# TEST_DIRS += compiled
 
 include $(DEPTH)/config/autoconf.mk
 MOCHITEST_FILES = \
 	test_canvas.html \
 	image_transparent50.png \
 	image_redtransparent.png \
 	image_yellow.png \
 	image_anim-poster-gr.png \
--- a/content/canvas/test/compiled/TestWebGLElementArrayCache.cpp
+++ b/content/canvas/test/compiled/TestWebGLElementArrayCache.cpp
@@ -26,24 +26,21 @@ void VerifyImplFunction(bool condition, 
   }
 }
 
 #define VERIFY(condition) \
     VerifyImplFunction((condition), __FILE__, __LINE__)
 
 void MakeRandomVector(nsTArray<uint8_t>& a, size_t size) {
   a.SetLength(size);
-  // only the most-significant bits of rand() are reasonably random
-  // 16 here is arbitrary, may fail on platforms where RAND_MAX is low,
-  // but guarded by an assertion.
-  enum { bitsToIgnore = 16 };
-  MOZ_STATIC_ASSERT((unsigned int)(RAND_MAX) >> (8 + bitsToIgnore),
-                    "Didn't expect RAND_MAX to be so low");
+  // only the most-significant bits of rand() are reasonably random.
+  // RAND_MAX can be as low as 0x7fff, and we need 8 bits for the result, so we can only
+  // ignore the 7 least significant bits.
   for (size_t i = 0; i < size; i++)
-    a[i] = static_cast<uint8_t>((unsigned int)(rand()) >> bitsToIgnore);
+    a[i] = static_cast<uint8_t>((unsigned int)(rand()) >> 7);
 }
 
 template<typename T>
 T RandomInteger(T a, T b)
 {
   T result(a + rand() % (b - a + 1));
   return result;
 }