refactor CPDF_Font and subclasses

refactor CPDF_Font and subclasses

Get rid of types; use virutal functions instead of type based checking;
remove unused functions; change casts; simplify load()/UnicodeFromCharCode()/CharCodeFromUnicode() calling.

R=thestig@chromium.org

Committed: https://pdfium.googlesource.com/pdfium/+/6b7b5a52f42aa9b37be5307915885d5f4566c326

[Wei Li, 2016-02-25 00:03:15]

Patchset #2 (id:20001) has been deleted

[Wei Li, 2016-02-25 00:06:20]

Description was changed from

==========
refactor CPDF_Font and subclasses

Get rid of types; use virutal functions instead of type based checking;
remove unused functions; remove some casts.
==========

to

==========
refactor CPDF_Font and subclasses

Get rid of types; use virutal functions instead of type based checking;
remove unused functions; change casts; simplify
load()/UnicodeFromCharCode()/CharCodeFromUnicode() calling.
==========

[Wei Li, 2016-02-25 00:14:26]

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

[Wei Li, 2016-02-25 00:14:26]

PTAL, thanks

[Lei Zhang, 2016-02-25 01:54:02]

lgtm

https://codereview.chromium.org/1729823004/diff/1/core/include/fpdfapi/fpdf_r...
File core/include/fpdfapi/fpdf_resource.h (right):

https://codereview.chromium.org/1729823004/diff/1/core/include/fpdfapi/fpdf_r...
core/include/fpdfapi/fpdf_resource.h:287: // CPDF_Font
nit: add a colon, ditto below.

https://codereview.chromium.org/1729823004/diff/1/core/src/fpdfapi/fpdf_font/...
File core/src/fpdfapi/fpdf_font/fpdf_font.cpp (right):

https://codereview.chromium.org/1729823004/diff/1/core/src/fpdfapi/fpdf_font/...
core/src/fpdfapi/fpdf_font/fpdf_font.cpp:25: struct _GlyphNameMap {
No leading underscore. Ditto below.

https://codereview.chromium.org/1729823004/diff/1/core/src/fpdfapi/fpdf_font/...
core/src/fpdfapi/fpdf_font/fpdf_font.cpp:34: extern "C" {
not needed

https://codereview.chromium.org/1729823004/diff/1/core/src/fpdfapi/fpdf_font/...
core/src/fpdfapi/fpdf_font/fpdf_font.cpp:41: _GlyphNameMap* found =
(_GlyphNameMap*)FXSYS_bsearch(
Just do std::find() given the size of the array?

https://codereview.chromium.org/1729823004/diff/1/core/src/fpdfapi/fpdf_font/...
core/src/fpdfapi/fpdf_font/fpdf_font.cpp:73: static FX_BOOL
FT_UseType1Charmap(FXFT_Face face) {
no need for static inside anonymous namespace

[Wei Li, 2016-02-25 01:58:24]

Patchset #4 (id:80001) has been deleted

[Wei Li, 2016-02-25 17:56:30]

Patchset #5 (id:120001) has been deleted

[Wei Li, 2016-02-25 17:57:37]

Patchset #5 (id:140001) has been deleted

[Wei Li, 2016-02-25 18:48:43]

Thanks for reviewing it.

https://codereview.chromium.org/1729823004/diff/1/core/include/fpdfapi/fpdf_r...
File core/include/fpdfapi/fpdf_resource.h (right):

https://codereview.chromium.org/1729823004/diff/1/core/include/fpdfapi/fpdf_r...
core/include/fpdfapi/fpdf_resource.h:287: // CPDF_Font
On 2016/02/25 01:54:02, Lei Zhang wrote:
> nit: add a colon, ditto below.

Done.

https://codereview.chromium.org/1729823004/diff/1/core/src/fpdfapi/fpdf_font/...
File core/src/fpdfapi/fpdf_font/fpdf_font.cpp (right):

https://codereview.chromium.org/1729823004/diff/1/core/src/fpdfapi/fpdf_font/...
core/src/fpdfapi/fpdf_font/fpdf_font.cpp:25: struct _GlyphNameMap {
On 2016/02/25 01:54:02, Lei Zhang wrote:
> No leading underscore. Ditto below.

Done.

https://codereview.chromium.org/1729823004/diff/1/core/src/fpdfapi/fpdf_font/...
core/src/fpdfapi/fpdf_font/fpdf_font.cpp:34: extern "C" {
On 2016/02/25 01:54:02, Lei Zhang wrote:
> not needed

Done.

https://codereview.chromium.org/1729823004/diff/1/core/src/fpdfapi/fpdf_font/...
core/src/fpdfapi/fpdf_font/fpdf_font.cpp:41: _GlyphNameMap* found =
(_GlyphNameMap*)FXSYS_bsearch(
On 2016/02/25 01:54:02, Lei Zhang wrote:
> Just do std::find() given the size of the array?

For such a small array, plain linear search should be fine. I was also surprised
that find/find_if seems having more overhead. I will leave it as is for now.

https://codereview.chromium.org/1729823004/diff/1/core/src/fpdfapi/fpdf_font/...
core/src/fpdfapi/fpdf_font/fpdf_font.cpp:73: static FX_BOOL
FT_UseType1Charmap(FXFT_Face face) {
On 2016/02/25 01:54:02, Lei Zhang wrote:
> no need for static inside anonymous namespace

Missed this, thanks

[Wei Li, 2016-02-25 19:35:44]

Description was changed from

==========
refactor CPDF_Font and subclasses

Get rid of types; use virutal functions instead of type based checking;
remove unused functions; change casts; simplify
load()/UnicodeFromCharCode()/CharCodeFromUnicode() calling.
==========

to

==========
refactor CPDF_Font and subclasses

Get rid of types; use virutal functions instead of type based checking;
remove unused functions; change casts; simplify
load()/UnicodeFromCharCode()/CharCodeFromUnicode() calling.

R=thestig@chromium.org

Committed:
https://pdfium.googlesource.com/pdfium/+/6b7b5a52f42aa9b37be5307915885d5f4566...
==========

[Wei Li, 2016-02-25 19:35:46]

Committed patchset #5 (id:160001) manually as
6b7b5a52f42aa9b37be5307915885d5f4566c326 (presubmit successful).