Remove IFGAS_FontMgr and clean up (the renamed) CFGAS_FontMgr a little.

Remove IFGAS_FontMgr and clean up (the renamed) CFGAS_FontMgr a little.

IFGAS_FontMgr is an interface only for a class only defined on Windows,
plus a class only defined for non-Windows. I'm removing the interface,
renaming the class to have the same name in both cases, and cleaning up
a bit of unused methods.

Committed: https://pdfium.googlesource.com/pdfium/+/8f3eb6051c0465923d6694bd0f0b6dc5bb51cbe3

[npm, 2016-11-10 23:28:46]

The CQ bit was checked by npm@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-10 23:28:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-10 23:36:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-10 23:36:25]

Dry run: Try jobs failed on following builders:
  win_xfa_32 on master.tryserver.client.pdfium (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.pdfium/builders/win_xfa_32/buil...)
  win_xfa_clang_32 on master.tryserver.client.pdfium (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.pdfium/builders/win_xfa_clang_3...)

[npm, 2016-11-10 23:42:56]

The CQ bit was checked by npm@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-10 23:43:04]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-10 23:46:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-10 23:46:15]

Dry run: Try jobs failed on following builders:
  win_xfa_clang_32 on master.tryserver.client.pdfium (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.pdfium/builders/win_xfa_clang_3...)
  win_xfa_rel on master.tryserver.client.pdfium (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.pdfium/builders/win_xfa_rel/bui...)

[npm, 2016-11-11 00:08:23]

The CQ bit was checked by npm@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-11 00:08:25]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[npm, 2016-11-11 00:20:16]

Description was changed from

==========
Remove IFGAS_FontMgr and clean up (the renamed) CFGAS_FontMgr a little.

IFGAS_FontMgr is an interface only for a class only defined on Windows,
plus a class only defined for non-Windows. I'm removing the interface,
renaming the class to have the same name in both cases, and cleaning up
a bit of unused methods.
==========

to

==========
Remove IFGAS_FontMgr and clean up (the renamed) CFGAS_FontMgr a little.

IFGAS_FontMgr is an interface only for a class only defined on Windows,
plus a class only defined for non-Windows. I'm removing the interface,
renaming the class to have the same name in both cases, and cleaning up
a bit of unused methods.
==========

[npm, 2016-11-11 00:20:16]

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

[npm, 2016-11-11 00:20:39]

PTAL

https://codereview.chromium.org/2494883002/diff/40001/xfa/fgas/font/cfgas_fon...
File xfa/fgas/font/cfgas_fontmgr.cpp (left):

https://codereview.chromium.org/2494883002/diff/40001/xfa/fgas/font/cfgas_fon...
xfa/fgas/font/cfgas_fontmgr.cpp:1223: #define GetUInt16(p) ((uint16_t)((p)[0] <<
8 | (p)[1]))
Moved these two to inline namespace methods. GetUInt32 was unused.

https://codereview.chromium.org/2494883002/diff/40001/xfa/fgas/font/cfgas_fon...
File xfa/fgas/font/cfgas_fontmgr.cpp (right):

https://codereview.chromium.org/2494883002/diff/40001/xfa/fgas/font/cfgas_fon...
xfa/fgas/font/cfgas_fontmgr.cpp:628: return pFontMgr;
Removed std::move here because compiler got mad

[commit-bot: I haz the power, 2016-11-11 00:20:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-11 00:20:56]

Dry run: This issue passed the CQ dry run.

[Tom Sepez, 2016-11-11 18:43:16]

This is much better. Just a few nits mostly pre-existing.

https://codereview.chromium.org/2494883002/diff/40001/xfa/fgas/font/cfgas_fon...
File xfa/fgas/font/cfgas_fontmgr.cpp (right):

https://codereview.chromium.org/2494883002/diff/40001/xfa/fgas/font/cfgas_fon...
xfa/fgas/font/cfgas_fontmgr.cpp:23: int32_t FX_GetSimilarValue(FX_FONTDESCRIPTOR
const* pFont,
nit: no need for FX_, also can we call this GetSimilarityScore() or
GetSimilarityMetric()? Current name sounds like we return a value similar to one
of the inputs :).

