Add EmbedderTest::LoadScopedPage()
Implement a LoadPage() variant that returns a ScopedEmbedderTestPage.
This scoper automatically calls EmbedderTest::UnloadPage() when it goes
out of scope. Convert a few FPDFViewEmbedderTest test cases to call
LoadScopedPage() instead of LoadPage() and get rid of the UnloadPage()
calls.
This also makes tests more robust if they return early due to gtest
assertion failures. The UnloadPage() will still run and the test won't
hit a CHECK() due to the unbalanced LoadPage() / UnloadPage() counts.
Change-Id: I45770f06777d18bd31c5e4e772b69caf2d3e8515
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/122010
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Tom Sepez <tsepez@google.com>
diff --git a/fpdfsdk/fpdf_view_embeddertest.cpp b/fpdfsdk/fpdf_view_embeddertest.cpp
index 852b446..86711ba 100644
--- a/fpdfsdk/fpdf_view_embeddertest.cpp
+++ b/fpdfsdk/fpdf_view_embeddertest.cpp
@@ -283,7 +283,7 @@
// Test for conversion of a point in device coordinates to page coordinates
TEST_F(FPDFViewEmbedderTest, DeviceCoordinatesToPageCoordinates) {
ASSERT_TRUE(OpenDocument("about_blank.pdf"));
- FPDF_PAGE page = LoadPage(0);
+ ScopedEmbedderTestPage page = LoadScopedPage(0);
EXPECT_TRUE(page);
// Error tolerance for floating point comparison
@@ -304,29 +304,29 @@
double page_x = 0.0;
double page_y = 0.0;
- EXPECT_TRUE(FPDF_DeviceToPage(page, start_x, start_y, size_x, size_y, rotate,
- device_x, device_y, &page_x, &page_y));
+ EXPECT_TRUE(FPDF_DeviceToPage(page.get(), start_x, start_y, size_x, size_y,
+ rotate, device_x, device_y, &page_x, &page_y));
EXPECT_NEAR(9.5625, page_x, kTolerance);
EXPECT_NEAR(775.5, page_y, kTolerance);
// Rotate 90 degrees clockwise
rotate = 1;
- EXPECT_TRUE(FPDF_DeviceToPage(page, start_x, start_y, size_x, size_y, rotate,
- device_x, device_y, &page_x, &page_y));
+ EXPECT_TRUE(FPDF_DeviceToPage(page.get(), start_x, start_y, size_x, size_y,
+ rotate, device_x, device_y, &page_x, &page_y));
EXPECT_NEAR(12.75, page_x, kTolerance);
EXPECT_NEAR(12.375, page_y, kTolerance);
// Rotate 180 degrees
rotate = 2;
- EXPECT_TRUE(FPDF_DeviceToPage(page, start_x, start_y, size_x, size_y, rotate,
- device_x, device_y, &page_x, &page_y));
+ EXPECT_TRUE(FPDF_DeviceToPage(page.get(), start_x, start_y, size_x, size_y,
+ rotate, device_x, device_y, &page_x, &page_y));
EXPECT_NEAR(602.4374, page_x, kTolerance);
EXPECT_NEAR(16.5, page_y, kTolerance);
// Rotate 90 degrees counter-clockwise
rotate = 3;
- EXPECT_TRUE(FPDF_DeviceToPage(page, start_x, start_y, size_x, size_y, rotate,
- device_x, device_y, &page_x, &page_y));
+ EXPECT_TRUE(FPDF_DeviceToPage(page.get(), start_x, start_y, size_x, size_y,
+ rotate, device_x, device_y, &page_x, &page_y));
EXPECT_NEAR(599.25, page_x, kTolerance);
EXPECT_NEAR(779.625, page_y, kTolerance);
@@ -334,16 +334,16 @@
// modulo by 4. A value of 4 is expected to be converted into 0 (normal
// rotation)
rotate = 4;
- EXPECT_TRUE(FPDF_DeviceToPage(page, start_x, start_y, size_x, size_y, rotate,
- device_x, device_y, &page_x, &page_y));
+ EXPECT_TRUE(FPDF_DeviceToPage(page.get(), start_x, start_y, size_x, size_y,
+ rotate, device_x, device_y, &page_x, &page_y));
EXPECT_NEAR(9.5625, page_x, kTolerance);
EXPECT_NEAR(775.5, page_y, kTolerance);
// FPDF_DeviceToPage returns untransformed coordinates if |rotate| % 4 is
// negative.
rotate = -1;
- EXPECT_TRUE(FPDF_DeviceToPage(page, start_x, start_y, size_x, size_y, rotate,
- device_x, device_y, &page_x, &page_y));
+ EXPECT_TRUE(FPDF_DeviceToPage(page.get(), start_x, start_y, size_x, size_y,
+ rotate, device_x, device_y, &page_x, &page_y));
EXPECT_NEAR(device_x, page_x, kTolerance);
EXPECT_NEAR(device_y, page_y, kTolerance);
@@ -357,10 +357,8 @@
EXPECT_NEAR(5678.0, page_y, kTolerance);
// Negative case - invalid output parameters
- EXPECT_FALSE(FPDF_DeviceToPage(page, start_x, start_y, size_x, size_y, rotate,
- device_x, device_y, nullptr, nullptr));
-
- UnloadPage(page);
+ EXPECT_FALSE(FPDF_DeviceToPage(page.get(), start_x, start_y, size_x, size_y,
+ rotate, device_x, device_y, nullptr, nullptr));
}
// Test for conversion of a point in page coordinates to device coordinates.
@@ -910,9 +908,8 @@
TEST_F(FPDFViewEmbedderTest, Crasher_452455) {
ASSERT_TRUE(OpenDocument("bug_452455.pdf"));
- FPDF_PAGE page = LoadPage(0);
+ ScopedEmbedderTestPage page = LoadScopedPage(0);
EXPECT_TRUE(page);
- UnloadPage(page);
}
TEST_F(FPDFViewEmbedderTest, Crasher_454695) {
@@ -2151,22 +2148,21 @@
TEST_F(FPDFViewEmbedderTest, RenderTransparencyOnWhiteBackground) {
ASSERT_TRUE(OpenDocument("bug_1302355.pdf"));
- FPDF_PAGE page = LoadPage(0);
+ ScopedEmbedderTestPage page = LoadScopedPage(0);
ASSERT_TRUE(page);
constexpr int kWidth = 200;
constexpr int kHeight = 200;
- EXPECT_EQ(kWidth, static_cast<int>(FPDF_GetPageWidthF(page)));
- EXPECT_EQ(kHeight, static_cast<int>(FPDF_GetPageHeightF(page)));
- EXPECT_TRUE(FPDFPage_HasTransparency(page));
+ EXPECT_EQ(kWidth, static_cast<int>(FPDF_GetPageWidthF(page.get())));
+ EXPECT_EQ(kHeight, static_cast<int>(FPDF_GetPageHeightF(page.get())));
+ EXPECT_TRUE(FPDFPage_HasTransparency(page.get()));
ScopedFPDFBitmap bitmap(FPDFBitmap_Create(kWidth, kHeight, /*alpha=*/true));
FPDFBitmap_FillRect(bitmap.get(), 0, 0, kWidth, kHeight, 0xFFFFFFFF);
- FPDF_RenderPageBitmap(bitmap.get(), page, /*start_x=*/0, /*start_y=*/0,
- kWidth, kHeight, /*rotate=*/0, /*flags=*/0);
+ FPDF_RenderPageBitmap(bitmap.get(), page.get(), /*start_x=*/0,
+ /*start_y=*/0, kWidth, kHeight, /*rotate=*/0,
+ /*flags=*/0);
// TODO(crbug.com/1302355): This page should not render blank.
EXPECT_EQ("eee4600ac08b458ac7ac2320e225674c", HashBitmap(bitmap.get()));
-
- UnloadPage(page);
}
TEST_F(FPDFViewEmbedderTest, Bug2112) {
diff --git a/testing/embedder_test.cpp b/testing/embedder_test.cpp
index 848c707..6ed273b 100644
--- a/testing/embedder_test.cpp
+++ b/testing/embedder_test.cpp
@@ -509,6 +509,11 @@
return page_count;
}
+EmbedderTest::ScopedEmbedderTestPage EmbedderTest::LoadScopedPage(
+ int page_index) {
+ return ScopedEmbedderTestPage(this, page_index);
+}
+
FPDF_PAGE EmbedderTest::LoadPage(int page_index) {
return LoadPageCommon(page_index, /*do_events=*/true);
}
@@ -889,3 +894,11 @@
filestream_.close();
}
#endif
+
+EmbedderTest::ScopedEmbedderTestPage::ScopedEmbedderTestPage(EmbedderTest* test,
+ int page_index)
+ : test_(test), page_(test->LoadPage(page_index)) {}
+
+EmbedderTest::ScopedEmbedderTestPage::~ScopedEmbedderTestPage() {
+ test_->UnloadPage(page_);
+}
diff --git a/testing/embedder_test.h b/testing/embedder_test.h
index 2368c14..a065623 100644
--- a/testing/embedder_test.h
+++ b/testing/embedder_test.h
@@ -15,6 +15,7 @@
#include "build/build_config.h"
#include "core/fxcrt/span.h"
+#include "core/fxcrt/unowned_ptr.h"
#include "public/cpp/fpdf_scopers.h"
#include "public/fpdf_dataavail.h"
#include "public/fpdf_ext.h"
@@ -88,6 +89,20 @@
int modifiers) {}
};
+ class ScopedEmbedderTestPage {
+ public:
+ ScopedEmbedderTestPage(EmbedderTest* test, int page_index);
+ ~ScopedEmbedderTestPage();
+
+ FPDF_PAGE get() { return page_; }
+
+ explicit operator bool() const { return !!page_; }
+
+ private:
+ UnownedPtr<EmbedderTest> const test_;
+ const FPDF_PAGE page_;
+ };
+
EmbedderTest();
~EmbedderTest() override;
@@ -150,6 +165,19 @@
int GetPageCount();
// Load a specific page of the open document with a given non-negative
+ // `page_index`. On success, fire form events for the page and return a
+ // ScopedEmbedderTestPage with the page handle. On failure, return an empty
+ // ScopedEmbedderTestPage.
+ // The caller needs to let the ScopedEmbedderTestPage go out of scope to
+ // properly unload the page, and must do so before the page's document and
+ // `this` get destroyed.
+ // The caller cannot call this for a `page_index` if it already obtained and
+ // holds the page handle for that page.
+ ScopedEmbedderTestPage LoadScopedPage(int page_index);
+
+ // Prefer LoadScopedPage() above.
+ //
+ // Load a specific page of the open document with a given non-negative
// `page_index`. On success, fire form events for the page and return a page
// handle. On failure, return nullptr.
// The caller does not own the returned page handle, but must call
