Return unique_ptrs from CFX_DIBitmap::Clone().

core/fxge/dib/fx_dib_main.cpp

 // Copyright 2014 PDFium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
 
 #include "core/fxge/fx_dib.h"
 
 #include <limits.h>
 
 #include <algorithm>
 #include <memory>
 #include <utility>
 
 #include "core/fxcodec/fx_codec.h"
+#include "core/fxcrt/cfx_maybe_owned.h"
 #include "core/fxge/cfx_gemodule.h"
 #include "core/fxge/dib/dib_int.h"
 #include "core/fxge/ge/cfx_cliprgn.h"
 #include "third_party/base/ptr_util.h"
 
 void CmykDecode(uint32_t cmyk, int& c, int& m, int& y, int& k) {
   c = FXSYS_GetCValue(cmyk);
   m = FXSYS_GetMValue(cmyk);
   y = FXSYS_GetYValue(cmyk);
   k = FXSYS_GetKValue(cmyk);
@@ skipping 141 matching lines @@
   pSrcBitmap->m_pBuffer = nullptr;
   pSrcBitmap->m_pAlphaMask = nullptr;
   m_bpp = pSrcBitmap->m_bpp;
   m_bExtBuf = pSrcBitmap->m_bExtBuf;
   m_AlphaFlag = pSrcBitmap->m_AlphaFlag;
   m_Width = pSrcBitmap->m_Width;
   m_Height = pSrcBitmap->m_Height;
   m_Pitch = pSrcBitmap->m_Pitch;
 }
 
