Remove core/include/fpdfdoc/fpdf_vt.h

Remove core/include/fpdfdoc/fpdf_vt.h

This CL removes the core/include/fpdfdoc/fpdf_vt.h file. The IPDF_VariableText
class was removed in favour of the only concrete class of CPD_VariableText.

CPVT_Provider was moved to CPDF_VariableText::Provider and
CPDF_VariableText_Iterator was moved to CPDF_VariableText::Iterator.

The IFX_Edit_FontMap interface was merged into the IPVT_FontMap interface so
there is only one.

Other classes were split out as needed in order to keep things compiling.

BUG=pdfium:468
Committed: https://pdfium.googlesource.com/pdfium/+/c7a7349cf316af37d4ad4b71c5742159deccbf33

[dsinclair, 2016-04-05 16:21:22]

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

[dsinclair, 2016-04-05 16:21:24]

PTAL.

https://codereview.chromium.org/1860063002/diff/1/fpdfsdk/include/fxedit/fxet...
File fpdfsdk/include/fxedit/fxet_edit.h (right):

https://codereview.chromium.org/1860063002/diff/1/fpdfsdk/include/fxedit/fxet...
fpdfsdk/include/fxedit/fxet_edit.h:781: class CFX_Edit_Provider : public
CPDF_VariableText::Provider {
This is the one other subclass of IPDF_VariableText::Provider. I made it a
subclass of CPDF_VariableText::Provider.

https://codereview.chromium.org/1860063002/diff/1/fpdfsdk/include/fxedit/fxet...
fpdfsdk/include/fxedit/fxet_edit.h:783: explicit CFX_Edit_Provider(IPVT_FontMap*
pFontMap);
This constructor is what necessitated the merge if IFX_Edit_FontMap and
IPVT_FontMap.

[dsinclair, 2016-04-05 16:22:52]

Description was changed from

==========
Remove core/include/fpdfdoc/fpdf_vt.h

This CL removes the core/include/fpdfdoc/fpdf_vt.h file. The IPDF_VariableText
class was removed in favour of the only concrete class of CPD_VariableText.

CPVT_Provider was moved to CPDF_VariableText::Provider and
CPDF_VariableText_Iterator was moved to CPDF_VariableText::Iterator.

The IFX_Edit_FontMap interface was merged into the IPVT_FontMap interface so
there is only one.

Other classes were split out as needed in order to keep things compiling.
==========

to

==========
Remove core/include/fpdfdoc/fpdf_vt.h

This CL removes the core/include/fpdfdoc/fpdf_vt.h file. The IPDF_VariableText
class was removed in favour of the only concrete class of CPD_VariableText.

CPVT_Provider was moved to CPDF_VariableText::Provider and
CPDF_VariableText_Iterator was moved to CPDF_VariableText::Iterator.

The IFX_Edit_FontMap interface was merged into the IPVT_FontMap interface so
there is only one.

Other classes were split out as needed in order to keep things compiling.

BUG=pdfium:468
==========

[Tom Sepez, 2016-04-05 16:58:23]

Looks OK, though I didn't confirm the blocks that were copied verbatim.

https://codereview.chromium.org/1860063002/diff/1/core/fpdfdoc/cpdf_variablet...
File core/fpdfdoc/cpdf_variabletext.cpp (right):

https://codereview.chromium.org/1860063002/diff/1/core/fpdfdoc/cpdf_variablet...
core/fpdfdoc/cpdf_variabletext.cpp:19: 
nit: no need for a blank line here.

https://codereview.chromium.org/1860063002/diff/1/core/fpdfdoc/cpvt_sectionin...
File core/fpdfdoc/cpvt_sectioninfo.h (right):

https://codereview.chromium.org/1860063002/diff/1/core/fpdfdoc/cpvt_sectionin...
core/fpdfdoc/cpvt_sectioninfo.h:16: : rcSection(), nTotalLine(0),
pSecProps(NULL), pWordProps(NULL) {}
nit: nullptr.

https://codereview.chromium.org/1860063002/diff/1/core/fpdfdoc/cpvt_sectionin...
core/fpdfdoc/cpvt_sectioninfo.h:19: delete pSecProps;
can these be unique_ptr?

https://codereview.chromium.org/1860063002/diff/1/core/fpdfdoc/cpvt_wordinfo.h
File core/fpdfdoc/cpvt_wordinfo.h (right):

https://codereview.chromium.org/1860063002/diff/1/core/fpdfdoc/cpvt_wordinfo....
core/fpdfdoc/cpvt_wordinfo.h:46: ~CPVT_WordInfo() { delete pWordProps; }
Can this be unique_ptr?

https://codereview.chromium.org/1860063002/diff/1/core/fpdfdoc/ctypeset.h
File core/fpdfdoc/ctypeset.h (right):

https://codereview.chromium.org/1860063002/diff/1/core/fpdfdoc/ctypeset.h#new...
core/fpdfdoc/ctypeset.h:30: CSection* m_pSection;
can this be CSection* const m_pSection?

https://codereview.chromium.org/1860063002/diff/1/core/fpdfdoc/include/cpdf_v...
File core/fpdfdoc/include/cpdf_variabletext.h (right):

https://codereview.chromium.org/1860063002/diff/1/core/fpdfdoc/include/cpdf_v...
core/fpdfdoc/include/cpdf_variabletext.h:58: CPDF_VariableText* m_pVT;
can this be CPDF_VariableText* m_pVT?

https://codereview.chromium.org/1860063002/diff/1/fpdfsdk/fxedit/fxet_edit.cpp
File fpdfsdk/fxedit/fxet_edit.cpp (right):

https://codereview.chromium.org/1860063002/diff/1/fpdfsdk/fxedit/fxet_edit.cp...
fpdfsdk/fxedit/fxet_edit.cpp:1126: FX_BOOL
CFX_Edit::SetRichTextScript(CPDF_VariableText::ScriptType nScriptType) {
nit: the Hungarian notation for enums would be eScriptType though its used
inconsistently.

https://codereview.chromium.org/1860063002/diff/1/fpdfsdk/fxedit/fxet_module.cpp
File fpdfsdk/fxedit/fxet_module.cpp (right):

https://codereview.chromium.org/1860063002/diff/1/fpdfsdk/fxedit/fxet_module....
fpdfsdk/fxedit/fxet_module.cpp:17: delete static_cast<CFX_Edit*>(pEdit);
Do we need the cast, or does whatever pEdit points at have a virtual dtor?  If
not, it should ...

https://codereview.chromium.org/1860063002/diff/1/fpdfsdk/include/fxedit/fx_e...
File fpdfsdk/include/fxedit/fx_edit.h (right):

https://codereview.chromium.org/1860063002/diff/1/fpdfsdk/include/fxedit/fx_e...
fpdfsdk/include/fxedit/fx_edit.h:169: // set a IPVT_FontMap pointer implemented
by user.
nit: blank line before each new // comment here, as long as we're mucking with
it.

https://codereview.chromium.org/1860063002/diff/1/fpdfsdk/include/fxedit/fxet...
File fpdfsdk/include/fxedit/fxet_edit.h (right):

https://codereview.chromium.org/1860063002/diff/1/fpdfsdk/include/fxedit/fxet...
fpdfsdk/include/fxedit/fxet_edit.h:548: FX_BOOL
SetRichTextScript(CPDF_VariableText::ScriptType nScriptType) override;
nit: eScriptType

[dsinclair, 2016-04-05 17:29:21]

https://codereview.chromium.org/1860063002/diff/1/core/fpdfdoc/cpdf_variablet...
File core/fpdfdoc/cpdf_variabletext.cpp (right):

https://codereview.chromium.org/1860063002/diff/1/core/fpdfdoc/cpdf_variablet...
core/fpdfdoc/cpdf_variabletext.cpp:19: 
On 2016/04/05 16:58:23, Tom Sepez wrote:
> nit: no need for a blank line here.

Done.

https://codereview.chromium.org/1860063002/diff/1/core/fpdfdoc/cpvt_sectionin...
File core/fpdfdoc/cpvt_sectioninfo.h (right):

https://codereview.chromium.org/1860063002/diff/1/core/fpdfdoc/cpvt_sectionin...
core/fpdfdoc/cpvt_sectioninfo.h:16: : rcSection(), nTotalLine(0),
pSecProps(NULL), pWordProps(NULL) {}
On 2016/04/05 16:58:23, Tom Sepez wrote:
> nit: nullptr.

Done.

https://codereview.chromium.org/1860063002/diff/1/core/fpdfdoc/cpvt_sectionin...
core/fpdfdoc/cpvt_sectioninfo.h:19: delete pSecProps;
On 2016/04/05 16:58:23, Tom Sepez wrote:
> can these be unique_ptr?

Will attempt as a followup.

https://codereview.chromium.org/1860063002/diff/1/core/fpdfdoc/cpvt_wordinfo.h
File core/fpdfdoc/cpvt_wordinfo.h (right):

https://codereview.chromium.org/1860063002/diff/1/core/fpdfdoc/cpvt_wordinfo....
core/fpdfdoc/cpvt_wordinfo.h:46: ~CPVT_WordInfo() { delete pWordProps; }
On 2016/04/05 16:58:23, Tom Sepez wrote:
> Can this be unique_ptr?

On followup list.

https://codereview.chromium.org/1860063002/diff/1/core/fpdfdoc/ctypeset.h
File core/fpdfdoc/ctypeset.h (right):

https://codereview.chromium.org/1860063002/diff/1/core/fpdfdoc/ctypeset.h#new...
core/fpdfdoc/ctypeset.h:30: CSection* m_pSection;
On 2016/04/05 16:58:23, Tom Sepez wrote:
> can this be CSection* const m_pSection?

Done.

https://codereview.chromium.org/1860063002/diff/1/core/fpdfdoc/include/cpdf_v...
File core/fpdfdoc/include/cpdf_variabletext.h (right):

https://codereview.chromium.org/1860063002/diff/1/core/fpdfdoc/include/cpdf_v...
core/fpdfdoc/include/cpdf_variabletext.h:58: CPDF_VariableText* m_pVT;
On 2016/04/05 16:58:23, Tom Sepez wrote:
> can this be CPDF_VariableText* m_pVT?

I assume you meant CPDF_VariableText* const m_pVT.

https://codereview.chromium.org/1860063002/diff/1/fpdfsdk/fxedit/fxet_edit.cpp
File fpdfsdk/fxedit/fxet_edit.cpp (right):

https://codereview.chromium.org/1860063002/diff/1/fpdfsdk/fxedit/fxet_edit.cp...
fpdfsdk/fxedit/fxet_edit.cpp:1126: FX_BOOL
CFX_Edit::SetRichTextScript(CPDF_VariableText::ScriptType nScriptType) {
On 2016/04/05 16:58:23, Tom Sepez wrote:
> nit: the Hungarian notation for enums would be eScriptType though its used
> inconsistently.

I'm going to leave this for now as I saw a lot of nScriptType names.

https://codereview.chromium.org/1860063002/diff/1/fpdfsdk/fxedit/fxet_module.cpp
File fpdfsdk/fxedit/fxet_module.cpp (right):

https://codereview.chromium.org/1860063002/diff/1/fpdfsdk/fxedit/fxet_module....
fpdfsdk/fxedit/fxet_module.cpp:17: delete static_cast<CFX_Edit*>(pEdit);
On 2016/04/05 16:58:23, Tom Sepez wrote:
> Do we need the cast, or does whatever pEdit points at have a virtual dtor?  If
> not, it should ...

Yea, virtual destructor. We can remove IFX_Edit and this gets fixed as CFX_Edit
is the only implementation.

https://codereview.chromium.org/1860063002/diff/1/fpdfsdk/include/fxedit/fx_e...
File fpdfsdk/include/fxedit/fx_edit.h (right):

https://codereview.chromium.org/1860063002/diff/1/fpdfsdk/include/fxedit/fx_e...
fpdfsdk/include/fxedit/fx_edit.h:169: // set a IPVT_FontMap pointer implemented
by user.
On 2016/04/05 16:58:23, Tom Sepez wrote:
> nit: blank line before each new // comment here, as long as we're mucking with
> it.

Done.

https://codereview.chromium.org/1860063002/diff/1/fpdfsdk/include/fxedit/fxet...
File fpdfsdk/include/fxedit/fxet_edit.h (right):

https://codereview.chromium.org/1860063002/diff/1/fpdfsdk/include/fxedit/fxet...
fpdfsdk/include/fxedit/fxet_edit.h:548: FX_BOOL
SetRichTextScript(CPDF_VariableText::ScriptType nScriptType) override;
On 2016/04/05 16:58:23, Tom Sepez wrote:
> nit: eScriptType

Acknowledged.

[Tom Sepez, 2016-04-05 19:01:09]

The CQ bit was checked by tsepez@chromium.org

[Tom Sepez, 2016-04-05 19:01:10]

lgtm

[commit-bot: I haz the power, 2016-04-05 19:01:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1860063002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1860063002/20001

[commit-bot: I haz the power, 2016-04-05 19:01:45]

Description was changed from

==========
Remove core/include/fpdfdoc/fpdf_vt.h

This CL removes the core/include/fpdfdoc/fpdf_vt.h file. The IPDF_VariableText
class was removed in favour of the only concrete class of CPD_VariableText.

CPVT_Provider was moved to CPDF_VariableText::Provider and
CPDF_VariableText_Iterator was moved to CPDF_VariableText::Iterator.

The IFX_Edit_FontMap interface was merged into the IPVT_FontMap interface so
there is only one.

Other classes were split out as needed in order to keep things compiling.

BUG=pdfium:468
==========

to

==========
Remove core/include/fpdfdoc/fpdf_vt.h

This CL removes the core/include/fpdfdoc/fpdf_vt.h file. The IPDF_VariableText
class was removed in favour of the only concrete class of CPD_VariableText.

CPVT_Provider was moved to CPDF_VariableText::Provider and
CPDF_VariableText_Iterator was moved to CPDF_VariableText::Iterator.

The IFX_Edit_FontMap interface was merged into the IPVT_FontMap interface so
there is only one.

Other classes were split out as needed in order to keep things compiling.

BUG=pdfium:468

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

[commit-bot: I haz the power, 2016-04-05 19:01:46]

Committed patchset #2 (id:20001) as
https://pdfium.googlesource.com/pdfium/+/c7a7349cf316af37d4ad4b71c5742159decc...