Move all of ccodec_bmpmodule state to its context

This avoids a stale delegate pointer issue in the module. In theory,
it should also allow for multiple decodes at the same time from
different contexts within the same module, but this isn't used.

Rename associated context, and use |new| to create it.  Along the
way, resolve a subtle FX_Alloc() vs. user-supplied callback
free() issue, and remove supporting code.

Bug: 728323
Change-Id: I7bb66bb5d5b4fa173bec2b445a8e71ab691fdf5c
Reviewed-on: https://pdfium-review.googlesource.com/6133
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>

4 files changed, 76 insertions, 88 deletions

diff --git a/core/fxcodec/codec/ccodec_bmpmodule.cpp b/core/fxcodec/codec/ccodec_bmpmodule.cpp
index 003f5d3d49..ae0a30c4b4 100644
--- a/core/fxcodec/codec/ccodec_bmpmodule.cpp
+++ b/core/fxcodec/codec/ccodec_bmpmodule.cpp
@@ -9,81 +9,78 @@
 #include "core/fxcodec/codec/codec_int.h"
 #include "core/fxcodec/fx_codec.h"
 #include "core/fxcodec/lbmp/fx_bmp.h"
+#include "core/fxcrt/cfx_unowned_ptr.h"
 #include "core/fxge/fx_dib.h"
 
