Fix page leaks in an embedder test
Embedder test's delegate function GetPage() calls FPDF_LoadPage() to
load a page which may be already loaded by embedder test itself.
Thus the page's ref count is increased unnecessarily.
This causes the page to be leaked. Fix this by putting the page map in
embedder test class and guarantee the page is loaded only once.
Also, fix leaks in this embedder tests by unloading the loaded pages to
properly release the resource.
BUG=pdfium:242
Review-Url: https://codereview.chromium.org/2258333002
diff --git a/fpdfsdk/fpdfformfill_embeddertest.cpp b/fpdfsdk/fpdfformfill_embeddertest.cpp
index 3a6a744..baf4853 100644
--- a/fpdfsdk/fpdfformfill_embeddertest.cpp
+++ b/fpdfsdk/fpdfformfill_embeddertest.cpp
@@ -45,7 +45,7 @@
SetDelegate(&delegate);
EXPECT_TRUE(OpenDocument("bug_507316.pdf"));
- FPDF_PAGE page = LoadAndCachePage(2);
+ FPDF_PAGE page = LoadPage(2);
EXPECT_TRUE(page);
DoOpenActions();
delegate.AdvanceTime(4000);
diff --git a/fpdfsdk/fsdk_baseform_embeddertest.cpp b/fpdfsdk/fsdk_baseform_embeddertest.cpp
index 622f1a8..7af8262 100644
--- a/fpdfsdk/fsdk_baseform_embeddertest.cpp
+++ b/fpdfsdk/fsdk_baseform_embeddertest.cpp
@@ -26,9 +26,12 @@
TEST_F(FSDKBaseFormEmbeddertest, CBA_AnnotIterator) {
EXPECT_TRUE(OpenDocument("annotiter.pdf"));
- EXPECT_TRUE(LoadPage(0));
- EXPECT_TRUE(LoadPage(1));
- EXPECT_TRUE(LoadPage(2));
+ FPDF_PAGE page0 = LoadPage(0);
+ FPDF_PAGE page1 = LoadPage(1);
+ FPDF_PAGE page2 = LoadPage(2);
+ EXPECT_TRUE(page0);
+ EXPECT_TRUE(page1);
+ EXPECT_TRUE(page2);
CFX_FloatRect LeftBottom(200, 200, 220, 220);
CFX_FloatRect RightBottom(400, 201, 420, 221);
@@ -112,4 +115,7 @@
pAnnot = iter.GetPrevAnnot(pAnnot);
EXPECT_EQ(iter.GetLastAnnot(), pAnnot);
}
+ UnloadPage(page2);
+ UnloadPage(page1);
+ UnloadPage(page0);
}
diff --git a/testing/embedder_test.cpp b/testing/embedder_test.cpp
index b1207fb..e10e2a8 100644
--- a/testing/embedder_test.cpp
+++ b/testing/embedder_test.cpp
@@ -242,21 +242,19 @@
}
FPDF_PAGE EmbedderTest::LoadPage(int page_number) {
+ // First check whether it is loaded already.
+ auto it = page_map_.find(page_number);
+ if (it != page_map_.end())
+ return it->second;
+
FPDF_PAGE page = FPDF_LoadPage(document_, page_number);
if (!page) {
return nullptr;
}
FORM_OnAfterLoadPage(page, form_handle_);
FORM_DoPageAAction(page, form_handle_, FPDFPAGE_AACTION_OPEN);
- return page;
-}
-
-FPDF_PAGE EmbedderTest::LoadAndCachePage(int page_number) {
- FPDF_PAGE page = delegate_->GetPage(form_handle_, document_, page_number);
- if (!page) {
- return nullptr;
- }
- FORM_DoPageAAction(page, form_handle_, FPDFPAGE_AACTION_OPEN);
+ // Cache the page.
+ page_map_[page_number] = page;
return page;
}
@@ -278,20 +276,12 @@
FPDF_ClosePage(page);
}
-FPDF_PAGE EmbedderTest::Delegate::GetPage(FPDF_FORMHANDLE form_handle,
+FPDF_PAGE EmbedderTest::Delegate::GetPage(FPDF_FORMFILLINFO* info,
FPDF_DOCUMENT document,
int page_index) {
- auto it = m_pageMap.find(page_index);
- if (it != m_pageMap.end()) {
- return it->second;
- }
- FPDF_PAGE page = FPDF_LoadPage(document, page_index);
- if (!page) {
- return nullptr;
- }
- m_pageMap[page_index] = page;
- FORM_OnAfterLoadPage(page, form_handle);
- return page;
+ EmbedderTest* test = static_cast<EmbedderTest*>(info);
+ auto it = test->page_map_.find(page_index);
+ return it != test->page_map_.end() ? it->second : nullptr;
}
// static
@@ -329,8 +319,8 @@
FPDF_PAGE EmbedderTest::GetPageTrampoline(FPDF_FORMFILLINFO* info,
FPDF_DOCUMENT document,
int page_index) {
- EmbedderTest* test = static_cast<EmbedderTest*>(info);
- return test->delegate_->GetPage(test->form_handle(), document, page_index);
+ return static_cast<EmbedderTest*>(info)->delegate_->GetPage(info, document,
+ page_index);
}
// Can't use gtest-provided main since we need to stash the path to the
diff --git a/testing/embedder_test.h b/testing/embedder_test.h
index 6b814a7..153ca6e 100644
--- a/testing/embedder_test.h
+++ b/testing/embedder_test.h
@@ -51,12 +51,9 @@
virtual void KillTimer(int id) {}
// Equivalent to FPDF_FORMFILLINFO::FFI_GetPage().
- virtual FPDF_PAGE GetPage(FPDF_FORMHANDLE form_handle,
+ virtual FPDF_PAGE GetPage(FPDF_FORMFILLINFO* info,
FPDF_DOCUMENT document,
int page_index);
-
- private:
- std::map<int, FPDF_PAGE> m_pageMap;
};
EmbedderTest();
@@ -101,10 +98,6 @@
// Load a specific page of the open document.
virtual FPDF_PAGE LoadPage(int page_number);
- // Load a specific page of the open document using delegate_->GetPage.
- // delegate_->GetPage also caches loaded page.
- virtual FPDF_PAGE LoadAndCachePage(int page_number);
-
// Convert a loaded page into a bitmap.
virtual FPDF_BITMAP RenderPage(FPDF_PAGE page);
@@ -130,6 +123,7 @@
TestLoader* loader_;
size_t file_length_;
std::unique_ptr<char, pdfium::FreeDeleter> file_contents_;
+ std::map<int, FPDF_PAGE> page_map_;
private:
static void UnsupportedHandlerTrampoline(UNSUPPORT_INFO*, int type);