Split fpdf_parser_objects.cpp into per-class .cpp/.h files.

Split fpdf_parser_objects.cpp into per-class .cpp/.h files.

R=dsinclair@chromium.org

Committed: https://pdfium.googlesource.com/pdfium/+/5fc239a0e64a5e16a23b99e128f88cc494d5b65c

[Tom Sepez, 2016-03-10 20:59:32]

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

[Tom Sepez, 2016-03-10 20:59:33]

Dan, last of the parser code.

[dsinclair, 2016-03-10 21:23:46]

lgtm w/ nits (and I think cdpf_null.cpp is named wrong ...)

https://codereview.chromium.org/1776913007/diff/1/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/1776913007/diff/1/BUILD.gn#newcode351
BUILD.gn:351: "core/src/fpdfapi/fpdf_parser/cdpf_null.cpp",
cpdf.... heh

https://codereview.chromium.org/1776913007/diff/1/core/include/fpdfapi/cpdf_a...
File core/include/fpdfapi/cpdf_array.h (right):

https://codereview.chromium.org/1776913007/diff/1/core/include/fpdfapi/cpdf_a...
core/include/fpdfapi/cpdf_array.h:1: // Copyright 2014 PDFium Authors. All
rights reserved.
nit: 2016

https://codereview.chromium.org/1776913007/diff/1/core/include/fpdfapi/cpdf_b...
File core/include/fpdfapi/cpdf_boolean.h (right):

https://codereview.chromium.org/1776913007/diff/1/core/include/fpdfapi/cpdf_b...
core/include/fpdfapi/cpdf_boolean.h:17: explicit CPDF_Boolean(FX_BOOL value) :
m_bValue(value) {}
nit: move the constructors to the .cpp file for chromium-style happy-ness

https://codereview.chromium.org/1776913007/diff/1/core/include/fpdfapi/cpdf_n...
File core/include/fpdfapi/cpdf_name.h (right):

https://codereview.chromium.org/1776913007/diff/1/core/include/fpdfapi/cpdf_n...
core/include/fpdfapi/cpdf_name.h:16: explicit CPDF_Name(const FX_CHAR* str) :
m_Name(str) {}
nit: it constructors in .cpp for chromium-style

https://codereview.chromium.org/1776913007/diff/1/core/include/fpdfapi/cpdf_n...
File core/include/fpdfapi/cpdf_null.h (right):

https://codereview.chromium.org/1776913007/diff/1/core/include/fpdfapi/cpdf_n...
core/include/fpdfapi/cpdf_null.h:14: CPDF_Null() {}
ditto

https://codereview.chromium.org/1776913007/diff/1/core/include/fpdfapi/cpdf_n...
File core/include/fpdfapi/cpdf_number.h (right):

https://codereview.chromium.org/1776913007/diff/1/core/include/fpdfapi/cpdf_n...
core/include/fpdfapi/cpdf_number.h:18: explicit CPDF_Number(FX_FLOAT value) :
m_bInteger(FALSE), m_Float(value) {}
ditto

https://codereview.chromium.org/1776913007/diff/1/core/include/fpdfapi/cpdf_r...
File core/include/fpdfapi/cpdf_reference.h (right):

https://codereview.chromium.org/1776913007/diff/1/core/include/fpdfapi/cpdf_r...
core/include/fpdfapi/cpdf_reference.h:17: : m_pObjList(pDoc),
m_RefObjNum(objnum) {}
ditto

https://codereview.chromium.org/1776913007/diff/1/core/include/fpdfapi/cpdf_s...
File core/include/fpdfapi/cpdf_stream.h (right):

