Refactor CPDF_Object and its subclasses.

Refactor CPDF_Object and its subclasses.

Mainly use virtual functions to replace functions with switch statements.
Also remove a few unused functions and make some format changes.

BUG=pdfium:63
R=thestig@chromium.org

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

[Wei Li, 2016-01-27 02:54:46]

Description was changed from

==========
Refactoring CPDF_Object and subclasses.

Mainly use virtual functions to replace switch statements based on types.
Also removed a few unused functions and did some format changes.

BUG=pdfium:63
==========

to

==========
Refactor CPDF_Object and its subclasses.

Mainly use virtual functions to replace switch statements based on types.
Also removed a few unused functions and did some format changes.

BUG=pdfium:63
==========

[Wei Li, 2016-01-27 02:55:25]

Description was changed from

==========
Refactor CPDF_Object and its subclasses.

Mainly use virtual functions to replace switch statements based on types.
Also removed a few unused functions and did some format changes.

BUG=pdfium:63
==========

to

==========
Refactor CPDF_Object and its subclasses.

Mainly used virtual functions to replace switch statements based on types.
Also removed a few unused functions and did some format changes.

BUG=pdfium:63
==========

[Wei Li, 2016-01-27 02:56:24]

Description was changed from

==========
Refactor CPDF_Object and its subclasses.

Mainly used virtual functions to replace switch statements based on types.
Also removed a few unused functions and did some format changes.

BUG=pdfium:63
==========

to

==========
Refactor CPDF_Object and its subclasses.

Mainly use virtual functions to replace switch statements based on types.
Also remove a few unused functions and make some format changes.

BUG=pdfium:63
==========

[Wei Li, 2016-01-27 02:59:55]

weili@chromium.org changed reviewers:
+ thestig@chromium.org

[Wei Li, 2016-01-27 02:59:56]

PTAL, thanks

[Wei Li, 2016-01-27 03:01:45]

Description was changed from

==========
Refactor CPDF_Object and its subclasses.

Mainly use virtual functions to replace switch statements based on types.
Also remove a few unused functions and make some format changes.

BUG=pdfium:63
==========

to

==========
Refactor CPDF_Object and its subclasses.

Mainly use virtual functions to replace functions with switch statements.
Also remove a few unused functions and make some format changes.

BUG=pdfium:63
==========

[Lei Zhang, 2016-01-27 23:17:00]

Thank you for taking on this bug.

https://codereview.chromium.org/1634373003/diff/20001/core/include/fpdfapi/fp...
File core/include/fpdfapi/fpdf_objects.h (right):

