Move core/include/fpdfapi/fpdf_pageobj.h into core/fpdfapi.

Move core/include/fpdfapi/fpdf_pageobj.h into core/fpdfapi.

This CL splits the file into individual classes and moves them into
core/fpdfapi/fpdf_page as needed.

R=tsepez@chromium.org

Committed: https://pdfium.googlesource.com/pdfium/+/584b1e679f41a580e2b38d5534f126355c78043b

[dsinclair, 2016-03-17 18:18:55]

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

[dsinclair, 2016-03-17 18:18:55]

PTAL.

[Tom Sepez, 2016-03-17 20:15:39]

https://codereview.chromium.org/1811053002/diff/20001/core/fpdfapi/fpdf_edit/...
File core/fpdfapi/fpdf_edit/include/cpdf_creator.h (right):

https://codereview.chromium.org/1811053002/diff/20001/core/fpdfapi/fpdf_edit/...
core/fpdfapi/fpdf_edit/include/cpdf_creator.h:15: class CPDF_Object;
nit: alphabetize all of these.

https://codereview.chromium.org/1811053002/diff/20001/core/fpdfapi/fpdf_page/...
File core/fpdfapi/fpdf_page/cpdf_allstates.h (right):

https://codereview.chromium.org/1811053002/diff/20001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/cpdf_allstates.h:14: class CPDF_Dictionary;
nit: alphabetize.

https://codereview.chromium.org/1811053002/diff/20001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/cpdf_allstates.h:26: CFX_Matrix m_TextMatrix, m_CTM,
m_ParentMatrix;
nit: blank line here, then one per line.

https://codereview.chromium.org/1811053002/diff/20001/core/fpdfapi/fpdf_page/...
File core/fpdfapi/fpdf_page/cpdf_clippathdata.h (right):

https://codereview.chromium.org/1811053002/diff/20001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/cpdf_clippathdata.h:10: #include <stdint.h>
Yeah, ok, not sure if we want fx_system.h instead just because.

https://codereview.chromium.org/1811053002/diff/20001/core/fpdfapi/fpdf_page/...
File core/fpdfapi/fpdf_page/cpdf_textstate.h (right):

