Bug 925448 - Stop CGImageRef external data being deleted prematurely. r=bgirard,bas a=bajaj
authorSteven Michaud <smichaud@pobox.com>
Wed, 04 Dec 2013 13:48:38 -0600
changeset 166661 73fdfabcc2be46efad1cd38f3e7fde95d931c042
parent 166660 77ac974d69d609ad333f2ceb06b3b0ea78d178dc
child 166662 45db33cf269ba7195316994595f7f729b88b1106
push id3066
push userakeybl@mozilla.com
push dateMon, 09 Dec 2013 19:58:46 +0000
treeherdermozilla-beta@a31a0dce83aa [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbgirard, bas, bajaj
bugs925448
milestone27.0a2
Bug 925448 - Stop CGImageRef external data being deleted prematurely. r=bgirard,bas a=bajaj
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 *);