Change IFWL_Widget to store a single delegate.

Change IFWL_Widget to store a single delegate.

Currently the IFWL_Widget will store a current and previous delegate. This
was because the CXFA_FF* widgets would add |this| as a delegate which can not
be cleaned up by the IFWL_Widget classes.

This CL moves the CXFA_FF* code to actual Delegate classes which receive the
previous delegate as a parameter. Then, the IFWL_Widget just holds a pointer
to that delegate.

[dsinclair, 2016-11-01 20:40:33]

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

[commit-bot: I haz the power, 2016-11-01 20:40:42]

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

[dsinclair, 2016-11-01 20:40:52]

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

[dsinclair, 2016-11-01 20:40:53]

PTAL.

[Tom Sepez, 2016-11-01 20:54:05]

https://codereview.chromium.org/2466273003/diff/20001/xfa/fwl/core/ifwl_widget.h
File xfa/fwl/core/ifwl_widget.h (right):

https://codereview.chromium.org/2466273003/diff/20001/xfa/fwl/core/ifwl_widge...
xfa/fwl/core/ifwl_widget.h:121: return
pdfium::WrapUnique<IFWL_WidgetDelegate>(m_pDelegate.release());
nit: maybe return std::move(m_pDelegate);

https://codereview.chromium.org/2466273003/diff/20001/xfa/fxfa/app/cxfa_ffche...
File xfa/fxfa/app/cxfa_ffcheckbuttondelegate.h (right):

https://codereview.chromium.org/2466273003/diff/20001/xfa/fxfa/app/cxfa_ffche...
xfa/fxfa/app/cxfa_ffcheckbuttondelegate.h:1: // Copyright 2016 PDFium Authors.
All rights reserved.
rather than adding all these tiny files, how about we first convert
IFWL_WidgetDelegate to IFWL_Widget::Delegate nested classes. Then
this can go in the same file as the class the delegate refers back to.

It feels kinda weird for a class to own the delegate handed to it, since it
must outlive it.  In that case, you'd save a lot of trouble by making it
never own the delegate instead of always owning it.

[dsinclair, 2016-11-01 20:54:29]

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

[commit-bot: I haz the power, 2016-11-01 20:54:32]

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

[dsinclair, 2016-11-01 20:58:10]

https://codereview.chromium.org/2466273003/diff/20001/xfa/fxfa/app/cxfa_ffche...
File xfa/fxfa/app/cxfa_ffcheckbuttondelegate.h (right):

https://codereview.chromium.org/2466273003/diff/20001/xfa/fxfa/app/cxfa_ffche...
xfa/fxfa/app/cxfa_ffcheckbuttondelegate.h:1: // Copyright 2016 PDFium Authors.
All rights reserved.
On 2016/11/01 20:54:04, Tom Sepez wrote:
> rather than adding all these tiny files, how about we first convert
> IFWL_WidgetDelegate to IFWL_Widget::Delegate nested classes. Then
> this can go in the same file as the class the delegate refers back to.
> 
> It feels kinda weird for a class to own the delegate handed to it, since it
> must outlive it.  In that case, you'd save a lot of trouble by making it
> never own the delegate instead of always owning it.

Changing IFWL_WidgetDelegate to IFWL_Widget::Delegate is a much larger change as
there are a whole bunch of delegates that live in the IFWL_* hierarchy. Seems
like it would be better as two steps. I can move these back into their parent
classes as public/private inner classes depending on inheritance if that's
desired. I'd prefer to do IFWL_Widget::Delegate as a separate change afterwards.

Making which class never own the delegate, the IFWL_Widget? In that case, you'd
make the creating widget own the delegate passed to IFWL_Widget?

[Tom Sepez, 2016-11-01 21:03:28]

> 
> Making which class never own the delegate, the IFWL_Widget? In that case,
you'd
> make the creating widget own the delegate passed to IFWL_Widget?

Right.  In which case it can be |this|.  Its pretty normal for a class a layer
above to also be a delegate, and I'd hate to have to force separate objects.

[dsinclair, 2016-11-01 21:07:40]

On 2016/11/01 21:03:28, Tom Sepez wrote:
> > 
> > Making which class never own the delegate, the IFWL_Widget? In that case,
> you'd
> > make the creating widget own the delegate passed to IFWL_Widget?
> 
> Right.  In which case it can be |this|.  Its pretty normal for a class a layer
> above to also be a delegate, and I'd hate to have to force separate objects.


K, I'll close this and go at it the other route of moving m_pDelegate ownership
up to the calling classes. Then this isn't needed as the CXFA_FF* widgets can
continue to pass |this| as the delegate.

[Tom Sepez, 2016-11-01 21:15:04]

> K, I'll close this and go at it the other route of moving m_pDelegate
ownership
> up to the calling classes. Then this isn't needed as the CXFA_FF* widgets can
> continue to pass |this| as the delegate.

Thanks.  Let me know how it goes; I'm guessing ok as |this| usually has all the
state to complete the delegated request anyways ...