https://codereview.chromium.org/1811053002/diff/20001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/cpdf_textstate.h:15: class CPDF_TextState : public
CFX_CountRef<CPDF_TextStateData> {
It dawns on me that it might be OK to put the CPDF_XXXXXData and the CPDF_XXXXX
in the same file, to prevent flipping back and forth a lot ... HMM.

https://codereview.chromium.org/1811053002/diff/20001/core/fpdfapi/fpdf_page/...
File core/fpdfapi/fpdf_page/include/cpdf_generalstate.h (right):

https://codereview.chromium.org/1811053002/diff/20001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/include/cpdf_generalstate.h:22: return m_pObject ?
FXSYS_round((bStroke ? m_pObject->m_StrokeAlpha
nit: too much nesting. I'd add a quick if (!m_pObject) return 255;

https://codereview.chromium.org/1811053002/diff/20001/core/fpdfapi/fpdf_page/...
File core/fpdfapi/fpdf_page/pageint.h (right):

https://codereview.chromium.org/1811053002/diff/20001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/pageint.h:25: class CPDF_Image;
nit: alphabetize.

[dsinclair, 2016-03-17 20:37:41]

https://codereview.chromium.org/1811053002/diff/20001/core/fpdfapi/fpdf_edit/...
File core/fpdfapi/fpdf_edit/include/cpdf_creator.h (right):

https://codereview.chromium.org/1811053002/diff/20001/core/fpdfapi/fpdf_edit/...
core/fpdfapi/fpdf_edit/include/cpdf_creator.h:15: class CPDF_Object;
On 2016/03/17 20:15:38, Tom Sepez wrote:
> nit: alphabetize all of these.

Done.

https://codereview.chromium.org/1811053002/diff/20001/core/fpdfapi/fpdf_page/...
File core/fpdfapi/fpdf_page/cpdf_allstates.h (right):

https://codereview.chromium.org/1811053002/diff/20001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/cpdf_allstates.h:14: class CPDF_Dictionary;
On 2016/03/17 20:15:38, Tom Sepez wrote:
> nit: alphabetize.

Done.

https://codereview.chromium.org/1811053002/diff/20001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/cpdf_allstates.h:26: CFX_Matrix m_TextMatrix, m_CTM,
m_ParentMatrix;
On 2016/03/17 20:15:38, Tom Sepez wrote:
> nit: blank line here, then one per line.

Done.

https://codereview.chromium.org/1811053002/diff/20001/core/fpdfapi/fpdf_page/...
File core/fpdfapi/fpdf_page/cpdf_clippathdata.h (right):

https://codereview.chromium.org/1811053002/diff/20001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/cpdf_clippathdata.h:10: #include <stdint.h>
On 2016/03/17 20:15:38, Tom Sepez wrote:
> Yeah, ok, not sure if we want fx_system.h instead just because.

Acknowledged. Including the lower level seems like it would trigger less
re-compiling on fx_system.h change.

https://codereview.chromium.org/1811053002/diff/20001/core/fpdfapi/fpdf_page/...
File core/fpdfapi/fpdf_page/cpdf_textstate.h (right):

https://codereview.chromium.org/1811053002/diff/20001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/cpdf_textstate.h:15: class CPDF_TextState : public
CFX_CountRef<CPDF_TextStateData> {
On 2016/03/17 20:15:38, Tom Sepez wrote:
> It dawns on me that it might be OK to put the CPDF_XXXXXData and the
CPDF_XXXXX
> in the same file, to prevent flipping back and forth a lot ... HMM.


Do we know why these are separated? Seems like the direction in the long run
would be to fold the CPDF_XXXData classes back into the CPDF_XXX classes and
remove the CPDF_XXXData, yea? If that seems sane, can file a followup bug.

https://codereview.chromium.org/1811053002/diff/20001/core/fpdfapi/fpdf_page/...
File core/fpdfapi/fpdf_page/include/cpdf_generalstate.h (right):

https://codereview.chromium.org/1811053002/diff/20001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/include/cpdf_generalstate.h:22: return m_pObject ?
FXSYS_round((bStroke ? m_pObject->m_StrokeAlpha
On 2016/03/17 20:15:38, Tom Sepez wrote:
> nit: too much nesting. I'd add a quick if (!m_pObject) return 255;

Done.

https://codereview.chromium.org/1811053002/diff/20001/core/fpdfapi/fpdf_page/...
File core/fpdfapi/fpdf_page/pageint.h (right):

https://codereview.chromium.org/1811053002/diff/20001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/pageint.h:25: class CPDF_Image;
On 2016/03/17 20:15:38, Tom Sepez wrote:
> nit: alphabetize.

Done.

[Tom Sepez, 2016-03-18 16:21:58]

https://codereview.chromium.org/1811053002/diff/20001/core/fpdfapi/fpdf_page/...
File core/fpdfapi/fpdf_page/cpdf_textstate.h (right):

https://codereview.chromium.org/1811053002/diff/20001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/cpdf_textstate.h:15: class CPDF_TextState : public
CFX_CountRef<CPDF_TextStateData> {
On 2016/03/17 20:37:41, dsinclair wrote:
> On 2016/03/17 20:15:38, Tom Sepez wrote:
> > It dawns on me that it might be OK to put the CPDF_XXXXXData and the
> CPDF_XXXXX
> > in the same file, to prevent flipping back and forth a lot ... HMM.
> 
> 
> Do we know why these are separated? Seems like the direction in the long run
> would be to fold the CPDF_XXXData classes back into the CPDF_XXX classes and
> remove the CPDF_XXXData, yea? If that seems sane, can file a followup bug.

They're separate because of inconsistency with the ownership model.  Sometimes
they are (externally) refcounted, sometimes they aren't and are just used
directly. I much prefer the case where the refcount is in the object, rather
than going throguh this layer of indirection, which would be the "folding
together" you propose. But there are places where callers just blatantly  set
the m_pObject to null, and count on this then acting as a kind of weak ptr.

[Tom Sepez, 2016-03-18 16:24:34]

lgtm

[dsinclair, 2016-03-21 13:15:48]

Description was changed from

==========
Move core/include/fpdfapi/fpdf_pageobj.h into core/fpdfapi.

This CL splits the file into individual classes and moves them into
core/fpdfapi/fpdf_page as needed.
==========

to

==========
Move core/include/fpdfapi/fpdf_pageobj.h into core/fpdfapi.

This CL splits the file into individual classes and moves them into
core/fpdfapi/fpdf_page as needed.

R=tsepez@chromium.org

Committed:
https://pdfium.googlesource.com/pdfium/+/584b1e679f41a580e2b38d5534f126355c78...
==========

[dsinclair, 2016-03-21 13:15:49]

Committed patchset #3 (id:40001) manually as
584b1e679f41a580e2b38d5534f126355c78043b (presubmit successful).