Split CPDF_SyntaxParser into its own named .cpp/.h files.

Split CPDF_SyntaxParser into its own named .cpp/.h files.

BUG=
R=dsinclair@chromium.org

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

[Tom Sepez, 2016-03-08 00:59:30]

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

[Tom Sepez, 2016-03-08 00:59:30]

A little patch to make Dan happy.

[dsinclair, 2016-03-08 01:15:06]

Yaaa. lgtm w/ comments.

https://codereview.chromium.org/1774753002/diff/60001/core/include/fpdfapi/fp...
File core/include/fpdfapi/fpdf_parser.h (right):

https://codereview.chromium.org/1774753002/diff/60001/core/include/fpdfapi/fp...
core/include/fpdfapi/fpdf_parser.h:31: class CPDF_SyntaxParser;
What's our thoughts on forward declarations? The style guide says to minimize
them (https://google.github.io/styleguide/cppguide.html#Forward_Declarations)
but we've got a lot of them  already.

https://codereview.chromium.org/1774753002/diff/60001/core/src/fpdfapi/fpdf_p...
File core/src/fpdfapi/fpdf_parser/cpdf_syntax_parser.cpp (right):

https://codereview.chromium.org/1774753002/diff/60001/core/src/fpdfapi/fpdf_p...
core/src/fpdfapi/fpdf_parser/cpdf_syntax_parser.cpp:1: // Copyright 2014 PDFium
Authors. All rights reserved.
nit: 2016?

https://codereview.chromium.org/1774753002/diff/60001/core/src/fpdfapi/fpdf_p...
core/src/fpdfapi/fpdf_parser/cpdf_syntax_parser.cpp:14: struct SearchTagRecord {
Can this go in namespace {}?

https://codereview.chromium.org/1774753002/diff/60001/core/src/fpdfapi/fpdf_p...
File core/src/fpdfapi/fpdf_parser/cpdf_syntax_parser.h (right):

https://codereview.chromium.org/1774753002/diff/60001/core/src/fpdfapi/fpdf_p...
core/src/fpdfapi/fpdf_parser/cpdf_syntax_parser.h:63: static const int
kParserMaxRecursionDepth = 64;
Can these move into the .cpp file now as they will only apply to this one class?

https://codereview.chromium.org/1774753002/diff/60001/core/src/fpdfapi/fpdf_p...
File core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp (right):

https://codereview.chromium.org/1774753002/diff/60001/core/src/fpdfapi/fpdf_p...
core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp:288:
m_pSyntax->m_MetadataObjnum = pMetadata->GetRefObjNum();
Why switch to a pointer from an object?

[Tom Sepez, 2016-03-08 19:35:42]

https://codereview.chromium.org/1774753002/diff/60001/core/include/fpdfapi/fp...
File core/include/fpdfapi/fpdf_parser.h (right):

https://codereview.chromium.org/1774753002/diff/60001/core/include/fpdfapi/fp...
core/include/fpdfapi/fpdf_parser.h:31: class CPDF_SyntaxParser;
On 2016/03/08 01:15:05, dsinclair wrote:
> What's our thoughts on forward declarations? The style guide says to minimize
> them (https://google.github.io/styleguide/cppguide.html#Forward_Declarations)
> but we've got a lot of them  already.

In a header file, if the class is only used as a pointer or reference, then use
a forward declaration if it avoids including a larger header, whose details
aren't needed.  That's my take. YMMV.

https://codereview.chromium.org/1774753002/diff/60001/core/src/fpdfapi/fpdf_p...
File core/src/fpdfapi/fpdf_parser/cpdf_syntax_parser.cpp (right):

https://codereview.chromium.org/1774753002/diff/60001/core/src/fpdfapi/fpdf_p...
core/src/fpdfapi/fpdf_parser/cpdf_syntax_parser.cpp:1: // Copyright 2014 PDFium
Authors. All rights reserved.
On 2016/03/08 01:15:05, dsinclair wrote:
> nit: 2016?

Done.

https://codereview.chromium.org/1774753002/diff/60001/core/src/fpdfapi/fpdf_p...
core/src/fpdfapi/fpdf_parser/cpdf_syntax_parser.cpp:14: struct SearchTagRecord {
On 2016/03/08 01:15:05, dsinclair wrote:
> Can this go in namespace {}?

Done.

https://codereview.chromium.org/1774753002/diff/60001/core/src/fpdfapi/fpdf_p...
File core/src/fpdfapi/fpdf_parser/cpdf_syntax_parser.h (right):

https://codereview.chromium.org/1774753002/diff/60001/core/src/fpdfapi/fpdf_p...
core/src/fpdfapi/fpdf_parser/cpdf_syntax_parser.h:63: static const int
kParserMaxRecursionDepth = 64;
On 2016/03/08 01:15:05, dsinclair wrote:
> Can these move into the .cpp file now as they will only apply to this one
class?

probably.

https://codereview.chromium.org/1774753002/diff/60001/core/src/fpdfapi/fpdf_p...
File core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp (right):

https://codereview.chromium.org/1774753002/diff/60001/core/src/fpdfapi/fpdf_p...
core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp:288:
m_pSyntax->m_MetadataObjnum = pMetadata->GetRefObjNum();
On 2016/03/08 01:15:06, dsinclair wrote:
> Why switch to a pointer from an object?

So that the .h file needs only the forward declaration, not the full details. 
The .cpp file includes the syntax parser header details.

[Tom Sepez, 2016-03-08 20:15:34]

Description was changed from

==========
Split CPDF_SyntaxParser into its own named .cpp/.h files.

BUG=
==========

to

==========
Split CPDF_SyntaxParser into its own named .cpp/.h files.

BUG=
R=dsinclair@chromium.org

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

[Tom Sepez, 2016-03-08 20:15:35]

Committed patchset #6 (id:100001) manually as
d50095fb151688e119a7097aabce11e4b3d36244 (presubmit successful).