Make SkCanvas handling more robust
- Make sure CFX_DefaultRenderDevice::AttachCanvas() callers check the
return value.
- Change the SkCanvas parameter in CFX_SkiaDeviceDriver::Create() and
CFX_DefaultRenderDevice::AttachCanvas() to references, since all
callers pass in a non-null pointer anyway.
Change-Id: Icab1a045b7d89c12fbc12936821adc4c6cb3242b
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/123030
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Tom Sepez <tsepez@google.com>
Commit-Queue: Lei Zhang <thestig@chromium.org>
diff --git a/core/fxge/cfx_defaultrenderdevice.h b/core/fxge/cfx_defaultrenderdevice.h
index b63e7d9..ee87539 100644
--- a/core/fxge/cfx_defaultrenderdevice.h
+++ b/core/fxge/cfx_defaultrenderdevice.h
@@ -28,7 +28,7 @@
RetainPtr<CFX_DIBitmap> pBackdropBitmap,
bool bGroupKnockout);
#if defined(PDF_USE_SKIA)
- bool AttachCanvas(SkCanvas* canvas);
+ [[nodiscard]] bool AttachCanvas(SkCanvas& canvas);
#endif
bool Create(int width, int height, FXDIB_Format format);
diff --git a/core/fxge/skia/fx_skia_device.cpp b/core/fxge/skia/fx_skia_device.cpp
index 4a3f51a..038dfa3 100644
--- a/core/fxge/skia/fx_skia_device.cpp
+++ b/core/fxge/skia/fx_skia_device.cpp
@@ -681,11 +681,7 @@
// static
std::unique_ptr<CFX_SkiaDeviceDriver> CFX_SkiaDeviceDriver::Create(
- SkCanvas* canvas) {
- if (!canvas) {
- return nullptr;
- }
-
+ SkCanvas& canvas) {
auto driver = pdfium::WrapUnique(new CFX_SkiaDeviceDriver(canvas));
if (!driver->m_pBitmap || !driver->m_pBackdropBitmap) {
return nullptr;
@@ -747,8 +743,8 @@
m_pCanvas = surface_->getCanvas();
}
-CFX_SkiaDeviceDriver::CFX_SkiaDeviceDriver(SkCanvas* canvas)
- : m_pCanvas(canvas), m_bGroupKnockout(false) {
+CFX_SkiaDeviceDriver::CFX_SkiaDeviceDriver(SkCanvas& canvas)
+ : m_pCanvas(&canvas), m_bGroupKnockout(false) {
int width = m_pCanvas->imageInfo().width();
int height = m_pCanvas->imageInfo().height();
DCHECK_EQ(kUnknown_SkColorType, m_pCanvas->imageInfo().colorType());
@@ -1659,7 +1655,7 @@
return true;
}
-bool CFX_DefaultRenderDevice::AttachCanvas(SkCanvas* canvas) {
+bool CFX_DefaultRenderDevice::AttachCanvas(SkCanvas& canvas) {
auto driver = CFX_SkiaDeviceDriver::Create(canvas);
if (!driver) {
return false;
diff --git a/core/fxge/skia/fx_skia_device.h b/core/fxge/skia/fx_skia_device.h
index 7a8caba..e575e1e 100644
--- a/core/fxge/skia/fx_skia_device.h
+++ b/core/fxge/skia/fx_skia_device.h
@@ -42,7 +42,7 @@
bool bRgbByteOrder,
RetainPtr<CFX_DIBitmap> pBackdropBitmap,
bool bGroupKnockout);
- static std::unique_ptr<CFX_SkiaDeviceDriver> Create(SkCanvas* canvas);
+ static std::unique_ptr<CFX_SkiaDeviceDriver> Create(SkCanvas& canvas);
~CFX_SkiaDeviceDriver() override;
@@ -168,7 +168,7 @@
bool bRgbByteOrder,
RetainPtr<CFX_DIBitmap> pBackdropBitmap,
bool bGroupKnockout);
- explicit CFX_SkiaDeviceDriver(SkCanvas* canvas);
+ explicit CFX_SkiaDeviceDriver(SkCanvas& canvas);
bool TryDrawText(pdfium::span<const TextCharPos> char_pos,
const CFX_Font* pFont,
diff --git a/core/fxge/skia/fx_skia_device_embeddertest.cpp b/core/fxge/skia/fx_skia_device_embeddertest.cpp
index 5d64a1c..3682d11 100644
--- a/core/fxge/skia/fx_skia_device_embeddertest.cpp
+++ b/core/fxge/skia/fx_skia_device_embeddertest.cpp
@@ -168,7 +168,7 @@
int start_y,
int size_x,
int size_y,
- SkCanvas* canvas) {
+ SkCanvas& canvas) {
CPDF_Page* cpdf_page = CPDFPageFromFPDFPage(page);
auto context = std::make_unique<CPDF_PageRenderContext>();
@@ -178,7 +178,7 @@
cpdf_page->SetRenderContext(std::move(context));
auto default_device = std::make_unique<CFX_DefaultRenderDevice>();
- default_device->AttachCanvas(canvas);
+ CHECK(default_device->AttachCanvas(canvas));
unowned_context->m_pDevice = std::move(default_device);
CPDFSDK_RenderPageWithContext(unowned_context, cpdf_page, start_x, start_y,
@@ -283,11 +283,11 @@
// Render top half.
RenderPageToSkCanvas(page, /*start_x=*/0, /*start_y=*/0,
- /*size_x=*/kPageWidth, /*size_y=*/kPageHeight, &canvas);
+ /*size_x=*/kPageWidth, /*size_y=*/kPageHeight, canvas);
// Render bottom half.
RenderPageToSkCanvas(page, /*start_x=*/0, /*start_y=*/-kPageHeight / 2,
- /*size_x=*/kPageWidth, /*size_y=*/kPageHeight, &canvas);
+ /*size_x=*/kPageWidth, /*size_y=*/kPageHeight, canvas);
EXPECT_THAT(image_ids, SizeIs(1));
diff --git a/fpdfsdk/fpdf_formfill.cpp b/fpdfsdk/fpdf_formfill.cpp
index 17ecbdc..7d741d0 100644
--- a/fpdfsdk/fpdf_formfill.cpp
+++ b/fpdfsdk/fpdf_formfill.cpp
@@ -203,7 +203,9 @@
#if defined(PDF_USE_SKIA)
SkCanvas* sk_canvas = SkCanvasFromFPDFSkiaCanvas(canvas);
if (sk_canvas && CFX_DefaultRenderDevice::UseSkiaRenderer()) {
- pDevice->AttachCanvas(sk_canvas);
+ if (!pDevice->AttachCanvas(*sk_canvas)) {
+ return;
+ }
}
#endif
diff --git a/fpdfsdk/fpdf_view.cpp b/fpdfsdk/fpdf_view.cpp
index 537b11a..4943746 100644
--- a/fpdfsdk/fpdf_view.cpp
+++ b/fpdfsdk/fpdf_view.cpp
@@ -794,7 +794,9 @@
cpdf_page->SetRenderContext(std::move(owned_context));
auto device = std::make_unique<CFX_DefaultRenderDevice>();
- device->AttachCanvas(sk_canvas);
+ if (!device->AttachCanvas(*sk_canvas)) {
+ return;
+ }
context->m_pDevice = std::move(device);
CPDFSDK_RenderPageWithContext(context, cpdf_page, 0, 0, size_x, size_y, 0, 0,
