Remove some #ifdef XFA in favor of runtime checks.chromium/3457

Make LoadDocumentImpl() logic match FPDFAvail_GetDocument() logic, so
that the XFA extension is loaded before checking unsupported features.
Add some comments along the way.

Change-Id: I040e40fcca872f7c0a46e921bce1146f0fe42588
Reviewed-on: https://pdfium-review.googlesource.com/34931
Reviewed-by: dsinclair <dsinclair@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>

5 files changed, 24 insertions, 26 deletions

diff --git a/core/fpdfapi/page/ipdf_page.h b/core/fpdfapi/page/ipdf_page.h
index 7c9fce7726..9b28559a94 100644
--- a/core/fpdfapi/page/ipdf_page.h
+++ b/core/fpdfapi/page/ipdf_page.h
@@ -20,6 +20,11 @@ class CPDFXFA_Page;
 // Interface implemented by both page types (CPDF_Page and CPDFXFA_Page).
 class IPDF_Page : public Retainable {
  public:
+  // There are actually 3 cases: a PDF page, an XFA page backed by a PDF page,
+  // and an XFA page not backed by a PDF page. AsPDFPage() will return the
+  // PDF page in either of the first two cases. AsXFAPage() is a straight
+  // downcast and is null if not either of the last two cases. Hence, both
+  // of these may return non-null on a given page.
   virtual CPDF_Page* AsPDFPage() = 0;
   virtual CPDFXFA_Page* AsXFAPage() = 0;
 
diff --git a/fpdfsdk/cpdfsdk_helpers.cpp b/fpdfsdk/cpdfsdk_helpers.cpp
index 2f42f146cd..391e79758f 100644
--- a/fpdfsdk/cpdfsdk_helpers.cpp
+++ b/fpdfsdk/cpdfsdk_helpers.cpp
@@ -245,12 +245,9 @@ void CheckUnSupportError(CPDF_Document* pDoc, uint32_t err_code) {
     }
   }
 
-#ifndef PDF_ENABLE_XFA
   // XFA Forms
-  CPDF_InterForm interform(pDoc);
-  if (interform.HasXFAForm())
+  if (!pDoc->GetExtension() && CPDF_InterForm(pDoc).HasXFAForm())
     RaiseUnSupportError(FPDF_UNSP_DOC_XFAFORM);
-#endif  // PDF_ENABLE_XFA
 }
 
 #ifndef _WIN32
