Fix / simplify CFX_FolderFontInfo::GetFontData() behavior.

Fix / simplify CFX_FolderFontInfo::GetFontData() behavior.

The GetFontData() behavior in FPDF_SYSFONTINFO seems reasonable.

Also do some code cleanup.

R=tsepez@chromium.org

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

[Lei Zhang, 2015-11-09 17:47:04]

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

[Tom Sepez, 2015-11-09 18:27:51]

https://codereview.chromium.org/1425023003/diff/20001/core/src/fxge/ge/fx_ge_...
File core/src/fxge/ge/fx_ge_fontmap.cpp (right):

https://codereview.chromium.org/1425023003/diff/20001/core/src/fxge/ge/fx_ge_...
core/src/fxge/ge/fx_ge_fontmap.cpp:48: const FX_DWORD kTableNAME = 0x6e616d65;
nit: again, want to use a macro to make a big-endian value from "name".  I think
its FXDWORD_GET_MSBFIRST("name");

https://codereview.chromium.org/1425023003/diff/20001/core/src/fxge/ge/fx_ge_...
core/src/fxge/ge/fx_ge_fontmap.cpp:49: const FX_DWORD kTableTTCF = 0x74746366;
nit: and FXDWORD_GET_MSBFIRST("ttcf")
but doublecheck my arithmetic/translations.

https://codereview.chromium.org/1425023003/diff/20001/core/src/fxge/ge/fx_ge_...
core/src/fxge/ge/fx_ge_fontmap.cpp:523: CFX_ByteString result;
nit: no need for "result" local, just return CFX_ByteString() during the early
returns, or GetNameFromTT() directly.

https://codereview.chromium.org/1425023003/diff/20001/core/src/fxge/ge/fx_ge_...
core/src/fxge/ge/fx_ge_fontmap.cpp:536: result = GetNameFromTT(buffer_ptr, 6);
nit: prefer early return on error.

[Tom Sepez, 2015-11-09 18:28:13]

lgtm

[Lei Zhang, 2015-11-09 19:20:00]

https://codereview.chromium.org/1425023003/diff/20001/core/src/fxge/ge/fx_ge_...
File core/src/fxge/ge/fx_ge_fontmap.cpp (right):

https://codereview.chromium.org/1425023003/diff/20001/core/src/fxge/ge/fx_ge_...
core/src/fxge/ge/fx_ge_fontmap.cpp:48: const FX_DWORD kTableNAME = 0x6e616d65;
On 2015/11/09 18:27:51, Tom Sepez wrote:
> nit: again, want to use a macro to make a big-endian value from "name".  I
think
> its FXDWORD_GET_MSBFIRST("name");

Done.

I locally added a static_assert(kFoo, kFooMacro, "fail") to sanity check before
making the change. However, FXDWORD_GET_MSBFIRST() as is did not play well with
static_assert, so I simplified FXDWORD_GET_MSBFIRST().

https://codereview.chromium.org/1425023003/diff/20001/core/src/fxge/ge/fx_ge_...
core/src/fxge/ge/fx_ge_fontmap.cpp:49: const FX_DWORD kTableTTCF = 0x74746366;
On 2015/11/09 18:27:51, Tom Sepez wrote:
> nit: and FXDWORD_GET_MSBFIRST("ttcf")
> but doublecheck my arithmetic/translations.

Done.

https://codereview.chromium.org/1425023003/diff/20001/core/src/fxge/ge/fx_ge_...
core/src/fxge/ge/fx_ge_fontmap.cpp:523: CFX_ByteString result;
On 2015/11/09 18:27:51, Tom Sepez wrote:
> nit: no need for "result" local, just return CFX_ByteString() during the early
> returns, or GetNameFromTT() directly.

Done. Some say, it makes it easier for compilers to do RVO, but I'll just assume
compilers will be smart enough to figure it out.

https://codereview.chromium.org/1425023003/diff/20001/core/src/fxge/ge/fx_ge_...
core/src/fxge/ge/fx_ge_fontmap.cpp:536: result = GetNameFromTT(buffer_ptr, 6);
On 2015/11/09 18:27:51, Tom Sepez wrote:
> nit: prefer early return on error.

Acknowledged.

[Lei Zhang, 2015-11-09 19:41:04]

To be explicit, PTAL again.

[Lei Zhang, 2015-11-11 18:19:25]

bump

[Tom Sepez, 2015-11-11 21:21:10]

lgtm

[Lei Zhang, 2015-11-12 00:51:58]

Committed patchset #5 (id:80001) manually as
e0c16d7461ca8317433d93835e4c36a84157cbef (presubmit successful).