https://codereview.chromium.org/1776913007/diff/1/core/include/fpdfapi/cpdf_s...
core/include/fpdfapi/cpdf_stream.h:68: class CPDF_StreamAcc {
Move to its own file?

https://codereview.chromium.org/1776913007/diff/1/core/include/fpdfapi/cpdf_s...
File core/include/fpdfapi/cpdf_string.h (right):

https://codereview.chromium.org/1776913007/diff/1/core/include/fpdfapi/cpdf_s...
core/include/fpdfapi/cpdf_string.h:16: CPDF_String() : m_bHex(FALSE) {}
ditto

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
File core/src/fpdfapi/fpdf_parser/cpdf_array.cpp (right):

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/cpdf_array.cpp:1: // Copyright 2014 PDFium Authors.
All rights reserved.
2016

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
File core/src/fpdfapi/fpdf_parser/cpdf_dictionary.cpp (right):

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/cpdf_dictionary.cpp:23: }
nit: no {}s

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/cpdf_dictionary.cpp:72: return CFX_ByteString();
return p ? p->GetString() : CFX_ByteString();

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/cpdf_dictionary.cpp:81: return CFX_ByteStringC();
return p ? p->GetConstString() : CFX_ByteStringC();

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/cpdf_dictionary.cpp:98: return CFX_ByteString(def);
return p ? p->GetString() : CFX_ByteString(def);

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/cpdf_dictionary.cpp:108: return
CFX_ByteStringC(def);
return p ? p->GetConstString() : CFX_ByteStringC(def);

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/cpdf_dictionary.cpp:116: return 0;
return p ? p->GetInteger() : 0;

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/cpdf_dictionary.cpp:124: return def;
return p ? p->GetInteger() : def;

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/cpdf_dictionary.cpp:132: return 0;
return p ? p->GetNumber() : 0;

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/cpdf_dictionary.cpp:140: return bDefault;
return ToBoolean(p) ? p->GetInteger() : bDefault;

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/cpdf_dictionary.cpp:197: }
nit: {}s

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
File core/src/fpdfapi/fpdf_parser/cpdf_indirect_object_holder.cpp (right):

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/cpdf_indirect_object_holder.cpp:20: }
nit: {}s

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/cpdf_indirect_object_holder.cpp:50: }
nit: {}s

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
File core/src/fpdfapi/fpdf_parser/cpdf_object.cpp (right):

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/cpdf_object.cpp:28: }
nit: {}s

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
File core/src/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp (right):

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp:13: #include
"core/include/fpdfapi/cpdf_stream.h"
Can this test be split to per object type? Or, file a bug to split it later?

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
File core/src/fpdfapi/fpdf_parser/cpdf_stream.cpp (right):

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/cpdf_stream.cpp:67: }
nit: {}s

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/cpdf_stream.cpp:70: m_pDict->SetAtInteger("Length",
size);
nit: {}s

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/cpdf_stream.cpp:81: }
nit: {}s

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/cpdf_stream.cpp:128: }
nit: {}

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/cpdf_stream.cpp:137:
CPDF_StreamAcc::CPDF_StreamAcc() {
Move to its own cpp file?

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/cpdf_stream.cpp:138: m_bNewBuf = FALSE;
Move these to initializer list.

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
File core/src/fpdfapi/fpdf_parser/cpdf_string.cpp (right):

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/cpdf_string.cpp:1: // Copyright 2014 PDFium
Authors. All rights reserved.
2016

https://codereview.chromium.org/1776913007/diff/1/fpdfsdk/src/fpdfdoc.cpp
File fpdfsdk/src/fpdfdoc.cpp (right):

https://codereview.chromium.org/1776913007/diff/1/fpdfsdk/src/fpdfdoc.cpp#new...
fpdfsdk/src/fpdfdoc.cpp:18: int THISMODULE = 0;
Ah, this is wacky, can it be removed?

https://codereview.chromium.org/1776913007/diff/1/pdfium.gyp
File pdfium.gyp (right):

https://codereview.chromium.org/1776913007/diff/1/pdfium.gyp#newcode366
pdfium.gyp:366: 'core/src/fpdfapi/fpdf_parser/cdpf_null.cpp',
cpdf ...

[Tom Sepez, 2016-03-10 22:05:19]

https://codereview.chromium.org/1776913007/diff/1/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/1776913007/diff/1/BUILD.gn#newcode351
BUILD.gn:351: "core/src/fpdfapi/fpdf_parser/cdpf_null.cpp",
On 2016/03/10 21:23:44, dsinclair wrote:
> cpdf.... heh

Done.

https://codereview.chromium.org/1776913007/diff/1/core/include/fpdfapi/cpdf_a...
File core/include/fpdfapi/cpdf_array.h (right):

https://codereview.chromium.org/1776913007/diff/1/core/include/fpdfapi/cpdf_a...
core/include/fpdfapi/cpdf_array.h:1: // Copyright 2014 PDFium Authors. All
rights reserved.
On 2016/03/10 21:23:44, dsinclair wrote:
> nit: 2016

Done.

https://codereview.chromium.org/1776913007/diff/1/core/include/fpdfapi/cpdf_b...
File core/include/fpdfapi/cpdf_boolean.h (right):

https://codereview.chromium.org/1776913007/diff/1/core/include/fpdfapi/cpdf_b...
core/include/fpdfapi/cpdf_boolean.h:17: explicit CPDF_Boolean(FX_BOOL value) :
m_bValue(value) {}
On 2016/03/10 21:23:44, dsinclair wrote:
> nit: move the constructors to the .cpp file for chromium-style happy-ness

Done.

https://codereview.chromium.org/1776913007/diff/1/core/include/fpdfapi/cpdf_n...
File core/include/fpdfapi/cpdf_name.h (right):

https://codereview.chromium.org/1776913007/diff/1/core/include/fpdfapi/cpdf_n...
core/include/fpdfapi/cpdf_name.h:16: explicit CPDF_Name(const FX_CHAR* str) :
m_Name(str) {}
On 2016/03/10 21:23:44, dsinclair wrote:
> nit: it constructors in .cpp for chromium-style

Done.

https://codereview.chromium.org/1776913007/diff/1/core/include/fpdfapi/cpdf_n...
File core/include/fpdfapi/cpdf_null.h (right):

https://codereview.chromium.org/1776913007/diff/1/core/include/fpdfapi/cpdf_n...
core/include/fpdfapi/cpdf_null.h:14: CPDF_Null() {}
On 2016/03/10 21:23:44, dsinclair wrote:
> ditto

Done.

https://codereview.chromium.org/1776913007/diff/1/core/include/fpdfapi/cpdf_r...
File core/include/fpdfapi/cpdf_reference.h (right):

https://codereview.chromium.org/1776913007/diff/1/core/include/fpdfapi/cpdf_r...
core/include/fpdfapi/cpdf_reference.h:17: : m_pObjList(pDoc),
m_RefObjNum(objnum) {}
On 2016/03/10 21:23:44, dsinclair wrote:
> ditto

Done.

https://codereview.chromium.org/1776913007/diff/1/core/include/fpdfapi/cpdf_s...
File core/include/fpdfapi/cpdf_stream.h (right):

https://codereview.chromium.org/1776913007/diff/1/core/include/fpdfapi/cpdf_s...
core/include/fpdfapi/cpdf_stream.h:68: class CPDF_StreamAcc {
On 2016/03/10 21:23:44, dsinclair wrote:
> Move to its own file?

I think that long-term this is becomes a CPDF_Stream::Accessor nested class
which is why I left it here.

https://codereview.chromium.org/1776913007/diff/1/core/include/fpdfapi/cpdf_s...
File core/include/fpdfapi/cpdf_string.h (right):

https://codereview.chromium.org/1776913007/diff/1/core/include/fpdfapi/cpdf_s...
core/include/fpdfapi/cpdf_string.h:16: CPDF_String() : m_bHex(FALSE) {}
On 2016/03/10 21:23:44, dsinclair wrote:
> ditto

Done.

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
File core/src/fpdfapi/fpdf_parser/cpdf_dictionary.cpp (right):

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/cpdf_dictionary.cpp:23: }
On 2016/03/10 21:23:44, dsinclair wrote:
> nit: no {}s

Done.

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/cpdf_dictionary.cpp:72: return CFX_ByteString();
On 2016/03/10 21:23:45, dsinclair wrote:
> return p ? p->GetString() : CFX_ByteString();

Done.

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
File core/src/fpdfapi/fpdf_parser/cpdf_indirect_object_holder.cpp (right):

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/cpdf_indirect_object_holder.cpp:20: }
On 2016/03/10 21:23:45, dsinclair wrote:
> nit: {}s

