Clean up fpf_skiafontmgr

Clean up fpf_skiafontmgr

Remove unused methods, create namespace, nits.

Committed: https://pdfium.googlesource.com/pdfium/+/83828032e3fa26dd98185523dfdba5f1f4e94dfc

[npm, 2016-10-18 22:33:43]

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

[commit-bot: I haz the power, 2016-10-18 22:33:48]

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-10-18 22:40:58]

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

[commit-bot: I haz the power, 2016-10-18 22:40:59]

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

[npm, 2016-10-18 22:43:28]

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

[commit-bot: I haz the power, 2016-10-18 22:43:38]

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-10-18 22:59:48]

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

[commit-bot: I haz the power, 2016-10-18 22:59:49]

Dry run: This issue passed the CQ dry run.

[npm, 2016-10-18 23:12:09]

npm@chromium.org changed reviewers:
+ caryclark@chromium.org, dsinclair@chromium.org, thestig@chromium.org

[npm, 2016-10-18 23:20:09]

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

[commit-bot: I haz the power, 2016-10-18 23:20:17]

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-10-18 23:28:06]

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

[commit-bot: I haz the power, 2016-10-18 23:28:06]

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

[npm, 2016-10-18 23:29:14]

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

[commit-bot: I haz the power, 2016-10-18 23:29:16]

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

[npm, 2016-10-18 23:29:19]

PTAL

[Lei Zhang, 2016-10-18 23:30:21]

Description was changed from

==========
Remove unused methods, create namespace, nits.
==========

to

==========
Clean up fpf_skiafontmgr

Remove unused methods, create namespace, nits.
==========

[Lei Zhang, 2016-10-18 23:32:41]

Are you going to rename core/fxge/android/fpf_skiafontmgr.h later?

I edited the CL description, BTW.

[Lei Zhang, 2016-10-18 23:39:15]

https://codereview.chromium.org/2433543002/diff/60001/core/fxge/android/cfpf_...
File core/fxge/android/cfpf_skiafontmgr.cpp (right):

https://codereview.chromium.org/2433543002/diff/60001/core/fxge/android/cfpf_...
core/fxge/android/cfpf_skiafontmgr.cpp:74: skFontMap + sizeof(skFontMap) /
sizeof(FPF_SKIAFONTMAP);
The sizeof() part doesn't smell right.

https://codereview.chromium.org/2433543002/diff/60001/core/fxge/android/cfpf_...
core/fxge/android/cfpf_skiafontmgr.cpp:200: uint32_t g_FPFSkiaFontCharsets[] = {
Is this missing a const now?

https://codereview.chromium.org/2433543002/diff/60001/core/fxge/android/cfpf_...
core/fxge/android/cfpf_skiafontmgr.cpp:257: for (auto it = m_FontFaces.rbegin();
it != m_FontFaces.rend(); ++it)
I wonder if this has to delete in reverse order, and if we can switch to vector
of unique_ptrs.

https://codereview.chromium.org/2433543002/diff/60001/core/fxge/android/cfpf_...
core/fxge/android/cfpf_skiafontmgr.cpp:288: uint32_t dwSubst =
FPF_SkiaGetSubstFont(dwFaceName, g_SkiaFontmap);
Is the second parameter always going to be |g_SkiaFontmap| ?

[commit-bot: I haz the power, 2016-10-18 23:42:39]

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

[commit-bot: I haz the power, 2016-10-18 23:42:40]

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

[npm, 2016-10-21 13:53:38]

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

[commit-bot: I haz the power, 2016-10-21 13:53:42]

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-10-21 14:07:49]

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

[commit-bot: I haz the power, 2016-10-21 14:07:50]

Dry run: This issue passed the CQ dry run.

[npm, 2016-10-21 14:08:51]

There are several class definitions in fpf_skiafontmgr.h.
Not renaming for now.

https://codereview.chromium.org/2433543002/diff/60001/core/fxge/android/cfpf_...
File core/fxge/android/cfpf_skiafontmgr.cpp (right):

https://codereview.chromium.org/2433543002/diff/60001/core/fxge/android/cfpf_...
core/fxge/android/cfpf_skiafontmgr.cpp:74: skFontMap + sizeof(skFontMap) /
sizeof(FPF_SKIAFONTMAP);
On 2016/10/18 23:39:14, Lei Zhang wrote:
> The sizeof() part doesn't smell right.