https://codereview.chromium.org/2494883002/diff/40001/xfa/fgas/font/cfgas_fon...
xfa/fgas/font/cfgas_fontmgr.cpp:45: FX_FONTDESCRIPTOR const*
FX_DefFontMatcher(FX_LPFONTMATCHPARAMS pParams,
nit: can we call this MatchDefaultFont()?  A "matcher" is a noun and implies a
class instead of a loose function.

https://codereview.chromium.org/2494883002/diff/40001/xfa/fgas/font/cfgas_fon...
xfa/fgas/font/cfgas_fontmgr.cpp:440: static const FX_BitCodePage
g_Bit2CodePage[] = {
nit: static inside namespace redundant.

https://codereview.chromium.org/2494883002/diff/40001/xfa/fgas/font/cfgas_fon...
xfa/fgas/font/cfgas_fontmgr.cpp:467: inline uint8_t GetUInt8(uint8_t* p) {
nit: const uint8_t* p here and below.

https://codereview.chromium.org/2494883002/diff/40001/xfa/fgas/font/cfgas_fon...
xfa/fgas/font/cfgas_fontmgr.cpp:479: FX_BIT2CHARSET g_FX_Bit2Charset1[16] = {
nit: can these be const FX_BIT2CHARSET ... ?

https://codereview.chromium.org/2494883002/diff/40001/xfa/fgas/font/cfgas_fon...
xfa/fgas/font/cfgas_fontmgr.cpp:628: return pFontMgr;
On 2016/11/11 00:20:39, npm wrote:
> Removed std::move here because compiler got mad

That's correct to remove it, if its just a variable name and not an expresion
being returned.

[npm, 2016-11-11 19:15:35]

The CQ bit was checked by npm@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-11 19:15:37]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[npm, 2016-11-11 19:24:46]

PTAL

https://codereview.chromium.org/2494883002/diff/40001/xfa/fgas/font/cfgas_fon...
File xfa/fgas/font/cfgas_fontmgr.cpp (right):

https://codereview.chromium.org/2494883002/diff/40001/xfa/fgas/font/cfgas_fon...
xfa/fgas/font/cfgas_fontmgr.cpp:23: int32_t FX_GetSimilarValue(FX_FONTDESCRIPTOR
const* pFont,
On 2016/11/11 18:43:16, Tom Sepez wrote:
> nit: no need for FX_, also can we call this GetSimilarityScore() or
> GetSimilarityMetric()? Current name sounds like we return a value similar to
one
> of the inputs :).

Done.

https://codereview.chromium.org/2494883002/diff/40001/xfa/fgas/font/cfgas_fon...
xfa/fgas/font/cfgas_fontmgr.cpp:45: FX_FONTDESCRIPTOR const*
FX_DefFontMatcher(FX_LPFONTMATCHPARAMS pParams,
On 2016/11/11 18:43:16, Tom Sepez wrote:
> nit: can we call this MatchDefaultFont()?  A "matcher" is a noun and implies a
> class instead of a loose function.

Done.

https://codereview.chromium.org/2494883002/diff/40001/xfa/fgas/font/cfgas_fon...
xfa/fgas/font/cfgas_fontmgr.cpp:440: static const FX_BitCodePage
g_Bit2CodePage[] = {
On 2016/11/11 18:43:16, Tom Sepez wrote:
> nit: static inside namespace redundant.

Done.

https://codereview.chromium.org/2494883002/diff/40001/xfa/fgas/font/cfgas_fon...
xfa/fgas/font/cfgas_fontmgr.cpp:467: inline uint8_t GetUInt8(uint8_t* p) {
On 2016/11/11 18:43:16, Tom Sepez wrote:
> nit: const uint8_t* p here and below.

Done.

https://codereview.chromium.org/2494883002/diff/40001/xfa/fgas/font/cfgas_fon...
xfa/fgas/font/cfgas_fontmgr.cpp:479: FX_BIT2CHARSET g_FX_Bit2Charset1[16] = {
On 2016/11/11 18:43:15, Tom Sepez wrote:
> nit: can these be const FX_BIT2CHARSET ... ?

Done.

[commit-bot: I haz the power, 2016-11-11 19:32:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-11 19:32:42]

Dry run: This issue passed the CQ dry run.

[Tom Sepez, 2016-11-12 01:03:27]

lgtm

[npm, 2016-11-12 01:15:53]

The CQ bit was checked by npm@chromium.org

[commit-bot: I haz the power, 2016-11-12 01:16:02]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-12 01:16:27]

Description was changed from

==========
Remove IFGAS_FontMgr and clean up (the renamed) CFGAS_FontMgr a little.

IFGAS_FontMgr is an interface only for a class only defined on Windows,
plus a class only defined for non-Windows. I'm removing the interface,
renaming the class to have the same name in both cases, and cleaning up
a bit of unused methods.
==========

to

==========
Remove IFGAS_FontMgr and clean up (the renamed) CFGAS_FontMgr a little.

IFGAS_FontMgr is an interface only for a class only defined on Windows,
plus a class only defined for non-Windows. I'm removing the interface,
renaming the class to have the same name in both cases, and cleaning up
a bit of unused methods.

Committed:
https://pdfium.googlesource.com/pdfium/+/8f3eb6051c0465923d6694bd0f0b6dc5bb51...
==========

[commit-bot: I haz the power, 2016-11-12 01:16:28]

Committed patchset #4 (id:60001) as
https://pdfium.googlesource.com/pdfium/+/8f3eb6051c0465923d6694bd0f0b6dc5bb51...