Clean up some image decoder classes:

- Use std::vector<uint8_t> instead of raw uint8_t*
- Make ICodec_ScanlineDecoder::GetScanline() return const uint8_t*
- Add FxFreeDeleter, use it in CCodec_ImageDataCache.
- Make CCodec_ImageDataCache encapsulate its data members.

R=tsepez@chromium.org

Review URL: https://codereview.chromium.org/1361053002 .

3 files changed, 160 insertions, 126 deletions

diff --git a/core/src/fxcodec/codec/codec_int.h b/core/src/fxcodec/codec/codec_int.h
index 125f36c9eb..682b5d0dbb 100644
--- a/core/src/fxcodec/codec/codec_int.h
+++ b/core/src/fxcodec/codec/codec_int.h
@@ -11,6 +11,7 @@
 #include <list>
 #include <map>
 
+#include "../../../../third_party/base/nonstd_unique_ptr.h"
 #include "../../../../third_party/libopenjpeg20/openjpeg.h"  // For OPJ_SIZE_T.
 #include "../../../include/fxcodec/fx_codec.h"
 #include "../jbig2/JBig2_Context.h"
@@ -36,12 +37,6 @@ class CCodec_BasicModule : public ICodec_BasicModule {
                                                          int bpc);
 };
 
-struct CCodec_ImageDataCache {
-  int m_Width, m_Height;
-  int m_nCachedLines;
-  uint8_t m_Data;
-};
-
 class CCodec_ScanlineDecoder : public ICodec_ScanlineDecoder {
  public:
   CCodec_ScanlineDecoder();
@@ -50,50 +45,58 @@ class CCodec_ScanlineDecoder : public ICodec_ScanlineDecoder {
   // ICodec_ScanlineDecoder
   FX_DWORD GetSrcOffset() override { return -1; }
   void DownScale(int dest_width, int dest_height) override;
-  uint8_t* GetScanline(int line) override;
+  const uint8_t* GetScanline(int line) override;
   FX_BOOL SkipToScanline(int line, IFX_Pause* pPause) override;
   int GetWidth() override { return m_OutputWidth; }
   int GetHeight() override { return m_OutputHeight; }
   int CountComps() override { return m_nComps; }
   int GetBPC() override { return m_bpc; }
   FX_BOOL IsColorTransformed() override { return m_bColorTransformed; }
-  void ClearImageData() override {
-    FX_Free(m_pDataCache);
-    m_pDataCache = NULL;
-  }
+  void ClearImageData() override { m_pDataCache.reset(); }
 
  protected:
-  int m_OrigWidth;
+  class ImageDataCache {
+   public:
+    ImageDataCache(int width, int height, int pitch);
+    ~ImageDataCache();
+
+    bool AllocateCache();
+    void AppendLine(const uint8_t* line);
+
+    int NumLines() const { return m_nCachedLines; }
+    const uint8_t* GetLine(int line) const;
+    bool IsSameDimensions(int width, int height) const {
+      return width == m_Width && height == m_Height;
+    }
+
+   private:
+    bool IsValid() const { return m_Data.get() != nullptr; }
+
+    const int m_Width;
+    const int m_Height;
+    const int m_Pitch;
+    int m_nCachedLines;
+    nonstd::unique_ptr<uint8_t, FxFreeDeleter> m_Data;
+  };
 
-  int m_OrigHeight;
+  virtual FX_BOOL v_Rewind() = 0;
+  virtual uint8_t* v_GetNextLine() = 0;
+  virtual void v_DownScale(int dest_width, int dest_height) = 0;
 
-  int m_DownScale;
+  uint8_t* ReadNextLine();
 
+  int m_OrigWidth;
+  int m_OrigHeight;
+  int m_DownScale;
   int m_OutputWidth;
-
   int m_OutputHeight;
-
   int m_nComps;
-
   int m_bpc;
-
   int m_Pitch;
-
   FX_BOOL m_bColorTransformed;
-
-  uint8_t* ReadNextLine();
-
-  virtual FX_BOOL v_Rewind() = 0;
-
-  virtual uint8_t* v_GetNextLine() = 0;
-
-  virtual void v_DownScale(int dest_width, int dest_height) = 0;
-
   int m_NextLine;
-
   uint8_t* m_pLastScanline;
-
-  CCodec_ImageDataCache* m_pDataCache;
+  nonstd::unique_ptr<ImageDataCache> m_pDataCache;
 };
 
 class CCodec_FaxModule : public ICodec_FaxModule {
@@ -257,10 +260,10 @@ class CCodec_JpxModule : public ICodec_JpxModule {
                     FX_DWORD* width,
                     FX_DWORD* height,
                     FX_DWORD* components) override;
-  FX_BOOL Decode(CJPX_Decoder* pDecoder,
-                 uint8_t* dest_data,
-                 int pitch,
-                 uint8_t* offsets) override;
+  bool Decode(CJPX_Decoder* pDecoder,
+              uint8_t* dest_data,
+              int pitch,
+              const std::vector<uint8_t>& offsets) override;
   void DestroyDecoder(CJPX_Decoder* pDecoder) override;
 };
 
diff --git a/core/src/fxcodec/codec/fx_codec.cpp b/core/src/fxcodec/codec/fx_codec.cpp
index 0d89ea6e98..46f479e0b1 100644
--- a/core/src/fxcodec/codec/fx_codec.cpp
+++ b/core/src/fxcodec/codec/fx_codec.cpp
@@ -5,7 +5,13 @@
 // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
 
 #include "../../../include/fxcodec/fx_codec.h"
+
+#include <cmath>
+
+#include "../../../../third_party/base/logging.h"
+#include "../../../include/fxcrt/fx_safe_types.h"
 #include "codec_int.h"
+
 CCodec_ModuleMgr::CCodec_ModuleMgr()
     : m_pBasicModule(new CCodec_BasicModule),
       m_pFaxModule(new CCodec_FaxModule),
@@ -14,25 +20,66 @@ CCodec_ModuleMgr::CCodec_ModuleMgr()
       m_pJbig2Module(new CCodec_Jbig2Module),
       m_pIccModule(new CCodec_IccModule),
       m_pFlateModule(new CCodec_FlateModule) {}
-CCodec_ScanlineDecoder::CCodec_ScanlineDecoder() {
-  m_NextLine = -1;
-  m_pDataCache = NULL;
-  m_pLastScanline = NULL;
+
+CCodec_ScanlineDecoder::ImageDataCache::ImageDataCache(int width,
+                                                       int height,
+                                                       int pitch)
+    : m_Width(width), m_Height(height), m_Pitch(pitch), m_nCachedLines(0) {
 }
-CCodec_ScanlineDecoder::~CCodec_ScanlineDecoder() {
-  FX_Free(m_pDataCache);
+
+CCodec_ScanlineDecoder::ImageDataCache::~ImageDataCache() {
 }
-uint8_t* CCodec_ScanlineDecoder::GetScanline(int line) {
-  if (m_pDataCache && line < m_pDataCache->m_nCachedLines) {
-    return &m_pDataCache->m_Data + line * m_Pitch;
+
+bool CCodec_ScanlineDecoder::ImageDataCache::AllocateCache() {
+  if (m_Pitch <= 0 || m_Height < 0)
+    return false;
+
+  FX_SAFE_SIZE_T size = m_Pitch;
+  size *= m_Height;
+  if (!size.IsValid())
+    return false;
+
+  m_Data.reset(FX_TryAlloc(uint8_t, size.ValueOrDie()));
+  return IsValid();
+}
+
+void CCodec_ScanlineDecoder::ImageDataCache::AppendLine(const uint8_t* line) {
+  // If the callers adds more lines than there is room, fail.
+  if (m_Pitch <= 0 || m_nCachedLines >= m_Height) {
+    NOTREACHED();
+    return;
   }
-  if (m_NextLine == line + 1) {
+
+  size_t offset = m_Pitch;
+  FXSYS_memcpy(m_Data.get() + offset * m_nCachedLines, line, m_Pitch);
+  ++m_nCachedLines;
+}
+
+const uint8_t* CCodec_ScanlineDecoder::ImageDataCache::GetLine(int line) const {
+  if (m_Pitch <= 0 || line < 0 || line >= m_nCachedLines)
+    return nullptr;
+
+  size_t offset = m_Pitch;
+  return m_Data.get() + offset * line;
+}
+
+CCodec_ScanlineDecoder::CCodec_ScanlineDecoder()
+    : m_NextLine(-1), m_pLastScanline(nullptr) {
+}
+
+CCodec_ScanlineDecoder::~CCodec_ScanlineDecoder() {
+}
+
+const uint8_t* CCodec_ScanlineDecoder::GetScanline(int line) {
+  if (m_pDataCache && line < m_pDataCache->NumLines())
+    return m_pDataCache->GetLine(line);
+
+  if (m_NextLine == line + 1)
     return m_pLastScanline;
-  }
+
   if (m_NextLine < 0 || m_NextLine > line) {
-    if (!v_Rewind()) {
-      return NULL;
-    }
+    if (!v_Rewind())
+      return nullptr;
     m_NextLine = 0;
   }
   while (m_NextLine < line) {
@@ -43,18 +90,19 @@ uint8_t* CCodec_ScanlineDecoder::GetScanline(int line) {
   m_NextLine++;
   return m_pLastScanline;
 }
+
 FX_BOOL CCodec_ScanlineDecoder::SkipToScanline(int line, IFX_Pause* pPause) {
-  if (m_pDataCache && line < m_pDataCache->m_nCachedLines) {
+  if (m_pDataCache && line < m_pDataCache->NumLines())
     return FALSE;
-  }
-  if (m_NextLine == line || m_NextLine == line + 1) {
+
+  if (m_NextLine == line || m_NextLine == line + 1)
     return FALSE;
-  }
+
   if (m_NextLine < 0 || m_NextLine > line) {
     v_Rewind();
     m_NextLine = 0;
   }
-  m_pLastScanline = NULL;
+  m_pLastScanline = nullptr;
   while (m_NextLine < line) {
     m_pLastScanline = ReadNextLine();
     m_NextLine++;
@@ -64,42 +112,35 @@ FX_BOOL CCodec_ScanlineDecoder::SkipToScanline(int line, IFX_Pause* pPause) {
   }
   return FALSE;
 }
+
 uint8_t* CCodec_ScanlineDecoder::ReadNextLine() {
   uint8_t* pLine = v_GetNextLine();
-  if (pLine == NULL) {
-    return NULL;
-  }
-  if (m_pDataCache && m_NextLine == m_pDataCache->m_nCachedLines) {
-    FXSYS_memcpy(&m_pDataCache->m_Data + m_NextLine * m_Pitch, pLine, m_Pitch);
-    m_pDataCache->m_nCachedLines++;
-  }
+  if (!pLine)
+    return nullptr;
+
+  if (m_pDataCache && m_NextLine == m_pDataCache->NumLines())
+    m_pDataCache->AppendLine(pLine);
   return pLine;
 }
+
 void CCodec_ScanlineDecoder::DownScale(int dest_width, int dest_height) {
-  if (dest_width < 0) {
-    dest_width = -dest_width;
-  }
-  if (dest_height < 0) {
-    dest_height = -dest_height;
-  }
+  dest_width = std::abs(dest_width);
+  dest_height = std::abs(dest_height);
   v_DownScale(dest_width, dest_height);
-  if (m_pDataCache) {
-    if (m_pDataCache->m_Height == m_OutputHeight &&
-        m_pDataCache->m_Width == m_OutputWidth) {
-      return;
-    }
-    FX_Free(m_pDataCache);
-    m_pDataCache = NULL;
-  }
-  m_pDataCache = (CCodec_ImageDataCache*)FX_TryAlloc(
-      uint8_t, sizeof(CCodec_ImageDataCache) + m_Pitch * m_OutputHeight);
-  if (m_pDataCache == NULL) {
+
+  if (m_pDataCache &&
+      m_pDataCache->IsSameDimensions(m_OutputWidth, m_OutputHeight)) {
     return;
   }
-  m_pDataCache->m_Height = m_OutputHeight;
-  m_pDataCache->m_Width = m_OutputWidth;
-  m_pDataCache->m_nCachedLines = 0;
+
+  nonstd::unique_ptr<ImageDataCache> cache(
+      new ImageDataCache(m_OutputWidth, m_OutputHeight, m_Pitch));
+  if (!cache->AllocateCache())
+    return;
+
+  m_pDataCache = nonstd::move(cache);
 }
+
 FX_BOOL CCodec_BasicModule::RunLengthEncode(const uint8_t* src_buf,
                                             FX_DWORD src_size,
                                             uint8_t*& dest_buf,
diff --git a/core/src/fxcodec/codec/fx_codec_jpx_opj.cpp b/core/src/fxcodec/codec/fx_codec_jpx_opj.cpp
index b7e032e614..fdffcb66de 100644
--- a/core/src/fxcodec/codec/fx_codec_jpx_opj.cpp
+++ b/core/src/fxcodec/codec/fx_codec_jpx_opj.cpp
@@ -6,6 +6,7 @@
 
 #include <algorithm>
 #include <limits>
+#include <vector>
 
 #include "../../../../third_party/lcms2-2.6/include/lcms2.h"
 #include "../../../../third_party/libopenjpeg20/openjpeg.h"
@@ -617,9 +618,9 @@ class CJPX_Decoder {
   ~CJPX_Decoder();
   FX_BOOL Init(const unsigned char* src_data, int src_size);
   void GetInfo(FX_DWORD* width, FX_DWORD* height, FX_DWORD* components);
-  FX_BOOL Decode(uint8_t* dest_buf,
-                 int pitch,
-                 uint8_t* offsets);
+  bool Decode(uint8_t* dest_buf,
+              int pitch,
+              const std::vector<uint8_t>& offsets);
 
  private:
   const uint8_t* m_SrcData;
@@ -738,45 +739,39 @@ void CJPX_Decoder::GetInfo(FX_DWORD* width,
   *components = (FX_DWORD)image->numcomps;
 }
 
-FX_BOOL CJPX_Decoder::Decode(uint8_t* dest_buf,
-                             int pitch,
-                             uint8_t* offsets) {
-  int i, wid, hei, row, col, channel, src;
-  uint8_t* pChannel;
-  uint8_t* pScanline;
-  uint8_t* pPixel;
+bool CJPX_Decoder::Decode(uint8_t* dest_buf,
+                          int pitch,
+                          const std::vector<uint8_t>& offsets) {
+  if (image->comps[0].w != image->x1 || image->comps[0].h != image->y1)
+    return false;
+
+  if (pitch<(int)(image->comps[0].w * 8 * image->numcomps + 31)>> 5 << 2)
+    return false;
 
-  if (image->comps[0].w != image->x1 || image->comps[0].h != image->y1) {
-    return FALSE;
-  }
-  if (pitch<(int)(image->comps[0].w * 8 * image->numcomps + 31)>> 5 << 2) {
-    return FALSE;
-  }
   FXSYS_memset(dest_buf, 0xff, image->y1 * pitch);
-  uint8_t** channel_bufs = FX_Alloc(uint8_t*, image->numcomps);
-  FX_BOOL result = FALSE;
-  int* adjust_comps = FX_Alloc(int, image->numcomps);
-  for (i = 0; i < (int)image->numcomps; i++) {
+  std::vector<uint8_t*> channel_bufs(image->numcomps);
+  std::vector<int> adjust_comps(image->numcomps);
+  for (uint32_t i = 0; i < image->numcomps; i++) {
     channel_bufs[i] = dest_buf + offsets[i];
     adjust_comps[i] = image->comps[i].prec - 8;
     if (i > 0) {
       if (image->comps[i].dx != image->comps[i - 1].dx ||
           image->comps[i].dy != image->comps[i - 1].dy ||
           image->comps[i].prec != image->comps[i - 1].prec) {
-        goto done;
+        return false;
       }
     }
   }
-  wid = image->comps[0].w;
-  hei = image->comps[0].h;
-  for (channel = 0; channel < (int)image->numcomps; channel++) {
-    pChannel = channel_bufs[channel];
+  int width = image->comps[0].w;
+  int height = image->comps[0].h;
+  for (uint32_t channel = 0; channel < image->numcomps; ++channel) {
+    uint8_t* pChannel = channel_bufs[channel];
     if (adjust_comps[channel] < 0) {
-      for (row = 0; row < hei; row++) {
-        pScanline = pChannel + row * pitch;
-        for (col = 0; col < wid; col++) {
-          pPixel = pScanline + col * image->numcomps;
-          src = image->comps[channel].data[row * wid + col];
+      for (int row = 0; row < height; ++row) {
+        uint8_t* pScanline = pChannel + row * pitch;
+        for (int col = 0; col < width; ++col) {
+          uint8_t* pPixel = pScanline + col * image->numcomps;
+          int src = image->comps[channel].data[row * width + col];
           src += image->comps[channel].sgnd
                      ? 1 << (image->comps[channel].prec - 1)
                      : 0;
@@ -788,14 +783,14 @@ FX_BOOL CJPX_Decoder::Decode(uint8_t* dest_buf,
         }
       }
     } else {
-      for (row = 0; row < hei; row++) {
-        pScanline = pChannel + row * pitch;
-        for (col = 0; col < wid; col++) {
-          pPixel = pScanline + col * image->numcomps;
+      for (int row = 0; row < height; ++row) {
+        uint8_t* pScanline = pChannel + row * pitch;
+        for (int col = 0; col < width; ++col) {
+          uint8_t* pPixel = pScanline + col * image->numcomps;
           if (!image->comps[channel].data) {
             continue;
           }
-          src = image->comps[channel].data[row * wid + col];
+          int src = image->comps[channel].data[row * width + col];
           src += image->comps[channel].sgnd
                      ? 1 << (image->comps[channel].prec - 1)
                      : 0;
@@ -815,12 +810,7 @@ FX_BOOL CJPX_Decoder::Decode(uint8_t* dest_buf,
       }
     }
   }
-  result = TRUE;
-
-done:
-  FX_Free(channel_bufs);
-  FX_Free(adjust_comps);
-  return result;
+  return true;
 }
 
 CCodec_JpxModule::CCodec_JpxModule() {}
@@ -841,10 +831,10 @@ void CCodec_JpxModule::GetImageInfo(CJPX_Decoder* pDecoder,
   pDecoder->GetInfo(width, height, components);
 }
 
-FX_BOOL CCodec_JpxModule::Decode(CJPX_Decoder* pDecoder,
-                                 uint8_t* dest_data,
-                                 int pitch,
-                                 uint8_t* offsets) {
+bool CCodec_JpxModule::Decode(CJPX_Decoder* pDecoder,
+                              uint8_t* dest_data,
+                              int pitch,
+                              const std::vector<uint8_t>& offsets) {
   return pDecoder->Decode(dest_data, pitch, offsets);
 }