Merge to XFA: Make CFX_FontMgr member variables private. (try 2)

Merge to XFA: Make CFX_FontMgr member variables private. (try 2)

R=tsepez@chromium.org

Review URL: https://codereview.chromium.org/1412733011 .

(cherry picked from commit 5a5b6fb3759daf1201ffc9702f626f4b243922d0)

Committed: https://pdfium.googlesource.com/pdfium/+/122ee21e38a15554c8c61edd21fda8c110be6072

[Lei Zhang, 2015-10-21 00:51:24]

Please review. There's non-trivial changes.

https://codereview.chromium.org/1414463006/diff/20001/xfa/src/fgas/src/font/f...
File xfa/src/fgas/src/font/fx_stdfontmgr.cpp (left):

https://codereview.chromium.org/1414463006/diff/20001/xfa/src/fgas/src/font/f...
xfa/src/fgas/src/font/fx_stdfontmgr.cpp:614: FXFT_Library& library =
CFX_GEModule::Get()->GetFontMgr()->m_FTLibrary;
|library| is not used here - initialization not needed?

https://codereview.chromium.org/1414463006/diff/20001/xfa/src/fgas/src/font/f...
xfa/src/fgas/src/font/fx_stdfontmgr.cpp:1039: BadRet:
Die gotos die!

https://codereview.chromium.org/1414463006/diff/20001/xfa/src/fgas/src/font/f...
File xfa/src/fgas/src/font/fx_stdfontmgr.cpp (right):

https://codereview.chromium.org/1414463006/diff/20001/xfa/src/fgas/src/font/f...
xfa/src/fgas/src/font/fx_stdfontmgr.cpp:1027: FX_Free(ftStream);
Not sure if |ftStream| needs to be freed if FXFT_Open_Face() returns 0, AKA
succeeds. We may be leaking memory...

[Tom Sepez, 2015-10-21 20:03:55]

https://codereview.chromium.org/1414463006/diff/20001/core/src/fxge/ge/fx_ge_...
File core/src/fxge/ge/fx_ge_font.cpp (left):

https://codereview.chromium.org/1414463006/diff/20001/core/src/fxge/ge/fx_ge_...
core/src/fxge/ge/fx_ge_font.cpp:189: if
(CFX_GEModule::Get()->GetFontMgr()->m_FTLibrary == NULL)
Do we still need this?

https://codereview.chromium.org/1414463006/diff/20001/xfa/src/fgas/src/font/f...
File xfa/src/fgas/src/font/fx_stdfontmgr.cpp (left):

https://codereview.chromium.org/1414463006/diff/20001/xfa/src/fgas/src/font/f...
xfa/src/fgas/src/font/fx_stdfontmgr.cpp:614: FXFT_Library& library =
CFX_GEModule::Get()->GetFontMgr()->m_FTLibrary;
On 2015/10/21 00:51:24, Lei Zhang wrote:
> |library| is not used here - initialization not needed?
May still need it in case any of our callees expect to see m_FTLibrary.

https://codereview.chromium.org/1414463006/diff/20001/xfa/src/fgas/src/font/f...
File xfa/src/fgas/src/font/fx_stdfontmgr.cpp (right):

https://codereview.chromium.org/1414463006/diff/20001/xfa/src/fgas/src/font/f...
xfa/src/fgas/src/font/fx_stdfontmgr.cpp:1027: FX_Free(ftStream);
On 2015/10/21 00:51:24, Lei Zhang wrote:
> Not sure if |ftStream| needs to be freed if FXFT_Open_Face() returns 0, AKA
> succeeds. We may be leaking memory...

Acknowledged.

[Lei Zhang, 2015-10-21 23:29:02]

https://codereview.chromium.org/1414463006/diff/20001/core/src/fxge/ge/fx_ge_...
File core/src/fxge/ge/fx_ge_font.cpp (left):

https://codereview.chromium.org/1414463006/diff/20001/core/src/fxge/ge/fx_ge_...
core/src/fxge/ge/fx_ge_font.cpp:189: if
(CFX_GEModule::Get()->GetFontMgr()->m_FTLibrary == NULL)
On 2015/10/21 20:03:55, Tom Sepez wrote:
> Do we still need this?

Yes, we pass |library| to _LoadFile(). I tried erasing CFX_Font::LoadFile(), but
no luck.

https://codereview.chromium.org/1414463006/diff/20001/xfa/src/fgas/src/font/f...
File xfa/src/fgas/src/font/fx_stdfontmgr.cpp (left):

https://codereview.chromium.org/1414463006/diff/20001/xfa/src/fgas/src/font/f...
xfa/src/fgas/src/font/fx_stdfontmgr.cpp:614: FXFT_Library& library =
CFX_GEModule::Get()->GetFontMgr()->m_FTLibrary;
On 2015/10/21 20:03:55, Tom Sepez wrote:
> On 2015/10/21 00:51:24, Lei Zhang wrote:
> > |library| is not used here - initialization not needed?
> May still need it in case any of our callees expect to see m_FTLibrary.

I'll just call it anyway.

[Tom Sepez, 2015-10-21 23:36:52]

lgtm

[Lei Zhang, 2015-10-22 00:07:32]

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