Done.

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/cpdf_indirect_object_holder.cpp:50: }
On 2016/03/10 21:23:45, dsinclair wrote:
> nit: {}s

Done.

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
File core/src/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp (right):

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/cpdf_object_unittest.cpp:13: #include
"core/include/fpdfapi/cpdf_stream.h"
On 2016/03/10 21:23:45, dsinclair wrote:
> Can this test be split to per object type? Or, file a bug to split it later?

we'll split it later, that was  my intent.

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
File core/src/fpdfapi/fpdf_parser/cpdf_stream.cpp (right):

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/cpdf_stream.cpp:67: }
On 2016/03/10 21:23:45, dsinclair wrote:
> nit: {}s

Done.

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/cpdf_stream.cpp:70: m_pDict->SetAtInteger("Length",
size);
On 2016/03/10 21:23:45, dsinclair wrote:
> nit: {}s

Done.

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/cpdf_stream.cpp:81: }
On 2016/03/10 21:23:45, dsinclair wrote:
> nit: {}s

Done.

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/cpdf_stream.cpp:137:
CPDF_StreamAcc::CPDF_StreamAcc() {
On 2016/03/10 21:23:45, dsinclair wrote:
> Move to its own cpp file?
See previous answer.

https://codereview.chromium.org/1776913007/diff/1/core/src/fpdfapi/fpdf_parse...
core/src/fpdfapi/fpdf_parser/cpdf_stream.cpp:138: m_bNewBuf = FALSE;
On 2016/03/10 21:23:45, dsinclair wrote:
> Move these to initializer list.

Done.

https://codereview.chromium.org/1776913007/diff/1/fpdfsdk/src/fpdfdoc.cpp
File fpdfsdk/src/fpdfdoc.cpp (right):

https://codereview.chromium.org/1776913007/diff/1/fpdfsdk/src/fpdfdoc.cpp#new...
fpdfsdk/src/fpdfdoc.cpp:18: int THISMODULE = 0;
On 2016/03/10 21:23:45, dsinclair wrote:
> Ah, this is wacky, can it be removed?

Not yet.  The so-called SetPrivateData allows one to bind a piece of data to an
arbirtary address, so they need something that's sure not to conflict ...

https://codereview.chromium.org/1776913007/diff/1/pdfium.gyp
File pdfium.gyp (right):

https://codereview.chromium.org/1776913007/diff/1/pdfium.gyp#newcode366
pdfium.gyp:366: 'core/src/fpdfapi/fpdf_parser/cdpf_null.cpp',
On 2016/03/10 21:23:45, dsinclair wrote:
> cpdf ...

Done.

[Tom Sepez, 2016-03-10 22:10:45]

Description was changed from

==========
Split fpdf_parser_objects.cpp into per-class .cpp/.h files.
==========

to

==========
Split fpdf_parser_objects.cpp into per-class .cpp/.h files.

R=dsinclair@chromium.org

Committed:
https://pdfium.googlesource.com/pdfium/+/5fc239a0e64a5e16a23b99e128f88cc494d5...
==========

[Tom Sepez, 2016-03-10 22:10:47]

Committed patchset #2 (id:20001) manually as
5fc239a0e64a5e16a23b99e128f88cc494d5b65c (presubmit successful).