Fold IFWL*::{Initialize|Finalize} into constructor/destructor

Fold IFWL*::{Initialize|Finalize} into constructor/destructor

This Cl moves the ::Initialize method into the constructors and the
::Finalize method into the destructors. The |m_pDelegate| is made private and
a unique_ptr. A |SetDelegate| and |GetDelegate| method are added.

Committed: https://pdfium.googlesource.com/pdfium/+/6fe8795d9022105a0061a0a81be9a49d49fda345

[dsinclair, 2016-10-31 19:48:17]

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

[dsinclair, 2016-10-31 19:48:18]

PTAL.

https://codereview.chromium.org/2464703006/diff/1/xfa/fwl/core/ifwl_datetimep...
File xfa/fwl/core/ifwl_datetimepicker.cpp (left):

https://codereview.chromium.org/2464703006/diff/1/xfa/fwl/core/ifwl_datetimep...
xfa/fwl/core/ifwl_datetimepicker.cpp:332: pSpin->Initialize();
This never did anything with pSpin so it would just leak. Removed the creation
which means all this block goes away.

[dsinclair, 2016-10-31 19:48:21]

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

[commit-bot: I haz the power, 2016-10-31 19:48:27]

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

[Tom Sepez, 2016-10-31 20:26:11]

Are we sure there aren't some subtleties of deletion order that will get
re-arranged?  Otherwise LGTM.

https://codereview.chromium.org/2464703006/diff/20001/xfa/fwl/core/ifwl_barco...
File xfa/fwl/core/ifwl_barcode.cpp (right):

https://codereview.chromium.org/2464703006/diff/20001/xfa/fwl/core/ifwl_barco...
xfa/fwl/core/ifwl_barcode.cpp:23: m_pBarcodeEngine.reset();
does deletion order matter?  Is there a reason we are explicitly resetting this?

https://codereview.chromium.org/2464703006/diff/20001/xfa/fwl/core/ifwl_edit.cpp
File xfa/fwl/core/ifwl_edit.cpp (right):

https://codereview.chromium.org/2464703006/diff/20001/xfa/fwl/core/ifwl_edit....
xfa/fwl/core/ifwl_edit.cpp:1461: (bVert ? &m_pVertScrollBar : &m_pHorzScrollBar)
nit: I've always hated this line of code, but never bothered to make it a
full-blown if.. else

https://codereview.chromium.org/2464703006/diff/20001/xfa/fwl/core/ifwl_toolt...
File xfa/fwl/core/ifwl_tooltip.cpp (right):

https://codereview.chromium.org/2464703006/diff/20001/xfa/fwl/core/ifwl_toolt...
xfa/fwl/core/ifwl_tooltip.cpp:33: m_pProperties->m_dwStyles |=
FWL_WGTSTYLE_Popup;
nit: can we flip these two lines around? Just because ... clear the old then set
the new ... or if the bitfields overlapped each other ... or

[dsinclair, 2016-11-01 15:39:18]

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

[commit-bot: I haz the power, 2016-11-01 15:39:25]

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 15:39:45]

https://codereview.chromium.org/2464703006/diff/20001/xfa/fwl/core/ifwl_barco...
File xfa/fwl/core/ifwl_barcode.cpp (right):

https://codereview.chromium.org/2464703006/diff/20001/xfa/fwl/core/ifwl_barco...
xfa/fwl/core/ifwl_barcode.cpp:23: m_pBarcodeEngine.reset();
On 2016/10/31 20:26:11, Tom Sepez wrote:
> does deletion order matter?  Is there a reason we are explicitly resetting
this?


Nope, as far as I know we can just get it get destroyed normally. Copied from
the Finalize without thinking.

https://codereview.chromium.org/2464703006/diff/20001/xfa/fwl/core/ifwl_edit.cpp
File xfa/fwl/core/ifwl_edit.cpp (right):

https://codereview.chromium.org/2464703006/diff/20001/xfa/fwl/core/ifwl_edit....
xfa/fwl/core/ifwl_edit.cpp:1461: (bVert ? &m_pVertScrollBar : &m_pHorzScrollBar)
On 2016/10/31 20:26:11, Tom Sepez wrote:
> nit: I've always hated this line of code, but never bothered to make it a
> full-blown if.. else

Done.

https://codereview.chromium.org/2464703006/diff/20001/xfa/fwl/core/ifwl_toolt...
File xfa/fwl/core/ifwl_tooltip.cpp (right):

https://codereview.chromium.org/2464703006/diff/20001/xfa/fwl/core/ifwl_toolt...
xfa/fwl/core/ifwl_tooltip.cpp:33: m_pProperties->m_dwStyles |=
FWL_WGTSTYLE_Popup;
On 2016/10/31 20:26:11, Tom Sepez wrote:
> nit: can we flip these two lines around? Just because ... clear the old then
set
> the new ... or if the bitfields overlapped each other ... or 

Done.

[dsinclair, 2016-11-01 15:40:40]

On 2016/10/31 20:26:11, Tom Sepez wrote:
> Are we sure there aren't some subtleties of deletion order that will get
> re-arranged?  Otherwise LGTM.


It passes under ASAN. If there are destruction order issues, it's impossible to
say for sure given how tangled everything is in the code base. Still LGTM?

[commit-bot: I haz the power, 2016-11-01 15:52:15]

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

[commit-bot: I haz the power, 2016-11-01 15:52:16]

Dry run: This issue passed the CQ dry run.

[Tom Sepez, 2016-11-01 21:34:09]

still lgtm

[dsinclair, 2016-11-02 01:47:55]

The CQ bit was checked by dsinclair@chromium.org

[commit-bot: I haz the power, 2016-11-02 01:47:57]

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

[commit-bot: I haz the power, 2016-11-02 01:48:22]

Description was changed from

==========
Fold IFWL*::{Initialize|Finalize} into constructor/destructor

This Cl moves the ::Initialize method into the constructors and the
::Finalize method into the destructors. The |m_pDelegate| is made private and
a unique_ptr. A |SetDelegate| and |GetDelegate| method are added.
==========

to

==========
Fold IFWL*::{Initialize|Finalize} into constructor/destructor

This Cl moves the ::Initialize method into the constructors and the
::Finalize method into the destructors. The |m_pDelegate| is made private and
a unique_ptr. A |SetDelegate| and |GetDelegate| method are added.

Committed:
https://pdfium.googlesource.com/pdfium/+/6fe8795d9022105a0061a0a81be9a49d49fd...
==========

[commit-bot: I haz the power, 2016-11-02 01:48:23]

Committed patchset #5 (id:80001) as
https://pdfium.googlesource.com/pdfium/+/6fe8795d9022105a0061a0a81be9a49d49fd...