Done.

https://codereview.chromium.org/2433543002/diff/60001/core/fxge/android/cfpf_...
core/fxge/android/cfpf_skiafontmgr.cpp:200: uint32_t g_FPFSkiaFontCharsets[] = {
On 2016/10/18 23:39:14, Lei Zhang wrote:
> Is this missing a const now?

Done.

https://codereview.chromium.org/2433543002/diff/60001/core/fxge/android/cfpf_...
core/fxge/android/cfpf_skiafontmgr.cpp:257: for (auto it = m_FontFaces.rbegin();
it != m_FontFaces.rend(); ++it)
On 2016/10/18 23:39:14, Lei Zhang wrote:
> I wonder if this has to delete in reverse order, and if we can switch to
vector
> of unique_ptrs.

It looks like they can be deleted in normal order. The unique_ptr change would
be
nontrivial because CFPF_SkiaFontDescriptor is cast into font and file
descriptors

https://codereview.chromium.org/2433543002/diff/60001/core/fxge/android/cfpf_...
core/fxge/android/cfpf_skiafontmgr.cpp:288: uint32_t dwSubst =
FPF_SkiaGetSubstFont(dwFaceName, g_SkiaFontmap);
On 2016/10/18 23:39:14, Lei Zhang wrote:
> Is the second parameter always going to be |g_SkiaFontmap| ?

No, in the line right below the parameter is |g_SkiaSansFontMap|

[Lei Zhang, 2016-10-21 18:26:21]

lgtm

https://codereview.chromium.org/2433543002/diff/60001/core/fxge/android/cfpf_...
File core/fxge/android/cfpf_skiafontmgr.cpp (right):

https://codereview.chromium.org/2433543002/diff/60001/core/fxge/android/cfpf_...
core/fxge/android/cfpf_skiafontmgr.cpp:288: uint32_t dwSubst =
FPF_SkiaGetSubstFont(dwFaceName, g_SkiaFontmap);
On 2016/10/21 14:08:50, npm wrote:
> On 2016/10/18 23:39:14, Lei Zhang wrote:
> > Is the second parameter always going to be |g_SkiaFontmap| ?
> 
> No, in the line right below the parameter is |g_SkiaSansFontMap|

*blink* *blink*

https://codereview.chromium.org/2433543002/diff/80001/core/fxge/android/cfpf_...
File core/fxge/android/cfpf_skiafontmgr.cpp (right):

https://codereview.chromium.org/2433543002/diff/80001/core/fxge/android/cfpf_...
core/fxge/android/cfpf_skiafontmgr.cpp:73: int length) {
size_t?

[npm, 2016-10-21 18:41:02]

https://codereview.chromium.org/2433543002/diff/80001/core/fxge/android/cfpf_...
File core/fxge/android/cfpf_skiafontmgr.cpp (right):

https://codereview.chromium.org/2433543002/diff/80001/core/fxge/android/cfpf_...
core/fxge/android/cfpf_skiafontmgr.cpp:73: int length) {
On 2016/10/21 18:26:21, Lei Zhang wrote:
> size_t?

Done.

[npm, 2016-10-21 18:41:07]

The CQ bit was checked by npm@chromium.org

[npm, 2016-10-21 18:41:07]

The patchset sent to the CQ was uploaded after l-g-t-m from thestig@chromium.org
Link to the patchset: https://codereview.chromium.org/2433543002/#ps100001
(title: "Use size_t")

[commit-bot: I haz the power, 2016-10-21 18:41:19]

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-10-21 18:56:56]

Description was changed from

==========
Clean up fpf_skiafontmgr

Remove unused methods, create namespace, nits.
==========

to

==========
Clean up fpf_skiafontmgr

Remove unused methods, create namespace, nits.

Committed:
https://pdfium.googlesource.com/pdfium/+/83828032e3fa26dd98185523dfdba5f1f4e9...
==========

[commit-bot: I haz the power, 2016-10-21 18:56:57]

Committed patchset #6 (id:100001) as
https://pdfium.googlesource.com/pdfium/+/83828032e3fa26dd98185523dfdba5f1f4e9...