-CFX_DIBitmap* CFX_DIBSource::Clone(const FX_RECT* pClip) const {
+std::unique_ptr<CFX_DIBitmap> CFX_DIBSource::Clone(const FX_RECT* pClip) const {
   FX_RECT rect(0, 0, m_Width, m_Height);
   if (pClip) {
     rect.Intersect(*pClip);
-    if (rect.IsEmpty()) {
+    if (rect.IsEmpty())
       return nullptr;
-    }
   }
-  std::unique_ptr<CFX_DIBitmap> pNewBitmap(new CFX_DIBitmap);
+  auto pNewBitmap = pdfium::MakeUnique<CFX_DIBitmap>();
   if (!pNewBitmap->Create(rect.Width(), rect.Height(), GetFormat()))
     return nullptr;
 
   pNewBitmap->CopyPalette(m_pPalette.get());
   pNewBitmap->CopyAlphaMask(m_pAlphaMask, pClip);
   if (GetBPP() == 1 && rect.left % 8 != 0) {
     int left_shift = rect.left % 32;
     int right_shift = 32 - left_shift;
     int dword_count = pNewBitmap->m_Pitch / 4;
     for (int row = rect.top; row < rect.bottom; row++) {
       uint32_t* src_scan = (uint32_t*)GetScanline(row) + rect.left / 32;
       uint32_t* dest_scan = (uint32_t*)pNewBitmap->GetScanline(row - rect.top);
       for (int i = 0; i < dword_count; i++) {
         dest_scan[i] =
             (src_scan[i] << left_shift) | (src_scan[i + 1] >> right_shift);
       }
     }
   } else {
     int copy_len = (pNewBitmap->GetWidth() * pNewBitmap->GetBPP() + 7) / 8;
     if (m_Pitch < (uint32_t)copy_len) {

[npm, 2016/12/05 20:37:23]

Nit: {}

[Tom Sepez, 2016/12/05 21:38:13]

Done.

       copy_len = m_Pitch;
     }
     for (int row = rect.top; row < rect.bottom; row++) {
       const uint8_t* src_scan = GetScanline(row) + rect.left * m_bpp / 8;
       uint8_t* dest_scan = (uint8_t*)pNewBitmap->GetScanline(row - rect.top);
       FXSYS_memcpy(dest_scan, src_scan, copy_len);
     }
   }
-  return pNewBitmap.release();
+  return pNewBitmap;
 }
 
 void CFX_DIBSource::BuildPalette() {
-  if (m_pPalette) {
+  if (m_pPalette)
     return;
-  }
+
   if (GetBPP() == 1) {
     m_pPalette.reset(FX_Alloc(uint32_t, 2));
     if (IsCmykImage()) {
       m_pPalette.get()[0] = 0xff;
       m_pPalette.get()[1] = 0;
     } else {
       m_pPalette.get()[0] = 0xff000000;
       m_pPalette.get()[1] = 0xffffffff;
     }
   } else if (GetBPP() == 8) {
     m_pPalette.reset(FX_Alloc(uint32_t, 256));
     if (IsCmykImage()) {
-      for (int i = 0; i < 256; i++) {
+      for (int i = 0; i < 256; i++)
         m_pPalette.get()[i] = 0xff - i;
-      }
     } else {
-      for (int i = 0; i < 256; i++) {
+      for (int i = 0; i < 256; i++)
         m_pPalette.get()[i] = 0xff000000 | (i * 0x10101);
-      }
     }
   }
 }
 
 bool CFX_DIBSource::BuildAlphaMask() {
   if (m_pAlphaMask) {
     return true;
   }
   m_pAlphaMask = new CFX_DIBitmap;
   if (!m_pAlphaMask->Create(m_Width, m_Height, FXDIB_8bppMask)) {
@@ skipping 408 matching lines @@
   } else {
     m_pAlphaMask->Clear(0xff000000);
   }
   return true;
 }
 
 const int g_ChannelOffset[] = {0, 2, 1, 0, 0, 1, 2, 3, 3};
 bool CFX_DIBitmap::LoadChannel(FXDIB_Channel destChannel,
                                const CFX_DIBSource* pSrcBitmap,
                                FXDIB_Channel srcChannel) {
-  if (!m_pBuffer) {
+  if (!m_pBuffer)
     return false;
-  }
-  CFX_DIBSource* pSrcClone = (CFX_DIBSource*)pSrcBitmap;
-  CFX_DIBitmap* pDst = this;
-  int destOffset, srcOffset;
+
+  CFX_MaybeOwned<CFX_DIBSource> pSrcClone(
+      const_cast<CFX_DIBSource*>(pSrcBitmap));

[dsinclair, 2016/12/05 20:41:26]

Should we remove the const from the signature?

[Tom Sepez, 2016/12/05 21:38:13]

Ok. Not too many ripples ...

+  int srcOffset;
   if (srcChannel == FXDIB_Alpha) {
-    if (!pSrcBitmap->HasAlpha() && !pSrcBitmap->IsAlphaMask()) {
+    if (!pSrcBitmap->HasAlpha() && !pSrcBitmap->IsAlphaMask())
       return false;
-    }
+
     if (pSrcBitmap->GetBPP() == 1) {
       pSrcClone = pSrcBitmap->CloneConvert(FXDIB_8bppMask);
-      if (!pSrcClone) {
+      if (!pSrcClone)
         return false;
-      }
     }
-    if (pSrcBitmap->GetFormat() == FXDIB_Argb) {
-      srcOffset = 3;
-    } else {
-      srcOffset = 0;
-    }
+    srcOffset = pSrcBitmap->GetFormat() == FXDIB_Argb ? 3 : 0;
   } else {
-    if (pSrcBitmap->IsAlphaMask()) {
+    if (pSrcBitmap->IsAlphaMask())
       return false;
-    }
+
     if (pSrcBitmap->GetBPP() < 24) {
       if (pSrcBitmap->IsCmykImage()) {
         pSrcClone = pSrcBitmap->CloneConvert(
             (FXDIB_Format)((pSrcBitmap->GetFormat() & 0xff00) | 0x20));

[dsinclair, 2016/12/05 20:41:26]

static_cast? and below

[Tom Sepez, 2016/12/05 21:38:13]

Done.

       } else {
         pSrcClone = pSrcBitmap->CloneConvert(
             (FXDIB_Format)((pSrcBitmap->GetFormat() & 0xff00) | 0x18));
       }
-      if (!pSrcClone) {
+      if (!pSrcClone)
         return false;
-      }
     }
     srcOffset = g_ChannelOffset[srcChannel];
   }
+  int destOffset = 0;
   if (destChannel == FXDIB_Alpha) {
     if (IsAlphaMask()) {
-      if (!ConvertFormat(FXDIB_8bppMask)) {
-        if (pSrcClone != pSrcBitmap) {
-          delete pSrcClone;
-        }
+      if (!ConvertFormat(FXDIB_8bppMask))
         return false;
-      }
-      destOffset = 0;
     } else {
-      destOffset = 0;
-      if (!ConvertFormat(IsCmykImage() ? FXDIB_Cmyka : FXDIB_Argb)) {
-        if (pSrcClone != pSrcBitmap) {
-          delete pSrcClone;
-        }
+      if (!ConvertFormat(IsCmykImage() ? FXDIB_Cmyka : FXDIB_Argb))
         return false;
-      }
-      if (GetFormat() == FXDIB_Argb) {
+
+      if (GetFormat() == FXDIB_Argb)
         destOffset = 3;
-      }
     }
   } else {
-    if (IsAlphaMask()) {
-      if (pSrcClone != pSrcBitmap) {
-        delete pSrcClone;
-      }
+    if (IsAlphaMask())
       return false;
-    }
+
     if (GetBPP() < 24) {
       if (HasAlpha()) {
-        if (!ConvertFormat(IsCmykImage() ? FXDIB_Cmyka : FXDIB_Argb)) {
-          if (pSrcClone != pSrcBitmap) {
-            delete pSrcClone;
-          }
+        if (!ConvertFormat(IsCmykImage() ? FXDIB_Cmyka : FXDIB_Argb))
           return false;
-        }
 #if _FXM_PLATFORM_ == _FXM_PLATFORM_APPLE_
       } else if (!ConvertFormat(IsCmykImage() ? FXDIB_Cmyk : FXDIB_Rgb32)) {
 #else
       } else if (!ConvertFormat(IsCmykImage() ? FXDIB_Cmyk : FXDIB_Rgb)) {
 #endif
-        if (pSrcClone != pSrcBitmap) {
-          delete pSrcClone;
-        }
         return false;
       }
     }
     destOffset = g_ChannelOffset[destChannel];
   }
   if (srcChannel == FXDIB_Alpha && pSrcClone->m_pAlphaMask) {
-    CFX_DIBitmap* pAlphaMask = pSrcClone->m_pAlphaMask;
+    CFX_MaybeOwned<CFX_DIBSource> pAlphaMask(pSrcClone->m_pAlphaMask);
     if (pSrcClone->GetWidth() != m_Width ||
         pSrcClone->GetHeight() != m_Height) {
       if (pAlphaMask) {
         pAlphaMask = pAlphaMask->StretchTo(m_Width, m_Height);
-        if (!pAlphaMask) {
-          if (pSrcClone != pSrcBitmap) {
-            delete pSrcClone;
-          }
+        if (!pAlphaMask)
           return false;
-        }
       }
     }
-    if (pSrcClone != pSrcBitmap) {
-      pSrcClone->m_pAlphaMask = nullptr;
-      delete pSrcClone;
-    }
-    pSrcClone = pAlphaMask;
+    pSrcClone = std::move(pAlphaMask);
     srcOffset = 0;
   } else if (pSrcClone->GetWidth() != m_Width ||
              pSrcClone->GetHeight() != m_Height) {
-    CFX_DIBitmap* pSrcMatched = pSrcClone->StretchTo(m_Width, m_Height);
-    if (pSrcClone != pSrcBitmap) {
-      delete pSrcClone;
-    }
-    if (!pSrcMatched) {
+    std::unique_ptr<CFX_DIBitmap> pSrcMatched =
+        pSrcClone->StretchTo(m_Width, m_Height);
+    if (!pSrcMatched)
       return false;
-    }
-    pSrcClone = pSrcMatched;
+
+    pSrcClone = std::move(pSrcMatched);
   }
+  CFX_DIBitmap* pDst = this;
   if (destChannel == FXDIB_Alpha && m_pAlphaMask) {
     pDst = m_pAlphaMask;
     destOffset = 0;
   }
   int srcBytes = pSrcClone->GetBPP() / 8;
   int destBytes = pDst->GetBPP() / 8;
   for (int row = 0; row < m_Height; row++) {
     uint8_t* dest_pos = (uint8_t*)pDst->GetScanline(row) + destOffset;
     const uint8_t* src_pos = pSrcClone->GetScanline(row) + srcOffset;
     for (int col = 0; col < m_Width; col++) {
       *dest_pos = *src_pos;
       dest_pos += destBytes;
       src_pos += srcBytes;
     }
   }
-  if (pSrcClone != pSrcBitmap && pSrcClone != pSrcBitmap->m_pAlphaMask) {
-    delete pSrcClone;
-  }
   return true;
 }
 
 bool CFX_DIBitmap::LoadChannel(FXDIB_Channel destChannel, int value) {
   if (!m_pBuffer) {
     return false;
   }
   int destOffset;
   if (destChannel == FXDIB_Alpha) {
     if (IsAlphaMask()) {
@@ skipping 43 matching lines @@
     uint8_t* scan_line = m_pBuffer + row * m_Pitch + destOffset;
     for (int col = 0; col < m_Width; col++) {
       *scan_line = value;
       scan_line += Bpp;
     }
   }
   return true;
 }
 
 bool CFX_DIBitmap::MultiplyAlpha(const CFX_DIBSource* pSrcBitmap) {
-  if (!m_pBuffer) {
+  if (!m_pBuffer)
     return false;
-  }
+
   ASSERT(pSrcBitmap->IsAlphaMask());
-  if (!pSrcBitmap->IsAlphaMask()) {
+  if (!pSrcBitmap->IsAlphaMask())
     return false;
-  }
-  if (!IsAlphaMask() && !HasAlpha()) {
+
+  if (!IsAlphaMask() && !HasAlpha())
     return LoadChannel(FXDIB_Alpha, pSrcBitmap, FXDIB_Alpha);
-  }
-  CFX_DIBitmap* pSrcClone = (CFX_DIBitmap*)pSrcBitmap;
+
+  CFX_MaybeOwned<CFX_DIBitmap> pSrcClone(
+      static_cast<CFX_DIBitmap*>(const_cast<CFX_DIBSource*>(pSrcBitmap)));

[npm, 2016/12/05 20:37:23]

Why 2 casts?

[Tom Sepez, 2016/12/05 21:38:13]

Because a const_cast can't change type, and a static_cast can't change
const-ness, and reinterpret_cast is dangerous as replacement.

   if (pSrcBitmap->GetWidth() != m_Width ||
       pSrcBitmap->GetHeight() != m_Height) {
     pSrcClone = pSrcBitmap->StretchTo(m_Width, m_Height);
-    if (!pSrcClone) {
+    if (!pSrcClone)
       return false;
-    }
   }
   if (IsAlphaMask()) {
-    if (!ConvertFormat(FXDIB_8bppMask)) {
-      if (pSrcClone != pSrcBitmap) {
-        delete pSrcClone;
-      }
+    if (!ConvertFormat(FXDIB_8bppMask))
       return false;
-    }
+
     for (int row = 0; row < m_Height; row++) {
       uint8_t* dest_scan = m_pBuffer + m_Pitch * row;
       uint8_t* src_scan = pSrcClone->m_pBuffer + pSrcClone->m_Pitch * row;
       if (pSrcClone->GetBPP() == 1) {
         for (int col = 0; col < m_Width; col++) {
           if (!((1 << (7 - col % 8)) & src_scan[col / 8])) {

[npm, 2016/12/05 20:37:23]

Nit: {}

[Tom Sepez, 2016/12/05 21:38:13]

Done.

             dest_scan[col] = 0;
           }
         }
       } else {
         for (int col = 0; col < m_Width; col++) {
           *dest_scan = (*dest_scan) * src_scan[col] / 255;
           dest_scan++;
         }
       }
     }
   } else {
     if (GetFormat() == FXDIB_Argb) {
-      if (pSrcClone->GetBPP() == 1) {
-        if (pSrcClone != pSrcBitmap) {
-          delete pSrcClone;
-        }
+      if (pSrcClone->GetBPP() == 1)
         return false;
-      }
+
       for (int row = 0; row < m_Height; row++) {
         uint8_t* dest_scan = m_pBuffer + m_Pitch * row + 3;
         uint8_t* src_scan = pSrcClone->m_pBuffer + pSrcClone->m_Pitch * row;
         for (int col = 0; col < m_Width; col++) {
           *dest_scan = (*dest_scan) * src_scan[col] / 255;
           dest_scan += 4;
         }
       }
     } else {
-      m_pAlphaMask->MultiplyAlpha(pSrcClone);
+      m_pAlphaMask->MultiplyAlpha(pSrcClone.Get());
     }
   }
-  if (pSrcClone != pSrcBitmap) {
-    delete pSrcClone;
-  }
   return true;
 }
 
 bool CFX_DIBitmap::GetGrayData(void* pIccTransform) {
   if (!m_pBuffer) {
     return false;
   }
   switch (GetFormat()) {
     case FXDIB_1bppRgb: {
       if (!m_pPalette) {
@@ skipping 527 matching lines @@
   if (pSrc->GetBuffer()) {
     m_pBitmap = pdfium::MakeUnique<CFX_DIBitmap>();
     if (!m_pBitmap->Create(pSrc->GetWidth(), pSrc->GetHeight(),
                            pSrc->GetFormat(), pSrc->GetBuffer())) {
       m_pBitmap.reset();
       return;
     }
     m_pBitmap->CopyPalette(pSrc->GetPalette());
     m_pBitmap->CopyAlphaMask(pSrc->m_pAlphaMask);
   } else {
-    m_pBitmap.reset(pSrc->Clone());
+    m_pBitmap = pSrc->Clone();
   }
 }
 
 CFX_DIBExtractor::~CFX_DIBExtractor() {}
 
 CFX_FilteredDIB::CFX_FilteredDIB() : m_pSrc(nullptr) {}
 
 CFX_FilteredDIB::~CFX_FilteredDIB() {
   if (m_bAutoDropSrc) {
     delete m_pSrc;
@@ skipping 198 matching lines @@
                                uint32_t* pSrcPalette) {
   m_pBitmap = pdfium::MakeUnique<CFX_DIBitmap>();
   if (!m_pBitmap->Create(width, height, src_format)) {
     m_pBitmap.reset();
     return false;
   }
   if (pSrcPalette)
     m_pBitmap->CopyPalette(pSrcPalette);
   return true;
 }