Backed out changeset 45dd83a63162 (bug 1339202) for crashing in image processing related tests, e.g. xpcshell test test_imgtools.js. r=backout

image/Decoder.cpp

@@ -64,7 +64,6 @@ Decoder::Decoder(RasterImage* aImage)
   , mDecodeDone(false)
   , mError(false)
   , mShouldReportError(false)
-  , mFinalizeFrames(true)
 { }
 
 Decoder::~Decoder()
@@ -456,7 +455,7 @@ Decoder::PostFrameStop(Opacity aFrameOpacity
   mFinishedNewFrame = true;
 
   mCurrentFrame->Finish(aFrameOpacity, aDisposalMethod, aTimeout,
-                        aBlendMethod, aBlendRect, mFinalizeFrames);
+                        aBlendMethod, aBlendRect);
 
   mProgress |= FLAG_FRAME_COMPLETE;
 

image/Decoder.h

@@ -262,10 +262,6 @@ public:
   bool HasError() const { return mError; }
   bool ShouldReportError() const { return mShouldReportError; }
 
-  // Finalize frames
-  void SetFinalizeFrames(bool aFinalize) { mFinalizeFrames = aFinalize; }
-  bool GetFinalizeFrames() const { return mFinalizeFrames; }
-
   /// Did we finish decoding enough that calling Decode() again would be useless?
   bool GetDecodeDone() const
   {
@@ -550,7 +546,6 @@ private:
   bool mDecodeDone : 1;
   bool mError : 1;
   bool mShouldReportError : 1;
-  bool mFinalizeFrames : 1;
 };
 
 } // namespace image

image/DecoderFactory.cpp

@@ -266,7 +266,6 @@ DecoderFactory::CreateDecoderForICOResource(DecoderType aType,
   decoder->SetOutputSize(aICODecoder->OutputSize());
   decoder->SetDecoderFlags(aICODecoder->GetDecoderFlags());
   decoder->SetSurfaceFlags(aICODecoder->GetSurfaceFlags());
-  decoder->SetFinalizeFrames(false);
 
   if (NS_FAILED(decoder->Init())) {
     return nullptr;

image/RasterImage.cpp

@@ -479,7 +479,7 @@ NS_IMETHODIMP_(already_AddRefed<SourceSurface>)
 RasterImage::GetFrame(uint32_t aWhichFrame,
                       uint32_t aFlags)
 {
-  return GetFrameAtSize(mSize, aWhichFrame, aFlags);
+  return GetFrameInternal(mSize, aWhichFrame, aFlags).second().forget();
 }
 
 NS_IMETHODIMP_(already_AddRefed<SourceSurface>)
@@ -487,12 +487,7 @@ RasterImage::GetFrameAtSize(const IntSize& aSize,
                             uint32_t aWhichFrame,
                             uint32_t aFlags)
 {
-  RefPtr<SourceSurface> surf =
-    GetFrameInternal(aSize, aWhichFrame, aFlags).second().forget();
-  // If we are here, it suggests the image is embedded in a canvas or some
-  // other path besides layers, and we won't need the file handle.
-  MarkSurfaceShared(surf);
-  return surf.forget();
+  return GetFrameInternal(aSize, aWhichFrame, aFlags).second().forget();
 }
 
 Pair<DrawResult, RefPtr<SourceSurface>>

image/decoders/nsICODecoder.cpp

@@ -593,13 +593,6 @@ nsICODecoder::FinishResource()
     return Transition::TerminateFailure();
   }
 
-  // Finalize the frame which we deferred to ensure we could modify the final
-  // result (e.g. to apply the BMP mask).
-  MOZ_ASSERT(!mContainedDecoder->GetFinalizeFrames());
-  if (mCurrentFrame) {
-    mCurrentFrame->FinalizeSurface();
-  }
-
   return Transition::TerminateSuccess();
 }
 

image/imgFrame.cpp

@@ -12,6 +12,7 @@
 
 #include "gfx2DGlue.h"
 #include "gfxPlatform.h"
+#include "gfxPrefs.h"
 #include "gfxUtils.h"
 #include "gfxAlphaRecovery.h"
 
