Cleanup some fxge font code.

Cleanup some fxge font code.

R=tsepez@chromium.org

Committed: https://pdfium.googlesource.com/pdfium/+/94a4956f7aadc10fe6dd8451e965bd7447985b76

[Lei Zhang, 2015-11-07 01:32:17]

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

[Lei Zhang, 2015-11-07 01:32:18]

https://codereview.chromium.org/1426403008/diff/1/core/src/fxge/ge/fx_ge_font...
File core/src/fxge/ge/fx_ge_fontmap.cpp (left):

https://codereview.chromium.org/1426403008/diff/1/core/src/fxge/ge/fx_ge_font...
core/src/fxge/ge/fx_ge_fontmap.cpp:498: CFX_ByteString
_FPDF_LoadTableFromTTStreamFile(IFX_FileStream* pFile,
Dead

https://codereview.chromium.org/1426403008/diff/1/core/src/fxge/ge/fx_ge_font...
core/src/fxge/ge/fx_ge_fontmap.cpp:632: FX_DWORD
_GetCodePageRangeFromCharset(int charset) {
Dead

[Tom Sepez, 2015-11-09 18:05:18]

lgtm

https://codereview.chromium.org/1426403008/diff/1/core/include/fxge/fx_font.h
File core/include/fxge/fx_font.h (right):

https://codereview.chromium.org/1426403008/diff/1/core/include/fxge/fx_font.h...
core/include/fxge/fx_font.h:403: FXFT_Face const m_Face;
nit: Someday we'll lose the *s in the typedefs, and this will make sense as
FXFT_FaceRec* const m_Face, but for now the const looks to be in a weird place,
though putting it first is misleading. Hmm.

https://codereview.chromium.org/1426403008/diff/1/core/include/fxge/fx_font.h...
core/include/fxge/fx_font.h:432: int m_OriginX, m_OriginY;
nit: one per line (next line too).

https://codereview.chromium.org/1426403008/diff/1/core/src/fxge/ge/fx_ge_font...
File core/src/fxge/ge/fx_ge_fontmap.cpp (right):

https://codereview.chromium.org/1426403008/diff/1/core/src/fxge/ge/fx_ge_font...
core/src/fxge/ge/fx_ge_fontmap.cpp:224: CFX_ByteString KeyNameFromFace(const
CFX_ByteString& face_name,
This is just moving stuff around, right?

https://codereview.chromium.org/1426403008/diff/1/core/src/fxge/ge/fx_ge_font...
core/src/fxge/ge/fx_ge_fontmap.cpp:1336: CFX_ByteString os2 =
FPDF_LoadTableFromTT(pFile, tables, nTables, 0x4f532f32);
nit: Isn't there some macro in somewhere in this codebase that would compute
0x4f532f32 regardless of endianness from "OS/2"? I was overjoyed to see that
string when I decoded this by hand.

https://codereview.chromium.org/1426403008/diff/1/core/src/fxge/ge/fx_ge_font...
core/src/fxge/ge/fx_ge_fontmap.cpp:1489: sizeof(AltFontName), CompareString));
nit: used more than once? else [](){} is your friend.

[Lei Zhang, 2015-11-09 18:41:03]

https://codereview.chromium.org/1426403008/diff/1/core/include/fxge/fx_font.h
File core/include/fxge/fx_font.h (right):

https://codereview.chromium.org/1426403008/diff/1/core/include/fxge/fx_font.h...
core/include/fxge/fx_font.h:403: FXFT_Face const m_Face;
On 2015/11/09 18:05:17, Tom Sepez wrote:
> nit: Someday we'll lose the *s in the typedefs, and this will make sense as
> FXFT_FaceRec* const m_Face, but for now the const looks to be in a weird
place,
> though putting it first is misleading. Hmm.

I reordered the methods / variables.

https://codereview.chromium.org/1426403008/diff/1/core/include/fxge/fx_font.h...
core/include/fxge/fx_font.h:432: int m_OriginX, m_OriginY;
On 2015/11/09 18:05:17, Tom Sepez wrote:
> nit: one per line (next line too).

Done.

https://codereview.chromium.org/1426403008/diff/1/core/src/fxge/ge/fx_ge_font...
File core/src/fxge/ge/fx_ge_fontmap.cpp (right):

https://codereview.chromium.org/1426403008/diff/1/core/src/fxge/ge/fx_ge_font...
core/src/fxge/ge/fx_ge_fontmap.cpp:224: CFX_ByteString KeyNameFromFace(const
CFX_ByteString& face_name,
On 2015/11/09 18:05:18, Tom Sepez wrote:
> This is just moving stuff around, right?  

Yep. Just shuffling code into the anonymous namespace.

https://codereview.chromium.org/1426403008/diff/1/core/src/fxge/ge/fx_ge_font...
core/src/fxge/ge/fx_ge_fontmap.cpp:1336: CFX_ByteString os2 =
FPDF_LoadTableFromTT(pFile, tables, nTables, 0x4f532f32);
On 2015/11/09 18:05:17, Tom Sepez wrote:
> nit: Isn't there some macro in somewhere in this codebase that would compute
> 0x4f532f32 regardless of endianness from "OS/2"? I was overjoyed to see that
> string when I decoded this by hand. 

I have other CLs to fix these.

https://codereview.chromium.org/1426403008/diff/1/core/src/fxge/ge/fx_ge_font...
core/src/fxge/ge/fx_ge_fontmap.cpp:1489: sizeof(AltFontName), CompareString));
On 2015/11/09 18:05:18, Tom Sepez wrote:
> nit: used more than once? else [](){} is your friend.

bsearch() is not std::binary_search(), so it does not take lambdas. Switching to
std::binary_search() would require a std::vector copy of |g_AltFontNames|, or
std::begin(), which is currently banned.

[Lei Zhang, 2015-11-09 18:42:05]

Committed patchset #3 (id:40001) manually as
94a4956f7aadc10fe6dd8451e965bd7447985b76 (presubmit successful).