Simplify some FPDF edit functions.

Fix an unlikely memory leak in FPDFPage_InsertObject().

BUG=pdfium:545

Review-Url: https://codereview.chromium.org/2195643002

3 files changed, 86 insertions, 85 deletions

diff --git a/core/fpdfapi/fpdf_page/include/cpdf_formobject.h b/core/fpdfapi/fpdf_page/include/cpdf_formobject.h
index 03e117a132..5b146b268d 100644
--- a/core/fpdfapi/fpdf_page/include/cpdf_formobject.h
+++ b/core/fpdfapi/fpdf_page/include/cpdf_formobject.h
@@ -28,6 +28,7 @@ class CPDF_FormObject : public CPDF_PageObject {
   const CPDF_FormObject* AsForm() const override;
 
   void CalcBoundingBox();
+  const CPDF_Form* form() const { return m_pForm.get(); }
 
   std::unique_ptr<CPDF_Form> m_pForm;
   CFX_Matrix m_FormMatrix;
diff --git a/fpdfsdk/fpdfeditimg.cpp b/fpdfsdk/fpdfeditimg.cpp
index 4429dcd201..b2c25779c6 100644
--- a/fpdfsdk/fpdfeditimg.cpp
+++ b/fpdfsdk/fpdfeditimg.cpp
@@ -17,9 +17,9 @@ FPDFPageObj_NewImgeObj(FPDF_DOCUMENT document) {
   CPDF_Document* pDoc = CPDFDocumentFromFPDFDocument(document);
   if (!pDoc)
     return nullptr;
+
   CPDF_ImageObject* pImageObj = new CPDF_ImageObject;
-  CPDF_Image* pImg = new CPDF_Image(pDoc);
-  pImageObj->m_pImage = pImg;
+  pImageObj->m_pImage = new CPDF_Image(pDoc);
   return pImageObj;
 }
 
@@ -32,13 +32,12 @@ FPDFImageObj_LoadJpegFile(FPDF_PAGE* pages,
     return FALSE;
 
   IFX_FileRead* pFile = new CPDF_CustomAccess(fileAccess);
-  CPDF_ImageObject* pImgObj = (CPDF_ImageObject*)image_object;
+  CPDF_ImageObject* pImgObj = reinterpret_cast<CPDF_ImageObject*>(image_object);
   pImgObj->m_GeneralState.GetModify();
   for (int index = 0; index < nCount; index++) {
     CPDF_Page* pPage = CPDFPageFromFPDFPage(pages[index]);
-    if (!pPage)
-      continue;
-    pImgObj->m_pImage->ResetCache(pPage, nullptr);
+    if (pPage)
+      pImgObj->m_pImage->ResetCache(pPage, nullptr);
   }
   pImgObj->m_pImage->SetJpegImage(pFile);
 
@@ -54,13 +53,14 @@ DLLEXPORT FPDF_BOOL STDCALL FPDFImageObj_SetMatrix(FPDF_PAGEOBJECT image_object,
                                                    double f) {
   if (!image_object)
     return FALSE;
-  CPDF_ImageObject* pImgObj = (CPDF_ImageObject*)image_object;
-  pImgObj->m_Matrix.a = (FX_FLOAT)a;
-  pImgObj->m_Matrix.b = (FX_FLOAT)b;
-  pImgObj->m_Matrix.c = (FX_FLOAT)c;
-  pImgObj->m_Matrix.d = (FX_FLOAT)d;
-  pImgObj->m_Matrix.e = (FX_FLOAT)e;
-  pImgObj->m_Matrix.f = (FX_FLOAT)f;
+
+  CPDF_ImageObject* pImgObj = reinterpret_cast<CPDF_ImageObject*>(image_object);
+  pImgObj->m_Matrix.a = static_cast<FX_FLOAT>(a);
+  pImgObj->m_Matrix.b = static_cast<FX_FLOAT>(b);
+  pImgObj->m_Matrix.c = static_cast<FX_FLOAT>(c);
+  pImgObj->m_Matrix.d = static_cast<FX_FLOAT>(d);
+  pImgObj->m_Matrix.e = static_cast<FX_FLOAT>(e);
+  pImgObj->m_Matrix.f = static_cast<FX_FLOAT>(f);
   pImgObj->CalcBoundingBox();
   return TRUE;
 }
@@ -71,17 +71,15 @@ DLLEXPORT FPDF_BOOL STDCALL FPDFImageObj_SetBitmap(FPDF_PAGE* pages,
                                                    FPDF_BITMAP bitmap) {
   if (!image_object || !bitmap || !pages)
     return FALSE;
-  CFX_DIBitmap* pBmp = nullptr;
-  pBmp = (CFX_DIBitmap*)bitmap;
-  CPDF_ImageObject* pImgObj = (CPDF_ImageObject*)image_object;
+
+  CPDF_ImageObject* pImgObj = reinterpret_cast<CPDF_ImageObject*>(image_object);
   pImgObj->m_GeneralState.GetModify();
   for (int index = 0; index < nCount; index++) {
     CPDF_Page* pPage = CPDFPageFromFPDFPage(pages[index]);
-    if (!pPage)
-      continue;
-    pImgObj->m_pImage->ResetCache(pPage, nullptr);
+    if (pPage)
+      pImgObj->m_pImage->ResetCache(pPage, nullptr);
   }
-  pImgObj->m_pImage->SetImage(pBmp, FALSE);
+  pImgObj->m_pImage->SetImage(reinterpret_cast<CFX_DIBitmap*>(bitmap), FALSE);
   pImgObj->CalcBoundingBox();
   return TRUE;
 }
diff --git a/fpdfsdk/fpdfeditpage.cpp b/fpdfsdk/fpdfeditpage.cpp
index 0d0dac4964..0daa459037 100644
--- a/fpdfsdk/fpdfeditpage.cpp
+++ b/fpdfsdk/fpdfeditpage.cpp
@@ -6,6 +6,10 @@
 
 #include "public/fpdf_edit.h"
 
+#include <algorithm>
+#include <memory>
+#include <utility>
+
 #include "core/fpdfapi/fpdf_edit/include/cpdf_pagecontentgenerator.h"
 #include "core/fpdfapi/fpdf_page/include/cpdf_form.h"
 #include "core/fpdfapi/fpdf_page/include/cpdf_formobject.h"
@@ -37,6 +41,29 @@
 #include <ctime>
 #endif
 
+namespace {
+
+static_assert(FPDF_PAGEOBJ_TEXT == CPDF_PageObject::TEXT,
+              "FPDF_PAGEOBJ_TEXT/CPDF_PageObject::TEXT mismatch");
+static_assert(FPDF_PAGEOBJ_PATH == CPDF_PageObject::PATH,
+              "FPDF_PAGEOBJ_PATH/CPDF_PageObject::PATH mismatch");
+static_assert(FPDF_PAGEOBJ_IMAGE == CPDF_PageObject::IMAGE,
+              "FPDF_PAGEOBJ_IMAGE/CPDF_PageObject::IMAGE mismatch");
+static_assert(FPDF_PAGEOBJ_SHADING == CPDF_PageObject::SHADING,
+              "FPDF_PAGEOBJ_SHADING/CPDF_PageObject::SHADING mismatch");
+static_assert(FPDF_PAGEOBJ_FORM == CPDF_PageObject::FORM,
+              "FPDF_PAGEOBJ_FORM/CPDF_PageObject::FORM mismatch");
+
+bool IsPageObject(CPDF_Page* pPage) {
+  if (!pPage || !pPage->m_pFormDict || !pPage->m_pFormDict->KeyExist("Type"))
+    return false;
+
+  CPDF_Object* pObject = pPage->m_pFormDict->GetObjectBy("Type")->GetDirect();
+  return pObject && !pObject->GetString().Compare("Page");
+}
+
+}  // namespace
+
 DLLEXPORT FPDF_DOCUMENT STDCALL FPDF_CreateNewDocument() {
   CPDF_Document* pDoc = new CPDF_Document(nullptr);
   pDoc->CreateNewDoc();
@@ -79,14 +106,11 @@ DLLEXPORT FPDF_PAGE STDCALL FPDFPage_New(FPDF_DOCUMENT document,
   if (!pDoc)
     return nullptr;
 
-  if (page_index < 0)
-    page_index = 0;
-  if (pDoc->GetPageCount() < page_index)
-    page_index = pDoc->GetPageCount();
-
+  page_index = std::min(std::max(page_index, 0), pDoc->GetPageCount());
   CPDF_Dictionary* pPageDict = pDoc->CreateNewPage(page_index);
   if (!pPageDict)
     return nullptr;
+
   CPDF_Array* pMediaBoxArray = new CPDF_Array;
   pMediaBoxArray->Add(new CPDF_Number(0));
   pMediaBoxArray->Add(new CPDF_Number(0));
@@ -111,16 +135,10 @@ DLLEXPORT FPDF_PAGE STDCALL FPDFPage_New(FPDF_DOCUMENT document,
 
 DLLEXPORT int STDCALL FPDFPage_GetRotation(FPDF_PAGE page) {
   CPDF_Page* pPage = CPDFPageFromFPDFPage(page);
-  if (!pPage || !pPage->m_pFormDict || !pPage->m_pFormDict->KeyExist("Type") ||
-      !pPage->m_pFormDict->GetObjectBy("Type")->GetDirect() ||
-      pPage->m_pFormDict->GetObjectBy("Type")->GetDirect()->GetString().Compare(
-          "Page")) {
-    return -1;
-  }
-  CPDF_Dictionary* pDict = pPage->m_pFormDict;
-  if (!pDict)
+  if (!IsPageObject(pPage))
     return -1;
 
+  CPDF_Dictionary* pDict = pPage->m_pFormDict;
   while (pDict) {
     if (pDict->KeyExist("Rotate")) {
       CPDF_Object* pRotateObj = pDict->GetObjectBy("Rotate")->GetDirect();
@@ -137,68 +155,59 @@ DLLEXPORT int STDCALL FPDFPage_GetRotation(FPDF_PAGE page) {
 
 DLLEXPORT void STDCALL FPDFPage_InsertObject(FPDF_PAGE page,
                                              FPDF_PAGEOBJECT page_obj) {
-  CPDF_Page* pPage = CPDFPageFromFPDFPage(page);
-  if (!pPage || !pPage->m_pFormDict || !pPage->m_pFormDict->KeyExist("Type") ||
-      !pPage->m_pFormDict->GetObjectBy("Type")->GetDirect() ||
-      pPage->m_pFormDict->GetObjectBy("Type")->GetDirect()->GetString().Compare(
-          "Page")) {
-    return;
-  }
-  CPDF_PageObject* pPageObj = (CPDF_PageObject*)page_obj;
+  CPDF_PageObject* pPageObj = reinterpret_cast<CPDF_PageObject*>(page_obj);
   if (!pPageObj)
     return;
 
-  pPage->GetPageObjectList()->push_back(
-      std::unique_ptr<CPDF_PageObject>(pPageObj));
+  std::unique_ptr<CPDF_PageObject> pPageObjHolder(pPageObj);
+  CPDF_Page* pPage = CPDFPageFromFPDFPage(page);
+  if (!IsPageObject(pPage))
+    return;
 
+  pPage->GetPageObjectList()->push_back(std::move(pPageObjHolder));
   switch (pPageObj->GetType()) {
-    case FPDF_PAGEOBJ_PATH: {
-      CPDF_PathObject* pPathObj = pPageObj->AsPath();
-      pPathObj->CalcBoundingBox();
+    case CPDF_PageObject::TEXT: {
       break;
     }
-    case FPDF_PAGEOBJ_TEXT: {
+    case CPDF_PageObject::PATH: {
+      CPDF_PathObject* pPathObj = pPageObj->AsPath();
+      pPathObj->CalcBoundingBox();
       break;
     }
-    case FPDF_PAGEOBJ_IMAGE: {
+    case CPDF_PageObject::IMAGE: {
       CPDF_ImageObject* pImageObj = pPageObj->AsImage();
       pImageObj->CalcBoundingBox();
       break;
     }
-    case FPDF_PAGEOBJ_SHADING: {
+    case CPDF_PageObject::SHADING: {
       CPDF_ShadingObject* pShadingObj = pPageObj->AsShading();
       pShadingObj->CalcBoundingBox();
       break;
     }
-    case FPDF_PAGEOBJ_FORM: {
+    case CPDF_PageObject::FORM: {
       CPDF_FormObject* pFormObj = pPageObj->AsForm();
       pFormObj->CalcBoundingBox();
       break;
     }
-    default:
+    default: {
+      ASSERT(false);
       break;
+    }
   }
 }
 
 DLLEXPORT int STDCALL FPDFPage_CountObject(FPDF_PAGE page) {
   CPDF_Page* pPage = CPDFPageFromFPDFPage(page);
-  if (!pPage || !pPage->m_pFormDict || !pPage->m_pFormDict->KeyExist("Type") ||
-      !pPage->m_pFormDict->GetObjectBy("Type")->GetDirect() ||
-      pPage->m_pFormDict->GetObjectBy("Type")->GetDirect()->GetString().Compare(
-          "Page")) {
+  if (!IsPageObject(pPage))
     return -1;
-  }
   return pdfium::CollectionSize<int>(*pPage->GetPageObjectList());
 }
 
 DLLEXPORT FPDF_PAGEOBJECT STDCALL FPDFPage_GetObject(FPDF_PAGE page,
                                                      int index) {
   CPDF_Page* pPage = CPDFPageFromFPDFPage(page);
-  if (!pPage || !pPage->m_pFormDict || !pPage->m_pFormDict->KeyExist("Type") ||
-      pPage->m_pFormDict->GetObjectBy("Type")->GetDirect()->GetString().Compare(
-          "Page")) {
+  if (!IsPageObject(pPage))
     return nullptr;
-  }
   return pPage->GetPageObjectList()->GetPageObjectByIndex(index);
 }
 
@@ -211,8 +220,8 @@ DLLEXPORT FPDF_BOOL STDCALL
 FPDFPageObj_HasTransparency(FPDF_PAGEOBJECT pageObject) {
   if (!pageObject)
     return FALSE;
-  CPDF_PageObject* pPageObj = (CPDF_PageObject*)pageObject;
 
+  CPDF_PageObject* pPageObj = reinterpret_cast<CPDF_PageObject*>(pageObject);
   const CPDF_GeneralStateData* pGeneralState =
       pPageObj->m_GeneralState.GetObject();
   int blend_type =
@@ -228,35 +237,30 @@ FPDFPageObj_HasTransparency(FPDF_PAGEOBJECT pageObject) {
   if (pGeneralState && pGeneralState->m_FillAlpha != 1.0f)
     return TRUE;
 
-  if (pPageObj->IsPath()) {
-    if (pGeneralState && pGeneralState->m_StrokeAlpha != 1.0f)
-      return TRUE;
+  if (pPageObj->IsPath() && pGeneralState &&
+      pGeneralState->m_StrokeAlpha != 1.0f) {
+    return TRUE;
   }
 
   if (pPageObj->IsForm()) {
-    CPDF_FormObject* pFormObj = pPageObj->AsForm();
-    if (pFormObj->m_pForm &&
-        (pFormObj->m_pForm->m_Transparency & PDFTRANS_ISOLATED))
-      return TRUE;
-    if (pFormObj->m_pForm &&
-        (!(pFormObj->m_pForm->m_Transparency & PDFTRANS_ISOLATED) &&
-         (pFormObj->m_pForm->m_Transparency & PDFTRANS_GROUP)))
-      return TRUE;
+    const CPDF_Form* pForm = pPageObj->AsForm()->form();
+    if (pForm) {
+      int trans = pForm->m_Transparency;
+      if ((trans & PDFTRANS_ISOLATED) || (trans & PDFTRANS_GROUP))
+        return TRUE;
+    }
   }
+
   return FALSE;
 }
 
 DLLEXPORT FPDF_BOOL STDCALL FPDFPage_GenerateContent(FPDF_PAGE page) {
   CPDF_Page* pPage = CPDFPageFromFPDFPage(page);
-  if (!pPage || !pPage->m_pFormDict || !pPage->m_pFormDict->KeyExist("Type") ||
-      !pPage->m_pFormDict->GetObjectBy("Type")->GetDirect() ||
-      pPage->m_pFormDict->GetObjectBy("Type")->GetDirect()->GetString().Compare(
-          "Page")) {
+  if (!IsPageObject(pPage))
     return FALSE;
-  }
+
   CPDF_PageContentGenerator CG(pPage);
   CG.GenerateContent();
-
   return TRUE;
 }
 
@@ -267,7 +271,7 @@ DLLEXPORT void STDCALL FPDFPageObj_Transform(FPDF_PAGEOBJECT page_object,
                                              double d,
                                              double e,
                                              double f) {
-  CPDF_PageObject* pPageObj = (CPDF_PageObject*)page_object;
+  CPDF_PageObject* pPageObj = reinterpret_cast<CPDF_PageObject*>(page_object);
   if (!pPageObj)
     return;
 
@@ -275,6 +279,7 @@ DLLEXPORT void STDCALL FPDFPageObj_Transform(FPDF_PAGEOBJECT page_object,
                     (FX_FLOAT)e, (FX_FLOAT)f);
   pPageObj->Transform(matrix);
 }
+
 DLLEXPORT void STDCALL FPDFPage_TransformAnnots(FPDF_PAGE page,
                                                 double a,
                                                 double b,
@@ -285,6 +290,7 @@ DLLEXPORT void STDCALL FPDFPage_TransformAnnots(FPDF_PAGE page,
   CPDF_Page* pPage = CPDFPageFromFPDFPage(page);
   if (!pPage)
     return;
+
   CPDF_AnnotList AnnotList(pPage);
   for (size_t i = 0; i < AnnotList.Count(); ++i) {
     CPDF_Annot* pAnnot = AnnotList.GetAt(i);
@@ -310,14 +316,10 @@ DLLEXPORT void STDCALL FPDFPage_TransformAnnots(FPDF_PAGE page,
 
 DLLEXPORT void STDCALL FPDFPage_SetRotation(FPDF_PAGE page, int rotate) {
   CPDF_Page* pPage = CPDFPageFromFPDFPage(page);
-  if (!pPage || !pPage->m_pFormDict || !pPage->m_pFormDict->KeyExist("Type") ||
-      !pPage->m_pFormDict->GetObjectBy("Type")->GetDirect() ||
-      pPage->m_pFormDict->GetObjectBy("Type")->GetDirect()->GetString().Compare(
-          "Page")) {
+  if (!IsPageObject(pPage))
     return;
-  }
+
   CPDF_Dictionary* pDict = pPage->m_pFormDict;
   rotate %= 4;
-
   pDict->SetAt("Rotate", new CPDF_Number(rotate * 90));
 }