Fixup page removal for XFA documents.

Fixup page removal for XFA documents.

Currently when you call DelegePage on a CPDFXFA_Document the page will not be
correctly removed. The page is left in the underlying document and, if
GetPage was never called for that page index nothing will happen.

This CL updates the code to always remove the page_index from the underlying
CPDF_Document then, if needed, the original code runs to clear out the
XFA page list for that page.

BUG=chromium:596373
Committed: https://pdfium.googlesource.com/pdfium/+/1f5f2fff688f4075a62c87a032c9a9070bde89fc

[dsinclair, 2016-04-25 20:40:30]

dsinclair@chromium.org changed reviewers:
+ thestig@chromium.org, tsepez@chromium.org

[dsinclair, 2016-04-25 20:40:30]

PTAL.

This should fix DOCTYPE_STATIC_XFA and DOCTYPE_PDF when using XFA, I don't think
DOCTYPE_DYNAMIC_XFA will be fixed with this.

[Lei Zhang, 2016-04-25 20:52:59]

https://codereview.chromium.org/1921033003/diff/20001/fpdfsdk/fpdfdoc_embedde...
File fpdfsdk/fpdfdoc_embeddertest.cpp (right):

https://codereview.chromium.org/1921033003/diff/20001/fpdfsdk/fpdfdoc_embedde...
fpdfsdk/fpdfdoc_embeddertest.cpp:146: FPDF_LoadXFA(document());
Check return value?

https://codereview.chromium.org/1921033003/diff/20001/fpdfsdk/fpdfdoc_embedde...
fpdfsdk/fpdfdoc_embeddertest.cpp:148: int count = FPDF_GetPageCount(document());
Check |count| before continuing?

https://codereview.chromium.org/1921033003/diff/20001/fpdfsdk/fpdfxfa/fpdfxfa...
File fpdfsdk/fpdfxfa/fpdfxfa_doc.cpp (right):

https://codereview.chromium.org/1921033003/diff/20001/fpdfsdk/fpdfxfa/fpdfxfa...
fpdfsdk/fpdfxfa/fpdfxfa_doc.cpp:204: // this page index then m_XFAPageList may
have size < page_index even if it's
nit: refer to variables as |var|.

[Tom Sepez, 2016-04-25 20:57:31]

LGTM, but see also https://codereview.chromium.org/1804163002 where this was
introduced.

[dsinclair, 2016-04-25 21:01:13]

https://codereview.chromium.org/1921033003/diff/20001/fpdfsdk/fpdfdoc_embedde...
File fpdfsdk/fpdfdoc_embeddertest.cpp (right):

https://codereview.chromium.org/1921033003/diff/20001/fpdfsdk/fpdfdoc_embedde...
fpdfsdk/fpdfdoc_embeddertest.cpp:146: FPDF_LoadXFA(document());
On 2016/04/25 20:52:59, Lei Zhang wrote:
> Check return value?

Removed, left over from initially trying to make the test work.

https://codereview.chromium.org/1921033003/diff/20001/fpdfsdk/fpdfdoc_embedde...
fpdfsdk/fpdfdoc_embeddertest.cpp:148: int count = FPDF_GetPageCount(document());
On 2016/04/25 20:52:59, Lei Zhang wrote:
> Check |count| before continuing?

Done.

https://codereview.chromium.org/1921033003/diff/20001/fpdfsdk/fpdfxfa/fpdfxfa...
File fpdfsdk/fpdfxfa/fpdfxfa_doc.cpp (right):

https://codereview.chromium.org/1921033003/diff/20001/fpdfsdk/fpdfxfa/fpdfxfa...
fpdfsdk/fpdfxfa/fpdfxfa_doc.cpp:204: // this page index then m_XFAPageList may
have size < page_index even if it's
On 2016/04/25 20:52:59, Lei Zhang wrote:
> nit: refer to variables as |var|.

Done.

[dsinclair, 2016-04-25 21:02:32]

On 2016/04/25 20:57:31, Tom Sepez wrote:
> LGTM, but see also https://codereview.chromium.org/1804163002 where this was
> introduced.


Right, that CL should still work fine. The problem is, pPage->Release will call
back to ::RemovePage in CPDFXFA_Document which does a SetAt on the page array.
Nothing in there will cause the original page to actually be deleted from the
document. So, the new call will make sure the page is deleted from the document
and we'll continue and cleanup the XFA structures as needed.

[dsinclair, 2016-04-25 21:06:13]

The CQ bit was checked by dsinclair@chromium.org

[dsinclair, 2016-04-25 21:06:13]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org
Link to the patchset: https://codereview.chromium.org/1921033003/#ps40001
(title: " ")

[commit-bot: I haz the power, 2016-04-25 21:06:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1921033003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1921033003/40001

[commit-bot: I haz the power, 2016-04-25 21:14:59]

Description was changed from

==========
Fixup page removal for XFA documents.

Currently when you call DelegePage on a CPDFXFA_Document the page will not be
correctly removed. The page is left in the underlying document and, if
GetPage was never called for that page index nothing will happen.

This CL updates the code to always remove the page_index from the underlying
CPDF_Document then, if needed, the original code runs to clear out the
XFA page list for that page.

BUG=chromium:596373
==========

to

==========
Fixup page removal for XFA documents.

Currently when you call DelegePage on a CPDFXFA_Document the page will not be
correctly removed. The page is left in the underlying document and, if
GetPage was never called for that page index nothing will happen.

This CL updates the code to always remove the page_index from the underlying
CPDF_Document then, if needed, the original code runs to clear out the
XFA page list for that page.

BUG=chromium:596373

Committed:
https://pdfium.googlesource.com/pdfium/+/1f5f2fff688f4075a62c87a032c9a9070bde...
==========

[commit-bot: I haz the power, 2016-04-25 21:15:00]

Committed patchset #3 (id:40001) as
https://pdfium.googlesource.com/pdfium/+/1f5f2fff688f4075a62c87a032c9a9070bde...