Refactor fx_font part 2

Move CFX_UnicodeEncoding and CFX_UnicodeEncodingEx to their own files.

Committed: https://pdfium.googlesource.com/pdfium/+/1a8946b09884393d7dc4941e59b3172a7e473b01

[npm, 2016-08-18 14:44:36]

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

[commit-bot: I haz the power, 2016-08-18 14:44:39]

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

[npm, 2016-08-18 14:46:28]

Description was changed from

==========
Fix ifndef


Refactor fx_font part 2

BUG=
==========

to

==========
Move CFX_UnicodeEncoding and CFX_UnicodeEncodingEx to their own files.
==========

[npm, 2016-08-18 14:46:28]

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

[npm, 2016-08-18 14:58:55]

PTAL

[commit-bot: I haz the power, 2016-08-18 14:59:28]

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

[commit-bot: I haz the power, 2016-08-18 14:59:30]

Dry run: This issue passed the CQ dry run.

[dsinclair, 2016-08-18 15:14:52]

https://codereview.chromium.org/2260533002/diff/1/core/fxge/ge/cfx_unicodeenc...
File core/fxge/ge/cfx_unicodeencodingex.cpp (right):

https://codereview.chromium.org/2260533002/diff/1/core/fxge/ge/cfx_unicodeenc...
core/fxge/ge/cfx_unicodeencodingex.cpp:13: #ifdef PDF_ENABLE_XFA
Instead of wrapping the whole file in an ENABLE_XFA, can you just move the file
in the BUILD.gn to a if (pdf_enable_xfa) section and then remove the ifdef.

https://codereview.chromium.org/2260533002/diff/1/core/fxge/ge/cfx_unicodeenc...
core/fxge/ge/cfx_unicodeencodingex.cpp:15: const uint32_t g_EncodingID[] = {
Move into an anonymous namespace.

https://codereview.chromium.org/2260533002/diff/1/core/fxge/ge/cfx_unicodeenc...
core/fxge/ge/cfx_unicodeencodingex.cpp:25: CFX_UnicodeEncodingEx*
_FXFM_CreateFontEncoding(CFX_Font* pFont,
Remove _ prefix as it's reserved by the compiler.  (and put in an anonymous
namespace if only used in this file)

https://codereview.chromium.org/2260533002/diff/1/core/fxge/ge/cfx_unicodeenc...
core/fxge/ge/cfx_unicodeencodingex.cpp:43: }
nit: {}'s

https://codereview.chromium.org/2260533002/diff/1/core/fxge/include/cfx_unico...
File core/fxge/include/cfx_unicodeencoding.h (right):

https://codereview.chromium.org/2260533002/diff/1/core/fxge/include/cfx_unico...
core/fxge/include/cfx_unicodeencoding.h:43: // Unowned, not nullptr.
Put this after the param:

CFX_Font* m_pFont;  // unowned, not nullptr.

https://codereview.chromium.org/2260533002/diff/1/core/fxge/include/cfx_unico...
File core/fxge/include/cfx_unicodeencodingex.h (right):

https://codereview.chromium.org/2260533002/diff/1/core/fxge/include/cfx_unico...
core/fxge/include/cfx_unicodeencodingex.h:19: #ifdef PDF_ENABLE_XFA
Do in build, remove ifdef.

https://codereview.chromium.org/2260533002/diff/1/xfa/fgas/font/fgas_gefont.h
File xfa/fgas/font/fgas_gefont.h (right):

https://codereview.chromium.org/2260533002/diff/1/xfa/fgas/font/fgas_gefont.h...
xfa/fgas/font/fgas_gefont.h:19: class CFX_UnicodeEncoding;
nit: sorted order.

[npm, 2016-08-18 17:36:42]

PTAL

https://codereview.chromium.org/2260533002/diff/1/core/fxge/ge/cfx_unicodeenc...
File core/fxge/ge/cfx_unicodeencodingex.cpp (right):

https://codereview.chromium.org/2260533002/diff/1/core/fxge/ge/cfx_unicodeenc...
core/fxge/ge/cfx_unicodeencodingex.cpp:13: #ifdef PDF_ENABLE_XFA
On 2016/08/18 15:14:52, dsinclair wrote:
> Instead of wrapping the whole file in an ENABLE_XFA, can you just move the
file
> in the BUILD.gn to a if (pdf_enable_xfa) section and then remove the ifdef.

Done.

https://codereview.chromium.org/2260533002/diff/1/core/fxge/ge/cfx_unicodeenc...
core/fxge/ge/cfx_unicodeencodingex.cpp:15: const uint32_t g_EncodingID[] = {
On 2016/08/18 15:14:52, dsinclair wrote:
> Move into an anonymous namespace.

Done.

https://codereview.chromium.org/2260533002/diff/1/core/fxge/ge/cfx_unicodeenc...
core/fxge/ge/cfx_unicodeencodingex.cpp:25: CFX_UnicodeEncodingEx*
_FXFM_CreateFontEncoding(CFX_Font* pFont,
On 2016/08/18 15:14:52, dsinclair wrote:
> Remove _ prefix as it's reserved by the compiler.  (and put in an anonymous
> namespace if only used in this file)

Done.

https://codereview.chromium.org/2260533002/diff/1/core/fxge/ge/cfx_unicodeenc...
core/fxge/ge/cfx_unicodeencodingex.cpp:43: }
On 2016/08/18 15:14:52, dsinclair wrote:
> nit: {}'s

