Remove NoDestructor usage in progressive image decoders
Add static methods to initialize/destroy {Bmp,Gif,Jpeg} progressive
decoders when the XFA module is initialized/destroyed. This lets
FPDF_DestroyLibrary() free more memory and work as it was originally
intended.
Bug: pdfium:1876
Change-Id: I4c3ca311b408e0befe837b52e7c0934dcd815088
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/113111
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
diff --git a/core/fxcodec/bmp/bmp_progressive_decoder.cpp b/core/fxcodec/bmp/bmp_progressive_decoder.cpp
index 4ab9fda..61e770a 100644
--- a/core/fxcodec/bmp/bmp_progressive_decoder.cpp
+++ b/core/fxcodec/bmp/bmp_progressive_decoder.cpp
@@ -8,13 +8,31 @@
#include "core/fxcodec/bmp/bmp_decoder.h"
#include "core/fxcodec/cfx_codec_memory.h"
+#include "third_party/base/check.h"
namespace fxcodec {
+namespace {
+
+BmpProgressiveDecoder* g_bmp_decoder = nullptr;
+
+} // namespace
+
+// static
+void BmpProgressiveDecoder::InitializeGlobals() {
+ CHECK(!g_bmp_decoder);
+ g_bmp_decoder = new BmpProgressiveDecoder();
+}
+
+// static
+void BmpProgressiveDecoder::DestroyGlobals() {
+ delete g_bmp_decoder;
+ g_bmp_decoder = nullptr;
+}
+
// static
BmpProgressiveDecoder* BmpProgressiveDecoder::GetInstance() {
- static pdfium::base::NoDestructor<BmpProgressiveDecoder> s;
- return s.get();
+ return g_bmp_decoder;
}
BmpProgressiveDecoder::BmpProgressiveDecoder() = default;
diff --git a/core/fxcodec/bmp/bmp_progressive_decoder.h b/core/fxcodec/bmp/bmp_progressive_decoder.h
index f2040c8..865e112 100644
--- a/core/fxcodec/bmp/bmp_progressive_decoder.h
+++ b/core/fxcodec/bmp/bmp_progressive_decoder.h
@@ -8,7 +8,6 @@
#define CORE_FXCODEC_BMP_BMP_PROGRESSIVE_DECODER_H_
#include "core/fxcodec/progressive_decoder_iface.h"
-#include "third_party/base/no_destructor.h"
#ifndef PDF_ENABLE_XFA_BMP
#error "BMP must be enabled"
@@ -18,6 +17,9 @@
class BmpProgressiveDecoder final : public ProgressiveDecoderIface {
public:
+ static void InitializeGlobals();
+ static void DestroyGlobals();
+
static BmpProgressiveDecoder* GetInstance();
// ProgressiveDecoderIface:
@@ -26,8 +28,6 @@
RetainPtr<CFX_CodecMemory> codec_memory) override;
private:
- friend pdfium::base::NoDestructor<BmpProgressiveDecoder>;
-
BmpProgressiveDecoder();
~BmpProgressiveDecoder() override;
};
diff --git a/core/fxcodec/gif/gif_progressive_decoder.cpp b/core/fxcodec/gif/gif_progressive_decoder.cpp
index 78c6a8c..8816ea6 100644
--- a/core/fxcodec/gif/gif_progressive_decoder.cpp
+++ b/core/fxcodec/gif/gif_progressive_decoder.cpp
@@ -8,13 +8,31 @@
#include "core/fxcodec/cfx_codec_memory.h"
#include "core/fxcodec/gif/gif_decoder.h"
+#include "third_party/base/check.h"
namespace fxcodec {
+namespace {
+
+GifProgressiveDecoder* g_gif_decoder = nullptr;
+
+} // namespace
+
+// static
+void GifProgressiveDecoder::InitializeGlobals() {
+ CHECK(!g_gif_decoder);
+ g_gif_decoder = new GifProgressiveDecoder();
+}
+
+// static
+void GifProgressiveDecoder::DestroyGlobals() {
+ delete g_gif_decoder;
+ g_gif_decoder = nullptr;
+}
+
// static
GifProgressiveDecoder* GifProgressiveDecoder::GetInstance() {
- static pdfium::base::NoDestructor<GifProgressiveDecoder> s;
- return s.get();
+ return g_gif_decoder;
}
GifProgressiveDecoder::GifProgressiveDecoder() = default;
diff --git a/core/fxcodec/gif/gif_progressive_decoder.h b/core/fxcodec/gif/gif_progressive_decoder.h
index 6b58c3c..52075a6 100644
--- a/core/fxcodec/gif/gif_progressive_decoder.h
+++ b/core/fxcodec/gif/gif_progressive_decoder.h
@@ -8,7 +8,6 @@
#define CORE_FXCODEC_GIF_GIF_PROGRESSIVE_DECODER_H_
#include "core/fxcodec/progressive_decoder_iface.h"
-#include "third_party/base/no_destructor.h"
#ifndef PDF_ENABLE_XFA_GIF
#error "GIF must be enabled"
@@ -18,6 +17,9 @@
class GifProgressiveDecoder final : public ProgressiveDecoderIface {
public:
+ static void InitializeGlobals();
+ static void DestroyGlobals();
+
static GifProgressiveDecoder* GetInstance();
// ProgressiveDecoderIface:
@@ -26,8 +28,6 @@
RetainPtr<CFX_CodecMemory> codec_memory) override;
private:
- friend pdfium::base::NoDestructor<GifProgressiveDecoder>;
-
GifProgressiveDecoder();
~GifProgressiveDecoder() override;
};
diff --git a/core/fxcodec/jpeg/jpeg_progressive_decoder.cpp b/core/fxcodec/jpeg/jpeg_progressive_decoder.cpp
index 1f1a7c3..d4d8839 100644
--- a/core/fxcodec/jpeg/jpeg_progressive_decoder.cpp
+++ b/core/fxcodec/jpeg/jpeg_progressive_decoder.cpp
@@ -84,10 +84,27 @@
namespace fxcodec {
+namespace {
+
+JpegProgressiveDecoder* g_jpeg_decoder = nullptr;
+
+} // namespace
+
+// static
+void JpegProgressiveDecoder::InitializeGlobals() {
+ CHECK(!g_jpeg_decoder);
+ g_jpeg_decoder = new JpegProgressiveDecoder();
+}
+
+// static
+void JpegProgressiveDecoder::DestroyGlobals() {
+ delete g_jpeg_decoder;
+ g_jpeg_decoder = nullptr;
+}
+
// static
JpegProgressiveDecoder* JpegProgressiveDecoder::GetInstance() {
- static pdfium::base::NoDestructor<JpegProgressiveDecoder> s;
- return s.get();
+ return g_jpeg_decoder;
}
// static
diff --git a/core/fxcodec/jpeg/jpeg_progressive_decoder.h b/core/fxcodec/jpeg/jpeg_progressive_decoder.h
index 6429d01..a94e33a 100644
--- a/core/fxcodec/jpeg/jpeg_progressive_decoder.h
+++ b/core/fxcodec/jpeg/jpeg_progressive_decoder.h
@@ -12,7 +12,6 @@
#include <memory>
#include "core/fxcodec/progressive_decoder_iface.h"
-#include "third_party/base/no_destructor.h"
namespace fxcodec {
@@ -20,6 +19,9 @@
class JpegProgressiveDecoder final : public ProgressiveDecoderIface {
public:
+ static void InitializeGlobals();
+ static void DestroyGlobals();
+
static JpegProgressiveDecoder* GetInstance();
static std::unique_ptr<Context> Start();
@@ -41,8 +43,6 @@
RetainPtr<CFX_CodecMemory> codec_memory) override;
private:
- friend pdfium::base::NoDestructor<JpegProgressiveDecoder>;
-
JpegProgressiveDecoder();
~JpegProgressiveDecoder() override;
};
diff --git a/core/fxcodec/progressive_decoder_unittest.cpp b/core/fxcodec/progressive_decoder_unittest.cpp
index ccb0c03..682cd19 100644
--- a/core/fxcodec/progressive_decoder_unittest.cpp
+++ b/core/fxcodec/progressive_decoder_unittest.cpp
@@ -14,6 +14,7 @@
#include "core/fxcodec/fx_codec.h"
#include "core/fxcodec/fx_codec_def.h"
+#include "core/fxcodec/jpeg/jpeg_progressive_decoder.h"
#include "core/fxcrt/cfx_read_only_span_stream.h"
#include "core/fxcrt/cfx_read_only_vector_stream.h"
#include "core/fxcrt/data_vector.h"
@@ -26,10 +27,12 @@
#ifdef PDF_ENABLE_XFA_BMP
#include "core/fxcodec/bmp/bmp_decoder.h"
+#include "core/fxcodec/bmp/bmp_progressive_decoder.h"
#endif // PDF_ENABLE_XFA_BMP
#ifdef PDF_ENABLE_XFA_GIF
#include "core/fxcodec/gif/gif_decoder.h"
+#include "core/fxcodec/gif/gif_progressive_decoder.h"
#endif // PDF_ENABLE_XFA_GIF
namespace fxcodec {
@@ -55,10 +58,31 @@
return status;
}
+class ProgressiveDecoderTest : public testing::Test {
+ void SetUp() override {
+#ifdef PDF_ENABLE_XFA_BMP
+ BmpProgressiveDecoder::InitializeGlobals();
+#endif
+#ifdef PDF_ENABLE_XFA_GIF
+ GifProgressiveDecoder::InitializeGlobals();
+#endif
+ JpegProgressiveDecoder::InitializeGlobals();
+ }
+ void TearDown() override {
+ JpegProgressiveDecoder::DestroyGlobals();
+#ifdef PDF_ENABLE_XFA_GIF
+ GifProgressiveDecoder::DestroyGlobals();
+#endif
+#ifdef PDF_ENABLE_XFA_BMP
+ BmpProgressiveDecoder::DestroyGlobals();
+#endif
+ }
+};
+
} // namespace
#ifdef PDF_ENABLE_XFA_BMP
-TEST(ProgressiveDecoder, Indexed8Bmp) {
+TEST_F(ProgressiveDecoderTest, Indexed8Bmp) {
static constexpr uint8_t kInput[] = {
0x42, 0x4d, 0x3e, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x3a,
0x00, 0x00, 0x00, 0x28, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00,
@@ -91,7 +115,7 @@
EXPECT_THAT(bitmap->GetScanline(0), ElementsAre(0xc0, 0x80, 0x40, 0x00));
}
-TEST(ProgressiveDecoder, Indexed8BmpWithInvalidIndex) {
+TEST_F(ProgressiveDecoderTest, Indexed8BmpWithInvalidIndex) {
static constexpr uint8_t kInput[] = {
0x42, 0x4d, 0x3e, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x3a,
0x00, 0x00, 0x00, 0x28, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00,
@@ -123,7 +147,7 @@
EXPECT_EQ(FXCODEC_STATUS::kError, status);
}
-TEST(ProgressiveDecoder, Direct24Bmp) {
+TEST_F(ProgressiveDecoderTest, Direct24Bmp) {
static constexpr uint8_t kInput[] = {
0x42, 0x4d, 0x3a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x36, 0x00,
0x00, 0x00, 0x28, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x00,
@@ -155,7 +179,7 @@
EXPECT_THAT(bitmap->GetScanline(0), ElementsAre(0xc0, 0x80, 0x40, 0x00));
}
-TEST(ProgressiveDecoder, Direct32Bmp) {
+TEST_F(ProgressiveDecoderTest, Direct32Bmp) {
static constexpr uint8_t kInput[] = {
0x42, 0x4d, 0x3a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x36, 0x00,
0x00, 0x00, 0x28, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x00,
@@ -187,7 +211,7 @@
EXPECT_THAT(bitmap->GetScanline(0), ElementsAre(0xc0, 0x80, 0x40, 0x00));
}
-TEST(ProgressiveDecoder, BmpWithDataOffsetBeforeEndOfHeader) {
+TEST_F(ProgressiveDecoderTest, BmpWithDataOffsetBeforeEndOfHeader) {
static constexpr uint8_t kInput[] = {
0x42, 0x4d, 0x3a, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x35, 0x00,
0x00, 0x00, 0x28, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x00,
@@ -219,7 +243,7 @@
EXPECT_THAT(bitmap->GetScanline(0), ElementsAre(0xc0, 0x80, 0x40, 0x00));
}
-TEST(ProgressiveDecoder, BmpWithDataOffsetAfterEndOfHeader) {
+TEST_F(ProgressiveDecoderTest, BmpWithDataOffsetAfterEndOfHeader) {
static constexpr uint8_t kInput[] = {
0x42, 0x4d, 0x3b, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x37, 0x00,
0x00, 0x00, 0x28, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x00,
@@ -251,7 +275,7 @@
EXPECT_THAT(bitmap->GetScanline(0), ElementsAre(0xc0, 0x80, 0x40, 0x00));
}
-TEST(ProgressiveDecoder, LargeBmp) {
+TEST_F(ProgressiveDecoderTest, LargeBmp) {
// Construct a 24-bit BMP larger than `kBlockSize` (4096 bytes).
static constexpr uint8_t kWidth = 37;
static constexpr uint8_t kHeight = 38;
@@ -304,7 +328,7 @@
#endif // PDF_ENABLE_XFA_BMP
#ifdef PDF_ENABLE_XFA_GIF
-TEST(ProgressiveDecoder, Gif87a) {
+TEST_F(ProgressiveDecoderTest, Gif87a) {
static constexpr uint8_t kInput[] = {
0x47, 0x49, 0x46, 0x38, 0x37, 0x61, 0x01, 0x00, 0x01, 0x00, 0x80, 0x01,
0x00, 0x40, 0x80, 0xc0, 0x80, 0x80, 0x80, 0x2c, 0x00, 0x00, 0x00, 0x00,
@@ -334,7 +358,7 @@
EXPECT_THAT(bitmap->GetScanline(0), ElementsAre(0xc0, 0x80, 0x40, 0xff));
}
-TEST(ProgressiveDecoder, Gif89a) {
+TEST_F(ProgressiveDecoderTest, Gif89a) {
static constexpr uint8_t kInput[] = {
0x47, 0x49, 0x46, 0x38, 0x39, 0x61, 0x01, 0x00, 0x01, 0x00, 0x80,
0x01, 0x00, 0x40, 0x80, 0xc0, 0x80, 0x80, 0x80, 0x21, 0xf9, 0x04,
@@ -365,7 +389,7 @@
EXPECT_THAT(bitmap->GetScanline(0), ElementsAre(0xc0, 0x80, 0x40, 0xff));
}
-TEST(ProgressiveDecoder, GifInsufficientCodeSize) {
+TEST_F(ProgressiveDecoderTest, GifInsufficientCodeSize) {
// This GIF causes `LZWDecompressor::Create()` to fail because the minimum
// code size is too small for the palette.
static constexpr uint8_t kInput[] = {
@@ -398,7 +422,7 @@
EXPECT_EQ(FXCODEC_STATUS::kError, status);
}
-TEST(ProgressiveDecoder, GifDecodeAcrossScanlines) {
+TEST_F(ProgressiveDecoderTest, GifDecodeAcrossScanlines) {
// This GIF contains an LZW code unit split across 2 scanlines. The decoder
// must continue decoding the second scanline using the residual data.
static constexpr uint8_t kInput[] = {
@@ -435,7 +459,7 @@
0x80, 0x40, 0xff, 0xc0, 0x80, 0x40, 0xff));
}
-TEST(ProgressiveDecoder, GifDecodeAcrossSubblocks) {
+TEST_F(ProgressiveDecoderTest, GifDecodeAcrossSubblocks) {
// This GIF contains a scanline split across 2 data sub-blocks. The decoder
// must continue decoding in the second sub-block.
static constexpr uint8_t kInput[] = {
diff --git a/fpdfsdk/fpdfxfa/BUILD.gn b/fpdfsdk/fpdfxfa/BUILD.gn
index b3d5fe0..4374f59 100644
--- a/fpdfsdk/fpdfxfa/BUILD.gn
+++ b/fpdfsdk/fpdfxfa/BUILD.gn
@@ -27,6 +27,7 @@
"../../core/fpdfapi/page",
"../../core/fpdfapi/parser",
"../../core/fpdfapi/render",
+ "../../core/fxcodec",
"../../core/fxcrt",
"../../fxbarcode",
"../../fxjs",
diff --git a/fpdfsdk/fpdfxfa/cpdfxfa_context.cpp b/fpdfsdk/fpdfxfa/cpdfxfa_context.cpp
index 8d48750..1271fbd 100644
--- a/fpdfsdk/fpdfxfa/cpdfxfa_context.cpp
+++ b/fpdfsdk/fpdfxfa/cpdfxfa_context.cpp
@@ -15,6 +15,7 @@
#include "core/fpdfapi/parser/cpdf_dictionary.h"
#include "core/fpdfapi/parser/cpdf_document.h"
#include "core/fpdfapi/parser/cpdf_seekablemultistream.h"
+#include "core/fxcodec/jpeg/jpeg_progressive_decoder.h"
#include "core/fxcrt/autonuller.h"
#include "core/fxcrt/fixed_zeroed_data_vector.h"
#include "core/fxcrt/stl_util.h"
@@ -40,6 +41,14 @@
#include "xfa/fxfa/cxfa_fontmgr.h"
#include "xfa/fxfa/cxfa_readynodeiterator.h"
+#ifdef PDF_ENABLE_XFA_BMP
+#include "core/fxcodec/bmp/bmp_progressive_decoder.h"
+#endif
+
+#ifdef PDF_ENABLE_XFA_GIF
+#include "core/fxcodec/gif/gif_progressive_decoder.h"
+#endif
+
namespace {
bool IsValidAlertButton(int type) {
@@ -94,9 +103,23 @@
void CPDFXFA_ModuleInit() {
CFGAS_GEModule::Create();
BC_Library_Init();
+#ifdef PDF_ENABLE_XFA_BMP
+ fxcodec::BmpProgressiveDecoder::InitializeGlobals();
+#endif
+#ifdef PDF_ENABLE_XFA_GIF
+ fxcodec::GifProgressiveDecoder::InitializeGlobals();
+#endif
+ fxcodec::JpegProgressiveDecoder::InitializeGlobals();
}
void CPDFXFA_ModuleDestroy() {
+ fxcodec::JpegProgressiveDecoder::DestroyGlobals();
+#ifdef PDF_ENABLE_XFA_GIF
+ fxcodec::GifProgressiveDecoder::DestroyGlobals();
+#endif
+#ifdef PDF_ENABLE_XFA_BMP
+ fxcodec::BmpProgressiveDecoder::DestroyGlobals();
+#endif
BC_Library_Destroy();
CFGAS_GEModule::Destroy();
}
