Rename CPDFXFA_Document::GetPageCount

Rename CPDFXFA_Document::GetPageCount

Both CPDF_Document and CPDFXFA_Document have a GetPageCount method. For
CPDF_Document this maps to the length of the m_PageList array. It is possible
for the page count values for the two types to be different.

This CL renames the CPDFXFA_Document::GetPageCount to
CPDFXFA_Document::GetXFAPageCount and marks the places where we need to validate
the code will work correctly with XFA forms.

[dsinclair, 2016-09-20 14:16:42]

The CQ bit was checked by dsinclair@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-20 14:16:55]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dsinclair, 2016-09-20 14:17:59]

dsinclair@chromium.org changed reviewers:
+ npm@chromium.org, tsepez@chromium.org, weili@chromium.org

[dsinclair, 2016-09-20 14:18:00]

PTAL.

[commit-bot: I haz the power, 2016-09-20 14:31:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-20 14:31:14]

Dry run: This issue passed the CQ dry run.

[Tom Sepez, 2016-09-20 16:46:20]

On 2016/09/20 14:18:00, dsinclair wrote:
> PTAL.

I'm kinda confused as to the direction this takes us -- in the end state,
wouldn't GetPageCount be a virtual method overridden in the XFA doc?  If so, why
do we rename it first?  Can you fill me in?

[dsinclair, 2016-09-20 17:24:34]

On 2016/09/20 16:46:20, Tom Sepez wrote:
> On 2016/09/20 14:18:00, dsinclair wrote:
> > PTAL.
> 
> I'm kinda confused as to the direction this takes us -- in the end state,
> wouldn't GetPageCount be a virtual method overridden in the XFA doc?  If so,
why
> do we rename it first?  Can you fill me in?


In the limit, yes GetPageCount should be virtual. At the moment, it will cause
issues. GetPageCount for XFA documents is different from non-XFA. But, there are
places where we currently assume the CPDF_Document version is the one we want.
That version will be smaller then what the XFA version would give us. This also
relates to having two page lists. There is a m_PageList in CPDF_Document and an
XFA page list in CPDFXFA_Document. The PageCount is used to determine the size
of m_PageList at times.

This caused an issue for CPDF_DataAvail (the prior Cl fixed it by adding the
IsPageLoaded method). But, it will still be using the wrong values and we need
to fix before we can combine the two. Similar with the page combining code. We
currently merge the CPDF_Document pages, but not the XFA pages. Once these
things are worked out, we should be able to overload GetPageCount and remove
GetXFAPageCount.

So, I'm making the XFA version explicit so we know which one we're using at each
point and can audit as needed.

[dsinclair, 2016-09-21 18:56:49]

On 2016/09/20 17:24:34, dsinclair wrote:
> On 2016/09/20 16:46:20, Tom Sepez wrote:
> > On 2016/09/20 14:18:00, dsinclair wrote:
> > > PTAL.
> > 
> > I'm kinda confused as to the direction this takes us -- in the end state,
> > wouldn't GetPageCount be a virtual method overridden in the XFA doc?  If so,
> why
> > do we rename it first?  Can you fill me in?
> 
> 
> In the limit, yes GetPageCount should be virtual. At the moment, it will cause
> issues. GetPageCount for XFA documents is different from non-XFA. But, there
are
> places where we currently assume the CPDF_Document version is the one we want.
> That version will be smaller then what the XFA version would give us. This
also
> relates to having two page lists. There is a m_PageList in CPDF_Document and
an
> XFA page list in CPDFXFA_Document. The PageCount is used to determine the size
> of m_PageList at times.
> 
> This caused an issue for CPDF_DataAvail (the prior Cl fixed it by adding the
> IsPageLoaded method). But, it will still be using the wrong values and we need
> to fix before we can combine the two. Similar with the page combining code. We
> currently merge the CPDF_Document pages, but not the XFA pages. Once these
> things are worked out, we should be able to overload GetPageCount and remove
> GetXFAPageCount.
> 
> So, I'm making the XFA version explicit so we know which one we're using at
each
> point and can audit as needed.


ping

[npm, 2016-09-21 21:59:23]

lgtm if I understand correctly and this is just a rename

https://codereview.chromium.org/2354933002/diff/1/core/fpdfapi/fpdf_parser/cp...
File core/fpdfapi/fpdf_parser/cpdf_data_avail.cpp (right):

https://codereview.chromium.org/2354933002/diff/1/core/fpdfapi/fpdf_parser/cp...
core/fpdfapi/fpdf_parser/cpdf_data_avail.cpp:1427: // the document does not
contain XFA pages.
I think I don't understand. You are not changing the way this part
works, since m_pDocument is a CPDF_Document. Is this for when you
change the CXFA_Document to be a subclass of CPDF_Document?

[dsinclair, 2016-09-22 14:06:02]

https://codereview.chromium.org/2354933002/diff/1/core/fpdfapi/fpdf_parser/cp...
File core/fpdfapi/fpdf_parser/cpdf_data_avail.cpp (right):

https://codereview.chromium.org/2354933002/diff/1/core/fpdfapi/fpdf_parser/cp...
core/fpdfapi/fpdf_parser/cpdf_data_avail.cpp:1427: // the document does not
contain XFA pages.
On 2016/09/21 21:59:22, npm wrote:
> I think I don't understand. You are not changing the way this part
> works, since m_pDocument is a CPDF_Document. Is this for when you
> change the CXFA_Document to be a subclass of CPDF_Document?


The problem I see here is that, the page count for an XFA document != page count
for the PDF document. So, if we're loading an XFA document with 5 pages, and two
of those pages are XFA pages, this method will never return _DONE for those last
two pages, I think. The TODO is to make sure this will work correctly with XFA
in general.

[dsinclair, 2016-09-22 18:09:13]

tsepez@ does this CL make more sense with the explanation above?