Done.

https://codereview.chromium.org/2260533002/diff/1/core/fxge/include/cfx_unico...
File core/fxge/include/cfx_unicodeencoding.h (right):

https://codereview.chromium.org/2260533002/diff/1/core/fxge/include/cfx_unico...
core/fxge/include/cfx_unicodeencoding.h:43: // Unowned, not nullptr.
On 2016/08/18 15:14:52, dsinclair wrote:
> Put this after the param:
> 
> CFX_Font* m_pFont;  // unowned, not nullptr.

Done.

https://codereview.chromium.org/2260533002/diff/1/core/fxge/include/cfx_unico...
File core/fxge/include/cfx_unicodeencodingex.h (right):

https://codereview.chromium.org/2260533002/diff/1/core/fxge/include/cfx_unico...
core/fxge/include/cfx_unicodeencodingex.h:19: #ifdef PDF_ENABLE_XFA
On 2016/08/18 15:14:52, dsinclair wrote:
> Do in build, remove ifdef.

Done.

https://codereview.chromium.org/2260533002/diff/1/xfa/fgas/font/fgas_gefont.h
File xfa/fgas/font/fgas_gefont.h (right):

https://codereview.chromium.org/2260533002/diff/1/xfa/fgas/font/fgas_gefont.h...
xfa/fgas/font/fgas_gefont.h:19: class CFX_UnicodeEncoding;
On 2016/08/18 15:14:52, dsinclair wrote:
> nit: sorted order.

Done.

[npm, 2016-08-18 17:36:48]

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

[commit-bot: I haz the power, 2016-08-18 17:36:53]

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

[dsinclair, 2016-08-18 17:43:03]

https://codereview.chromium.org/2260533002/diff/40001/xfa/fgas/font/fgas_gefo...
File xfa/fgas/font/fgas_gefont.cpp (right):

https://codereview.chromium.org/2260533002/diff/40001/xfa/fgas/font/fgas_gefo...
xfa/fgas/font/fgas_gefont.cpp:10: #include
"core/fxge/include/cfx_unicodeencodingex.h"
This needs to be inside an #ifdef PDF_ENABLE_XFA block now.

https://codereview.chromium.org/2260533002/diff/40001/xfa/fxbarcode/oned/BC_O...
File xfa/fxbarcode/oned/BC_OneDimWriter.cpp (right):

https://codereview.chromium.org/2260533002/diff/40001/xfa/fxbarcode/oned/BC_O...
xfa/fxbarcode/oned/BC_OneDimWriter.cpp:33: #include
"core/fxge/include/cfx_unicodeencodingex.h"
ditto

[commit-bot: I haz the power, 2016-08-18 17:50:43]

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

[commit-bot: I haz the power, 2016-08-18 17:50:44]

Dry run: This issue passed the CQ dry run.

[npm, 2016-08-18 17:53:23]

https://codereview.chromium.org/2260533002/diff/40001/xfa/fgas/font/fgas_gefo...
File xfa/fgas/font/fgas_gefont.cpp (right):

https://codereview.chromium.org/2260533002/diff/40001/xfa/fgas/font/fgas_gefo...
xfa/fgas/font/fgas_gefont.cpp:10: #include
"core/fxge/include/cfx_unicodeencodingex.h"
On 2016/08/18 17:43:03, dsinclair wrote:
> This needs to be inside an #ifdef PDF_ENABLE_XFA block now.

Why? The whole xfa directory is under if(pdf_enable_xfa) already in Build.gn

[dsinclair, 2016-08-18 17:55:06]

The CQ bit was checked by dsinclair@chromium.org

[dsinclair, 2016-08-18 17:55:06]

lgtm

Doh, didn't realize they were in xfa/. Ignore me.

[commit-bot: I haz the power, 2016-08-18 17:55:17]

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-08-18 17:55:32]

Description was changed from

==========
Move CFX_UnicodeEncoding and CFX_UnicodeEncodingEx to their own files.
==========

to

==========
Move CFX_UnicodeEncoding and CFX_UnicodeEncodingEx to their own files.

Committed:
https://pdfium.googlesource.com/pdfium/+/1a8946b09884393d7dc4941e59b3172a7e47...
==========

[commit-bot: I haz the power, 2016-08-18 17:55:33]

Committed patchset #3 (id:40001) as
https://pdfium.googlesource.com/pdfium/+/1a8946b09884393d7dc4941e59b3172a7e47...