Split apart the remainder of fpdf_render_render.cpp into per-class files.

Split apart the remainder of fpdf_render_render.cpp into per-class files.

R=dsinclair@chromium.org

Committed: https://pdfium.googlesource.com/pdfium/+/f04089059b140ad50ed386b509ab200ce84b66e5

[Tom Sepez, 2016-03-10 02:28:17]

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

[Tom Sepez, 2016-03-10 02:28:27]

Dan, for review.

[dsinclair, 2016-03-10 14:15:25]

lgtm w/ nits

https://codereview.chromium.org/1778173002/diff/40001/core/include/fpdfapi/ip...
File core/include/fpdfapi/ipdf_data_avail.h (right):

https://codereview.chromium.org/1778173002/diff/40001/core/include/fpdfapi/ip...
core/include/fpdfapi/ipdf_data_avail.h:22: DataError = -1,        //
PDF_DATA_ERROR
nit: These comments and on the two enum's below aren't really useful as they,
basically, say the same thing as the name.

https://codereview.chromium.org/1778173002/diff/40001/core/include/fpdfapi/ip...
core/include/fpdfapi/ipdf_data_avail.h:48: virtual ~FileAvail() {}
nit: move virtual body to a .cpp (and below)

https://codereview.chromium.org/1778173002/diff/40001/core/src/fpdfapi/fpdf_p...
File core/src/fpdfapi/fpdf_parser/cpdf_data_avail.h (right):

