Bug 1133119 - ::Map should fail if the data is null, and add more null pointer checks. r=mattwoodrow, a=sledru
authorMilan Sreckovic <milan@mozilla.com>
Tue, 03 Mar 2015 11:17:55 -0500
changeset 260232 90d2538212ab
parent 260231 22f8fa3a9273
child 260233 fe8c5e74565f
push id724
push userryanvm@gmail.com
push date2015-04-23 01:08 +0000
treeherdermozilla-release@db41e8e267ed [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmattwoodrow, sledru
bugs1133119
milestone38.0
Bug 1133119 - ::Map should fail if the data is null, and add more null pointer checks. r=mattwoodrow, a=sledru
gfx/2d/2D.h
gfx/2d/DrawTargetCairo.cpp
gfx/2d/SourceSurfaceD2D.cpp
gfx/2d/SourceSurfaceD2D1.cpp
gfx/2d/SourceSurfaceD2DTarget.cpp
--- a/gfx/2d/2D.h
+++ b/gfx/2d/2D.h
@@ -404,22 +404,25 @@ public:
 
   /** @deprecated
    * Stride of the surface, distance in bytes between the start of the image
    * data belonging to row y and row y+1. This may be negative.
    * Can return 0 if there was OOM allocating surface data.
    */
   virtual int32_t Stride() = 0;
 
+  /**
+   * The caller is responsible for ensuring aMappedSurface is not null.
+   */
   virtual bool Map(MapType, MappedSurface *aMappedSurface)
   {
     aMappedSurface->mData = GetData();
     aMappedSurface->mStride = Stride();
-    mIsMapped = true;
-    return true;
+    mIsMapped = !!aMappedSurface->mData;
+    return mIsMapped;
   }
 
   virtual void Unmap()
   {
     MOZ_ASSERT(mIsMapped);
     mIsMapped = false;
   }
 
--- a/gfx/2d/DrawTargetCairo.cpp
+++ b/gfx/2d/DrawTargetCairo.cpp
@@ -205,16 +205,18 @@ ReleaseData(void* aData)
 }
 
 cairo_surface_t*
 CopyToImageSurface(unsigned char *aData,
                    const IntRect &aRect,
                    int32_t aStride,
                    SurfaceFormat aFormat)
 {
+  MOZ_ASSERT(aData);
+
   cairo_surface_t* surf = cairo_image_surface_create(GfxFormatToCairoFormat(aFormat),
                                                      aRect.width,
                                                      aRect.height);
   // In certain scenarios, requesting larger than 8k image fails.  Bug 803568
   // covers the details of how to run into it, but the full detailed
   // investigation hasn't been done to determine the underlying cause.  We
   // will just handle the failure to allocate the surface to avoid a crash.
   if (cairo_surface_status(surf)) {
@@ -257,16 +259,20 @@ cairo_surface_t* GetAsImageSurface(cairo
   return nullptr;
 }
 
 cairo_surface_t* CreateSubImageForData(unsigned char* aData,
                                        const IntRect& aRect,
                                        int aStride,
                                        SurfaceFormat aFormat)
 {
+  if (!aData) {
+    gfxWarning() << "DrawTargetCairo.CreateSubImageForData null aData";
+    return nullptr;
+  }
   unsigned char *data = aData +
                         aRect.y * aStride +
                         aRect.x * BytesPerPixel(aFormat);
 
   cairo_surface_t *image =
     cairo_image_surface_create_for_data(data,
                                         GfxFormatToCairoFormat(aFormat),
                                         aRect.width,
@@ -369,18 +375,18 @@ GetCairoSurfaceForSourceSurface(SourceSu
   cairo_surface_t* surf =
     CreateSubImageForData(map.mData, subimage,
                           map.mStride, data->GetFormat());
 
   // In certain scenarios, requesting larger than 8k image fails.  Bug 803568
   // covers the details of how to run into it, but the full detailed
   // investigation hasn't been done to determine the underlying cause.  We
   // will just handle the failure to allocate the surface to avoid a crash.
-  if (cairo_surface_status(surf)) {
-    if (cairo_surface_status(surf) == CAIRO_STATUS_INVALID_STRIDE) {
+  if (!surf || cairo_surface_status(surf)) {
+    if (surf && (cairo_surface_status(surf) == CAIRO_STATUS_INVALID_STRIDE)) {
       // If we failed because of an invalid stride then copy into
       // a new surface with a stride that cairo chooses. No need to
       // set user data since we're not dependent on the original
       // data.
       cairo_surface_t* result =
         CopyToImageSurface(map.mData,
                            subimage,
                            map.mStride,
@@ -1348,18 +1354,26 @@ DrawTargetCairo::CreateFilter(FilterType
 }
 
 TemporaryRef<SourceSurface>
 DrawTargetCairo::CreateSourceSurfaceFromData(unsigned char *aData,
                                              const IntSize &aSize,
                                              int32_t aStride,
                                              SurfaceFormat aFormat) const
 {
+  if (!aData) {
+    gfxWarning() << "DrawTargetCairo::CreateSourceSurfaceFromData null aData";
+    return nullptr;
+  }
+
   cairo_surface_t* surf = CopyToImageSurface(aData, IntRect(IntPoint(), aSize),
                                              aStride, aFormat);
+  if (!surf) {
+    return nullptr;
+  }
 
   RefPtr<SourceSurfaceCairo> source_surf = new SourceSurfaceCairo(surf, aSize, aFormat);
   cairo_surface_destroy(surf);
 
   return source_surf.forget();
 }
 
 #ifdef CAIRO_HAS_XLIB_SURFACE
--- a/gfx/2d/SourceSurfaceD2D.cpp
+++ b/gfx/2d/SourceSurfaceD2D.cpp
@@ -274,19 +274,19 @@ DataSourceSurfaceD2D::Map(MapType aMapTy
 
   if (FAILED(hr)) {
     gfxWarning() << "Texture map failed with code: " << hexa(hr);
     return false;
   }
 
   aMappedSurface->mData = (uint8_t*)map.pData;
   aMappedSurface->mStride = map.RowPitch;
-  mIsMapped = true;
+  mIsMapped = !!aMappedSurface->mData;
 
-  return true;
+  return mIsMapped;
 }
 
 void
 DataSourceSurfaceD2D::Unmap()
 {
   MOZ_ASSERT(mIsMapped);
 
   mIsMapped = false;
--- a/gfx/2d/SourceSurfaceD2D1.cpp
+++ b/gfx/2d/SourceSurfaceD2D1.cpp
@@ -181,18 +181,18 @@ DataSourceSurfaceD2D1::Map(MapType aMapT
     MOZ_CRASH("No support for Write maps on D2D1 DataSourceSurfaces yet!");
   }
 
   D2D1_MAPPED_RECT map;
   mBitmap->Map(D2D1_MAP_OPTIONS_READ, &map);
   aMappedSurface->mData = map.bits;
   aMappedSurface->mStride = map.pitch;
 
-  mIsMapped = true;
-  return true;
+  mIsMapped = !!aMappedSurface->mData;
+  return mIsMapped;
 }
 
 void
 DataSourceSurfaceD2D1::Unmap()
 {
   MOZ_ASSERT(mIsMapped);
 
   mIsMapped = false;
--- a/gfx/2d/SourceSurfaceD2DTarget.cpp
+++ b/gfx/2d/SourceSurfaceD2DTarget.cpp
@@ -277,19 +277,19 @@ DataSourceSurfaceD2DTarget::Map(MapType 
 
   if (FAILED(hr)) {
     gfxWarning() << "Texture map failed with code: " << hexa(hr);
     return false;
   }
 
   aMappedSurface->mData = (uint8_t*)map.pData;
   aMappedSurface->mStride = map.RowPitch;
-  mIsMapped = true;
+  mIsMapped = !!aMappedSurface->mData;
 
-  return true;
+  return mIsMapped;
 }
 
 void
 DataSourceSurfaceD2DTarget::Unmap()
 {
   MOZ_ASSERT(mIsMapped);
 
   mIsMapped = false;