From 2d11d72e326140b9abeb6de2db1e28e5bf9d7e64 Mon Sep 17 00:00:00 2001
From: Tom Sepez <tsepez@chromium.org>
Date: Thu, 31 May 2018 23:38:32 +0000
Subject: Make CPDF_Document own its Extension.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Inverting the ownership from the current situation makes cleanup
much more intuitive.

Change-Id: Iad9a7ca70c0746170ba753297732e3e34f96c5ba
Reviewed-on: https://pdfium-review.googlesource.com/33190
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: dsinclair <dsinclair@chromium.org>
Reviewed-by: Nicolás Peña Moreno <npm@chromium.org>
---
 core/fpdfapi/parser/cpdf_document.h |  8 +++++---
 fpdfsdk/cpdfsdk_helpers.cpp         |  2 +-
 fpdfsdk/fpdf_view.cpp               | 14 +++-----------
 fpdfsdk/fpdfxfa/cpdfxfa_context.cpp | 11 +++++------
 fpdfsdk/fpdfxfa/cpdfxfa_context.h   |  4 ++--
 5 files changed, 16 insertions(+), 23 deletions(-)

diff --git a/core/fpdfapi/parser/cpdf_document.h b/core/fpdfapi/parser/cpdf_document.h
index c240e77567..c096e89e28 100644
--- a/core/fpdfapi/parser/cpdf_document.h
+++ b/core/fpdfapi/parser/cpdf_document.h
@@ -56,8 +56,10 @@ class CPDF_Document : public CPDF_IndirectObjectHolder {
   explicit CPDF_Document(std::unique_ptr<CPDF_Parser> pParser);
   ~CPDF_Document() override;
 
-  Extension* GetExtension() const { return m_pExtension.Get(); }
-  void SetExtension(Extension* pExt) { m_pExtension = pExt; }
+  Extension* GetExtension() const { return m_pExtension.get(); }
+  void SetExtension(std::unique_ptr<Extension> pExt) {
+    m_pExtension = std::move(pExt);
+  }
 
   CPDF_Parser* GetParser() const { return m_pParser.get(); }
   const CPDF_Dictionary* GetRoot() const { return m_pRootDict; }
@@ -176,7 +178,7 @@ class CPDF_Document : public CPDF_IndirectObjectHolder {
   // Dict {objnum, gennum} to page mapping.
   std::map<std::pair<uint32_t, uint32_t>, CPDF_Page::ObservedPtr> m_PageMap;
 
-  UnownedPtr<Extension> m_pExtension;
+  std::unique_ptr<Extension> m_pExtension;
 };
 
 #endif  // CORE_FPDFAPI_PARSER_CPDF_DOCUMENT_H_
diff --git a/fpdfsdk/cpdfsdk_helpers.cpp b/fpdfsdk/cpdfsdk_helpers.cpp
index c6861bc05a..210fcf8b84 100644
--- a/fpdfsdk/cpdfsdk_helpers.cpp
+++ b/fpdfsdk/cpdfsdk_helpers.cpp
@@ -158,7 +158,7 @@ CPDF_Document* CPDFDocumentFromFPDFDocument(FPDF_DOCUMENT doc) {
 FPDF_DOCUMENT FPDFDocumentFromCPDFDocument(CPDF_Document* doc) {
 #ifdef PDF_ENABLE_XFA
   if (doc && !doc->GetExtension())
-    doc->SetExtension(new CPDFXFA_Context(pdfium::WrapUnique(doc)));
+    doc->SetExtension(pdfium::MakeUnique<CPDFXFA_Context>(doc));
 #endif  // PDF_ENABLE_XFA
   return reinterpret_cast<FPDF_DOCUMENT>(doc);
 }
diff --git a/fpdfsdk/fpdf_view.cpp b/fpdfsdk/fpdf_view.cpp
index 5304e330b7..ef242295b7 100644
--- a/fpdfsdk/fpdf_view.cpp
+++ b/fpdfsdk/fpdf_view.cpp
@@ -6,6 +6,7 @@
 
 #include "public/fpdfview.h"
 
+#include <memory>
 #include <utility>
 #include <vector>
 
@@ -748,17 +749,8 @@ FPDF_EXPORT void FPDF_CALLCONV FPDF_ClosePage(FPDF_PAGE page) {
 }
 
 FPDF_EXPORT void FPDF_CALLCONV FPDF_CloseDocument(FPDF_DOCUMENT document) {
-  auto* pDoc = CPDFDocumentFromFPDFDocument(document);
-  if (!pDoc)
-    return;
-
-  // Deleting the extension will delete the document
-  if (pDoc->GetExtension()) {
-    delete pDoc->GetExtension();
-    return;
-  }
-
-  delete pDoc;
+  // Take it back across the API and throw it away,
+  std::unique_ptr<CPDF_Document>(CPDFDocumentFromFPDFDocument(document));
 }
 
 FPDF_EXPORT unsigned long FPDF_CALLCONV FPDF_GetLastError() {
diff --git a/fpdfsdk/fpdfxfa/cpdfxfa_context.cpp b/fpdfsdk/fpdfxfa/cpdfxfa_context.cpp
index 3b10f0d51e..36d86650b7 100644
--- a/fpdfsdk/fpdfxfa/cpdfxfa_context.cpp
+++ b/fpdfsdk/fpdfxfa/cpdfxfa_context.cpp
@@ -36,11 +36,10 @@ extern void SetLastError(int err);
 extern int GetLastError();
 #endif
 
-CPDFXFA_Context::CPDFXFA_Context(std::unique_ptr<CPDF_Document> pPDFDoc)
-    : m_pPDFDoc(std::move(pPDFDoc)),
+CPDFXFA_Context::CPDFXFA_Context(CPDF_Document* pPDFDoc)
+    : m_pPDFDoc(pPDFDoc),
       m_pXFAApp(pdfium::MakeUnique<CXFA_FFApp>(this)),
-      m_DocEnv(this) {
-}
+      m_DocEnv(this) {}
 
 CPDFXFA_Context::~CPDFXFA_Context() {
   m_nLoadStatus = FXFA_LOADSTATUS_CLOSING;
@@ -99,7 +98,7 @@ bool CPDFXFA_Context::LoadXFADoc() {
     return false;
 
   m_pXFADoc = pdfium::MakeUnique<CXFA_FFDoc>(pApp, &m_DocEnv);
-  if (!m_pXFADoc->OpenDoc(m_pPDFDoc.get())) {
+  if (!m_pXFADoc->OpenDoc(m_pPDFDoc.Get())) {
     SetLastError(FPDF_ERR_XFALOAD);
     return false;
   }
@@ -189,7 +188,7 @@ RetainPtr<CPDFXFA_Page> CPDFXFA_Context::GetXFAPage(
 }
 
 CPDF_Document* CPDFXFA_Context::GetPDFDoc() const {
-  return m_pPDFDoc.get();
+  return m_pPDFDoc.Get();
 }
 
 void CPDFXFA_Context::DeletePage(int page_index) {
diff --git a/fpdfsdk/fpdfxfa/cpdfxfa_context.h b/fpdfsdk/fpdfxfa/cpdfxfa_context.h
index a01990cce4..b240e7ee11 100644
--- a/fpdfsdk/fpdfxfa/cpdfxfa_context.h
+++ b/fpdfsdk/fpdfxfa/cpdfxfa_context.h
@@ -36,7 +36,7 @@ enum LoadStatus {
 class CPDFXFA_Context : public CPDF_Document::Extension,
                         public IXFA_AppProvider {
  public:
-  explicit CPDFXFA_Context(std::unique_ptr<CPDF_Document> pPDFDoc);
+  explicit CPDFXFA_Context(CPDF_Document* pPDFDoc);
   ~CPDFXFA_Context() override;
 
   bool LoadXFADoc();
@@ -112,7 +112,7 @@ class CPDFXFA_Context : public CPDF_Document::Extension,
   void CloseXFADoc();
 
   FormType m_FormType = FormType::kNone;
-  std::unique_ptr<CPDF_Document> m_pPDFDoc;
+  UnownedPtr<CPDF_Document> m_pPDFDoc;
   std::unique_ptr<CXFA_FFDoc> m_pXFADoc;
   Observable<CPDFSDK_FormFillEnvironment>::ObservedPtr m_pFormFillEnv;
   UnownedPtr<CXFA_FFDocView> m_pXFADocView;
-- 
cgit v1.2.3