Backed out 3 changesets (bug 1456558) for crashtest assertion failures on a CLOSED TREE
authorAndreea Pavel <apavel@mozilla.com>
Fri, 21 Sep 2018 02:13:41 +0300
changeset 496030 fac7810a8f317798d8d78e6cdd4034bf18385001
parent 496029 70d8f11cf6e8278b8e1509a69bd02adc0168e606
child 496031 dba2140cb3663a6a15ddbb0af423c39ba45d9fd4
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1456558
milestone64.0a1
backs out70d8f11cf6e8278b8e1509a69bd02adc0168e606
af9fc3daf97ca1ef1fe09587553deae1fa9c279c
f209a9d848f42a377c15bf6a0b0d0255a4ebd580
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
Backed out 3 changesets (bug 1456558) for crashtest assertion failures on a CLOSED TREE Backed out changeset 70d8f11cf6e8 (bug 1456558) Backed out changeset af9fc3daf97c (bug 1456558) Backed out changeset f209a9d848f4 (bug 1456558)
gfx/thebes/gfxPrefs.h
image/Image.cpp
image/Image.h
image/LookupResult.h
image/SVGDrawingParameters.h
image/SurfaceCache.cpp
image/SurfaceCache.h
image/VectorImage.cpp
image/VectorImage.h
image/imgFrame.cpp
modules/libpref/init/all.js
--- a/gfx/thebes/gfxPrefs.h
+++ b/gfx/thebes/gfxPrefs.h
@@ -541,17 +541,16 @@ private:
   DECL_GFX_PREF(Live, "gl.require-hardware",                   RequireHardwareGL, bool, false);
   DECL_GFX_PREF(Live, "gl.use-tls-is-current",                 UseTLSIsCurrent, int32_t, 0);
 
   DECL_GFX_PREF(Live, "image.animated.decode-on-demand.threshold-kb", ImageAnimatedDecodeOnDemandThresholdKB, uint32_t, 20480);
   DECL_GFX_PREF(Live, "image.animated.decode-on-demand.batch-size", ImageAnimatedDecodeOnDemandBatchSize, uint32_t, 6);
   DECL_GFX_PREF(Live, "image.animated.generate-full-frames",   ImageAnimatedGenerateFullFrames, bool, false);
   DECL_GFX_PREF(Live, "image.animated.resume-from-last-displayed", ImageAnimatedResumeFromLastDisplayed, bool, false);
   DECL_GFX_PREF(Live, "image.cache.factor2.threshold-surfaces", ImageCacheFactor2ThresholdSurfaces, int32_t, -1);
-  DECL_GFX_PREF(Live, "image.cache.max-rasterized-svg-threshold-kb", ImageCacheMaxRasterizedSVGThresholdKB, int32_t, 90*1024);
   DECL_GFX_PREF(Once, "image.cache.size",                      ImageCacheSize, int32_t, 5*1024*1024);
   DECL_GFX_PREF(Once, "image.cache.timeweight",                ImageCacheTimeWeight, int32_t, 500);
   DECL_GFX_PREF(Live, "image.decode-immediately.enabled",      ImageDecodeImmediatelyEnabled, bool, false);
   DECL_GFX_PREF(Live, "image.downscale-during-decode.enabled", ImageDownscaleDuringDecodeEnabled, bool, true);
   DECL_GFX_PREF(Live, "image.infer-src-animation.threshold-ms", ImageInferSrcAnimationThresholdMS, uint32_t, 2000);
   DECL_GFX_PREF(Live, "image.layout_network_priority",         ImageLayoutNetworkPriority, bool, true);
   DECL_GFX_PREF(Once, "image.mem.decode_bytes_at_a_time",      ImageMemDecodeBytesAtATime, uint32_t, 200000);
   DECL_GFX_PREF(Live, "image.mem.discardable",                 ImageMemDiscardable, bool, false);
