Bug 1267260 - Change CanvasImageCache to lookup cache images based on imgIContainer instead of imgIRequest. r=seth

--HG--
extra : rebase_source : 872ee783a05d0319ed69ce3c494dbf2a3206e26d

dom/canvas/CanvasImageCache.cpp

@@ -20,60 +20,65 @@ namespace mozilla {
 using namespace dom;
 using namespace gfx;
 
-struct ImageCacheKey {
+/**
-  ImageCacheKey(Element* aImage,
+ * Used for images specific to this one canvas. Required
+ * due to CORS security.
+ */
+struct ImageCacheKey
+{
+  ImageCacheKey(imgIContainer* aImage,
                 HTMLCanvasElement* aCanvas,
                 bool aIsAccelerated)
     : mImage(aImage)
     , mCanvas(aCanvas)
     , mIsAccelerated(aIsAccelerated)
   {}
-  Element* mImage;
+  nsCOMPtr<imgIContainer> mImage;
   HTMLCanvasElement* mCanvas;
   bool mIsAccelerated;
 };
 
-struct ImageCacheEntryData {
+/**
+ * Cache data needs to be separate from the entry
+ * for nsExpirationTracker.
+ */
+struct ImageCacheEntryData
+{
   ImageCacheEntryData(const ImageCacheEntryData& aOther)
     : mImage(aOther.mImage)
-    , mILC(aOther.mILC)
     , mCanvas(aOther.mCanvas)
     , mIsAccelerated(aOther.mIsAccelerated)
-    , mRequest(aOther.mRequest)
     , mSourceSurface(aOther.mSourceSurface)
     , mSize(aOther.mSize)
   {}
   explicit ImageCacheEntryData(const ImageCacheKey& aKey)
     : mImage(aKey.mImage)
-    , mILC(nullptr)
     , mCanvas(aKey.mCanvas)
     , mIsAccelerated(aKey.mIsAccelerated)
   {}
 
   nsExpirationState* GetExpirationState() { return &mState; }
-
   size_t SizeInBytes() { return mSize.width * mSize.height * 4; }
 
   // Key
-  RefPtr<Element> mImage;
+  nsCOMPtr<imgIContainer> mImage;
-  nsIImageLoadingContent* mILC;
+  HTMLCanvasElement* mCanvas;
-  RefPtr<HTMLCanvasElement> mCanvas;
   bool mIsAccelerated;
   // Value
-  nsCOMPtr<imgIRequest> mRequest;
   RefPtr<SourceSurface> mSourceSurface;
   IntSize mSize;
   nsExpirationState mState;
 };
 
-class ImageCacheEntry : public PLDHashEntryHdr {
+class ImageCacheEntry : public PLDHashEntryHdr
+{
 public:
   typedef ImageCacheKey KeyType;
   typedef const ImageCacheKey* KeyTypePointer;
 
   explicit ImageCacheEntry(const KeyType* aKey) :
       mData(new ImageCacheEntryData(*aKey)) {}
-  ImageCacheEntry(const ImageCacheEntry &toCopy) :
+  ImageCacheEntry(const ImageCacheEntry& toCopy) :
       mData(new ImageCacheEntryData(*toCopy.mData)) {}
   ~ImageCacheEntry() {}
 
@@ -87,52 +92,61 @@ public:
   static KeyTypePointer KeyToPointer(KeyType& key) { return &key; }
   static PLDHashNumber HashKey(KeyTypePointer key)
   {
-    return HashGeneric(key->mImage, key->mCanvas, key->mIsAccelerated);
+    return HashGeneric(key->mImage.get(), key->mCanvas, key->mIsAccelerated);
   }
   enum { ALLOW_MEMMOVE = true };
 
   nsAutoPtr<ImageCacheEntryData> mData;
 };
 
-struct SimpleImageCacheKey {
+
-  SimpleImageCacheKey(const imgIRequest* aImage,
+/**
-                      bool aIsAccelerated)
+ * Used for all images across all canvases.
+ */
+struct AllCanvasImageCacheKey
+{
+  AllCanvasImageCacheKey(imgIContainer* aImage,
+                         bool aIsAccelerated)
     : mImage(aImage)
     , mIsAccelerated(aIsAccelerated)
   {}
-  const imgIRequest* mImage;
+
+  nsCOMPtr<imgIContainer> mImage;
   bool mIsAccelerated;
 };
 
-class SimpleImageCacheEntry : public PLDHashEntryHdr {
+class AllCanvasImageCacheEntry : public PLDHashEntryHdr {
 public:
-  typedef SimpleImageCacheKey KeyType;
+  typedef AllCanvasImageCacheKey KeyType;
-  typedef const SimpleImageCacheKey* KeyTypePointer;
+  typedef const AllCanvasImageCacheKey* KeyTypePointer;
 
-  explicit SimpleImageCacheEntry(KeyTypePointer aKey)
+  explicit AllCanvasImageCacheEntry(const KeyType* aKey)
-    : mRequest(const_cast<imgIRequest*>(aKey->mImage))
+    : mImage(aKey->mImage)
     , mIsAccelerated(aKey->mIsAccelerated)
   {}
-  SimpleImageCacheEntry(const SimpleImageCacheEntry &toCopy)
+
-    : mRequest(toCopy.mRequest)
+  AllCanvasImageCacheEntry(const AllCanvasImageCacheEntry &toCopy)
+    : mImage(toCopy.mImage)
     , mIsAccelerated(toCopy.mIsAccelerated)
     , mSourceSurface(toCopy.mSourceSurface)
   {}
-  ~SimpleImageCacheEntry() {}
+
+  ~AllCanvasImageCacheEntry() {}
 
   bool KeyEquals(KeyTypePointer key) const
   {
-    return key->mImage == mRequest && key->mIsAccelerated == mIsAccelerated;
+    return mImage == key->mImage &&
+           mIsAccelerated == key->mIsAccelerated;
   }
 
   static KeyTypePointer KeyToPointer(KeyType& key) { return &key; }
   static PLDHashNumber HashKey(KeyTypePointer key)
   {
-    return HashGeneric(key->mImage, key->mIsAccelerated);
+    return HashGeneric(key->mImage.get(), key->mIsAccelerated);
   }
   enum { ALLOW_MEMMOVE = true };
 
-  nsCOMPtr<imgIRequest> mRequest;
+  nsCOMPtr<imgIContainer> mImage;
   bool mIsAccelerated;
   RefPtr<SourceSurface> mSourceSurface;
 };
@@ -142,7 +156,8 @@ static int32_t sCanvasImageCacheLimit = 0;
 
 class ImageCacheObserver;
 
-class ImageCache final : public nsExpirationTracker<ImageCacheEntryData,4> {
+class ImageCache final : public nsExpirationTracker<ImageCacheEntryData,4>
+{
 public:
   // We use 3 generations of 1 second each to get a 2-3 seconds timeout.
   enum { GENERATION_MS = 1000 };
@@ -153,20 +168,24 @@ public:
   {
     mTotal -= aObject->SizeInBytes();
     RemoveObject(aObject);
-    // Deleting the entry will delete aObject since the entry owns aObject
+
-    mSimpleCache.RemoveEntry(SimpleImageCacheKey(aObject->mRequest, aObject->mIsAccelerated));
+    // Remove from the all canvas cache entry first since nsExpirationTracker
+    // will delete aObject.
+    mAllCanvasCache.RemoveEntry(AllCanvasImageCacheKey(aObject->mImage, aObject->mIsAccelerated));
+
+    // Deleting the entry will delete aObject since the entry owns aObject.
     mCache.RemoveEntry(ImageCacheKey(aObject->mImage, aObject->mCanvas, aObject->mIsAccelerated));
   }
 
   nsTHashtable<ImageCacheEntry> mCache;
-  nsTHashtable<SimpleImageCacheEntry> mSimpleCache;
+  nsTHashtable<AllCanvasImageCacheEntry> mAllCanvasCache;
   size_t mTotal;
   RefPtr<ImageCacheObserver> mImageCacheObserver;
 };
 
 static ImageCache* gImageCache = nullptr;
 
-// Listen memory-pressure event for image cache purge
+// Listen memory-pressure event for image cache purge.
 class ImageCacheObserver final : public nsIObserver
 {
 public:
@@ -256,10 +275,33 @@ ImageCache::~ImageCache() {
   mImageCacheObserver->Destroy();
 }
 
+static already_AddRefed<imgIContainer>
+GetImageContainer(dom::Element* aImage)
+{
+  nsCOMPtr<imgIRequest> request;
+  nsCOMPtr<nsIImageLoadingContent> ilc = do_QueryInterface(aImage);
+  if (!ilc) {
+    return nullptr;
+  }
+
+  ilc->GetRequest(nsIImageLoadingContent::CURRENT_REQUEST,
+                  getter_AddRefs(request));
+  if (!request) {
+    return nullptr;
+  }
+
+  nsCOMPtr<imgIContainer> imgContainer;
+  request->GetImage(getter_AddRefs(imgContainer));
+  if (!imgContainer) {
+    return nullptr;
+  }
+
+  return imgContainer.forget();
+}
+
 void
 CanvasImageCache::NotifyDrawImage(Element* aImage,
                                   HTMLCanvasElement* aCanvas,
-                                  imgIRequest* aRequest,
                                   SourceSurface* aSource,
                                   const IntSize& aSize,
                                   bool aIsAccelerated)
@@ -269,32 +311,31 @@ CanvasImageCache::NotifyDrawImage(Element* aImage,
     nsContentUtils::RegisterShutdownObserver(new CanvasImageCacheShutdownObserver());
   }
 
-  ImageCacheEntry* entry =
+  nsCOMPtr<imgIContainer> imgContainer = GetImageContainer(aImage);
-    gImageCache->mCache.PutEntry(ImageCacheKey(aImage, aCanvas, aIsAccelerated));
+  if (!imgContainer) {
+    return;
+  }
+
+  AllCanvasImageCacheKey allCanvasCacheKey(imgContainer, aIsAccelerated);
+  ImageCacheKey canvasCacheKey(imgContainer, aCanvas, aIsAccelerated);
+  ImageCacheEntry* entry = gImageCache->mCache.PutEntry(canvasCacheKey);
+
   if (entry) {
     if (entry->mData->mSourceSurface) {
       // We are overwriting an existing entry.
       gImageCache->mTotal -= entry->mData->SizeInBytes();
       gImageCache->RemoveObject(entry->mData);
-      gImageCache->mSimpleCache.RemoveEntry(SimpleImageCacheKey(entry->mData->mRequest, entry->mData->mIsAccelerated));
+      gImageCache->mAllCanvasCache.RemoveEntry(allCanvasCacheKey);
     }
-    gImageCache->AddObject(entry->mData);
 
-    nsCOMPtr<nsIImageLoadingContent> ilc = do_QueryInterface(aImage);
+    gImageCache->AddObject(entry->mData);
-    if (ilc) {
-      ilc->GetRequest(nsIImageLoadingContent::CURRENT_REQUEST,
-                      getter_AddRefs(entry->mData->mRequest));
-    }
-    entry->mData->mILC = ilc;
     entry->mData->mSourceSurface = aSource;
     entry->mData->mSize = aSize;
-
     gImageCache->mTotal += entry->mData->SizeInBytes();
 
-    if (entry->mData->mRequest) {
+    AllCanvasImageCacheEntry* allEntry = gImageCache->mAllCanvasCache.PutEntry(allCanvasCacheKey);
-      SimpleImageCacheEntry* simpleentry =
+    if (allEntry) {
-        gImageCache->mSimpleCache.PutEntry(SimpleImageCacheKey(entry->mData->mRequest, aIsAccelerated));
+      allEntry->mSourceSurface = aSource;
-      simpleentry->mSourceSurface = aSource;
     }
   }
 
@@ -307,54 +348,56 @@ CanvasImageCache::NotifyDrawImage(Element* aImage,
 }
 
 SourceSurface*
-CanvasImageCache::Lookup(Element* aImage,
+CanvasImageCache::LookupAllCanvas(Element* aImage,
-                         HTMLCanvasElement* aCanvas,
+                                  bool aIsAccelerated)
-                         gfx::IntSize* aSize,
-                         bool aIsAccelerated)
 {
-  if (!gImageCache)
+  if (!gImageCache) {
     return nullptr;
+  }
 
-  ImageCacheEntry* entry =
+  nsCOMPtr<imgIContainer> imgContainer = GetImageContainer(aImage);
-    gImageCache->mCache.GetEntry(ImageCacheKey(aImage, aCanvas, aIsAccelerated));
+  if (!imgContainer) {
-  if (!entry || !entry->mData->mILC)
     return nullptr;
+  }
 
-  nsCOMPtr<imgIRequest> request;
+  AllCanvasImageCacheEntry* entry =
-  entry->mData->mILC->GetRequest(nsIImageLoadingContent::CURRENT_REQUEST, getter_AddRefs(request));
+    gImageCache->mAllCanvasCache.GetEntry(AllCanvasImageCacheKey(imgContainer, aIsAccelerated));
-  if (request != entry->mData->mRequest)
+  if (!entry) {
-    return nullptr;
-
-  gImageCache->MarkUsed(entry->mData);
-
-  *aSize = entry->mData->mSize;
-  return entry->mData->mSourceSurface;
-}
-
-SourceSurface*
-CanvasImageCache::SimpleLookup(Element* aImage,
-                               bool aIsAccelerated)
-{
-  if (!gImageCache)
-    return nullptr;
-
-  nsCOMPtr<imgIRequest> request;
-  nsCOMPtr<nsIImageLoadingContent> ilc = do_QueryInterface(aImage);
-  if (!ilc)
-    return nullptr;
-
-  ilc->GetRequest(nsIImageLoadingContent::CURRENT_REQUEST,
-                  getter_AddRefs(request));
-  if (!request)
-    return nullptr;
-
-  SimpleImageCacheEntry* entry = gImageCache->mSimpleCache.GetEntry(SimpleImageCacheKey(request, aIsAccelerated));
-  if (!entry)
     return nullptr;
+  }
 
   return entry->mSourceSurface;
 }
 
+SourceSurface*
+CanvasImageCache::LookupCanvas(Element* aImage,
+                               HTMLCanvasElement* aCanvas,
+                               IntSize* aSizeOut,
+                               bool aIsAccelerated)
+{
+  if (!gImageCache) {
+    return nullptr;
+  }
+
+  nsCOMPtr<imgIContainer> imgContainer = GetImageContainer(aImage);
+  if (!imgContainer) {
+    return nullptr;
+  }
+
+  ImageCacheEntry* entry =
+    gImageCache->mCache.GetEntry(ImageCacheKey(imgContainer, aCanvas, aIsAccelerated));
+  if (!entry) {
+    return nullptr;
+  }
+
+  MOZ_ASSERT(aSizeOut);
+
+  gImageCache->MarkUsed(entry->mData);
+  *aSizeOut = entry->mData->mSize;
+  return entry->mData->mSourceSurface;
+}
+
+
 NS_IMPL_ISUPPORTS(CanvasImageCacheShutdownObserver, nsIObserver)
 
 NS_IMETHODIMP

dom/canvas/CanvasImageCache.h

@@ -6,6 +6,7 @@
 #ifndef CANVASIMAGECACHE_H_
 #define CANVASIMAGECACHE_H_
 
+#include "mozilla/RefPtr.h"
 #include "nsSize.h"
 
 namespace mozilla {
@@ -17,7 +18,7 @@ namespace gfx {
 class SourceSurface;
 } // namespace gfx
 } // namespace mozilla
-class imgIRequest;
+class imgIContainer;
 
 namespace mozilla {
 
@@ -25,34 +26,30 @@ class CanvasImageCache {
   typedef mozilla::gfx::SourceSurface SourceSurface;
 public:
   /**
-   * Notify that image element aImage was (or is about to be) drawn to aCanvas
+   * Notify that image element aImage was drawn to aCanvas element
    * using the first frame of aRequest's image. The data for the surface is
    * in aSurface, and the image size is in aSize.
    */
   static void NotifyDrawImage(dom::Element* aImage,
                               dom::HTMLCanvasElement* aCanvas,
-                              imgIRequest* aRequest,
                               SourceSurface* aSource,
                               const gfx::IntSize& aSize,
                               bool aIsAccelerated);
 
   /**
-   * Check whether aImage has recently been drawn into aCanvas. If we return
+   * Check whether aImage has recently been drawn any canvas. If we return
-   * a non-null surface, then the image was recently drawn into the canvas
+   * a non-null surface, then the same image was recently drawn into a canvas.
-   * (with the same image request) and the returned surface contains the image
-   * data, and the image size will be returned in aSize.
    */
-  static SourceSurface* Lookup(dom::Element* aImage,
+  static SourceSurface* LookupAllCanvas(dom::Element* aImage,
-                               dom::HTMLCanvasElement* aCanvas,
+                                        bool aIsAccelerated);
-                               gfx::IntSize* aSize,
-                               bool aIsAccelerated);
 
   /**
-   * This is the same as Lookup, except it works on any image recently drawn
+   * Like the top above, but restricts the lookup to only aCanvas. This is
-   * into any canvas. Security checks need to be done again if using the
+   * required for CORS security.
-   * results from this.
    */
-  static SourceSurface* SimpleLookup(dom::Element* aImage,
+  static SourceSurface* LookupCanvas(dom::Element* aImage,
+                                     dom::HTMLCanvasElement* aCanvas,
+                                     gfx::IntSize* aSizeOut,
                                      bool aIsAccelerated);
 };
 

dom/canvas/CanvasRenderingContext2D.cpp

@@ -4361,57 +4361,6 @@ ExtractSubrect(SourceSurface* aSurface, gfx::Rect* aSourceRect, DrawTarget* aTar
   return subrectDT->Snapshot();
 }
 
-// Acts like nsLayoutUtils::SurfaceFromElement, but it'll attempt
-// to pull a SourceSurface from our cache. This allows us to avoid
-// reoptimizing surfaces if content and canvas backends are different.
-nsLayoutUtils::SurfaceFromElementResult
-CanvasRenderingContext2D::CachedSurfaceFromElement(Element* aElement)
-{
-  nsLayoutUtils::SurfaceFromElementResult res;
-
-  nsCOMPtr<nsIImageLoadingContent> imageLoader = do_QueryInterface(aElement);
-  if (!imageLoader) {
-    return res;
-  }
-
-  nsCOMPtr<imgIRequest> imgRequest;
-  imageLoader->GetRequest(nsIImageLoadingContent::CURRENT_REQUEST,
-                          getter_AddRefs(imgRequest));
-  if (!imgRequest) {
-    return res;
-  }
-
-  uint32_t status;
-  if (NS_FAILED(imgRequest->GetImageStatus(&status)) ||
-      !(status & imgIRequest::STATUS_LOAD_COMPLETE)) {
-    return res;
-  }
-
-  nsCOMPtr<nsIPrincipal> principal;
-  if (NS_FAILED(imgRequest->GetImagePrincipal(getter_AddRefs(principal))) ||
-      !principal) {
-    return res;
-  }
-
-  res.mSourceSurface =
-    CanvasImageCache::SimpleLookup(aElement, mIsSkiaGL);
-  if (!res.mSourceSurface) {
-    return res;
-  }
-
-  int32_t corsmode = imgIRequest::CORS_NONE;
-  if (NS_SUCCEEDED(imgRequest->GetCORSMode(&corsmode))) {
-    res.mCORSUsed = corsmode != imgIRequest::CORS_NONE;
-  }
-
-  res.mSize = res.mSourceSurface->GetSize();
-  res.mPrincipal = principal.forget();
-  res.mIsWriteOnly = false;
-  res.mImageRequest = imgRequest.forget();
-
-  return res;
-}
-
 //
 // image
 //
@@ -4435,6 +4384,56 @@ ClipImageDimension(double& aSourceCoord, double& aSourceSize, int32_t aImageSize
   }
 }
 
+// Acts like nsLayoutUtils::SurfaceFromElement, but it'll attempt
+// to pull a SourceSurface from our cache. This allows us to avoid
+// reoptimizing surfaces if content and canvas backends are different.
+nsLayoutUtils::SurfaceFromElementResult
+CanvasRenderingContext2D::CachedSurfaceFromElement(Element* aElement)
+{
+  nsLayoutUtils::SurfaceFromElementResult res;
+  nsCOMPtr<nsIImageLoadingContent> imageLoader = do_QueryInterface(aElement);
+  if (!imageLoader) {
+    return res;
+  }
+
+  nsCOMPtr<imgIRequest> imgRequest;
+  imageLoader->GetRequest(nsIImageLoadingContent::CURRENT_REQUEST,
+                          getter_AddRefs(imgRequest));
+  if (!imgRequest) {
+    return res;
+  }
+
+  uint32_t status = 0;
+  if (NS_FAILED(imgRequest->GetImageStatus(&status)) ||
+      !(status & imgIRequest::STATUS_LOAD_COMPLETE)) {
+    return res;
+  }
+
+  nsCOMPtr<nsIPrincipal> principal;
+  if (NS_FAILED(imgRequest->GetImagePrincipal(getter_AddRefs(principal))) ||
+      !principal) {
+    return res;
+  }
+
+  res.mSourceSurface =
+    CanvasImageCache::LookupAllCanvas(aElement, mIsSkiaGL);
+  if (!res.mSourceSurface) {
+    return res;
+  }
+
+  int32_t corsmode = imgIRequest::CORS_NONE;
+  if (NS_SUCCEEDED(imgRequest->GetCORSMode(&corsmode))) {
+    res.mCORSUsed = corsmode != imgIRequest::CORS_NONE;
+  }
+
+  res.mSize = res.mSourceSurface->GetSize();
+  res.mPrincipal = principal.forget();
+  res.mIsWriteOnly = false;
+  res.mImageRequest = imgRequest.forget();
+
+  return res;
+}
+
 // drawImage(in HTMLImageElement image, in float dx, in float dy);
 //   -- render image from 0,0 at dx,dy top-left coords
 // drawImage(in HTMLImageElement image, in float dx, in float dy, in float dw, in float dh);
@@ -4504,7 +4503,7 @@ CanvasRenderingContext2D::DrawImage(const CanvasImageSource& aImage,
     }
 
     srcSurf =
-      CanvasImageCache::Lookup(element, mCanvasElement, &imgSize, mIsSkiaGL);
+     CanvasImageCache::LookupCanvas(element, mCanvasElement, &imgSize, mIsSkiaGL);
   }
 
   nsLayoutUtils::DirectDrawInfo drawInfo;
@@ -4613,15 +4612,13 @@ CanvasRenderingContext2D::DrawImage(const CanvasImageSource& aImage,
     // of animated images. We also don't want to rasterize vector images.
     uint32_t sfeFlags = nsLayoutUtils::SFE_WANT_FIRST_FRAME |
                         nsLayoutUtils::SFE_NO_RASTERIZING_VECTORS;
-    // The cache lookup can miss even if the image is already in the cache
+
-    // if the image is coming from a different element or cached for a
-    // different canvas. This covers the case when we miss due to caching
-    // for a different canvas, but CanvasImageCache should be fixed if we
-    // see misses due to different elements drawing the same image.
     nsLayoutUtils::SurfaceFromElementResult res =
-      CachedSurfaceFromElement(element);
+      CanvasRenderingContext2D::CachedSurfaceFromElement(element);
-    if (!res.mSourceSurface)
+
+    if (!res.mSourceSurface) {
       res = nsLayoutUtils::SurfaceFromElement(element, sfeFlags, mTarget);
+    }
 
     if (!res.mSourceSurface && !res.mDrawInfo.mImgContainer) {
       // The spec says to silently do nothing in the following cases:
@@ -4653,10 +4650,8 @@ CanvasRenderingContext2D::DrawImage(const CanvasImageSource& aImage,
 
     if (res.mSourceSurface) {
       if (res.mImageRequest) {
-        CanvasImageCache::NotifyDrawImage(element, mCanvasElement, res.mImageRequest,
+        CanvasImageCache::NotifyDrawImage(element, mCanvasElement, res.mSourceSurface, imgSize, mIsSkiaGL);
-                                          res.mSourceSurface, imgSize, mIsSkiaGL);
       }
-
       srcSurf = res.mSourceSurface;
     } else {
       drawInfo = res.mDrawInfo;

layout/base/nsLayoutUtils.cpp

@@ -7281,8 +7281,9 @@ nsLayoutUtils::SurfaceFromElement(nsIImageLoadingContent* aElement,
   nsCOMPtr<imgIRequest> imgRequest;
   rv = aElement->GetRequest(nsIImageLoadingContent::CURRENT_REQUEST,
                             getter_AddRefs(imgRequest));
-  if (NS_FAILED(rv) || !imgRequest)
+  if (NS_FAILED(rv) || !imgRequest) {
     return result;
+  }
 
   uint32_t status;
   imgRequest->GetImageStatus(&status);
@@ -7297,13 +7298,15 @@ nsLayoutUtils::SurfaceFromElement(nsIImageLoadingContent* aElement,
 
   nsCOMPtr<nsIPrincipal> principal;
   rv = imgRequest->GetImagePrincipal(getter_AddRefs(principal));
-  if (NS_FAILED(rv))
+  if (NS_FAILED(rv)) {
     return result;
+  }
 
   nsCOMPtr<imgIContainer> imgContainer;
   rv = imgRequest->GetImage(getter_AddRefs(imgContainer));
-  if (NS_FAILED(rv))
+  if (NS_FAILED(rv)) {
     return result;
+  }
 
   uint32_t noRasterize = aSurfaceFlags & SFE_NO_RASTERIZING_VECTORS;
 
@@ -7368,7 +7371,6 @@ nsLayoutUtils::SurfaceFromElement(nsIImageLoadingContent* aElement,
   // no images, including SVG images, can load content from another domain.
   result.mIsWriteOnly = false;
   result.mImageRequest = imgRequest.forget();
-
   return result;
 }