-struct FXBMP_Context {
-  bmp_decompress_struct_p bmp_ptr;
-  void* parent_ptr;
+class CCodec_BmpModule::Context {
+ public:
+  Context(bmp_decompress_struct_p pBmp,
+          CCodec_BmpModule* pModule,
+          CCodec_BmpModule::Delegate* pDelegate);
+  ~Context();
 
-  void* (*m_AllocFunc)(unsigned int);
-  void (*m_FreeFunc)(void*);
+  bmp_decompress_struct_p m_pBmp;
+  CFX_UnownedPtr<CCodec_BmpModule> const m_pModule;
+  CFX_UnownedPtr<CCodec_BmpModule::Delegate> const m_pDelegate;
+  char m_szLastError[256];
 };
+
 extern "C" {
-static void* bmp_alloc_func(unsigned int size) {
-  return FX_Alloc(char, size);
-}
-static void bmp_free_func(void* p) {
-  FX_Free(p);
-}
-};
-static void bmp_error_data(bmp_decompress_struct_p bmp_ptr,
-                           const char* err_msg) {
-  strncpy((char*)bmp_ptr->err_ptr, err_msg, BMP_MAX_ERROR_SIZE - 1);
-  longjmp(bmp_ptr->jmpbuf, 1);
+
+static void bmp_error_data(bmp_decompress_struct_p pBmp, const char* err_msg) {
+  strncpy(pBmp->err_ptr, err_msg, BMP_MAX_ERROR_SIZE - 1);
+  longjmp(pBmp->jmpbuf, 1);
 }
-static void bmp_read_scanline(bmp_decompress_struct_p bmp_ptr,
+
+static void bmp_read_scanline(bmp_decompress_struct_p pBmp,
                               int32_t row_num,
                               uint8_t* row_buf) {
-  FXBMP_Context* p = (FXBMP_Context*)bmp_ptr->context_ptr;
-  CCodec_BmpModule* pModule = (CCodec_BmpModule*)p->parent_ptr;
-  pModule->GetDelegate()->BmpReadScanline(row_num, row_buf);
+  auto* p = reinterpret_cast<CCodec_BmpModule::Context*>(pBmp->context_ptr);
+  p->m_pDelegate->BmpReadScanline(row_num, row_buf);
 }
-static bool bmp_get_data_position(bmp_decompress_struct_p bmp_ptr,
+
+static bool bmp_get_data_position(bmp_decompress_struct_p pBmp,
                                   uint32_t rcd_pos) {
-  FXBMP_Context* p = (FXBMP_Context*)bmp_ptr->context_ptr;
-  CCodec_BmpModule* pModule = (CCodec_BmpModule*)p->parent_ptr;
-  return pModule->GetDelegate()->BmpInputImagePositionBuf(rcd_pos);
+  auto* p = reinterpret_cast<CCodec_BmpModule::Context*>(pBmp->context_ptr);
+  return p->m_pDelegate->BmpInputImagePositionBuf(rcd_pos);
 }
 
-CCodec_BmpModule::CCodec_BmpModule() {
+}  // extern "C"
+
+CCodec_BmpModule::Context::Context(bmp_decompress_struct_p pBmp,
+                                   CCodec_BmpModule* pModule,
+                                   CCodec_BmpModule::Delegate* pDelegate)
+    : m_pBmp(pBmp), m_pModule(pModule), m_pDelegate(pDelegate) {
   memset(m_szLastError, 0, sizeof(m_szLastError));
 }
 
-CCodec_BmpModule::~CCodec_BmpModule() {}
+CCodec_BmpModule::Context::~Context() {
+  bmp_destroy_decompress(&m_pBmp);
+}
 
-FXBMP_Context* CCodec_BmpModule::Start() {
-  FXBMP_Context* p = FX_Alloc(FXBMP_Context, 1);
-  if (!p)
-    return nullptr;
+CCodec_BmpModule::CCodec_BmpModule() {}
 
-  memset(p, 0, sizeof(FXBMP_Context));
-  if (!p)
-    return nullptr;
+CCodec_BmpModule::~CCodec_BmpModule() {}
 
-  p->m_AllocFunc = bmp_alloc_func;
-  p->m_FreeFunc = bmp_free_func;
-  p->bmp_ptr = nullptr;
-  p->parent_ptr = this;
-  p->bmp_ptr = bmp_create_decompress();
-  if (!p->bmp_ptr) {
-    FX_Free(p);
+CCodec_BmpModule::Context* CCodec_BmpModule::Start(Delegate* pDelegate) {
+  bmp_decompress_struct_p pBmp = bmp_create_decompress();
+  if (!pBmp)
     return nullptr;
-  }
-  p->bmp_ptr->context_ptr = p;
-  p->bmp_ptr->err_ptr = m_szLastError;
-  p->bmp_ptr->bmp_error_fn = bmp_error_data;
-  p->bmp_ptr->bmp_get_row_fn = bmp_read_scanline;
-  p->bmp_ptr->bmp_get_data_position_fn = bmp_get_data_position;
+
+  auto* p = new Context(pBmp, this, pDelegate);
+  p->m_pBmp->context_ptr = p;
+  p->m_pBmp->err_ptr = p->m_szLastError;
+  p->m_pBmp->bmp_error_fn = bmp_error_data;
+  p->m_pBmp->bmp_get_row_fn = bmp_read_scanline;
+  p->m_pBmp->bmp_get_data_position_fn = bmp_get_data_position;
   return p;
 }
 
-void CCodec_BmpModule::Finish(FXBMP_Context* ctx) {
-  if (ctx) {
-    bmp_destroy_decompress(&ctx->bmp_ptr);
-    ctx->m_FreeFunc(ctx);
-  }
+void CCodec_BmpModule::Finish(Context* ctx) {
+  delete ctx;
 }
-int32_t CCodec_BmpModule::ReadHeader(FXBMP_Context* ctx,
+
+int32_t CCodec_BmpModule::ReadHeader(Context* ctx,
                                      int32_t* width,
                                      int32_t* height,
                                      bool* tb_flag,
@@ -91,41 +88,41 @@ int32_t CCodec_BmpModule::ReadHeader(FXBMP_Context* ctx,
                                      int32_t* pal_num,
                                      uint32_t** pal_pp,
                                      CFX_DIBAttribute* pAttribute) {
-  if (setjmp(ctx->bmp_ptr->jmpbuf)) {
+  if (setjmp(ctx->m_pBmp->jmpbuf)) {
     return 0;
   }
-  int32_t ret = bmp_read_header(ctx->bmp_ptr);
+  int32_t ret = bmp_read_header(ctx->m_pBmp);
   if (ret != 1) {
     return ret;
   }
-  *width = ctx->bmp_ptr->width;
-  *height = ctx->bmp_ptr->height;
-  *tb_flag = ctx->bmp_ptr->imgTB_flag;
-  *components = ctx->bmp_ptr->components;
-  *pal_num = ctx->bmp_ptr->pal_num;
-  *pal_pp = ctx->bmp_ptr->pal_ptr;
+  *width = ctx->m_pBmp->width;
+  *height = ctx->m_pBmp->height;
+  *tb_flag = ctx->m_pBmp->imgTB_flag;
+  *components = ctx->m_pBmp->components;
+  *pal_num = ctx->m_pBmp->pal_num;
+  *pal_pp = ctx->m_pBmp->pal_ptr;
   if (pAttribute) {
     pAttribute->m_wDPIUnit = FXCODEC_RESUNIT_METER;
-    pAttribute->m_nXDPI = ctx->bmp_ptr->dpi_x;
-    pAttribute->m_nYDPI = ctx->bmp_ptr->dpi_y;
-    pAttribute->m_nBmpCompressType = ctx->bmp_ptr->compress_flag;
+    pAttribute->m_nXDPI = ctx->m_pBmp->dpi_x;
+    pAttribute->m_nYDPI = ctx->m_pBmp->dpi_y;
+    pAttribute->m_nBmpCompressType = ctx->m_pBmp->compress_flag;
   }
   return 1;
 }
 
-int32_t CCodec_BmpModule::LoadImage(FXBMP_Context* ctx) {
-  if (setjmp(ctx->bmp_ptr->jmpbuf))
+int32_t CCodec_BmpModule::LoadImage(Context* ctx) {
+  if (setjmp(ctx->m_pBmp->jmpbuf))
     return 0;
-  return bmp_decode_image(ctx->bmp_ptr);
+  return bmp_decode_image(ctx->m_pBmp);
 }
 
-uint32_t CCodec_BmpModule::GetAvailInput(FXBMP_Context* ctx,
+uint32_t CCodec_BmpModule::GetAvailInput(Context* ctx,
                                          uint8_t** avail_buf_ptr) {
-  return bmp_get_avail_input(ctx->bmp_ptr, avail_buf_ptr);
+  return bmp_get_avail_input(ctx->m_pBmp, avail_buf_ptr);
 }
 
-void CCodec_BmpModule::Input(FXBMP_Context* ctx,
+void CCodec_BmpModule::Input(Context* ctx,
                              const uint8_t* src_buf,
                              uint32_t src_size) {
-  bmp_input_buffer(ctx->bmp_ptr, (uint8_t*)src_buf, src_size);
+  bmp_input_buffer(ctx->m_pBmp, (uint8_t*)src_buf, src_size);
 }
diff --git a/core/fxcodec/codec/ccodec_bmpmodule.h b/core/fxcodec/codec/ccodec_bmpmodule.h
index c9fcabb2bd..7893fba4c7 100644
--- a/core/fxcodec/codec/ccodec_bmpmodule.h
+++ b/core/fxcodec/codec/ccodec_bmpmodule.h
@@ -11,10 +11,10 @@
 #include "core/fxcrt/fx_system.h"
 
 class CFX_DIBAttribute;
-struct FXBMP_Context;
 
 class CCodec_BmpModule {
  public:
+  class Context;
   class Delegate {
    public:
     virtual bool BmpInputImagePositionBuf(uint32_t rcd_pos) = 0;
@@ -24,13 +24,11 @@ class CCodec_BmpModule {
   CCodec_BmpModule();
   ~CCodec_BmpModule();
 
-  FXBMP_Context* Start();
-  void Finish(FXBMP_Context* pContext);
-  uint32_t GetAvailInput(FXBMP_Context* pContext, uint8_t** avail_buf_ptr);
-  void Input(FXBMP_Context* pContext,
-             const uint8_t* src_buf,
-             uint32_t src_size);
-  int32_t ReadHeader(FXBMP_Context* pContext,
+  Context* Start(Delegate* pDelegate);
+  void Finish(Context* pContext);
+  uint32_t GetAvailInput(Context* pContext, uint8_t** avail_buf_ptr);
+  void Input(Context* pContext, const uint8_t* src_buf, uint32_t src_size);
+  int32_t ReadHeader(Context* pContext,
                      int32_t* width,
                      int32_t* height,
                      bool* tb_flag,
@@ -38,13 +36,7 @@ class CCodec_BmpModule {
                      int32_t* pal_num,
                      uint32_t** pal_pp,
                      CFX_DIBAttribute* pAttribute);
-  int32_t LoadImage(FXBMP_Context* pContext);
-  Delegate* GetDelegate() const { return m_pDelegate.Get(); }
-  void SetDelegate(Delegate* pDelegate) { m_pDelegate = pDelegate; }
-
- protected:
-  CFX_UnownedPtr<Delegate> m_pDelegate;
-  char m_szLastError[256];
+  int32_t LoadImage(Context* pContext);
 };
 
 #endif  // CORE_FXCODEC_CODEC_CCODEC_BMPMODULE_H_
diff --git a/core/fxcodec/codec/ccodec_progressivedecoder.h b/core/fxcodec/codec/ccodec_progressivedecoder.h
index 42d0ee3aba..f9a67ccee3 100644
--- a/core/fxcodec/codec/ccodec_progressivedecoder.h
+++ b/core/fxcodec/codec/ccodec_progressivedecoder.h
@@ -134,7 +134,7 @@ class CCodec_ProgressiveDecoder : public CCodec_BmpModule::Delegate,
   FXJPEG_Context* m_pJpegContext;
   FXPNG_Context* m_pPngContext;
   std::unique_ptr<CGifContext> m_pGifContext;
-  FXBMP_Context* m_pBmpContext;
+  CCodec_BmpModule::Context* m_pBmpContext;
   CFX_UnownedPtr<CCodec_TiffContext> m_pTiffContext;
   FXCODEC_IMAGE_TYPE m_imagType;
   uint32_t m_offSet;
diff --git a/core/fxcodec/codec/fx_codec_progress.cpp b/core/fxcodec/codec/fx_codec_progress.cpp
index dba0e20533..4cadb152d8 100644
--- a/core/fxcodec/codec/fx_codec_progress.cpp
+++ b/core/fxcodec/codec/fx_codec_progress.cpp
@@ -1020,8 +1020,7 @@ bool CCodec_ProgressiveDecoder::DetectImageType(FXCODEC_IMAGE_TYPE imageType,
         m_status = FXCODEC_STATUS_ERR_MEMORY;
         return false;
       }
-      pBmpModule->SetDelegate(this);
-      m_pBmpContext = pBmpModule->Start();
+      m_pBmpContext = pBmpModule->Start(this);
       if (!m_pBmpContext) {
         m_status = FXCODEC_STATUS_ERR_MEMORY;
         return false;