assert in BitmapInfoAccess if bitmap is empty or we can't read from it

and fixup the "make unique if two things want to write to bitmap at same
time" check, it was using maBitmap before maBitmap has been assigned
to.
Lets just make it check something more useful, like the share count of
the underlying SalBitmap.

Change-Id: I97a629457174da2e4df1edc9674290aa9c9a2b52
Reviewed-on: https://gerrit.libreoffice.org/55416Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>

emfio/source/reader/wmfreader.cxx

@@ -805,9 +805,9 @@ namespace emfio
                 mpInputStream->ReadUInt16( nFunction ).ReadUInt16( nFunction );
 
                 ReadDIB(aBmp, *mpInputStream, false);
-                Bitmap::ScopedReadAccess pBmp(aBmp);
-                if ( pBmp )
+                if ( !!aBmp )
                 {
+                    Bitmap::ScopedReadAccess pBmp(aBmp);
                     for ( long y = 0; y < pBmp->Height(); y++ )
                     {
                         for ( long x = 0; x < pBmp->Width(); x++ )
@@ -822,7 +822,6 @@ namespace emfio
                     nCount = pBmp->Height() * pBmp->Width();
                     if ( !nCount )
                         nCount++;
-                    pBmp.reset();
                 }
                 Color aColor( static_cast<sal_uInt8>( nRed / nCount ), static_cast<sal_uInt8>( nGreen / nCount ), static_cast<sal_uInt8>( nBlue / nCount ) );
                 CreateObject(o3tl::make_unique<WinMtfFillStyle>( aColor, false ));

vcl/source/gdi/bmpacc.cxx

@@ -34,38 +34,35 @@ BitmapInfoAccess::BitmapInfoAccess( Bitmap& rBitmap, BitmapAccessMode nMode ) :
 {
     std::shared_ptr<SalBitmap> xImpBmp = rBitmap.ImplGetSalBitmap();
 
-    SAL_WARN_IF( !xImpBmp, "vcl", "Forbidden Access to empty bitmap!" );
+    assert( xImpBmp && "Forbidden Access to empty bitmap!" );
 
-    if( xImpBmp )
+    if( !xImpBmp )
+        return;
+
+    if( mnAccessMode == BitmapAccessMode::Write && xImpBmp.use_count() > 2 )
     {
-        if( mnAccessMode == BitmapAccessMode::Write && !maBitmap.ImplGetSalBitmap() )
-        {
-            xImpBmp.reset();
-            rBitmap.ImplMakeUnique();
-            xImpBmp = rBitmap.ImplGetSalBitmap();
-        }
-        else
-        {
-            DBG_ASSERT( mnAccessMode != BitmapAccessMode::Write ||
-                        xImpBmp.use_count() == 2,
-                        "Unpredictable results: bitmap is referenced more than once!" );
-        }
+        SAL_WARN( "vcl", "two code paths trying to write to same underlying Bitmap" );
+        xImpBmp.reset();
+        rBitmap.ImplMakeUnique();
+        xImpBmp = rBitmap.ImplGetSalBitmap();
+    }
 
-        mpBuffer = xImpBmp->AcquireBuffer( mnAccessMode );
+    mpBuffer = xImpBmp->AcquireBuffer( mnAccessMode );
 
-        if( !mpBuffer )
+    if( !mpBuffer )
+    {
+        std::shared_ptr<SalBitmap> xNewImpBmp(ImplGetSVData()->mpDefInst->CreateSalBitmap());
+        if (xNewImpBmp->Create(*xImpBmp, rBitmap.GetBitCount()))
         {
-            std::shared_ptr<SalBitmap> xNewImpBmp(ImplGetSVData()->mpDefInst->CreateSalBitmap());
-            if (xNewImpBmp->Create(*xImpBmp, rBitmap.GetBitCount()))
-            {
-                xImpBmp = xNewImpBmp;
-                rBitmap.ImplSetSalBitmap( xImpBmp );
-                mpBuffer = xImpBmp->AcquireBuffer( mnAccessMode );
-            }
+            xImpBmp = xNewImpBmp;
+            rBitmap.ImplSetSalBitmap( xImpBmp );
+            mpBuffer = xImpBmp->AcquireBuffer( mnAccessMode );
         }
-
-        maBitmap = rBitmap;
     }
+
+    assert(mpBuffer);
+
+    maBitmap = rBitmap;
 }
 
 BitmapInfoAccess::~BitmapInfoAccess()
@@ -75,7 +72,6 @@ BitmapInfoAccess::~BitmapInfoAccess()
     if (mpBuffer && xImpBmp)
     {
         xImpBmp->ReleaseBuffer( mpBuffer, mnAccessMode );
-        mpBuffer = nullptr;
     }
 }