https://codereview.chromium.org/1634373003/diff/20001/core/include/fpdfapi/fp...
core/include/fpdfapi/fpdf_objects.h:61: virtual CFX_WideString
GetUnicodeText(CFX_CharMap* pCharMap = NULL) const {
NULL -> nullptr since we are changing this line anyway.

https://codereview.chromium.org/1634373003/diff/20001/core/include/fpdfapi/fp...
core/include/fpdfapi/fpdf_objects.h:118: // From CPDF_Object.
nit: We usually just write this as: // CPDF_Object:

https://codereview.chromium.org/1634373003/diff/20001/core/include/fpdfapi/fp...
core/include/fpdfapi/fpdf_objects.h:131: CPDF_Boolean* AsBoolean() override {
return const_cast<CPDF_Boolean*>(this); }
Do you need the const_cast?

https://codereview.chromium.org/1634373003/diff/20001/core/include/fpdfapi/fp...
core/include/fpdfapi/fpdf_objects.h:158: if (m_bInteger)
One liner: return m_bInteger ? new CPDF_Number(m_Integer) : new
CPDF_Number(m_Float);

https://codereview.chromium.org/1634373003/diff/20001/core/include/fpdfapi/fp...
core/include/fpdfapi/fpdf_objects.h:497: const CFX_ByteString& GetImageDecoder()
{ return m_ImageDecoder; }
Can these two methods be const?

https://codereview.chromium.org/1634373003/diff/20001/core/src/fpdfapi/fpdf_p...
File core/src/fpdfapi/fpdf_parser/fpdf_parser_objects_unittest.cpp (right):

https://codereview.chromium.org/1634373003/diff/20001/core/src/fpdfapi/fpdf_p...
core/src/fpdfapi/fpdf_parser/fpdf_parser_objects_unittest.cpp:64: for (int i =
0; i < m_IndirectObjs.size(); ++i) {
|i| should be size_t. Ditto below.

https://codereview.chromium.org/1634373003/diff/20001/core/src/fpdfapi/fpdf_p...
core/src/fpdfapi/fpdf_parser/fpdf_parser_objects_unittest.cpp:265: auto obj =
m_DirectObjs[i]->Clone();
Do we need to free |obj|?

https://codereview.chromium.org/1634373003/diff/20001/core/src/fpdfapi/fpdf_p...
core/src/fpdfapi/fpdf_parser/fpdf_parser_objects_unittest.cpp:298: const char*
set_values[] = {"true",    "fake", "3.125f", "097",
const char* const set_values[] = { ... }

Otherwise, it's not truly const and one can write x[0] = "foo";

[Wei Li, 2016-01-28 18:23:14]

https://codereview.chromium.org/1634373003/diff/20001/core/include/fpdfapi/fp...
File core/include/fpdfapi/fpdf_objects.h (right):

https://codereview.chromium.org/1634373003/diff/20001/core/include/fpdfapi/fp...
core/include/fpdfapi/fpdf_objects.h:61: virtual CFX_WideString
GetUnicodeText(CFX_CharMap* pCharMap = NULL) const {
On 2016/01/27 23:16:59, Lei Zhang wrote:
> NULL -> nullptr since we are changing this line anyway.

Done.

https://codereview.chromium.org/1634373003/diff/20001/core/include/fpdfapi/fp...
core/include/fpdfapi/fpdf_objects.h:118: // From CPDF_Object.
On 2016/01/27 23:16:59, Lei Zhang wrote:
> nit: We usually just write this as: // CPDF_Object:

Done.

https://codereview.chromium.org/1634373003/diff/20001/core/include/fpdfapi/fp...
core/include/fpdfapi/fpdf_objects.h:131: CPDF_Boolean* AsBoolean() override {
return const_cast<CPDF_Boolean*>(this); }
On 2016/01/27 23:16:59, Lei Zhang wrote:
> Do you need the const_cast?

Done.

https://codereview.chromium.org/1634373003/diff/20001/core/include/fpdfapi/fp...
core/include/fpdfapi/fpdf_objects.h:158: if (m_bInteger)
On 2016/01/27 23:16:59, Lei Zhang wrote:
> One liner: return m_bInteger ? new CPDF_Number(m_Integer) : new
> CPDF_Number(m_Float);

Done.

https://codereview.chromium.org/1634373003/diff/20001/core/include/fpdfapi/fp...
core/include/fpdfapi/fpdf_objects.h:497: const CFX_ByteString& GetImageDecoder()
{ return m_ImageDecoder; }
On 2016/01/27 23:16:59, Lei Zhang wrote:
> Can these two methods be const?

Done.

https://codereview.chromium.org/1634373003/diff/20001/core/src/fpdfapi/fpdf_p...
File core/src/fpdfapi/fpdf_parser/fpdf_parser_objects_unittest.cpp (right):

https://codereview.chromium.org/1634373003/diff/20001/core/src/fpdfapi/fpdf_p...
core/src/fpdfapi/fpdf_parser/fpdf_parser_objects_unittest.cpp:64: for (int i =
0; i < m_IndirectObjs.size(); ++i) {
On 2016/01/27 23:17:00, Lei Zhang wrote:
> |i| should be size_t. Ditto below.

Done.

https://codereview.chromium.org/1634373003/diff/20001/core/src/fpdfapi/fpdf_p...
core/src/fpdfapi/fpdf_parser/fpdf_parser_objects_unittest.cpp:265: auto obj =
m_DirectObjs[i]->Clone();
On 2016/01/27 23:17:00, Lei Zhang wrote:
> Do we need to free |obj|?

Good catch, done.

https://codereview.chromium.org/1634373003/diff/20001/core/src/fpdfapi/fpdf_p...
core/src/fpdfapi/fpdf_parser/fpdf_parser_objects_unittest.cpp:298: const char*
set_values[] = {"true",    "fake", "3.125f", "097",
On 2016/01/27 23:17:00, Lei Zhang wrote:
> const char* const set_values[] = { ... }
> 
> Otherwise, it's not truly const and one can write x[0] = "foo";

Done.

[Lei Zhang, 2016-01-28 22:54:03]

lgtm

[Wei Li, 2016-01-30 01:25:43]

Patchset #4 (id:60001) has been deleted

[Wei Li, 2016-01-30 01:27:49]

Patchset #4 (id:80001) has been deleted

[Wei Li, 2016-01-30 01:30:32]

Description was changed from

==========
Refactor CPDF_Object and its subclasses.

Mainly use virtual functions to replace functions with switch statements.
Also remove a few unused functions and make some format changes.

BUG=pdfium:63
==========

to

==========
Refactor CPDF_Object and its subclasses.

Mainly use virtual functions to replace functions with switch statements.
Also remove a few unused functions and make some format changes.

BUG=pdfium:63
R=thestig@chromium.org

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

[Wei Li, 2016-01-30 01:30:33]

Committed patchset #4 (id:100001) manually as
df1ab41e000069dda7bf21527308f3bf41e126ca (presubmit successful).