@@ -19,8 +20,6 @@
 #include "MainThreadUtils.h"
 #include "mozilla/CheckedInt.h"
 #include "mozilla/gfx/Tools.h"
-#include "mozilla/gfx/gfxVars.h"
-#include "mozilla/layers/SourceSurfaceSharedData.h"
 #include "mozilla/layers/SourceSurfaceVolatileData.h"
 #include "mozilla/Likely.h"
 #include "mozilla/MemoryReporting.h"
@@ -52,12 +51,6 @@ CreateLockedSurface(DataSourceSurface *aSurface,
                     const IntSize& size,
                     SurfaceFormat format)
 {
-  // Shared memory is never released until the surface itself is released
-  if (aSurface->GetType() == SurfaceType::DATA_SHARED) {
-    RefPtr<DataSourceSurface> surf(aSurface);
-    return surf.forget();
-  }
-
   DataSourceSurface::ScopedMap* smap =
     new DataSourceSurface::ScopedMap(aSurface, DataSourceSurface::READ_WRITE);
   if (smap->IsMapped()) {
@@ -84,17 +77,11 @@ AllocateBufferForImage(const IntSize& size,
                        bool aIsAnimated = false)
 {
   int32_t stride = VolatileSurfaceStride(size, format);
-  if (!aIsAnimated && gfxVars::UseWebRender()) {
-    RefPtr<SourceSurfaceSharedData> newSurf = new SourceSurfaceSharedData();
-    if (newSurf->Init(size, stride, format)) {
-      return newSurf.forget();
-    }
-  } else {
-    RefPtr<SourceSurfaceVolatileData> newSurf= new SourceSurfaceVolatileData();
-    if (newSurf->Init(size, stride, format)) {
-      return newSurf.forget();
-    }
+  RefPtr<SourceSurfaceVolatileData> newSurf = new SourceSurfaceVolatileData();
+  if (newSurf->Init(size, stride, format)) {
+    return newSurf.forget();
   }
+
   return nullptr;
 }
 
@@ -121,19 +108,6 @@ ClearSurface(DataSourceSurface* aSurface, const IntSize& aSize, SurfaceFormat aF
   return true;
 }
 
-void
-MarkSurfaceShared(SourceSurface* aSurface)
-{
-  // Depending on what requested the image decoding, the buffer may or may not
-  // end up being shared with another process (e.g. put in a painted layer,
-  // used inside a canvas). If not shared, we should ensure are not keeping the
-  // handle only because we have yet to share it.
-  if (aSurface && aSurface->GetType() == SurfaceType::DATA_SHARED) {
-    auto sharedSurface = static_cast<SourceSurfaceSharedData*>(aSurface);
-    sharedSurface->FinishedSharing();
-  }
-}
-
 // Returns true if an image of aWidth x aHeight is allowed and legal.
 static bool
 AllowedImageSize(int32_t aWidth, int32_t aHeight)
@@ -385,8 +359,6 @@ imgFrame::InitWithDrawable(gfxDrawable* aDrawable,
     // We used an offscreen surface, which is an "optimized" surface from
     // imgFrame's perspective.
     mOptSurface = target->Snapshot();
-  } else {
-    FinalizeSurface();
   }
 
   // If we reach this point, we should regard ourselves as complete.
@@ -579,10 +551,6 @@ bool imgFrame::Draw(gfxContext* aContext, const ImageRegion& aRegion,
                                imageRect.Size(), region, surfaceResult.mFormat,
                                aSamplingFilter, aImageFlags, aOpacity);
   }
-
-  // Image got put into a painted layer, it will not be shared with another
-  // process.
-  MarkSurfaceShared(surf);
   return true;
 }
 
@@ -613,8 +581,7 @@ imgFrame::Finish(Opacity aFrameOpacity /* = Opacity::SOME_TRANSPARENCY */,
                  FrameTimeout aTimeout
                    /* = FrameTimeout::FromRawMilliseconds(0) */,
                  BlendMethod aBlendMethod /* = BlendMethod::OVER */,
-                 const Maybe<IntRect>& aBlendRect /* = Nothing() */,
-                 bool aFinalize /* = true */)
+                 const Maybe<IntRect>& aBlendRect /* = Nothing() */)
 {
   MonitorAutoLock lock(mMonitor);
   MOZ_ASSERT(mLockCount > 0, "Image data should be locked");
@@ -624,11 +591,6 @@ imgFrame::Finish(Opacity aFrameOpacity /* = Opacity::SOME_TRANSPARENCY */,
   mBlendMethod = aBlendMethod;
   mBlendRect = aBlendRect;
   ImageUpdatedInternal(GetRect());
-
-  if (aFinalize) {
-    FinalizeSurfaceInternal();
-  }
-
   mFinished = true;
 
   // The image is now complete, wake up anyone who's waiting.
@@ -671,9 +633,6 @@ imgFrame::GetImageDataInternal(uint8_t** aData, uint32_t* aLength) const
   MOZ_ASSERT(mLockCount > 0, "Image data should be locked");
 
   if (mLockedSurface) {
-    // TODO: This is okay for now because we only realloc shared surfaces on
-    // the main thread after decoding has finished, but if animations want to
-    // read frame data off the main thread, we will need to reconsider this.
     *aData = mLockedSurface->GetData();
     MOZ_ASSERT(*aData,
       "mLockedSurface is non-null, but GetData is null in GetImageData");
@@ -794,27 +753,6 @@ imgFrame::SetOptimizable()
   mOptimizable = true;
 }
 
-void
-imgFrame::FinalizeSurface()
-{
-  MonitorAutoLock lock(mMonitor);
-  FinalizeSurfaceInternal();
-}
-
-void
-imgFrame::FinalizeSurfaceInternal()
-{
-  mMonitor.AssertCurrentThreadOwns();
-
-  // Not all images will have mRawSurface to finalize (i.e. paletted images).
-  if (!mRawSurface || mRawSurface->GetType() != SurfaceType::DATA_SHARED) {
-    return;
-  }
-
-  auto sharedSurf = static_cast<SourceSurfaceSharedData*>(mRawSurface.get());
-  sharedSurf->Finalize();
-}
-
 already_AddRefed<SourceSurface>
 imgFrame::GetSourceSurface()
 {

image/imgFrame.h

@@ -279,15 +279,12 @@ public:
    * @param aBlendRect       For animation frames, if present, the subrect in
    *                         which @aBlendMethod applies. Outside of this
    *                         subrect, BlendMethod::OVER is always used.
-   * @param aFinalize        Finalize the underlying surface (e.g. so that it
-   *                         may be marked as read only if possible).
    */
   void Finish(Opacity aFrameOpacity = Opacity::SOME_TRANSPARENCY,
               DisposalMethod aDisposalMethod = DisposalMethod::KEEP,
               FrameTimeout aTimeout = FrameTimeout::FromRawMilliseconds(0),
               BlendMethod aBlendMethod = BlendMethod::OVER,
-              const Maybe<IntRect>& aBlendRect = Nothing(),
-              bool aFinalize = true);
+              const Maybe<IntRect>& aBlendRect = Nothing());
 
   /**
    * Mark this imgFrame as aborted. This informs the imgFrame that if it isn't
@@ -344,7 +341,6 @@ public:
 
   void SetOptimizable();
 
-  void FinalizeSurface();
   already_AddRefed<SourceSurface> GetSourceSurface();
 
   void AddSizeOfExcludingThis(MallocSizeOf aMallocSizeOf, size_t& aHeapSizeOut,
@@ -365,7 +361,6 @@ private: // methods
   void GetImageDataInternal(uint8_t** aData, uint32_t* length) const;
   uint32_t GetImageBytesPerRow() const;
   uint32_t GetImageDataLength() const;
-  void FinalizeSurfaceInternal();
   already_AddRefed<SourceSurface> GetSourceSurfaceInternal();
 
   uint32_t PaletteDataLength() const
@@ -622,8 +617,6 @@ private:
   RefPtr<imgFrame> mFrame;
 };
 
-void MarkSurfaceShared(gfx::SourceSurface* aSurface);
-
 } // namespace image
 } // namespace mozilla