commit 38f725fb445ad4da15229208005d768fafc48a90
author Tom Sepez <tsepez@chromium.org> Wed Dec 04 00:26:34 2019 +0000
committer Chromium commit bot <commit-bot@chromium.org> Wed Dec 04 00:26:34 2019 +0000
tree 235dca8f4486f6c2382b264900ff97027a63a166
parent 96b67f6045b97cfe78a3d782bfb422ba77d39783

Cover some more nullptr-safe paths in fpdfview.h public functions.

Also introduce EmbedderTest::CloseDocument() public method, and
use it to check init/destroy idempotency.

Change-Id: I580a1180b447925d1a1bbc53f85ffc080c283b8d
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/62850
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>

fpdfsdk/fpdf_view_embeddertest.cpp

diff --git a/fpdfsdk/fpdf_view_embeddertest.cpp b/fpdfsdk/fpdf_view_embeddertest.cpp
index 1f52d68..cdbd8b3 100644
--- a/fpdfsdk/fpdf_view_embeddertest.cpp
+++ b/fpdfsdk/fpdf_view_embeddertest.cpp
@@ -266,6 +266,19 @@
   UnloadPage(page);
 }
 
+TEST_F(FPDFViewEmbedderTest, MultipleInitDestroy) {
+  FPDF_InitLibrary();  // Redundant given call in SetUp(), but safe.
+  FPDF_InitLibrary();  // Doubly-redundant even, but safe.
+
+  EXPECT_TRUE(OpenDocument("about_blank.pdf"));
+  CloseDocument();
+  CloseDocument();  // Redundant given above, but safe.
+  CloseDocument();  // Doubly-redundant even, but safe.
+
+  FPDF_DestroyLibrary();  // Doubly-redundant even, but safe.
+  FPDF_DestroyLibrary();  // Redundant given call in TearDown(), but safe.
+}
+
 TEST_F(FPDFViewEmbedderTest, Document) {
   EXPECT_TRUE(OpenDocument("about_blank.pdf"));
   EXPECT_EQ(1, GetPageCount());
@@ -364,7 +377,7 @@
 TEST_F(FPDFViewEmbedderTest, Page) {
   EXPECT_TRUE(OpenDocument("about_blank.pdf"));
   FPDF_PAGE page = LoadPage(0);
-  EXPECT_NE(nullptr, page);
+  EXPECT_TRUE(page);
 
   EXPECT_EQ(612.0, FPDF_GetPageWidth(page));
   EXPECT_EQ(792.0, FPDF_GetPageHeight(page));
@@ -376,8 +389,14 @@
   EXPECT_EQ(612.0, rect.right);
   EXPECT_EQ(792.0, rect.top);
 
+  // Null arguments return errors rather than crashing,
+  EXPECT_EQ(0.0, FPDF_GetPageWidth(nullptr));
+  EXPECT_EQ(0.0, FPDF_GetPageHeight(nullptr));
+  EXPECT_FALSE(FPDF_GetPageBoundingBox(nullptr, &rect));
+  EXPECT_FALSE(FPDF_GetPageBoundingBox(page, nullptr));
+
   UnloadPage(page);
-  EXPECT_EQ(nullptr, LoadPage(1));
+  EXPECT_FALSE(LoadPage(1));
 }
 
 TEST_F(FPDFViewEmbedderTest, ViewerRefDummy) {

testing/embedder_test.cpp

diff --git a/testing/embedder_test.cpp b/testing/embedder_test.cpp
index 4507bb9..dc9b04a 100644
--- a/testing/embedder_test.cpp
+++ b/testing/embedder_test.cpp
@@ -100,8 +100,7 @@
 
   if (document_) {
     FORM_DoDocumentAAction(form_handle_, FPDFDOC_AACTION_WC);
-    FPDFDOC_ExitFormFillEnvironment(form_handle_);
-    FPDF_CloseDocument(document_);
+    CloseDocument();
   }
 
   FPDFAvail_Destroy(avail_);
@@ -244,6 +243,14 @@
   return true;
 }
 
+void EmbedderTest::CloseDocument() {
+  FPDFDOC_ExitFormFillEnvironment(form_handle_);
+  form_handle_ = nullptr;
+
+  FPDF_CloseDocument(document_);
+  document_ = nullptr;
+}
+
 FPDF_FORMHANDLE EmbedderTest::SetupFormFillEnvironment(
     FPDF_DOCUMENT doc,
     JavaScriptOption javascript_option) {

testing/embedder_test.h

diff --git a/testing/embedder_test.h b/testing/embedder_test.h
index 0b9abbe..0711bfa 100644
--- a/testing/embedder_test.h
+++ b/testing/embedder_test.h
@@ -106,6 +106,11 @@
                                 const char* password);
   bool OpenDocumentWithoutJavaScript(const std::string& filename);
 
+  // Close the document from a previous OpenDocument() call. This happens
+  // automatically at tear-down, and is usually not explicitly required,
+  // unless testing multiple documents or duplicate destruction.
+  void CloseDocument();
+
   // Perform JavaScript actions that are to run at document open time.
   void DoOpenActions();