Bug 980415 - Fix multiple points in the codebase where we fail to call DataSourceSurface::Unmap(). r=Bas
authorJonathan Watt <jwatt@jwatt.org>
Fri, 07 Mar 2014 13:21:38 +0000
changeset 189733 b8f0eb5251eb6a097a06943eb8697b349ccf2414
parent 189732 af1a435e117a7035d5e9f1464663ab5f49ea464f
child 189734 856705b0c63ff9352032f07b8bdcec6a07213423
push id3503
push userraliiev@mozilla.com
push dateMon, 28 Apr 2014 18:51:11 +0000
treeherdermozilla-beta@c95ac01e332e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersBas
bugs980415
milestone30.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 980415 - Fix multiple points in the codebase where we fail to call DataSourceSurface::Unmap(). r=Bas
browser/components/shell/src/nsWindowsShellService.cpp
gfx/gl/GLScreenBuffer.cpp
gfx/layers/GrallocImages.cpp
image/src/imgTools.cpp
widget/qt/nsClipboard.cpp
--- a/browser/components/shell/src/nsWindowsShellService.cpp
+++ b/browser/components/shell/src/nsWindowsShellService.cpp
@@ -761,27 +761,20 @@ WriteBitmap(nsIFile* aFile, imgIContaine
   nsRefPtr<gfxImageSurface> thebesImageSurface =
     thebesSurface->GetAsReadableARGB32ImageSurface();
   NS_ENSURE_TRUE(thebesImageSurface, NS_ERROR_FAILURE);
 
   RefPtr<DataSourceSurface> dataSurface =
     thebesImageSurface->CopyToB8G8R8A8DataSourceSurface();
   NS_ENSURE_TRUE(dataSurface, NS_ERROR_FAILURE);
 
-  DataSourceSurface::MappedSurface map;
-  dataSurface->Map(DataSourceSurface::MapType::READ, &map);
-  if (!map.mData) {
-    return NS_ERROR_FAILURE;
-  }
-
   int32_t width = dataSurface->GetSize().width;
   int32_t height = dataSurface->GetSize().height;
   int32_t bytesPerPixel = 4 * sizeof(uint8_t);
   uint32_t bytesPerRow = bytesPerPixel * width;
-  uint32_t length = map.mStride * height;
 
   // initialize these bitmap structs which we will later
   // serialize directly to the head of the bitmap file
   BITMAPINFOHEADER bmi;
   bmi.biSize = sizeof(BITMAPINFOHEADER);
   bmi.biWidth = width;
   bmi.biHeight = height;
   bmi.biPlanes = 1;
@@ -800,43 +793,50 @@ WriteBitmap(nsIFile* aFile, imgIContaine
   bf.bfOffBits = sizeof(BITMAPFILEHEADER) + sizeof(BITMAPINFOHEADER);
   bf.bfSize = bf.bfOffBits + bmi.biSizeImage;
 
   // get a file output stream
   nsCOMPtr<nsIOutputStream> stream;
   rv = NS_NewLocalFileOutputStream(getter_AddRefs(stream), aFile);
   NS_ENSURE_SUCCESS(rv, rv);
 
+  DataSourceSurface::MappedSurface map;
+  if (!dataSurface->Map(DataSourceSurface::MapType::READ, &map)) {
+    return NS_ERROR_FAILURE;
+  }
+
   // write the bitmap headers and rgb pixel data to the file
   rv = NS_ERROR_FAILURE;
   if (stream) {
     uint32_t written;
     stream->Write((const char*)&bf, sizeof(BITMAPFILEHEADER), &written);
     if (written == sizeof(BITMAPFILEHEADER)) {
       stream->Write((const char*)&bmi, sizeof(BITMAPINFOHEADER), &written);
       if (written == sizeof(BITMAPINFOHEADER)) {
         // write out the image data backwards because the desktop won't
         // show bitmaps with negative heights for top-to-bottom
-        uint32_t i = length;
+        uint32_t i = map.mStride * height;
         do {
           i -= map.mStride;
           stream->Write(((const char*)map.mData) + i, bytesPerRow, &written);
           if (written == bytesPerRow) {
             rv = NS_OK;
           } else {
             rv = NS_ERROR_FAILURE;
             break;
           }
         } while (i != 0);
       }
     }
 
     stream->Close();
   }
 
+  dataSurface->Unmap();
+
   return rv;
 }
 
 NS_IMETHODIMP
 nsWindowsShellService::SetDesktopBackground(nsIDOMElement* aElement, 
                                             int32_t aPosition)
 {
   nsresult rv;
--- a/gfx/gl/GLScreenBuffer.cpp
+++ b/gfx/gl/GLScreenBuffer.cpp
@@ -485,16 +485,17 @@ GLScreenBuffer::Readback(SharedSurface_G
   DataSourceSurface::MappedSurface ms;
   dest->Map(DataSourceSurface::MapType::READ, &ms);
   nsRefPtr<gfxImageSurface> wrappedDest =
     new gfxImageSurface(ms.mData,
                         ThebesIntSize(dest->GetSize()),
                         ms.mStride,
                         SurfaceFormatToImageFormat(dest->GetFormat()));
   DeprecatedReadback(src, wrappedDest);
+  dest->Unmap();
 }
 
 void
 GLScreenBuffer::DeprecatedReadback(SharedSurface_GL* src, gfxImageSurface* dest)
 {
     MOZ_ASSERT(src && dest);
     MOZ_ASSERT(ToIntSize(dest->GetSize()) == src->Size());
     MOZ_ASSERT(dest->Format() == (src->HasAlpha() ? gfxImageFormat::ARGB32
--- a/gfx/layers/GrallocImages.cpp
+++ b/gfx/layers/GrallocImages.cpp
@@ -348,39 +348,45 @@ GrallocImage::GetAsSourceSurface()
     uint8_t* buffer_as_bytes = static_cast<uint8_t*>(buffer);
     ConvertYVU420SPToRGB565(buffer, alignedWidth,
                             buffer_as_bytes + uvOffset, uvStride,
                             mappedSurface.mData,
                             width, height);
 
     surface->Unmap();
     return surface;
-  } else if (format == HAL_PIXEL_FORMAT_YCrCb_420_SP) {
+  }
+
+  if (format == HAL_PIXEL_FORMAT_YCrCb_420_SP) {
     uint32_t uvOffset = height * width;
     ConvertYVU420SPToRGB565(buffer, width,
                             buffer + uvOffset, width,
                             mappedSurface.mData,
                             width, height);
 
     surface->Unmap();
     return surface;
-  } else if (format == HAL_PIXEL_FORMAT_YV12) {
+  }
+
+  if (format == HAL_PIXEL_FORMAT_YV12) {
     gfx::ConvertYCbCrToRGB(mData,
                            surface->GetFormat(),
                            mSize,
                            surface->GetData(),
                            surface->Stride());
+    surface->Unmap();
     return surface;
   }
 
   android::ColorConverter colorConverter((OMX_COLOR_FORMATTYPE)omxFormat,
                                          OMX_COLOR_Format16bitRGB565);
 
   if (!colorConverter.isValid()) {
     NS_WARNING("Invalid color conversion");
+    surface->Unmap();
     return nullptr;
   }
 
   rv = colorConverter.convert(buffer, width, height,
                               0, 0, width - 1, height - 1 /* source crop */,
                               mappedSurface.mData, width, height,
                               0, 0, width - 1, height - 1 /* dest crop */);
 
--- a/image/src/imgTools.cpp
+++ b/image/src/imgTools.cpp
@@ -129,32 +129,32 @@ static nsresult EncodeImageData(DataSour
   nsAutoCString encoderCID(
     NS_LITERAL_CSTRING("@mozilla.org/image/encoder;2?type=") + aMimeType);
 
   nsCOMPtr<imgIEncoder> encoder = do_CreateInstance(encoderCID.get());
   if (!encoder)
     return NS_IMAGELIB_ERROR_NO_ENCODER;
 
   DataSourceSurface::MappedSurface map;
-  aDataSurface->Map(DataSourceSurface::MapType::READ, &map);
-  if (!map.mData)
+  if (!aDataSurface->Map(DataSourceSurface::MapType::READ, &map)) {
     return NS_ERROR_FAILURE;
+  }
 
   IntSize size = aDataSurface->GetSize();
   uint32_t dataLength = map.mStride * size.height;
 
   // Encode the bitmap
   nsresult rv = encoder->InitFromData(map.mData,
                                       dataLength,
                                       size.width,
                                       size.height,
                                       map.mStride,
                                       imgIEncoder::INPUT_FORMAT_HOSTARGB,
                                       aOutputOptions);
-
+  aDataSurface->Unmap();
   NS_ENSURE_SUCCESS(rv, rv);
 
   return CallQueryInterface(encoder, aStream);
 }
 
 NS_IMETHODIMP imgTools::EncodeImage(imgIContainer *aContainer,
                                     const nsACString& aMimeType,
                                     const nsAString& aOutputOptions,
@@ -199,32 +199,34 @@ NS_IMETHODIMP imgTools::EncodeScaledImag
   } else if (aScaledHeight == 0) {
     aScaledHeight = frameHeight;
   }
 
   RefPtr<DataSourceSurface> dataSurface =
     Factory::CreateDataSourceSurface(IntSize(aScaledWidth, aScaledHeight),
                                      SurfaceFormat::B8G8R8A8);
   DataSourceSurface::MappedSurface map;
-  dataSurface->Map(DataSourceSurface::MapType::WRITE, &map);
-  if (!map.mData)
+  if (!dataSurface->Map(DataSourceSurface::MapType::WRITE, &map)) {
     return NS_ERROR_FAILURE;
+  }
 
   RefPtr<DrawTarget> dt =
     Factory::CreateDrawTargetForData(BackendType::CAIRO,
                                      map.mData,
                                      dataSurface->GetSize(),
                                      map.mStride,
                                      SurfaceFormat::B8G8R8A8);
   dt->DrawSurface(frame,
                   Rect(0, 0, aScaledWidth, aScaledHeight),
                   Rect(0, 0, frameWidth, frameHeight),
                   DrawSurfaceOptions(),
                   DrawOptions(1.0f, CompositionOp::OP_SOURCE));
 
+  dataSurface->Unmap();
+
   return EncodeImageData(dataSurface, aMimeType, aOutputOptions, aStream);
 }
 
 NS_IMETHODIMP imgTools::EncodeCroppedImage(imgIContainer *aContainer,
                                            const nsACString& aMimeType,
                                            int32_t aOffsetX,
                                            int32_t aOffsetY,
                                            int32_t aWidth,
@@ -262,30 +264,32 @@ NS_IMETHODIMP imgTools::EncodeCroppedIma
   // Check that the given crop rectangle is within image bounds.
   NS_ENSURE_ARG(frameWidth >= aOffsetX + aWidth &&
                 frameHeight >= aOffsetY + aHeight);
 
   RefPtr<DataSourceSurface> dataSurface =
     Factory::CreateDataSourceSurface(IntSize(aWidth, aHeight),
                                      SurfaceFormat::B8G8R8A8);
   DataSourceSurface::MappedSurface map;
-  dataSurface->Map(DataSourceSurface::MapType::WRITE, &map);
-  if (!map.mData)
+  if (!dataSurface->Map(DataSourceSurface::MapType::WRITE, &map)) {
     return NS_ERROR_FAILURE;
+  }
 
   RefPtr<DrawTarget> dt =
     Factory::CreateDrawTargetForData(BackendType::CAIRO,
                                      map.mData,
                                      dataSurface->GetSize(),
                                      map.mStride,
                                      SurfaceFormat::B8G8R8A8);
   dt->CopySurface(frame,
                   IntRect(aOffsetX, aOffsetY, aWidth, aHeight),
                   IntPoint(0, 0));
 
+  dataSurface->Unmap();
+
   return EncodeImageData(dataSurface, aMimeType, aOutputOptions, aStream);
 }
 
 NS_IMETHODIMP imgTools::CreateScriptedObserver(imgIScriptedNotificationObserver* aInner,
                                                imgINotificationObserver** aObserver)
 {
   NS_ADDREF(*aObserver = new ScriptedNotificationObserver(aInner));
   return NS_OK;
--- a/widget/qt/nsClipboard.cpp
+++ b/widget/qt/nsClipboard.cpp
@@ -192,26 +192,27 @@ nsClipboard::SetNativeClipboardData( nsI
                   continue;
 
                 RefPtr<DataSourceSurface> dataSurface =
                   surface->GetDataSurface();
                 if (!dataSurface)
                   continue;
 
                 DataSourceSurface::MappedSurface map;
-                dataSurface->Map(DataSourceSurface::MapType::READ, &map);
-                if (!map.mData)
+                if (!dataSurface->Map(DataSourceSurface::MapType::READ, &map))
                   continue;
 
                 QImage qImage(map.mData,
                               dataSurface->GetSize().width,
                               dataSurface->GetSize().height,
                               map.mStride,
                               _moz2dformat_to_qformat(dataSurface->GetFormat()));
 
+                dataSurface->Unmap();
+
                 // Add image to the mimeData
                 mimeData->setImageData(qImage);
                 imageAdded = true;
             }
 
             // Other flavors, adding data to clipboard "as is"
             else
             {