https://codereview.chromium.org/1778173002/diff/40001/core/src/fpdfapi/fpdf_p...
core/src/fpdfapi/fpdf_parser/cpdf_data_avail.h:20: enum PDF_DATAAVAIL_STATUS {
These are only used in cpdf_data_avail.cpp, move to a namespace {} in the .cpp

https://codereview.chromium.org/1778173002/diff/40001/core/src/fpdfapi/fpdf_p...
core/src/fpdfapi/fpdf_parser/cpdf_data_avail.h:45: enum PDF_PAGENODE_TYPE {
ditto

https://codereview.chromium.org/1778173002/diff/40001/core/src/fpdfapi/fpdf_p...
core/src/fpdfapi/fpdf_parser/cpdf_data_avail.h:74: class PageNode {
ditto

https://codereview.chromium.org/1778173002/diff/40001/core/src/fpdfapi/fpdf_p...
File core/src/fpdfapi/fpdf_parser/cpdf_syntax_parser_unittest.cpp (right):

https://codereview.chromium.org/1778173002/diff/40001/core/src/fpdfapi/fpdf_p...
core/src/fpdfapi/fpdf_parser/cpdf_syntax_parser_unittest.cpp:1: // Copyright
2015 PDFium Authors. All rights reserved.
nit: 2016?

https://codereview.chromium.org/1778173002/diff/40001/core/src/fpdfapi/fpdf_p...
File core/src/fpdfapi/fpdf_parser/fpdf_parser_decode_embeddertest.cpp (right):

https://codereview.chromium.org/1778173002/diff/40001/core/src/fpdfapi/fpdf_p...
core/src/fpdfapi/fpdf_parser/fpdf_parser_decode_embeddertest.cpp:5: #include
"core/include/fpdfapi/fpdf_parser_decode.h"
nit: I don't think this goes at the top in the test file does it?

[Tom Sepez, 2016-03-10 17:19:26]

https://codereview.chromium.org/1778173002/diff/40001/core/include/fpdfapi/ip...
File core/include/fpdfapi/ipdf_data_avail.h (right):

https://codereview.chromium.org/1778173002/diff/40001/core/include/fpdfapi/ip...
core/include/fpdfapi/ipdf_data_avail.h:22: DataError = -1,        //
PDF_DATA_ERROR
On 2016/03/10 14:15:25, dsinclair wrote:
> nit: These comments and on the two enum's below aren't really useful as they,
> basically, say the same thing as the name.

The are literally the name of a #define somewhere.

https://codereview.chromium.org/1778173002/diff/40001/core/include/fpdfapi/ip...
core/include/fpdfapi/ipdf_data_avail.h:48: virtual ~FileAvail() {}
On 2016/03/10 14:15:25, dsinclair wrote:
> nit: move virtual body to a .cpp (and below)

Done.

https://codereview.chromium.org/1778173002/diff/40001/core/src/fpdfapi/fpdf_p...
File core/src/fpdfapi/fpdf_parser/cpdf_data_avail.h (right):

https://codereview.chromium.org/1778173002/diff/40001/core/src/fpdfapi/fpdf_p...
core/src/fpdfapi/fpdf_parser/cpdf_data_avail.h:20: enum PDF_DATAAVAIL_STATUS {
On 2016/03/10 14:15:25, dsinclair wrote:
> These are only used in cpdf_data_avail.cpp, move to a namespace {} in the .cpp
Used in this file at line 172.  Should be private to the CPDF_DataAvail class,
but lots of stuff to move around.

https://codereview.chromium.org/1778173002/diff/40001/core/src/fpdfapi/fpdf_p...
core/src/fpdfapi/fpdf_parser/cpdf_data_avail.h:45: enum PDF_PAGENODE_TYPE {
On 2016/03/10 14:15:25, dsinclair wrote:
> ditto

Ditto, line 79. Should be part of pagenode.

https://codereview.chromium.org/1778173002/diff/40001/core/src/fpdfapi/fpdf_p...
core/src/fpdfapi/fpdf_parser/cpdf_data_avail.h:74: class PageNode {
On 2016/03/10 14:15:25, dsinclair wrote:
> ditto

ditto, line 210.

https://codereview.chromium.org/1778173002/diff/40001/core/src/fpdfapi/fpdf_p...
File core/src/fpdfapi/fpdf_parser/cpdf_syntax_parser_unittest.cpp (right):

https://codereview.chromium.org/1778173002/diff/40001/core/src/fpdfapi/fpdf_p...
core/src/fpdfapi/fpdf_parser/cpdf_syntax_parser_unittest.cpp:1: // Copyright
2015 PDFium Authors. All rights reserved.
On 2016/03/10 14:15:25, dsinclair wrote:
> nit: 2016?

Done.

https://codereview.chromium.org/1778173002/diff/40001/core/src/fpdfapi/fpdf_p...
File core/src/fpdfapi/fpdf_parser/fpdf_parser_decode_embeddertest.cpp (right):

https://codereview.chromium.org/1778173002/diff/40001/core/src/fpdfapi/fpdf_p...
core/src/fpdfapi/fpdf_parser/fpdf_parser_decode_embeddertest.cpp:5: #include
"core/include/fpdfapi/fpdf_parser_decode.h"
On 2016/03/10 14:15:25, dsinclair wrote:
> nit: I don't think this goes at the top in the test file does it?

Yes, for test files with the same name.

Per style guide:
In dir/foo.cc or dir/foo_test.cc, whose main purpose is to implement or test the
stuff in dir2/foo2.h, order your includes as follows:

dir2/foo2.h.
...

[Tom Sepez, 2016-03-10 17:20:08]

Description was changed from

==========
Split apart the remainder of fpdf_render_render.cpp into per-class files.
==========

to

==========
Split apart the remainder of fpdf_render_render.cpp into per-class files.

R=dsinclair@chromium.org

Committed:
https://pdfium.googlesource.com/pdfium/+/f04089059b140ad50ed386b509ab200ce84b...
==========

[Tom Sepez, 2016-03-10 17:20:11]

Committed patchset #4 (id:60001) manually as
f04089059b140ad50ed386b509ab200ce84b66e5 (presubmit successful).

[brucedawson, 2016-03-11 22:39:57]

brucedawson@chromium.org changed reviewers:
+ brucedawson@chromium.org

[brucedawson, 2016-03-11 22:39:58]

https://codereview.chromium.org/1778173002/diff/60001/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/1778173002/diff/60001/BUILD.gn#newcode353
BUILD.gn:353: "core/src/fpdfapi/fpdf_parser/fpdf_data_avail.cpp",
These two files (.cpp and .h) don't exist.

[Tom Sepez, 2016-03-11 22:57:34]

https://codereview.chromium.org/1778173002/diff/60001/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/1778173002/diff/60001/BUILD.gn#newcode353
BUILD.gn:353: "core/src/fpdfapi/fpdf_parser/fpdf_data_avail.cpp",
On 2016/03/11 22:39:58, brucedawson wrote:
> These two files (.cpp and .h) don't exist.
Argh, typo.  Should be cpdf_, and we don't have a standalone gn.

[brucedawson, 2016-03-11 22:59:10]

Ah. I looked in the .gyp file and thought they were the same. One letter
difference...