Bug 1428765 - Fix scenario where display item clip chains hash to different values even though they are equal. r?mstange draft
authorKartikaya Gupta <kgupta@mozilla.com>
Tue, 09 Jan 2018 12:31:58 -0500
changeset 717861 b74ec5e3eecb30e029acf119aa1feea919864aec
parent 717738 6f5fac320fcb6625603fa8a744ffa8523f8b3d71
child 745365 6e0b17edd3ed7412ab4f7688be6dba7cd0832bc3
push id94798
push userkgupta@mozilla.com
push dateTue, 09 Jan 2018 18:33:10 +0000
reviewersmstange
bugs1428765
milestone59.0a1
Bug 1428765 - Fix scenario where display item clip chains hash to different values even though they are equal. r?mstange MozReview-Commit-ID: KOBNjcuMUn
layout/painting/DisplayItemClipChain.cpp
--- a/layout/painting/DisplayItemClipChain.cpp
+++ b/layout/painting/DisplayItemClipChain.cpp
@@ -23,19 +23,23 @@ DisplayItemClipChain::Equal(const Displa
   if (aClip1 == aClip2) {
     return true;
   }
 
   if (!aClip1 || !aClip2) {
     return false;
   }
 
-  return aClip1->mASR == aClip2->mASR &&
+  bool ret = aClip1->mASR == aClip2->mASR &&
          aClip1->mClip == aClip2->mClip &&
          Equal(aClip1->mParent, aClip2->mParent);
+  // Sanity check: if two clip chains are equal they must hash to the same
+  // thing too, or Bad Things (TM) will happen.
+  MOZ_ASSERT(!ret || (Hash(aClip1) == Hash(aClip2)));
+  return ret;
 }
 
 uint32_t
 DisplayItemClipChain::Hash(const DisplayItemClipChain* aClip)
 {
   if (!aClip) {
     return 0;
   }
@@ -43,17 +47,22 @@ DisplayItemClipChain::Hash(const Display
   // We include the number of rounded rects in the hash but not their contents.
   // This is to keep the hash fast, because most clips will not have rounded
   // rects and including them will slow down the hash in the common case. Note
   // that the ::Equal check still checks the rounded rect contents, so in case
   // of hash collisions the clip chains can still be distinguished using that.
   uint32_t hash = HashGeneric(aClip->mASR, aClip->mClip.GetRoundedRectCount());
   if (aClip->mClip.HasClip()) {
     const nsRect& rect = aClip->mClip.GetClipRect();
-    hash = AddToHash(hash, rect.x, rect.y, rect.width, rect.height);
+    // empty rects are considered equal in DisplayItemClipChain::Equal, even
+    // though they may have different x and y coordinates. So make sure they
+    // hash to the same thing in those cases too.
+    if (!rect.IsEmpty()) {
+      hash = AddToHash(hash, rect.x, rect.y, rect.width, rect.height);
+    }
   }
 
   return hash;
 }
 
 /* static */ nsCString
 DisplayItemClipChain::ToString(const DisplayItemClipChain* aClipChain)
 {