Make creation of CPDFSDK_Document clearer

fpdfsdk/fpdfformfill.cpp

 // Copyright 2014 PDFium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
 
 #include "public/fpdf_formfill.h"
 
 #include <memory>
 #include <vector>
@@ skipping 225 matching lines @@
   const int kRequiredVersion = 1;
 #endif  // PDF_ENABLE_XFA
   if (!formInfo || formInfo->version != kRequiredVersion)
     return nullptr;
 
   UnderlyingDocumentType* pDocument = UnderlyingFromFPDFDocument(document);
   if (!pDocument)
     return nullptr;
 
   CPDFSDK_Environment* pEnv = new CPDFSDK_Environment(pDocument, formInfo);
+  CPDFSDK_Document* pSDKDoc = new CPDFSDK_Document(pDocument, pEnv);

[Tom Sepez, 2016/09/22 17:02:22]

I like how this adds credibility to the theory that these two should merge.  I
think this the only place we create these two classes.  If so, we can get rid of
setsdkdocument and make ctor do the new'ing of the document itself.

Otherwise, nit: local not needed.  

[dsinclair, 2016/09/22 17:32:12]

Done.

+  pEnv->SetSDKDocument(pSDKDoc);
+
 #ifdef PDF_ENABLE_XFA
-  pEnv->SetSDKDocument(pDocument->GetSDKDocument(pEnv));
-  CPDFXFA_App* pApp = CPDFXFA_App::GetInstance();
-  pApp->AddFormFillEnv(pEnv);
-#else  // PDF_ENABLE_XFA
-  pEnv->SetSDKDocument(new CPDFSDK_Document(pDocument, pEnv));
+  pDocument->SetSDKDoc(WrapUnique(pSDKDoc));
+  CPDFXFA_App::GetInstance()->AddFormFillEnv(pEnv);
 #endif  // PDF_ENABLE_XFA
+
   return pEnv;
 }
 
 DLLEXPORT void STDCALL
 FPDFDOC_ExitFormFillEnvironment(FPDF_FORMHANDLE hHandle) {
   if (!hHandle)
     return;
+
   CPDFSDK_Environment* pEnv = HandleToCPDFSDKEnvironment(hHandle);
+
 #ifdef PDF_ENABLE_XFA
-  CPDFXFA_App* pApp = CPDFXFA_App::GetInstance();
-  pApp->RemoveFormFillEnv(pEnv);
+  CPDFXFA_App::GetInstance()->RemoveFormFillEnv(pEnv);
 #else   // PDF_ENABLE_XFA
   if (CPDFSDK_Document* pSDKDoc = pEnv->GetSDKDocument()) {
     pEnv->SetSDKDocument(nullptr);

[Tom Sepez, 2016/09/22 17:02:22]

Similarly, this is the only place where this gets deleted.  The env dtor can
take care of this cleanup.  Maybe a unique_ptr even.

[dsinclair, 2016/09/22 17:32:12]

That we can't do. The environment only owns the SDKDocument in non-XFA.
CPDFXFA_Document owns it if we're in XFA.

     delete pSDKDoc;
   }
 #endif  // PDF_ENABLE_XFA
+
   delete pEnv;
 }
 
 DLLEXPORT FPDF_BOOL STDCALL FORM_OnMouseMove(FPDF_FORMHANDLE hHandle,
                                              FPDF_PAGE page,
                                              int modifier,
                                              double page_x,
                                              double page_y) {
   CPDFSDK_PageView* pPageView = FormHandleToPageView(hHandle, page);
   if (!pPageView)
@@ skipping 442 matching lines @@
       CPDF_Action action = aa.GetAction(CPDF_AAction::OpenPage);
       pActionHandler->DoAction_Page(action, CPDF_AAction::OpenPage, pSDKDoc);
     }
   } else {
     if (aa.ActionExist(CPDF_AAction::ClosePage)) {
       CPDF_Action action = aa.GetAction(CPDF_AAction::ClosePage);
       pActionHandler->DoAction_Page(action, CPDF_AAction::ClosePage, pSDKDoc);
     }
   }
 }