diff --git a/fpdfsdk/cpdfsdk_pageview.cpp b/fpdfsdk/cpdfsdk_pageview.cpp
index d2736b1d42..def911d4ae 100644
--- a/fpdfsdk/cpdfsdk_pageview.cpp
+++ b/fpdfsdk/cpdfsdk_pageview.cpp
@@ -33,24 +33,25 @@
 CPDFSDK_PageView::CPDFSDK_PageView(CPDFSDK_FormFillEnvironment* pFormFillEnv,
                                    IPDF_Page* page)
     : m_page(page), m_pFormFillEnv(pFormFillEnv) {
+  ASSERT(m_page);
   CPDF_Page* pPDFPage = ToPDFPage(page);
   if (pPDFPage) {
     CPDFSDK_InterForm* pInterForm = pFormFillEnv->GetInterForm();
     CPDF_InterForm* pPDFInterForm = pInterForm->GetInterForm();
     pPDFInterForm->FixPageFields(pPDFPage);
-#ifndef PDF_ENABLE_XFA
-    pPDFPage->SetView(this);
-#endif  // PDF_ENABLE_XFA
+    if (!page->AsXFAPage())
+      pPDFPage->SetView(this);
   }
 }
 
 CPDFSDK_PageView::~CPDFSDK_PageView() {
-#ifndef PDF_ENABLE_XFA
-  // The call to |ReleaseAnnot| can cause the page pointed to by |m_page| to
-  // be freed, which will cause issues if we try to cleanup the pageview pointer
-  // in |m_page|. So, reset the pageview pointer before doing anything else.
-  m_page->AsPDFPage()->SetView(nullptr);
-#endif  // PDF_ENABLE_XFA
+  if (!m_page->AsXFAPage()) {
+    // The call to |ReleaseAnnot| can cause the page pointed to by |m_page| to
+    // be freed, which will cause issues if we try to cleanup the pageview
+    // pointer in |m_page|. So, reset the pageview pointer before doing anything
+    // else.
+    m_page->AsPDFPage()->SetView(nullptr);
+  }
 
   CPDFSDK_AnnotHandlerMgr* pAnnotHandlerMgr =
       m_pFormFillEnv->GetAnnotHandlerMgr();
diff --git a/fpdfsdk/cpdfsdk_pageview.h b/fpdfsdk/cpdfsdk_pageview.h
index 38ff288607..46c35e1e2d 100644
--- a/fpdfsdk/cpdfsdk_pageview.h
+++ b/fpdfsdk/cpdfsdk_pageview.h
@@ -44,7 +44,6 @@ class CPDFSDK_PageView final : public CPDF_Page::View {
   bool DeleteAnnot(CPDFSDK_Annot* pAnnot);
   CPDFSDK_Annot* AddAnnot(CXFA_FFWidget* pPDFAnnot);
   CPDFSDK_Annot* GetAnnotByXFAWidget(CXFA_FFWidget* hWidget);
-
   CPDFXFA_Page* GetPDFXFAPage() { return ToXFAPage(m_page); }
 #endif  // PDF_ENABLE_XFA
 
@@ -66,10 +65,12 @@ class CPDFSDK_PageView final : public CPDF_Page::View {
   bool OnFocus(const CFX_PointF& point, uint32_t nFlag);
   bool OnLButtonDown(const CFX_PointF& point, uint32_t nFlag);
   bool OnLButtonUp(const CFX_PointF& point, uint32_t nFlag);
+
 #ifdef PDF_ENABLE_XFA
   bool OnRButtonDown(const CFX_PointF& point, uint32_t nFlag);
   bool OnRButtonUp(const CFX_PointF& point, uint32_t nFlag);
 #endif  // PDF_ENABLE_XFA
+
   bool OnChar(int nChar, uint32_t nFlag);
   bool OnKeyDown(int nKeyCode, int nFlag);
   bool OnKeyUp(int nKeyCode, int nFlag);
@@ -88,15 +89,10 @@ class CPDFSDK_PageView final : public CPDF_Page::View {
 
   void SetValid(bool bValid) { m_bValid = bValid; }
   bool IsValid() { return m_bValid; }
-
   bool IsLocked() { return m_bLocked; }
-
   void SetBeingDestroyed() { m_bBeingDestroyed = true; }
   bool IsBeingDestroyed() const { return m_bBeingDestroyed; }
-
-#ifndef PDF_ENABLE_XFA
   void TakePageOwnership() { m_pOwnsPage.Reset(ToPDFPage(m_page)); }
-#endif  // PDF_ENABLE_XFA
 
  private:
   CPDFSDK_Annot* GetFXAnnotAtPoint(const CFX_PointF& point);
@@ -117,9 +113,7 @@ class CPDFSDK_PageView final : public CPDF_Page::View {
   std::vector<CPDFSDK_Annot*> m_SDKAnnotArray;
   UnownedPtr<CPDFSDK_FormFillEnvironment> const m_pFormFillEnv;
   CPDFSDK_Annot::ObservedPtr m_pCaptureWidget;
-#ifndef PDF_ENABLE_XFA
   RetainPtr<CPDF_Page> m_pOwnsPage;
-#endif  // PDF_ENABLE_XFA
   bool m_bOnWidget = false;
   bool m_bValid = false;
   bool m_bLocked = false;
diff --git a/fpdfsdk/fpdf_view.cpp b/fpdfsdk/fpdf_view.cpp
index cbbfd9c71d..4f10bcb20a 100644
--- a/fpdfsdk/fpdf_view.cpp
+++ b/fpdfsdk/fpdf_view.cpp
@@ -67,7 +67,7 @@ static_assert(WindowsPrintMode::kModePostScript2PassThrough ==
 static_assert(WindowsPrintMode::kModePostScript3PassThrough ==
                   FPDF_PRINTMODE_POSTSCRIPT3_PASSTHROUGH,
               "WindowsPrintMode::kModePostScript3PassThrough value mismatch");
-#endif
+#endif  // _FX_PLATFORM_ == _FX_PLATFORM_WINDOWS_
 
 namespace {
 
@@ -151,12 +151,12 @@ FPDF_DOCUMENT LoadDocumentImpl(
     ProcessParseError(error);
     return nullptr;
   }
-  CheckUnSupportError(pDocument.get(), error);
 
 #ifdef PDF_ENABLE_XFA
   pDocument->SetExtension(pdfium::MakeUnique<CPDFXFA_Context>(pDocument.get()));
 #endif  // PDF_ENABLE_XFA
 
+  CheckUnSupportError(pDocument.get(), error);
   return FPDFDocumentFromCPDFDocument(pDocument.release());
 }
 
@@ -716,7 +716,7 @@ FPDF_EXPORT FPDF_RECORDER FPDF_CALLCONV FPDF_RenderPageSkp(FPDF_PAGE page,
   pPage->SetRenderContext(nullptr);
   return recorder;
 }
-#endif
+#endif  // _SKIA_SUPPORT_
 
 FPDF_EXPORT void FPDF_CALLCONV FPDF_ClosePage(FPDF_PAGE page) {
   if (!page)
@@ -726,7 +726,9 @@ FPDF_EXPORT void FPDF_CALLCONV FPDF_ClosePage(FPDF_PAGE page) {
   RetainPtr<IPDF_Page> pPage;
   pPage.Unleak(IPDFPageFromFPDFPage(page));
 
-#ifndef PDF_ENABLE_XFA
+  if (pPage->AsXFAPage())
+    return;
+
   CPDFSDK_PageView* pPageView =
       static_cast<CPDFSDK_PageView*>(pPage->AsPDFPage()->GetView());
   if (!pPageView || pPageView->IsBeingDestroyed())
@@ -741,7 +743,6 @@ FPDF_EXPORT void FPDF_CALLCONV FPDF_ClosePage(FPDF_PAGE page) {
   // first because it will attempt to reset the View on the |pPage| during
   // destruction.
   pPageView->GetFormFillEnv()->RemovePageView(pPage.Get());
-#endif  // PDF_ENABLE_XFA
 }
 
 FPDF_EXPORT void FPDF_CALLCONV FPDF_CloseDocument(FPDF_DOCUMENT document) {