Bug 1251742. Avoid overflow in computing area of surface sizes in SurfaceCache. r=dholbert
authorTimothy Nikkel <tnikkel@gmail.com>
Fri, 26 Feb 2016 17:13:59 -0600
changeset 285895 e4ae782bcc00c668b21edbd6ab3394f936507d9e
parent 285894 11ba2a4c1ad3cefde131d4e9b455ef3192077b6a
child 285896 a03ba8b50b93853954a37f72b45dbd2f2fd478b3
push id30036
push usercbook@mozilla.com
push dateMon, 29 Feb 2016 10:35:59 +0000
treeherderautoland@9da51cb4974e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert
bugs1251742, 1228314
milestone47.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 1251742. Avoid overflow in computing area of surface sizes in SurfaceCache. r=dholbert http://hg.mozilla.org/mozilla-central/rev/9727cdebb2ee (bug 1228314) fixed the first instance of this, but missed the next two for some reason.
image/SurfaceCache.cpp
image/SurfaceCache.h
--- a/image/SurfaceCache.cpp
+++ b/image/SurfaceCache.cpp
@@ -222,16 +222,21 @@ private:
   nsExpirationState  mExpirationState;
   RefPtr<imgFrame> mSurface;
   DrawableFrameRef   mDrawableRef;
   const Cost         mCost;
   const ImageKey     mImageKey;
   const SurfaceKey   mSurfaceKey;
 };
 
+static int64_t
+AreaOfIntSize(const IntSize& aSize) {
+  return static_cast<int64_t>(aSize.width) * static_cast<int64_t>(aSize.height);
+}
+
 /**
  * An ImageSurfaceCache is a per-image surface cache. For correctness we must be
  * able to remove all surfaces associated with an image when the image is
  * destroyed or invalidated. Since this will happen frequently, it makes sense
  * to make it cheap by storing the surfaces for each image separately.
  *
  * ImageSurfaceCache also keeps track of whether its associated image is locked
  * or unlocked.
@@ -322,21 +327,19 @@ public:
         continue;
       }
 
       SurfaceKey bestMatchKey = bestMatch->GetSurfaceKey();
 
       // Compare sizes. We use an area-based heuristic here instead of computing a
       // truly optimal answer, since it seems very unlikely to make a difference
       // for realistic sizes.
-      int64_t idealArea = static_cast<int64_t>(idealKey.Size().width) *
-        static_cast<int64_t>(idealKey.Size().height);
-      int64_t surfaceArea = aSurfaceKey.Size().width * aSurfaceKey.Size().height;
-      int64_t bestMatchArea =
-        bestMatchKey.Size().width * bestMatchKey.Size().height;
+      int64_t idealArea = AreaOfIntSize(idealKey.Size());
+      int64_t surfaceArea = AreaOfIntSize(aSurfaceKey.Size());
+      int64_t bestMatchArea = AreaOfIntSize(bestMatchKey.Size());
 
       // If the best match is smaller than the ideal size, prefer bigger sizes.
       if (bestMatchArea < idealArea) {
         if (surfaceArea > bestMatchArea) {
           bestMatch = surface;
         }
         continue;
       }
--- a/image/SurfaceCache.h
+++ b/image/SurfaceCache.h
@@ -59,17 +59,17 @@ public:
   uint32_t Hash() const
   {
     uint32_t hash = HashGeneric(mSize.width, mSize.height);
     hash = AddToHash(hash, mSVGContext.map(HashSIC).valueOr(0));
     hash = AddToHash(hash, mAnimationTime, uint32_t(mFlags));
     return hash;
   }
 
-  IntSize Size() const { return mSize; }
+  const IntSize& Size() const { return mSize; }
   Maybe<SVGImageContext> SVGContext() const { return mSVGContext; }
   float AnimationTime() const { return mAnimationTime; }
   SurfaceFlags Flags() const { return mFlags; }
 
 private:
   SurfaceKey(const IntSize& aSize,
              const Maybe<SVGImageContext>& aSVGContext,
              const float aAnimationTime,