Replace IFGAS_Font with underlying concrete type

Replace IFGAS_Font with underlying concrete type

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

[Lei Zhang, 2016-06-02 06:45:16]

Patchset #1 (id:1) has been deleted

[Lei Zhang, 2016-06-02 07:32:36]

Patchset #1 (id:20001) has been deleted

[Lei Zhang, 2016-06-02 08:39:58]

Patchset #3 (id:80001) has been deleted

[Lei Zhang, 2016-06-02 08:45:51]

Patchset #3 (id:100001) has been deleted

[Lei Zhang, 2016-06-02 08:51:15]

Patchset #3 (id:110001) has been deleted

[Lei Zhang, 2016-06-02 16:19:12]

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

[Lei Zhang, 2016-06-02 16:19:13]

Since you asked about CFX_GEFont yesterday...

[dsinclair, 2016-06-02 17:37:39]

lgtm w/ nits.

https://codereview.chromium.org/2037563002/diff/130001/xfa/fgas/font/fgas_font.h
File xfa/fgas/font/fgas_font.h (right):

https://codereview.chromium.org/2037563002/diff/130001/xfa/fgas/font/fgas_fon...
xfa/fgas/font/fgas_font.h:128: virtual CFX_GEFont* GetDefFontByCodePage(
This version of the class looks very similar to the above, can we move the
#ifdefs around a bit and combine these two definitions?

https://codereview.chromium.org/2037563002/diff/130001/xfa/fgas/font/fgas_gef...
File xfa/fgas/font/fgas_gefont.cpp (right):

https://codereview.chromium.org/2037563002/diff/130001/xfa/fgas/font/fgas_gef...
xfa/fgas/font/fgas_gefont.cpp:19: if (NULL != pFontMgr) {
nit: if (pFontMgr)

https://codereview.chromium.org/2037563002/diff/130001/xfa/fgas/font/fgas_gef...
xfa/fgas/font/fgas_gefont.cpp:22: return NULL;
nit: nullptr's in various places

https://codereview.chromium.org/2037563002/diff/130001/xfa/fgas/font/fgas_std...
File xfa/fgas/font/fgas_stdfontmgr.cpp (right):

https://codereview.chromium.org/2037563002/diff/130001/xfa/fgas/font/fgas_std...
xfa/fgas/font/fgas_stdfontmgr.cpp:52: CFX_GEFont* pFont = NULL;
nit: nullptr and others.

https://codereview.chromium.org/2037563002/diff/130001/xfa/fgas/font/fgas_std...
xfa/fgas/font/fgas_stdfontmgr.cpp:57: if ((pFD = FindFont(pszFontFamily,
dwFontStyles, TRUE, wCodePage)) == NULL) {
nit: if (!( ... and drop the == NULL

[Lei Zhang, 2016-06-07 01:17:15]

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

[Lei Zhang, 2016-06-07 01:17:16]

+tsepez as well since our CLs collided.

https://codereview.chromium.org/2037563002/diff/130001/xfa/fgas/font/fgas_font.h
File xfa/fgas/font/fgas_font.h (right):

https://codereview.chromium.org/2037563002/diff/130001/xfa/fgas/font/fgas_fon...
xfa/fgas/font/fgas_font.h:128: virtual CFX_GEFont* GetDefFontByCodePage(
On 2016/06/02 17:37:38, dsinclair wrote:
> This version of the class looks very similar to the above, can we move the
> #ifdefs around a bit and combine these two definitions?

I have a follow up CL.

https://codereview.chromium.org/2037563002/diff/130001/xfa/fgas/font/fgas_gef...
File xfa/fgas/font/fgas_gefont.cpp (right):

https://codereview.chromium.org/2037563002/diff/130001/xfa/fgas/font/fgas_gef...
xfa/fgas/font/fgas_gefont.cpp:19: if (NULL != pFontMgr) {
On 2016/06/02 17:37:38, dsinclair wrote:
> nit: if (pFontMgr)

I have a separate set of CLs to kill off NULLs.

[Tom Sepez, 2016-06-07 15:56:56]

Can we change the CL description to something like "Replace IFGAS_Font with
underlying concrete type".

LGTM otherwise.

[Lei Zhang, 2016-06-07 17:59:04]

Description was changed from

==========
Remove IFX_Font.
==========

to

==========
Replace IFGAS_Font with underlying concrete type
==========

[Lei Zhang, 2016-06-07 17:59:12]

The CQ bit was checked by thestig@chromium.org

[Lei Zhang, 2016-06-07 17:59:12]

The patchset sent to the CQ was uploaded after l-g-t-m from
dsinclair@chromium.org
Link to the patchset: https://codereview.chromium.org/2037563002/#ps170001
(title: "nit")

[commit-bot: I haz the power, 2016-06-07 17:59:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2037563002/170001

[commit-bot: I haz the power, 2016-06-07 17:59:42]

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

[commit-bot: I haz the power, 2016-06-07 17:59:44]

Failed to apply patch for xfa/fde/cfde_txtedttextset.cpp:
While running git apply --index -3 -p1;
  error: patch failed: xfa/fde/cfde_txtedttextset.cpp:49
  Falling back to three-way merge...
  Applied patch to 'xfa/fde/cfde_txtedttextset.cpp' with conflicts.
  U xfa/fde/cfde_txtedttextset.cpp

Patch:       xfa/fde/cfde_txtedttextset.cpp
Index: xfa/fde/cfde_txtedttextset.cpp
diff --git a/xfa/fde/cfde_txtedttextset.cpp b/xfa/fde/cfde_txtedttextset.cpp
index
800f8c88b1c41ba75e8e44f56a48625405b7240e..f79036602254e7bfd2d63f7dad97b7e664dd0c35
100644
--- a/xfa/fde/cfde_txtedttextset.cpp
+++ b/xfa/fde/cfde_txtedttextset.cpp
@@ -49,7 +49,7 @@ int32_t CFDE_TxtEdtTextSet::GetString(FDE_HVISUALOBJ hText,
   return pPiece->nCount;
 }
 
-IFGAS_Font* CFDE_TxtEdtTextSet::GetFont(FDE_HVISUALOBJ hText) {
+CFGAS_GEFont* CFDE_TxtEdtTextSet::GetFont(FDE_HVISUALOBJ hText) {
   return m_pPage->GetEngine()->GetEditParams()->pFont;
 }

[commit-bot: I haz the power, 2016-06-07 17:59:45]

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

[Lei Zhang, 2016-06-08 00:19:24]

The CQ bit was checked by thestig@chromium.org

[Lei Zhang, 2016-06-08 00:19:25]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
dsinclair@chromium.org
Link to the patchset: https://codereview.chromium.org/2037563002/#ps190001
(title: "rebase")

[commit-bot: I haz the power, 2016-06-08 00:19:26]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2037563002/190001

[commit-bot: I haz the power, 2016-06-08 00:33:42]

Description was changed from

==========
Replace IFGAS_Font with underlying concrete type
==========

to

==========
Replace IFGAS_Font with underlying concrete type

Committed:
https://pdfium.googlesource.com/pdfium/+/a4fdfc5ed0e8d2e6acc52cc34eac42c6072f...
==========

[commit-bot: I haz the power, 2016-06-08 00:33:43]

Committed patchset #6 (id:190001) as
https://pdfium.googlesource.com/pdfium/+/a4fdfc5ed0e8d2e6acc52cc34eac42c6072f...