--- a/image/Image.cpp
+++ b/image/Image.cpp
@@ -155,17 +155,16 @@ ImageResource::GetImageContainerImpl(Lay
     }
   }
 
   if (container) {
     switch (entry->mLastDrawResult) {
       case ImgDrawResult::SUCCESS:
       case ImgDrawResult::BAD_IMAGE:
       case ImgDrawResult::BAD_ARGS:
-      case ImgDrawResult::NOT_SUPPORTED:
         container.forget(aOutContainer);
         return entry->mLastDrawResult;
       case ImgDrawResult::NOT_READY:
       case ImgDrawResult::INCOMPLETE:
       case ImgDrawResult::TEMPORARY_ERROR:
         // Temporary conditions where we need to rerequest the frame to recover.
         break;
       case ImgDrawResult::WRONG_SIZE:
@@ -214,17 +213,16 @@ ImageResource::GetImageContainerImpl(Lay
       if (bestSize == entry->mSize && flags == entry->mFlags &&
           aSVGContext == entry->mSVGContext) {
         container = entry->mContainer.get();
         if (container) {
           switch (entry->mLastDrawResult) {
             case ImgDrawResult::SUCCESS:
             case ImgDrawResult::BAD_IMAGE:
             case ImgDrawResult::BAD_ARGS:
-            case ImgDrawResult::NOT_SUPPORTED:
               container.forget(aOutContainer);
               return entry->mLastDrawResult;
             case ImgDrawResult::NOT_READY:
             case ImgDrawResult::INCOMPLETE:
             case ImgDrawResult::TEMPORARY_ERROR:
               // Temporary conditions where we need to rerequest the frame to
               // recover. We have already done so!
               break;
--- a/image/Image.h
+++ b/image/Image.h
@@ -332,34 +332,16 @@ protected:
   TimeStamp                     mLastRefreshTime;
   uint64_t                      mInnerWindowId;
   uint32_t                      mAnimationConsumers;
   uint16_t                      mAnimationMode; // Enum values in imgIContainer
   bool                          mInitialized:1; // Have we been initalized?
   bool                          mAnimating:1;   // Are we currently animating?
   bool                          mError:1;       // Error handling
 
-  /**
-   * Attempt to find a matching cached surface in the SurfaceCache, and if not
-   * available, request the production of such a surface (either synchronously
-   * or asynchronously).
-   *
-   * If the draw result is BAD_IMAGE, BAD_ARGS or NOT_READY, the size will be
-   * the same as aSize. If it is TEMPORARY_ERROR, INCOMPLETE, or SUCCESS, the
-   * size is a hint as to what we expect the surface size to be, once the best
-   * fitting size is available. It may or may not match the size of the surface
-   * returned at this moment. This is useful for choosing how to store the final
-   * result (e.g. if going into an ImageContainer, ideally we would share the
-   * same container for many requested sizes, if they all end up with the same
-   * best fit size in the end).
-   *
-   * A valid surface should only be returned for SUCCESS and INCOMPLETE.
-   *
-   * Any other draw result is invalid.
-   */
   virtual Tuple<ImgDrawResult, gfx::IntSize, RefPtr<gfx::SourceSurface>>
     GetFrameInternal(const gfx::IntSize& aSize,
                      const Maybe<SVGImageContext>& aSVGContext,
                      uint32_t aWhichFrame,
                      uint32_t aFlags)
   {
     return MakeTuple(ImgDrawResult::BAD_IMAGE, aSize,
                      RefPtr<gfx::SourceSurface>());
--- a/image/LookupResult.h
+++ b/image/LookupResult.h
@@ -104,20 +104,17 @@ public:
 
 private:
   LookupResult(const LookupResult&) = delete;
   LookupResult& operator=(const LookupResult& aOther) = delete;
 
   DrawableSurface mSurface;
   MatchType mMatchType;
 
-  /// mSuggestedSize will be the size of the returned surface if the result is
-  /// SUBSTITUTE_BECAUSE_BEST. It will be empty for EXACT, and can contain a
-  /// non-empty size possibly different from the returned surface (if any) for
-  /// all other results. If non-empty, it will always be the size the caller
-  /// should request any decodes at.
+  /// If given, the size the caller should request a decode at. This may or may
+  /// not match the size the caller requested from the cache.
   gfx::IntSize mSuggestedSize;
 };
 
 } // namespace image
 } // namespace mozilla
 
 #endif // mozilla_image_LookupResult_h
--- a/image/SVGDrawingParameters.h
+++ b/image/SVGDrawingParameters.h
@@ -19,46 +19,43 @@ namespace mozilla {
 namespace image {
 
 struct SVGDrawingParameters
 {
   typedef mozilla::gfx::IntSize IntSize;
   typedef mozilla::gfx::SamplingFilter SamplingFilter;
 
   SVGDrawingParameters(gfxContext* aContext,
-                       const nsIntSize& aRasterSize,
-                       const nsIntSize& aDrawSize,
+                       const nsIntSize& aSize,
                        const ImageRegion& aRegion,
                        SamplingFilter aSamplingFilter,
                        const Maybe<SVGImageContext>& aSVGContext,
                        float aAnimationTime,
                        uint32_t aFlags,
                        float aOpacity)
     : context(aContext)
-    , size(aRasterSize)
-    , drawSize(aDrawSize)
+    , size(aSize.width, aSize.height)
     , region(aRegion)
     , samplingFilter(aSamplingFilter)
     , svgContext(aSVGContext)
-    , viewportSize(aRasterSize)
+    , viewportSize(aSize)
     , animationTime(aAnimationTime)
     , flags(aFlags)
     , opacity(aOpacity)
   {
     if (aSVGContext) {
       auto sz = aSVGContext->GetViewportSize();
       if (sz) {
         viewportSize = nsIntSize(sz->width, sz->height); // XXX losing unit
       }
     }
   }
 
   gfxContext*                   context;
-  IntSize                       size; // Size to rasterize a surface at.
-  IntSize                       drawSize; // Size to draw the given surface at.
+  IntSize                       size;
   ImageRegion                   region;
   SamplingFilter                samplingFilter;
   const Maybe<SVGImageContext>& svgContext;
   nsIntSize                     viewportSize;
   float                         animationTime;
   uint32_t                      flags;
   gfxFloat                      opacity;
 };
--- a/image/SurfaceCache.cpp
+++ b/image/SurfaceCache.cpp
@@ -7,17 +7,16 @@
  * SurfaceCache is a service for caching temporary surfaces in imagelib.
  */
 
 #include "SurfaceCache.h"
 
 #include <algorithm>
 #include "mozilla/Assertions.h"
 #include "mozilla/Attributes.h"
-#include "mozilla/CheckedInt.h"
 #include "mozilla/DebugOnly.h"
 #include "mozilla/Likely.h"
 #include "mozilla/Move.h"
 #include "mozilla/Pair.h"
 #include "mozilla/RefPtr.h"
 #include "mozilla/StaticMutex.h"
 #include "mozilla/StaticPtr.h"
 #include "mozilla/Tuple.h"
@@ -247,21 +246,20 @@ AreaOfIntSize(const IntSize& aSize) {
  * ideal size as the "best" available (as opposed to subsitution but not found).
  * This allows us to minimize memory consumption and CPU time spent decoding
  * when a website requires many variants of the same surface.
  */
 class ImageSurfaceCache
 {
   ~ImageSurfaceCache() { }
 public:
-  explicit ImageSurfaceCache(const ImageKey aImageKey)
+  ImageSurfaceCache()
     : mLocked(false)
     , mFactor2Mode(false)
     , mFactor2Pruned(false)
-    , mIsVectorImage(aImageKey->GetType() == imgIContainer::TYPE_VECTOR)
   { }
 
   MOZ_DECLARE_REFCOUNTED_TYPENAME(ImageSurfaceCache)
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(ImageSurfaceCache)
 
   typedef
     nsRefPtrHashtable<nsGenericHashKey<SurfaceKey>, CachedSurface> SurfaceTable;
 
@@ -327,17 +325,17 @@ public:
       // If no exact match is found, and we are not in factor of 2 mode, then
       // we know that we will trigger a decode because at best we will provide
       // a substitute. Make sure we switch now to factor of 2 mode if necessary.
       MaybeSetFactor2Mode();
     }
 
     // Try for a best match second, if using compact.
     IntSize suggestedSize = SuggestedSize(aIdealKey.Size());
-    if (suggestedSize != aIdealKey.Size()) {
+    if (mFactor2Mode) {
       if (!exactMatch) {
         SurfaceKey compactKey = aIdealKey.CloneWithSize(suggestedSize);
         mSurfaces.Get(compactKey, getter_AddRefs(exactMatch));
         if (exactMatch && exactMatch->IsDecoded()) {
           MOZ_ASSERT(suggestedSize != aIdealKey.Size());
           return MakeTuple(exactMatch.forget(),
                            MatchType::SUBSTITUTE_BECAUSE_BEST,
                            suggestedSize);
@@ -398,17 +396,17 @@ public:
                    "No exact match despite the fact the sizes match!");
         matchType = MatchType::SUBSTITUTE_BECAUSE_NOT_FOUND;
       } else if (exactMatch != bestMatch) {
         // The exact match is still decoding, but we found a substitute.
         matchType = MatchType::SUBSTITUTE_BECAUSE_PENDING;
       } else if (aIdealKey.Size() != bestMatch->GetSurfaceKey().Size()) {
         // The best factor of 2 match is still decoding, but the best we've got.
         MOZ_ASSERT(suggestedSize != aIdealKey.Size());
-        MOZ_ASSERT(mFactor2Mode || mIsVectorImage);
+        MOZ_ASSERT(mFactor2Mode);
         matchType = MatchType::SUBSTITUTE_BECAUSE_BEST;
       } else {
         // The exact match is still decoding, but it's the best we've got.
         matchType = MatchType::EXACT;
       }
     } else {
       if (exactMatch) {
         // We found an "exact match"; it must have been a placeholder.
@@ -430,28 +428,30 @@ public:
     // Typically an image cache will not have too many size-varying surfaces, so
     // if we exceed the given threshold, we should consider using a subset.
     int32_t thresholdSurfaces = gfxPrefs::ImageCacheFactor2ThresholdSurfaces();
     if (thresholdSurfaces < 0 ||
         mSurfaces.Count() <= static_cast<uint32_t>(thresholdSurfaces)) {
       return;
     }
 
-    // Determine how many native surfaces this image has. If it is zero, and it
-    // is a vector image, then we should impute a single native size. Otherwise,
-    // it may be zero because we don't know yet, or the image has an error, or
-    // it isn't supported.
+    // Determine how many native surfaces this image has. Zero means we either
+    // don't know yet (in which case do nothing), or we don't want to limit the
+    // number of surfaces for this image.
+    //
+    // XXX(aosmond): Vector images have zero native sizes. This is because they
+    // are regenerated at the given size. There isn't an equivalent concept to
+    // the native size (and w/h ratio) to provide a frame of reference to what
+    // are "good" sizes. While it is desirable to have a similar mechanism as
+    // that for raster images, it will need a different approach.
     auto first = ConstIter();
     NotNull<CachedSurface*> current = WrapNotNull(first.UserData());
     Image* image = static_cast<Image*>(current->GetImageKey());
     size_t nativeSizes = image->GetNativeSizesLength();
-    if (mIsVectorImage) {
-      MOZ_ASSERT(nativeSizes == 0);
-      nativeSizes = 1;
-    } else if (nativeSizes == 0) {
+    if (nativeSizes == 0) {
       return;
     }
 
     // Increase the threshold by the number of native sizes. This ensures that
     // we do not prevent decoding of the image at all its native sizes. It does
     // not guarantee we will provide a surface at that size however (i.e. many
     // other sized surfaces are requested, in addition to the native sizes).
     thresholdSurfaces += nativeSizes;
@@ -526,43 +526,16 @@ public:
     // We should never leave factor of 2 mode due to pruning in of itself, but
     // if we discarded surfaces due to the volatile buffers getting released,
     // it is possible.
     AfterMaybeRemove();
   }
 
   IntSize SuggestedSize(const IntSize& aSize) const
   {
-    IntSize suggestedSize = SuggestedSizeInternal(aSize);
-    MOZ_ASSERT(SurfaceCache::IsLegalSize(suggestedSize));
-
-    // Whether or not we are in factor of 2 mode, vector image rasterization is
-    // clamped at a configured maximum if the caller is willing to accept
-    // substitutes.
-    if (mIsVectorImage) {
-      // If we exceed the maximum, we need to scale the size downwards to fit.
-      // It shouldn't get here if it is significantly larger because
-      // VectorImage::UseSurfaceCacheForSize should prevent us from requesting
-      // a rasterized version of a surface greater than 4x the maximum.
-      int32_t maxSizeKB = gfxPrefs::ImageCacheMaxRasterizedSVGThresholdKB();
-      int32_t proposedKB = suggestedSize.width * suggestedSize.height / 256;
-      if (maxSizeKB >= proposedKB) {
-        return suggestedSize;
-      }
-
-      double scale = sqrt(double(maxSizeKB) / proposedKB);
-      suggestedSize.width = int32_t(scale * suggestedSize.width);
-      suggestedSize.height = int32_t(scale * suggestedSize.height);
-    }
-
-    return suggestedSize;
-  }
-
-  IntSize SuggestedSizeInternal(const IntSize& aSize) const
-  {
     // When not in factor of 2 mode, we can always decode at the given size.
     if (!mFactor2Mode) {
       return aSize;
     }
 
     // We cannot enter factor of 2 mode unless we have a minimum number of
     // surfaces, and we should have left it if the cache was emptied.
     if (MOZ_UNLIKELY(IsEmpty())) {
@@ -574,58 +547,21 @@ public:
     auto iter = ConstIter();
     NotNull<CachedSurface*> firstSurface = WrapNotNull(iter.UserData());
     Image* image = static_cast<Image*>(firstSurface->GetImageKey());
     IntSize factorSize;
     if (NS_FAILED(image->GetWidth(&factorSize.width)) ||
         NS_FAILED(image->GetHeight(&factorSize.height)) ||
         factorSize.IsEmpty()) {
       // We should not have entered factor of 2 mode without a valid size, and
-      // several successfully decoded surfaces. Note that valid vector images
-      // may have a default size of 0x0, and those are not yet supported.
+      // several successfully decoded surfaces.
       MOZ_ASSERT_UNREACHABLE("Expected valid native size!");
       return aSize;
     }
 
-    if (mIsVectorImage) {
-      // Ensure the aspect ratio matches the native size before forcing the
-      // caller to accept a factor of 2 size. The difference between the aspect
-      // ratios is:
-      //
-      //     delta = nativeWidth/nativeHeight - desiredWidth/desiredHeight
-      //
-      //     delta*nativeHeight*desiredHeight = nativeWidth*desiredHeight
-      //                                      - desiredWidth*nativeHeight
-      //
-      // Using the maximum accepted delta as a constant, we can avoid the
-      // floating point division and just compare after some integer ops.
-      int32_t delta = factorSize.width * aSize.height - aSize.width * factorSize.height;
-      int32_t maxDelta = (factorSize.height * aSize.height) >> 4;
-      if (delta > maxDelta || delta < -maxDelta) {
-        return aSize;
-      }
-
-      // If the requested size is bigger than the native size, we actually need
-      // to grow the native size instead of shrinking it.
-      if (factorSize.width < aSize.width) {
-        do {
-          IntSize candidate(factorSize.width * 2, factorSize.height * 2);
-          if (!SurfaceCache::IsLegalSize(candidate)) {
-            break;
-          }
-
-          factorSize = candidate;
-        } while (factorSize.width < aSize.width);
-
-        return factorSize;
-      }
-
-      // Otherwise we can find the best fit as normal.
-    }
-
     // Start with the native size as the best first guess.
     IntSize bestSize = factorSize;
     factorSize.width /= 2;
     factorSize.height /= 2;
 
     while (!factorSize.IsEmpty()) {
       if (!CompareArea(aSize, bestSize, factorSize)) {
         // This size is not better than the last. Since we proceed from largest
@@ -733,20 +669,16 @@ private:
   bool              mLocked;
 
   // True in "factor of 2" mode.
   bool              mFactor2Mode;
 
   // True if all non-factor of 2 surfaces have been removed from the cache. Note
   // that this excludes unsubstitutable sizes.
   bool              mFactor2Pruned;
-
-  // True if the surfaces are produced from a vector image. If so, it must match
-  // the aspect ratio when using factor of 2 mode.
-  bool              mIsVectorImage;
 };
 
 /**
  * SurfaceCacheImpl is responsible for determining which surfaces will be cached
  * and managing the surface cache data structures. Rather than interact with
  * SurfaceCacheImpl directly, client code interacts with SurfaceCache, which
  * maintains high-level invariants and encapsulates the details of the surface
  * cache's implementation.
@@ -823,20 +755,19 @@ public:
     while (cost > mAvailableCost) {
       MOZ_ASSERT(!mCosts.IsEmpty(),
                  "Removed everything and it still won't fit");
       Remove(mCosts.LastElement().Surface(), /* aStopTracking */ true, aAutoLock);
     }
 
     // Locate the appropriate per-image cache. If there's not an existing cache
     // for this image, create it.
-    const ImageKey imageKey = aProvider->GetImageKey();
-    RefPtr<ImageSurfaceCache> cache = GetImageCache(imageKey);
+    RefPtr<ImageSurfaceCache> cache = GetImageCache(aProvider->GetImageKey());
     if (!cache) {
-      cache = new ImageSurfaceCache(imageKey);
+      cache = new ImageSurfaceCache;
       mImageCaches.Put(aProvider->GetImageKey(), cache);
     }
 
     // If we were asked to mark the cache entry available, do so.
     if (aSetAvailable) {
       aProvider->Availability().SetAvailable();
     }
 
@@ -1078,17 +1009,17 @@ public:
     // to just update our data structures without reinserting.
     Insert(aProvider, /* aSetAvailable = */ true, aAutoLock);
   }
 
   void LockImage(const ImageKey aImageKey)
   {
     RefPtr<ImageSurfaceCache> cache = GetImageCache(aImageKey);
     if (!cache) {
-      cache = new ImageSurfaceCache(aImageKey);
+      cache = new ImageSurfaceCache;
       mImageCaches.Put(aImageKey, cache);
     }
 
     cache->SetLocked(true);
 
     // We don't relock this image's existing surfaces right away; instead, the
     // image should arrange for Lookup() to touch them if they are still useful.
   }
@@ -1694,35 +1625,10 @@ SurfaceCache::MaximumCapacity()
   StaticMutexAutoLock lock(sInstanceMutex);
   if (!sInstance) {
     return 0;
   }
 
   return sInstance->MaximumCapacity();
 }
 
-/* static */ bool
-SurfaceCache::IsLegalSize(const IntSize& aSize)
-{
-  // reject over-wide or over-tall images
-  const int32_t k64KLimit = 0x0000FFFF;
-  if (MOZ_UNLIKELY(aSize.width > k64KLimit || aSize.height > k64KLimit )) {
-    NS_WARNING("image too big");
-    return false;
-  }
-
-  // protect against invalid sizes
-  if (MOZ_UNLIKELY(aSize.height <= 0 || aSize.width <= 0)) {
-    return false;
-  }
-
-  // check to make sure we don't overflow a 32-bit
-  CheckedInt32 requiredBytes = CheckedInt32(aSize.width) *
-                               CheckedInt32(aSize.height) * 4;
-  if (MOZ_UNLIKELY(!requiredBytes.isValid())) {
-    NS_WARNING("width or height too large");
-    return false;
-  }
-  return true;
-}
-
 } // namespace image
 } // namespace mozilla
--- a/image/SurfaceCache.h
+++ b/image/SurfaceCache.h
@@ -438,21 +438,16 @@ struct SurfaceCache
                                     MallocSizeOf      aMallocSizeOf);
 
   /**
    * @return maximum capacity of the SurfaceCache in bytes. This is only exposed
    * for use by tests; normal code should use CanHold() instead.
    */
   static size_t MaximumCapacity();
 
-  /**
-   * @return true if the given size is valid.
-   */
-  static bool IsLegalSize(const IntSize& aSize);
-
 private:
   virtual ~SurfaceCache() = 0;  // Forbid instantiation.
 };
 
 } // namespace image
 } // namespace mozilla
 
 #endif // mozilla_image_SurfaceCache_h
--- a/image/VectorImage.cpp
+++ b/image/VectorImage.cpp
@@ -802,62 +802,55 @@ VectorImage::GetFrameInternal(const IntS
                      RefPtr<SourceSurface>());
   }
 
   if (!mIsFullyLoaded) {
     return MakeTuple(ImgDrawResult::NOT_READY, aSize,
                      RefPtr<SourceSurface>());
   }
 
-  // We don't allow large surfaces to be rasterized on the Draw and
-  // GetImageContainerAtSize paths, because those have alternatives. If we get
-  // here however, then we know it came from GetFrame(AtSize) and that path does
-  // not have any fallback method, so we don't check UseSurfaceCacheForSize.
-  RefPtr<SourceSurface> sourceSurface;
-  IntSize decodeSize;
-  Tie(sourceSurface, decodeSize) =
+  RefPtr<SourceSurface> sourceSurface =
     LookupCachedSurface(aSize, aSVGContext, aFlags);
   if (sourceSurface) {
-    return MakeTuple(ImgDrawResult::SUCCESS, decodeSize, std::move(sourceSurface));
+    return MakeTuple(ImgDrawResult::SUCCESS, aSize, std::move(sourceSurface));
   }
 
   if (mIsDrawing) {
     NS_WARNING("Refusing to make re-entrant call to VectorImage::Draw");
-    return MakeTuple(ImgDrawResult::TEMPORARY_ERROR, decodeSize,
+    return MakeTuple(ImgDrawResult::TEMPORARY_ERROR, aSize,
                      RefPtr<SourceSurface>());
   }
 
   // By using a null gfxContext, we ensure that we will always attempt to
   // create a surface, even if we aren't capable of caching it (e.g. due to our
   // flags, having an animation, etc). Otherwise CreateSurface will assume that
   // the caller is capable of drawing directly to its own draw target if we
   // cannot cache.
-  SVGDrawingParameters params(nullptr, decodeSize, aSize,
-                              ImageRegion::Create(decodeSize),
+  SVGDrawingParameters params(nullptr, aSize, ImageRegion::Create(aSize),
                               SamplingFilter::POINT, aSVGContext,
                               mSVGDocumentWrapper->GetCurrentTime(),
                               aFlags, 1.0);
 
   bool didCache; // Was the surface put into the cache?
   bool contextPaint = aSVGContext && aSVGContext->GetContextPaint();
 
   AutoRestoreSVGState autoRestore(params, mSVGDocumentWrapper,
                                   mIsDrawing, contextPaint);
 
   RefPtr<gfxDrawable> svgDrawable = CreateSVGDrawable(params);
   RefPtr<SourceSurface> surface =
     CreateSurface(params, svgDrawable, didCache);
   if (!surface) {
     MOZ_ASSERT(!didCache);
-    return MakeTuple(ImgDrawResult::TEMPORARY_ERROR, decodeSize,
+    return MakeTuple(ImgDrawResult::TEMPORARY_ERROR, aSize,
                      RefPtr<SourceSurface>());
   }
 
   SendFrameComplete(didCache, params.flags);
-  return MakeTuple(ImgDrawResult::SUCCESS, decodeSize, std::move(surface));
+  return MakeTuple(ImgDrawResult::SUCCESS, aSize, std::move(surface));
 }
 
 //******************************************************************************
 Tuple<ImgDrawResult, IntSize>
 VectorImage::GetImageContainerSize(LayerManager* aManager,
                                    const IntSize& aSize,
                                    uint32_t aFlags)
 {
@@ -906,39 +899,36 @@ VectorImage::GetImageContainer(LayerMana
 //******************************************************************************
 NS_IMETHODIMP_(bool)
 VectorImage::IsImageContainerAvailableAtSize(LayerManager* aManager,
                                              const IntSize& aSize,
                                              uint32_t aFlags)
 {
   // Since we only support image containers with WebRender, and it can handle
   // textures larger than the hw max texture size, we don't need to check aSize.
-  return !aSize.IsEmpty() &&
-         UseSurfaceCacheForSize(aSize) &&
-         IsImageContainerAvailable(aManager, aFlags);
+  return !aSize.IsEmpty() && IsImageContainerAvailable(aManager, aFlags);
 }
 
 //******************************************************************************
 NS_IMETHODIMP_(ImgDrawResult)
 VectorImage::GetImageContainerAtSize(layers::LayerManager* aManager,
                                      const gfx::IntSize& aSize,
                                      const Maybe<SVGImageContext>& aSVGContext,
                                      uint32_t aFlags,
                                      layers::ImageContainer** aOutContainer)
 {
-  if (!UseSurfaceCacheForSize(aSize)) {
-    return ImgDrawResult::NOT_SUPPORTED;
-  }
-
   Maybe<SVGImageContext> newSVGContext;
   MaybeRestrictSVGContext(newSVGContext, aSVGContext, aFlags);
 
-  // The aspect ratio flag was already handled as part of the SVG context
-  // restriction above.
-  uint32_t flags = aFlags & ~(FLAG_FORCE_PRESERVEASPECTRATIO_NONE);
+  // Since we do not support high quality scaling with SVG, we mask it off so
+  // that container requests with and without it map to the same container.
+  // Similarly the aspect ratio flag was already handled as part of the SVG
+  // context restriction above.
+  uint32_t flags = aFlags & ~(FLAG_HIGH_QUALITY_SCALING |
+                              FLAG_FORCE_PRESERVEASPECTRATIO_NONE);
   return GetImageContainerImpl(aManager, aSize,
                                newSVGContext ? newSVGContext : aSVGContext,
                                flags, aOutContainer);
 }
 
 bool
 VectorImage::MaybeRestrictSVGContext(Maybe<SVGImageContext>& aNewSVGContext,
                                      const Maybe<SVGImageContext>& aSVGContext,
@@ -1007,51 +997,47 @@ VectorImage::Draw(gfxContext* aContext,
   if (!mIsFullyLoaded) {
     return ImgDrawResult::NOT_READY;
   }
 
   if (mAnimationConsumers == 0) {
     SendOnUnlockedDraw(aFlags);
   }
 
-  // We should bypass the cache when:
-  // - We are using a DrawTargetRecording because we prefer the drawing commands
-  //   in general to the rasterized surface. This allows blob images to avoid
-  //   rasterized SVGs with WebRender.
-  // - The size exceeds what we are will to cache as a rasterized surface.
-  if (aContext->GetDrawTarget()->GetBackendType() == BackendType::RECORDING ||
-      !UseSurfaceCacheForSize(aSize)) {
+  // We should always bypass the cache when using DrawTargetRecording because
+  // we prefer the drawing commands in general to the rasterized surface. This
+  // allows blob images to avoid rasterized SVGs with WebRender.
+  if (aContext->GetDrawTarget()->GetBackendType() == BackendType::RECORDING) {
     aFlags |= FLAG_BYPASS_SURFACE_CACHE;
   }
 
   MOZ_ASSERT(!(aFlags & FLAG_FORCE_PRESERVEASPECTRATIO_NONE) ||
              (aSVGContext && aSVGContext->GetViewportSize()),
              "Viewport size is required when using "
              "FLAG_FORCE_PRESERVEASPECTRATIO_NONE");
 
   float animTime = (aWhichFrame == FRAME_FIRST)
                      ? 0.0f : mSVGDocumentWrapper->GetCurrentTime();
 
   Maybe<SVGImageContext> newSVGContext;
   bool contextPaint =
     MaybeRestrictSVGContext(newSVGContext, aSVGContext, aFlags);
 
-  SVGDrawingParameters params(aContext, aSize, aSize, aRegion, aSamplingFilter,
+  SVGDrawingParameters params(aContext, aSize, aRegion, aSamplingFilter,
                               newSVGContext ? newSVGContext : aSVGContext,
                               animTime, aFlags, aOpacity);
 
   // If we have an prerasterized version of this image that matches the
   // drawing parameters, use that.
-  RefPtr<SourceSurface> sourceSurface;
-  Tie(sourceSurface, params.size) =
+  RefPtr<SourceSurface> sourceSurface =
     LookupCachedSurface(aSize, params.svgContext, aFlags);
   if (sourceSurface) {
-    RefPtr<gfxDrawable> drawable =
-      new gfxSurfaceDrawable(sourceSurface, params.size);
-    Show(drawable, params);
+    RefPtr<gfxDrawable> svgDrawable =
+      new gfxSurfaceDrawable(sourceSurface, sourceSurface->GetSize());
+    Show(svgDrawable, params);
     return ImgDrawResult::SUCCESS;
   }
 
   // else, we need to paint the image:
 
   if (mIsDrawing) {
     NS_WARNING("Refusing to make re-entrant call to VectorImage::Draw");
     return ImgDrawResult::TEMPORARY_ERROR;
@@ -1085,77 +1071,50 @@ VectorImage::CreateSVGDrawable(const SVG
                            aParams.size,
                            aParams.flags);
 
   RefPtr<gfxDrawable> svgDrawable =
     new gfxCallbackDrawable(cb, aParams.size);
   return svgDrawable.forget();
 }
 
-bool
-VectorImage::UseSurfaceCacheForSize(const IntSize& aSize) const
-{
-  int32_t maxSizeKB = gfxPrefs::ImageCacheMaxRasterizedSVGThresholdKB();
-  if (maxSizeKB <= 0) {
-    return true;
-  }
-
-  if (!SurfaceCache::IsLegalSize(aSize)) {
-    return false;
-  }
-
-  // With factor of 2 mode, we should be willing to use a surface which is up
-  // to twice the width, and twice the height, of the maximum sized surface
-  // before switching to drawing to the target directly. That means the size in
-  // KB works out to be:
-  //   width * height * 4 [bytes/pixel] * / 1024 [bytes/KB] <= 2 * 2 * maxSizeKB
-  return aSize.width * aSize.height / 1024 <= maxSizeKB;
-}
-
-Tuple<RefPtr<SourceSurface>, IntSize>
+already_AddRefed<SourceSurface>
 VectorImage::LookupCachedSurface(const IntSize& aSize,
                                  const Maybe<SVGImageContext>& aSVGContext,
                                  uint32_t aFlags)
 {
   // If we're not allowed to use a cached surface, don't attempt a lookup.
   if (aFlags & FLAG_BYPASS_SURFACE_CACHE) {
-    return MakeTuple(RefPtr<SourceSurface>(), aSize);
+    return nullptr;
   }
 
   // We don't do any caching if we have animation, so don't bother with a lookup
   // in this case either.
   if (mHaveAnimations) {
-    return MakeTuple(RefPtr<SourceSurface>(), aSize);
+    return nullptr;
   }
 
-  LookupResult result(MatchType::NOT_FOUND);
-  SurfaceKey surfaceKey = VectorSurfaceKey(aSize, aSVGContext);
-  if ((aFlags & FLAG_SYNC_DECODE) || !(aFlags & FLAG_HIGH_QUALITY_SCALING)) {
-    result = SurfaceCache::Lookup(ImageKey(this), surfaceKey);
-  } else {
-    result = SurfaceCache::LookupBestMatch(ImageKey(this), surfaceKey);
-  }
+  LookupResult result =
+    SurfaceCache::Lookup(ImageKey(this),
+                         VectorSurfaceKey(aSize, aSVGContext));
 
-  IntSize rasterSize = result.SuggestedSize().IsEmpty()
-                       ? aSize : result.SuggestedSize();
-  MOZ_ASSERT(result.Type() != MatchType::SUBSTITUTE_BECAUSE_PENDING);
-  if (!result || result.Type() == MatchType::SUBSTITUTE_BECAUSE_NOT_FOUND) {
-    // No matching surface, or the OS freed the volatile buffer.
-    return MakeTuple(RefPtr<SourceSurface>(), rasterSize);
+  MOZ_ASSERT(result.SuggestedSize().IsEmpty(), "SVG should not substitute!");
+  if (!result) {
+    return nullptr;  // No matching surface, or the OS freed the volatile buffer.
   }
 
   RefPtr<SourceSurface> sourceSurface = result.Surface()->GetSourceSurface();
   if (!sourceSurface) {
     // Something went wrong. (Probably a GPU driver crash or device reset.)
     // Attempt to recover.
     RecoverFromLossOfSurfaces();
-    return MakeTuple(RefPtr<SourceSurface>(), rasterSize);
+    return nullptr;
   }
 
-  return MakeTuple(std::move(sourceSurface), rasterSize);
+  return sourceSurface.forget();
 }
 
 already_AddRefed<SourceSurface>
 VectorImage::CreateSurface(const SVGDrawingParameters& aParams,
                            gfxDrawable* aSVGDrawable,
                            bool& aWillCache)
 {
   MOZ_ASSERT(mIsDrawing);
@@ -1225,25 +1184,17 @@ VectorImage::CreateSurface(const SVGDraw
   if (!aWillCache) {
     return surface.forget();
   }
 
   // Attempt to cache the frame.
   SurfaceKey surfaceKey = VectorSurfaceKey(aParams.size, aParams.svgContext);
   NotNull<RefPtr<ISurfaceProvider>> provider =
     MakeNotNull<SimpleSurfaceProvider*>(ImageKey(this), surfaceKey, frame);
-
-  if (SurfaceCache::Insert(provider) == InsertOutcome::SUCCESS &&
-      aParams.size != aParams.drawSize) {
-    // We created a new surface that wasn't the size we requested, which means
-    // we entered factor-of-2 mode. We should purge any surfaces we no longer
-    // need rather than waiting for the cache to expire them.
-    SurfaceCache::PruneImage(ImageKey(this));
-  }
-
+  SurfaceCache::Insert(provider);
   return surface.forget();
 }
 
 void
 VectorImage::SendFrameComplete(bool aDidCache, uint32_t aFlags)
 {
   // If the cache was not updated, we have nothing to do.
   if (!aDidCache) {
@@ -1268,31 +1219,20 @@ VectorImage::SendFrameComplete(bool aDid
     }));
   }
 }
 
 
 void
 VectorImage::Show(gfxDrawable* aDrawable, const SVGDrawingParameters& aParams)
 {
-  // The surface size may differ from the size at which we wish to draw. As
-  // such, we may need to adjust the context/region to take this into account.
-  gfxContextMatrixAutoSaveRestore saveMatrix(aParams.context);
-  ImageRegion region(aParams.region);
-  if (aParams.drawSize != aParams.size) {
-    gfx::Size scale(double(aParams.drawSize.width) / aParams.size.width,
-                    double(aParams.drawSize.height) / aParams.size.height);
-    aParams.context->Multiply(gfxMatrix::Scaling(scale.width, scale.height));
-    region.Scale(1.0 / scale.width, 1.0 / scale.height);
-  }
-
   MOZ_ASSERT(aDrawable, "Should have a gfxDrawable by now");
   gfxUtils::DrawPixelSnapped(aParams.context, aDrawable,
                              SizeDouble(aParams.size),
-                             region,
+                             aParams.region,
                              SurfaceFormat::B8G8R8A8,
                              aParams.samplingFilter,
                              aParams.flags, aParams.opacity, false);
 
 #ifdef DEBUG
   NotifyDrawingObservers();
 #endif
 
--- a/image/VectorImage.h
+++ b/image/VectorImage.h
@@ -87,37 +87,30 @@ private:
                      uint32_t aWhichFrame,
                      uint32_t aFlags) override;
 
   Tuple<ImgDrawResult, IntSize>
     GetImageContainerSize(layers::LayerManager* aManager,
                           const IntSize& aSize,
                           uint32_t aFlags) override;
 
-  /**
-   * Attempt to find a matching cached surface in the SurfaceCache. Returns the
-   * cached surface, if found, and the size to rasterize at, if applicable.
-   * If we cannot rasterize, it will be the requested size to draw at (aSize).
-   */
-  Tuple<RefPtr<SourceSurface>, IntSize>
+  /// Attempt to find a matching cached surface in the SurfaceCache.
+  already_AddRefed<SourceSurface>
     LookupCachedSurface(const IntSize& aSize,
                         const Maybe<SVGImageContext>& aSVGContext,
                         uint32_t aFlags);
 
   bool MaybeRestrictSVGContext(Maybe<SVGImageContext>& aNewSVGContext,
                                const Maybe<SVGImageContext>& aSVGContext,
                                uint32_t aFlags);
 
   /// Create a gfxDrawable which callbacks into the SVG document.
   already_AddRefed<gfxDrawable>
     CreateSVGDrawable(const SVGDrawingParameters& aParams);
 
-  /// Returns true if we use the surface cache to store rasterized copies.
-  bool UseSurfaceCacheForSize(const IntSize& aSize) const;
-
   /// Rasterize the SVG into a surface. aWillCache will be set to whether or
   /// not the new surface was put into the cache.
   already_AddRefed<SourceSurface>
     CreateSurface(const SVGDrawingParameters& aParams,
                   gfxDrawable* aSVGDrawable,
                   bool& aWillCache);
 
   /// Send a frame complete notification if appropriate. Must be called only
--- a/image/imgFrame.cpp
+++ b/image/imgFrame.cpp
@@ -2,17 +2,16 @@
 /* vim: set ts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "imgFrame.h"
 #include "ImageRegion.h"
 #include "ShutdownTracker.h"
-#include "SurfaceCache.h"
 
 #include "prenv.h"
 
 #include "gfx2DGlue.h"
 #include "gfxPlatform.h"
 #include "gfxPrefs.h"
 #include "gfxUtils.h"
 
@@ -153,23 +152,48 @@ ClearSurface(DataSourceSurface* aSurface
     // Otherwise, it's allocated via mmap and refers to a zeroed page and will
     // be COW once it's written to.
     memset(data, 0, stride * aSize.height);
   }
 
   return true;
 }
 
+// Returns true if an image of aWidth x aHeight is allowed and legal.
+static bool
+AllowedImageSize(int32_t aWidth, int32_t aHeight)
+{
+  // reject over-wide or over-tall images
+  const int32_t k64KLimit = 0x0000FFFF;
+  if (MOZ_UNLIKELY(aWidth > k64KLimit || aHeight > k64KLimit )) {
+    NS_WARNING("image too big");
+    return false;
+  }
+
+  // protect against invalid sizes
+  if (MOZ_UNLIKELY(aHeight <= 0 || aWidth <= 0)) {
+    return false;
+  }
+
+  // check to make sure we don't overflow a 32-bit
+  CheckedInt32 requiredBytes = CheckedInt32(aWidth) * CheckedInt32(aHeight) * 4;
+  if (MOZ_UNLIKELY(!requiredBytes.isValid())) {
+    NS_WARNING("width or height too large");
+    return false;
+  }
+  return true;
+}
+
 static bool AllowedImageAndFrameDimensions(const nsIntSize& aImageSize,
                                            const nsIntRect& aFrameRect)
 {
-  if (!SurfaceCache::IsLegalSize(aImageSize)) {
+  if (!AllowedImageSize(aImageSize.width, aImageSize.height)) {
     return false;
   }
-  if (!SurfaceCache::IsLegalSize(aFrameRect.Size())) {
+  if (!AllowedImageSize(aFrameRect.Width(), aFrameRect.Height())) {
     return false;
   }
   nsIntRect imageRect(0, 0, aImageSize.width, aImageSize.height);
   if (!imageRect.Contains(aFrameRect)) {
     NS_WARNING("Animated image frame does not fit inside bounds of image");
   }
   return true;
 }
@@ -308,17 +332,17 @@ imgFrame::InitWithDrawable(gfxDrawable* 
                            const nsIntSize& aSize,
                            const SurfaceFormat aFormat,
                            SamplingFilter aSamplingFilter,
                            uint32_t aImageFlags,
                            gfx::BackendType aBackend)
 {
   // Assert for properties that should be verified by decoders,
   // warn for properties related to bad content.
-  if (!SurfaceCache::IsLegalSize(aSize)) {
+  if (!AllowedImageSize(aSize.width, aSize.height)) {
     NS_WARNING("Should have legal image size");
     mAborted = true;
     return NS_ERROR_FAILURE;
   }
 
   mImageSize = aSize;
   mFrameRect = IntRect(IntPoint(0, 0), aSize);
 
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -4615,20 +4615,16 @@ pref("image.animated.resume-from-last-di
 
 // Maximum number of surfaces for an image before entering "factor of 2" mode.
 // This in addition to the number of "native" sizes of an image. A native size
 // is a size for which we can decode a frame without up or downscaling. Most
 // images only have 1, but some (i.e. ICOs) may have multiple frames for the
 // same data at different sizes.
 pref("image.cache.factor2.threshold-surfaces", 4);
 
-// Maximum number of pixels in either dimension that we are willing to upscale
-// an SVG to when we are in "factor of 2" mode.
-pref("image.cache.max-rasterized-svg-threshold-kb", 92160);
-
 // The maximum size, in bytes, of the decoded images we cache
 pref("image.cache.size", 5242880);
 
 // A weight, from 0-1000, to place on time when comparing to size.
 // Size is given a weight of 1000 - timeweight.
 pref("image.cache.timeweight", 500);
 
 // Decode all images automatically on load, ignoring our normal heuristics.