Rename IFWL_Widget::SetDelegate

Rename IFWL_Widget::SetDelegate

The ::SetDelegate method was misleading. The primary use was
SetDelegate(nullptr) which returned the current delegate and didn't actually
set anything. When a value was passed it would set the
|m_pCurDelegate| not the |m_pDelegate|.

This Cl breaks ::SetDelegate into ::GetCurrentDelegate and ::SetCurrentDelegate
to make it clear what is happening and that this does not effect the
|m_pDelegate| variable.

Committed: https://pdfium.googlesource.com/pdfium/+/9ec22175f945d227e71c459e7e0b7c464159c18b

[dsinclair, 2016-10-31 19:03:26]

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

[dsinclair, 2016-10-31 19:03:26]

PTAL.

[dsinclair, 2016-10-31 19:03:34]

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:03:35]

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 19:12:33]

Better.

https://codereview.chromium.org/2459423003/diff/1/xfa/fwl/core/cfwl_widget.cpp
File xfa/fwl/core/cfwl_widget.cpp (right):

https://codereview.chromium.org/2459423003/diff/1/xfa/fwl/core/cfwl_widget.cp...
xfa/fwl/core/cfwl_widget.cpp:138: if (!m_pIface)
nit: maybe invert the test and remove early return.

https://codereview.chromium.org/2459423003/diff/1/xfa/fwl/core/ifwl_listbox.cpp
File xfa/fwl/core/ifwl_listbox.cpp (right):

https://codereview.chromium.org/2459423003/diff/1/xfa/fwl/core/ifwl_listbox.c...
xfa/fwl/core/ifwl_listbox.cpp:1078: if (!m_pOwner->IsShowScrollBar(TRUE))
nit: ditto

https://codereview.chromium.org/2459423003/diff/1/xfa/fwl/core/ifwl_widget.cpp
File xfa/fwl/core/ifwl_widget.cpp (right):

https://codereview.chromium.org/2459423003/diff/1/xfa/fwl/core/ifwl_widget.cp...
xfa/fwl/core/ifwl_widget.cpp:355: IFWL_WidgetDelegate*
IFWL_Widget::GetCurrentDelegate() {
This is still a little weird in that I'd expect GetXXX methods to be const. 
Here we're promoting m_pDelegate to default as well.  How often does this
happen?  Can we figure out the spots where it does? Or could the call happen at
any time and place?

[commit-bot: I haz the power, 2016-10-31 19:22:26]

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

[commit-bot: I haz the power, 2016-10-31 19:22:28]

Dry run: This issue passed the CQ dry run.

[dsinclair, 2016-10-31 19:54:50]

https://codereview.chromium.org/2459423003/diff/1/xfa/fwl/core/cfwl_widget.cpp
File xfa/fwl/core/cfwl_widget.cpp (right):

https://codereview.chromium.org/2459423003/diff/1/xfa/fwl/core/cfwl_widget.cp...
xfa/fwl/core/cfwl_widget.cpp:138: if (!m_pIface)
On 2016/10/31 19:12:33, Tom Sepez wrote:
> nit: maybe invert the test and remove early return.

Done.

https://codereview.chromium.org/2459423003/diff/1/xfa/fwl/core/ifwl_listbox.cpp
File xfa/fwl/core/ifwl_listbox.cpp (right):

https://codereview.chromium.org/2459423003/diff/1/xfa/fwl/core/ifwl_listbox.c...
xfa/fwl/core/ifwl_listbox.cpp:1078: if (!m_pOwner->IsShowScrollBar(TRUE))
On 2016/10/31 19:12:33, Tom Sepez wrote:
> nit: ditto

Done.

https://codereview.chromium.org/2459423003/diff/1/xfa/fwl/core/ifwl_widget.cpp
File xfa/fwl/core/ifwl_widget.cpp (right):

https://codereview.chromium.org/2459423003/diff/1/xfa/fwl/core/ifwl_widget.cp...
xfa/fwl/core/ifwl_widget.cpp:355: IFWL_WidgetDelegate*
IFWL_Widget::GetCurrentDelegate() {
On 2016/10/31 19:12:33, Tom Sepez wrote:
> This is still a little weird in that I'd expect GetXXX methods to be const. 
> Here we're promoting m_pDelegate to default as well.  How often does this
> happen?  Can we figure out the spots where it does? Or could the call happen
at
> any time and place?


It happens in the CXFA_FF* methods. The issue is that they set |this| as the
delegate, so things get wonky. I'm hoping to fix this by giving them real
delegates, passing m_pDelegate into those delegates to be managed and then
getting rid of m_pCurDelegate in favour of m_pDelegate directly.

(The next CL in this chain makes m_pDelegate a unique_ptr which is the first
step in making this happen.)

[Tom Sepez, 2016-10-31 21:55:33]

lgtm

[dsinclair, 2016-11-01 15:21:00]

The CQ bit was checked by dsinclair@chromium.org

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

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-01 15:33:07]

Description was changed from

==========
Rename IFWL_Widget::SetDelegate

The ::SetDelegate method was misleading. The primary use was
SetDelegate(nullptr) which returned the current delegate and didn't actually
set anything. When a value was passed it would set the
|m_pCurDelegate| not the |m_pDelegate|.

This Cl breaks ::SetDelegate into ::GetCurrentDelegate and ::SetCurrentDelegate
to make it clear what is happening and that this does not effect the
|m_pDelegate| variable.
==========

to

==========
Rename IFWL_Widget::SetDelegate

The ::SetDelegate method was misleading. The primary use was
SetDelegate(nullptr) which returned the current delegate and didn't actually
set anything. When a value was passed it would set the
|m_pCurDelegate| not the |m_pDelegate|.

This Cl breaks ::SetDelegate into ::GetCurrentDelegate and ::SetCurrentDelegate
to make it clear what is happening and that this does not effect the
|m_pDelegate| variable.

Committed:
https://pdfium.googlesource.com/pdfium/+/9ec22175f945d227e71c459e7e0b7c464159...
==========

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

Committed patchset #3 (id:40001) as
https://pdfium.googlesource.com/pdfium/+/9ec22175f945d227e71c459e7e0b7c464159...