Cleanup FDE interfaces.

Cleanup FDE interfaces.

This CL removes IFDE_TextOut, IFDE_Path, IFDE_RenderContext, IFDE_RenderDevice,
and IFDE_VisualSetIterator in favour of the concrete classes.

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

[dsinclair, 2016-04-19 12:49:51]

Description was changed from

==========
Cleanup FDE interfaces.

This CL removes IFDE_TextOut, IFDE_Path, IFDE_RenderContext, IFDE_RenderDevice,
and IFDE_VisualSetIterator in favour of the concrete classes.
==========

to

==========
Cleanup FDE interfaces.

This CL removes IFDE_TextOut, IFDE_Path, IFDE_RenderContext, IFDE_RenderDevice,
and IFDE_VisualSetIterator in favour of the concrete classes.

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

[dsinclair, 2016-04-19 13:42:13]

Patchset #1 (id:1) has been deleted

[dsinclair, 2016-04-19 13:45:26]

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

[dsinclair, 2016-04-19 13:45:27]

PTAL.

[Tom Sepez, 2016-04-19 19:02:38]

lgtm

https://codereview.chromium.org/1896893003/diff/20001/xfa/fde/fde_iterator.cpp
File xfa/fde/fde_iterator.cpp (right):

https://codereview.chromium.org/1896893003/diff/20001/xfa/fde/fde_iterator.cp...
xfa/fde/fde_iterator.cpp:44: return pCanvas->hPos != nullptr;
nit: return !!pCanvas->hPos;

https://codereview.chromium.org/1896893003/diff/20001/xfa/fde/tto/fde_textout.h
File xfa/fde/tto/fde_textout.h (right):

https://codereview.chromium.org/1896893003/diff/20001/xfa/fde/tto/fde_textout...
xfa/fde/tto/fde_textout.h:46: public:
nit: public redundant in struct.

https://codereview.chromium.org/1896893003/diff/20001/xfa/fde/tto/fde_textout...
xfa/fde/tto/fde_textout.h:64: FX_BOOL m_bNewReload;
nit: blank line between members and methods

[dsinclair, 2016-04-19 19:20:26]

PTAL.

Nasty merge with FDE_TextOut changes.

https://codereview.chromium.org/1896893003/diff/20001/xfa/fde/fde_iterator.cpp
File xfa/fde/fde_iterator.cpp (right):

https://codereview.chromium.org/1896893003/diff/20001/xfa/fde/fde_iterator.cp...
xfa/fde/fde_iterator.cpp:44: return pCanvas->hPos != nullptr;
On 2016/04/19 19:02:38, Tom Sepez wrote:
> nit: return !!pCanvas->hPos;

Done.

https://codereview.chromium.org/1896893003/diff/20001/xfa/fde/tto/fde_textout.h
File xfa/fde/tto/fde_textout.h (right):

https://codereview.chromium.org/1896893003/diff/20001/xfa/fde/tto/fde_textout...
xfa/fde/tto/fde_textout.h:46: public:
On 2016/04/19 19:02:38, Tom Sepez wrote:
> nit: public redundant in struct.

Done.

https://codereview.chromium.org/1896893003/diff/20001/xfa/fde/tto/fde_textout...
xfa/fde/tto/fde_textout.h:64: FX_BOOL m_bNewReload;
On 2016/04/19 19:02:38, Tom Sepez wrote:
> nit: blank line between members and methods

Done.

[dsinclair, 2016-04-21 13:10:21]

ping

[Tom Sepez, 2016-04-21 17:05:25]

Sorry, being picky.

https://codereview.chromium.org/1896893003/diff/60001/xfa/fwl/theme/cfwl_widg...
File xfa/fwl/theme/cfwl_widgettp.h (right):

https://codereview.chromium.org/1896893003/diff/60001/xfa/fwl/theme/cfwl_widg...
xfa/fwl/theme/cfwl_widgettp.h:23: Mon,
weeks start on sundays.  we should adjust the code accordinly.

https://codereview.chromium.org/1896893003/diff/60001/xfa/fxfa/app/xfa_textla...
File xfa/fxfa/app/xfa_textlayout.cpp (right):

https://codereview.chromium.org/1896893003/diff/60001/xfa/fxfa/app/xfa_textla...
xfa/fxfa/app/xfa_textlayout.cpp:34: CFX_WideStringC& wsAttr,
I'd be a lot happier if these returned CFX_WideStrings. Currently, the lifetime
of the returned WideStringC is that of the "assoc" in the table.  If it gets
removed, the WideStringC becomes invalid.  Since we already have a widestring,
returning it is cheap (no allocs involved) and allow refcounting to keep it
intact as needed.

