Bug 925448 - Stop CGImageRef external data being deleted prematurely. r=bgirard,bas
authorSteven Michaud <smichaud@pobox.com>
Tue, 26 Nov 2013 12:41:32 -0600
changeset 157579 8a5c832a8bbf5d6483050c888370eadd26119c29
parent 157578 fb5ae868b923ed5c20a17db82a5f851f565d517f
child 157580 b4238682735ab139baadd1313933e832f618ab13
push id36768
push usersmichaud@pobox.com
push dateTue, 26 Nov 2013 18:42:34 +0000
treeherdermozilla-inbound@8a5c832a8bbf [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbgirard, bas
bugs925448
milestone28.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 925448 - Stop CGImageRef external data being deleted prematurely. r=bgirard,bas
gfx/2d/SourceSurfaceCG.cpp
gfx/2d/SourceSurfaceCG.h
--- a/gfx/2d/SourceSurfaceCG.cpp
+++ b/gfx/2d/SourceSurfaceCG.cpp
@@ -273,31 +273,18 @@ void SourceSurfaceCGBitmapContext::Ensur
   // Instead of using CGBitmapContextCreateImage we create
   // a CGImage around the data associated with the CGBitmapContext
   // we do this to avoid the vm_copy that CGBitmapContextCreateImage.
   // vm_copy tends to cause all sorts of unexpected performance problems
   // because of the mm tricks that vm_copy does. Using a regular
   // memcpy when the bitmap context is modified gives us more predictable
   // performance characteristics.
   if (!mImage) {
-    void *info;
-    if (mCg) {
-      // if we have an mCg than it owns the data
-      // and we don't want to tranfer ownership
-      // to the CGDataProviderCreateWithData
-      info = nullptr;
-    } else {
-      // otherwise we transfer ownership to
-      // the dataProvider
-      info = mData;
-    }
-
     if (!mData) abort();
-
-    mImage = CreateCGImage(info, mData, mSize, mStride, mFormat);
+    mImage = CreateCGImage(nullptr, mData, mSize, mStride, mFormat);
   }
 }
 
 IntSize
 SourceSurfaceCGBitmapContext::GetSize() const
 {
   return mSize;
 }
@@ -305,18 +292,18 @@ SourceSurfaceCGBitmapContext::GetSize() 
 void
 SourceSurfaceCGBitmapContext::DrawTargetWillChange()
 {
   if (mDrawTarget) {
     // This will break the weak reference we hold to mCg
     size_t stride = CGBitmapContextGetBytesPerRow(mCg);
     size_t height = CGBitmapContextGetHeight(mCg);
 
-    //XXX: infalliable malloc?
-    mData = malloc(stride * height);
+    mDataHolder.Realloc(stride * height);
+    mData = mDataHolder;
 
     // copy out the data from the CGBitmapContext
     // we'll maintain ownership of mData until
     // we transfer it to mImage
     memcpy(mData, CGBitmapContextGetData(mCg), stride*height);
 
     // drop the current image for the data associated with the CGBitmapContext
     if (mImage)
@@ -325,20 +312,16 @@ SourceSurfaceCGBitmapContext::DrawTarget
 
     mCg = nullptr;
     mDrawTarget = nullptr;
   }
 }
 
 SourceSurfaceCGBitmapContext::~SourceSurfaceCGBitmapContext()
 {
-  if (!mImage && !mCg) {
-    // neither mImage or mCg owns the data
-    free(mData);
-  }
   if (mImage)
     CGImageRelease(mImage);
 }
 
 SourceSurfaceCGIOSurfaceContext::SourceSurfaceCGIOSurfaceContext(DrawTargetCG *aDrawTarget)
 {
   CGContextRef cg = (CGContextRef)aDrawTarget->GetNativeSurface(NATIVE_SURFACE_CGCONTEXT_ACCELERATED);
 
--- a/gfx/2d/SourceSurfaceCG.h
+++ b/gfx/2d/SourceSurfaceCG.h
@@ -89,16 +89,30 @@ class SourceSurfaceCGBitmapContext : pub
 {
 public:
   SourceSurfaceCGBitmapContext(DrawTargetCG *);
   ~SourceSurfaceCGBitmapContext();
 
   virtual SurfaceType GetType() const { return SURFACE_COREGRAPHICS_CGCONTEXT; }
   virtual IntSize GetSize() const;
   virtual SurfaceFormat GetFormat() const { return mFormat; }
+  virtual TemporaryRef<DataSourceSurface> GetDataSurface()
+  {
+    // This call to DrawTargetWillChange() is needed to make a local copy of
+    // the data from mDrawTarget.  If we don't do that, the data can end up
+    // getting deleted before the CGImageRef it belongs to.
+    //
+    // Another reason we need a local copy of the data is that the data in
+    // mDrawTarget could change when someone touches the original DrawTargetCG
+    // object.  But a SourceSurface object should be immutable.
+    //
+    // For more information see bug 925448.
+    DrawTargetWillChange();
+    return this;
+  }
 
   CGImageRef GetImage() { EnsureImage(); return mImage; }
 
   virtual unsigned char *GetData() { return static_cast<unsigned char*>(mData); }
 
   virtual int32_t Stride() { return mStride; }
 
 private:
@@ -114,16 +128,19 @@ private:
   SurfaceFormat mFormat;
 
   mutable CGImageRef mImage;
 
   // mData can be owned by three different things:
   // mImage, mCg or SourceSurfaceCGBitmapContext
   void *mData;
 
+  // The image buffer, if the buffer is owned by this class.
+  AlignedArray<uint8_t> mDataHolder;
+
   int32_t mStride;
   IntSize mSize;
 };
 
 class SourceSurfaceCGIOSurfaceContext : public SourceSurfaceCGContext
 {
 public:
   SourceSurfaceCGIOSurfaceContext(DrawTargetCG *);