Fix crashing in CPDFXFA_Document::GetPage()

fpdfsdk/src/fpdfxfa/fpdfxfa_doc.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 "../../include/fsdk_define.h"
 #include "../../include/fpdfxfa/fpdfxfa_doc.h"
 #include "../../include/fsdk_mgr.h"
 #include "../../include/fpdfxfa/fpdfxfa_app.h"
@@ skipping 126 matching lines @@
     case DOCTYPE_DYNAMIC_XFA:
       if (m_pXFADoc)
         return m_pXFADocView->CountPageViews();
     default:
       return 0;
   }
 
   return 0;
 }
 
 CPDFXFA_Page* CPDFXFA_Document::GetPage(int page_index) {

[Lei Zhang, 2015/12/21 17:47:59]

What happen if |page_index| is negative?

[jun_fang, 2016/01/04 09:30:23]

It shouldn't be less than 0. Will add a test to make sure |page_index| >= 0.

-  if (!m_pPDFDoc && !m_pXFADoc)

[Lei Zhang, 2015/12/21 17:47:58]

Do you still care about these being nullptrs?

-    return NULL;
-
-  CPDFXFA_Page* pPage = NULL;
-  if (m_XFAPageList.GetSize()) {
+  CPDFXFA_Page* pPage = nullptr;
+  int nCount = m_XFAPageList.GetSize();
+  if (nCount > 0 && page_index < nCount) {
     pPage = m_XFAPageList.GetAt(page_index);
     if (pPage)
       pPage->AddRef();
   } else {
     m_XFAPageList.SetSize(GetPageCount());
   }
-
-  if (!pPage) {
-    pPage = new CPDFXFA_Page(this, page_index);
-    FX_BOOL bRet = pPage->LoadPage();
-    if (!bRet) {
-      delete pPage;
-      return NULL;
-    }
-
-    m_XFAPageList.SetAt(page_index, pPage);
+  if (pPage)
+    return pPage;
+  pPage = new CPDFXFA_Page(this, page_index);
+  if (!pPage->LoadPage()) {
+    delete pPage;
+    return nullptr;
   }
-
+  m_XFAPageList.SetAt(page_index, pPage);
   return pPage;
 }
 
 CPDFXFA_Page* CPDFXFA_Document::GetPage(IXFA_PageView* pPage) {
   if (!pPage)
     return NULL;
 
   if (!m_pXFADoc)
     return NULL;
 
@@ skipping 302 matching lines @@
     menuFlag |= FXFA_MEMU_SELECTALL;
 
   CPDFDoc_Environment* pEnv = m_pSDKDoc->GetEnv();
   if (pEnv == NULL)
     return FALSE;
 
   return pEnv->FFI_PopupMenu(pPage, hWidget, menuFlag, ptPopup, NULL);
 }
 
 void CPDFXFA_Document::PageViewEvent(IXFA_PageView* pPageView,
                                      FX_DWORD dwFlags) {

[Lei Zhang, 2015/12/21 17:47:58]

Is |dwFlags| always just a single value, or can it be something like
(XFA_PAGEVIEWEVENT_PostAdded | XFA_PAGEVIEWEVENT_PostRemoved) ?

[jun_fang, 2016/01/04 09:30:23]

It's just a single value.

+  if (!pPageView || (FXFA_PAGEVIEWEVENT_POSTADDED != dwFlags &&

[jun_fang, 2015/12/21 07:19:59]

SDK needs to handle changes on page view.
The events include FXFA_PAGEVIEWEVENT_POSTADDED and
FXFA_PAGEVIEWEVENT_POSTREMOVED.

[Lei Zhang, 2015/12/21 17:47:59]

The posted value is XFA_PAGEVIEWEVENT_PostRemoved, but the checked value is
FXFA_PAGEVIEWEVENT_POSTREMOVED. Can we use one of the two consistently?

[jun_fang, 2016/01/04 09:30:23]

Acknowledged.

+                     FXFA_PAGEVIEWEVENT_POSTREMOVED != dwFlags)) {
+    return;
+  }
+  CPDFXFA_Page* pPage = nullptr;
+  if (FXFA_PAGEVIEWEVENT_POSTADDED == dwFlags) {

[Lei Zhang, 2015/12/21 17:47:58]

nit: Prefer to just write: var == value

I believe Clang at least, will warn if one accidentally writes: var = value

[jun_fang, 2016/01/04 09:30:23]

Acknowledged.

+    pPage = GetPage(pPageView->GetPageViewIndex());
+    if (pPage)
+      pPage->SetXFAPageView(pPageView);
+    return;
+  }
+  pPage = GetPage(pPageView);
+  if (!pPage)
+    return;
+  pPage->SetXFAPageView(nullptr);
+  m_pSDKDoc->GetPageView(pPage)->ClearFXAnnots();
 }
 
 void CPDFXFA_Document::WidgetEvent(IXFA_Widget* hWidget,
                                    CXFA_WidgetAcc* pWidgetData,
                                    FX_DWORD dwEvent,
                                    void* pParam,
                                    void* pAdditional) {
   if (m_iDocType != DOCTYPE_DYNAMIC_XFA || !hWidget)
     return;
 
@@ skipping 763 matching lines @@
   }
 
   return _GetHValueByName(szPropName, hValue,
                           m_pSDKDoc->GetEnv()->GetJSRuntime());
 }
 FX_BOOL CPDFXFA_Document::_GetHValueByName(const CFX_ByteStringC& utf8Name,
                                            FXJSE_HVALUE hValue,
                                            IJS_Runtime* runTime) {
   return runTime->GetHValueByName(utf8Name, hValue);
 }