That's kinda why I added AsStringC() - to show up places where we need to think
...

[dsinclair, 2016-04-21 17:09:24]

https://codereview.chromium.org/1896893003/diff/60001/xfa/fwl/theme/cfwl_widg...
File xfa/fwl/theme/cfwl_widgettp.h (right):

https://codereview.chromium.org/1896893003/diff/60001/xfa/fwl/theme/cfwl_widg...
xfa/fwl/theme/cfwl_widgettp.h:23: Mon,
On 2016/04/21 17:05:25, Tom Sepez wrote:
> weeks start on sundays.  we should adjust the code accordinly.

Done.

https://codereview.chromium.org/1896893003/diff/60001/xfa/fxfa/app/xfa_textla...
File xfa/fxfa/app/xfa_textlayout.cpp (right):

https://codereview.chromium.org/1896893003/diff/60001/xfa/fxfa/app/xfa_textla...
xfa/fxfa/app/xfa_textlayout.cpp:34: CFX_WideStringC& wsAttr,
On 2016/04/21 17:05:25, Tom Sepez wrote:
> I'd be a lot happier if these returned CFX_WideStrings. Currently, the
lifetime
> of the returned WideStringC is that of the "assoc" in the table.  If it gets
> removed, the WideStringC becomes invalid.  Since we already have a widestring,
> returning it is cheap (no allocs involved) and allow refcounting to keep it
> intact as needed.
> 
> That's kinda why I added AsStringC() - to show up places where we need to
think
> ...


Will do as a follow up as un-related to this CL.

[Tom Sepez, 2016-04-21 17:12:53]

On 2016/04/21 17:09:24, dsinclair wrote:
>
https://codereview.chromium.org/1896893003/diff/60001/xfa/fwl/theme/cfwl_widg...
> File xfa/fwl/theme/cfwl_widgettp.h (right):
> 
>
https://codereview.chromium.org/1896893003/diff/60001/xfa/fwl/theme/cfwl_widg...
> xfa/fwl/theme/cfwl_widgettp.h:23: Mon,
> On 2016/04/21 17:05:25, Tom Sepez wrote:
> > weeks start on sundays.  we should adjust the code accordinly.
> 
> Done.
> 
>
https://codereview.chromium.org/1896893003/diff/60001/xfa/fxfa/app/xfa_textla...
> File xfa/fxfa/app/xfa_textlayout.cpp (right):
> 
>
https://codereview.chromium.org/1896893003/diff/60001/xfa/fxfa/app/xfa_textla...
> xfa/fxfa/app/xfa_textlayout.cpp:34: CFX_WideStringC& wsAttr,
> On 2016/04/21 17:05:25, Tom Sepez wrote:
> > I'd be a lot happier if these returned CFX_WideStrings. Currently, the
> lifetime
> > of the returned WideStringC is that of the "assoc" in the table.  If it gets
> > removed, the WideStringC becomes invalid.  Since we already have a
widestring,
> > returning it is cheap (no allocs involved) and allow refcounting to keep it
> > intact as needed.
> > 
> > That's kinda why I added AsStringC() - to show up places where we need to
> think
> > ...
> 
> 
> Will do as a follow up as un-related to this CL.

++LGTM

[dsinclair, 2016-04-21 17:13:24]

The CQ bit was checked by dsinclair@chromium.org

[commit-bot: I haz the power, 2016-04-21 17:13:32]

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

[commit-bot: I haz the power, 2016-04-21 18:06:30]

Description was changed from

==========
Cleanup FDE interfaces.

This CL removes IFDE_TextOut, IFDE_Path, IFDE_RenderContext, IFDE_RenderDevice,
and IFDE_VisualSetIterator in favour of the concrete classes.

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

to

==========
Cleanup FDE interfaces.

This CL removes IFDE_TextOut, IFDE_Path, IFDE_RenderContext, IFDE_RenderDevice,
and IFDE_VisualSetIterator in favour of the concrete classes.

BUG=pdfium:468

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

[commit-bot: I haz the power, 2016-04-21 18:06:30]

Committed patchset #4 (id:80001) as
https://pdfium.googlesource.com/pdfium/+/acd0d59e3c09f04bffd178aecd3638d3e189...