Rename common methods between ifwl and cfwl.

Rename common methods between ifwl and cfwl.

This CL renames some methods in IFWL which have the same name as the CFWL
methods but the CFWL methods are not proxy methods.

Committed: https://pdfium.googlesource.com/pdfium/+/e50184c087ee6f90568ad1166a55f5d25be4d09e

[dsinclair, 2016-11-22 16:15:37]

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

[dsinclair, 2016-11-22 16:15:38]

PTAL.

[dsinclair, 2016-11-22 16:15:42]

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

[commit-bot: I haz the power, 2016-11-22 16:15:49]

Dry run: 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-22 16:25:39]

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

[commit-bot: I haz the power, 2016-11-22 16:25:41]

Dry run: This issue passed the CQ dry run.

[npm, 2016-11-22 16:36:16]

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

https://codereview.chromium.org/2520413002/diff/1/xfa/fwl/core/cfwl_listbox.c...
xfa/fwl/core/cfwl_listbox.cpp:85: void CFWL_ListBox::GetItemText(IFWL_Widget*
pWidget,
Widget not being used in this method

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

https://codereview.chromium.org/2520413002/diff/1/xfa/fwl/core/ifwl_combobox....
xfa/fwl/core/ifwl_combobox.cpp:375: if (fHeight > 0 && m_rtList.height >
GetDataProviderListHeight()) {
Do we need to call this method three times?

https://codereview.chromium.org/2520413002/diff/1/xfa/fxfa/app/xfa_ffchoiceli...
File xfa/fxfa/app/xfa_ffchoicelist.cpp (right):

https://codereview.chromium.org/2520413002/diff/1/xfa/fxfa/app/xfa_ffchoiceli...
xfa/fxfa/app/xfa_ffchoicelist.cpp:163: pListBox->GetItemText(nullptr,
pListBox->GetSelItem(0), eParam.m_wsNewText);
It's unclear to me why this is the same.

[dsinclair, 2016-11-22 16:42:10]

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

[dsinclair, 2016-11-22 16:42:15]

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

https://codereview.chromium.org/2520413002/diff/1/xfa/fwl/core/cfwl_listbox.c...
xfa/fwl/core/cfwl_listbox.cpp:85: void CFWL_ListBox::GetItemText(IFWL_Widget*
pWidget,
On 2016/11/22 16:36:16, npm wrote:
> Widget not being used in this method

It isn't being used in a bunch of methods. I'm going to cleanup the delegate
params in a followup once I get the classes merged.

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

https://codereview.chromium.org/2520413002/diff/1/xfa/fwl/core/ifwl_combobox....
xfa/fwl/core/ifwl_combobox.cpp:375: if (fHeight > 0 && m_rtList.height >
GetDataProviderListHeight()) {
On 2016/11/22 16:36:16, npm wrote:
> Do we need to call this method three times?

Done.

https://codereview.chromium.org/2520413002/diff/1/xfa/fxfa/app/xfa_ffchoiceli...
File xfa/fxfa/app/xfa_ffchoicelist.cpp (right):

https://codereview.chromium.org/2520413002/diff/1/xfa/fxfa/app/xfa_ffchoiceli...
xfa/fxfa/app/xfa_ffchoicelist.cpp:163: pListBox->GetItemText(nullptr,
pListBox->GetSelItem(0), eParam.m_wsNewText);
On 2016/11/22 16:36:16, npm wrote:
> It's unclear to me why this is the same.

pListBox had two GetItemText() methods. First first took the IFWL_Widget as the
first param, the second didn't. I removed the version that didn't and changed
these to pass a null widget.

[commit-bot: I haz the power, 2016-11-22 16:42:19]

Dry run: 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-22 16:51:29]

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

[commit-bot: I haz the power, 2016-11-22 16:51:29]

Dry run: This issue passed the CQ dry run.

[npm, 2016-11-22 16:52:13]

https://codereview.chromium.org/2520413002/diff/1/xfa/fxfa/app/xfa_ffchoiceli...
File xfa/fxfa/app/xfa_ffchoicelist.cpp (right):

https://codereview.chromium.org/2520413002/diff/1/xfa/fxfa/app/xfa_ffchoiceli...
xfa/fxfa/app/xfa_ffchoicelist.cpp:163: pListBox->GetItemText(nullptr,
pListBox->GetSelItem(0), eParam.m_wsNewText);
On 2016/11/22 16:42:15, dsinclair wrote:
> On 2016/11/22 16:36:16, npm wrote:
> > It's unclear to me why this is the same.
> 
> pListBox had two GetItemText() methods. First first took the IFWL_Widget as
the
> first param, the second didn't. I removed the version that didn't and changed
> these to pass a null widget.

Original:
if (GetWidget()) ToListBox(GetWidget())->GetItemText(pItem, wsText);
The GetITemText is from IFWL_ListBox, because that's what ToListBox returns.

Now:
if (pItem) wsText = static_cast<CFWL_ListItem*>(pItem)->m_wsText;

Not even sure if the if's will always match...

[dsinclair, 2016-11-22 16:53:49]

https://codereview.chromium.org/2520413002/diff/1/xfa/fxfa/app/xfa_ffchoiceli...
File xfa/fxfa/app/xfa_ffchoicelist.cpp (right):

https://codereview.chromium.org/2520413002/diff/1/xfa/fxfa/app/xfa_ffchoiceli...
xfa/fxfa/app/xfa_ffchoicelist.cpp:163: pListBox->GetItemText(nullptr,
pListBox->GetSelItem(0), eParam.m_wsNewText);
On 2016/11/22 16:52:13, npm wrote:
> On 2016/11/22 16:42:15, dsinclair wrote:
> > On 2016/11/22 16:36:16, npm wrote:
> > > It's unclear to me why this is the same.
> > 
> > pListBox had two GetItemText() methods. First first took the IFWL_Widget as
> the
> > first param, the second didn't. I removed the version that didn't and
changed
> > these to pass a null widget.
> 
> Original:
> if (GetWidget()) ToListBox(GetWidget())->GetItemText(pItem, wsText);
> The GetITemText is from IFWL_ListBox, because that's what ToListBox returns.
> 
> Now:
> if (pItem) wsText = static_cast<CFWL_ListItem*>(pItem)->m_wsText;
> 
> Not even sure if the if's will always match...


Right, but if you look at GetItemText in ListBox, it proxies the request back
this class passing this as the widget. We don't use the widget so nullptr should
be fine until I can remove the param.

[npm, 2016-11-22 16:59:55]

lgtm

[Tom Sepez, 2016-11-22 17:10:24]

lgtm

https://codereview.chromium.org/2520413002/diff/20001/xfa/fwl/core/cfwl_listb...
File xfa/fwl/core/cfwl_listbox.cpp (right):

https://codereview.chromium.org/2520413002/diff/20001/xfa/fwl/core/cfwl_listb...
xfa/fwl/core/cfwl_listbox.cpp:85: void CFWL_ListBox::GetItemText(IFWL_Widget*
pWidget,
can we return it instead of out params?

https://codereview.chromium.org/2520413002/diff/20001/xfa/fwl/core/ifwl_listb...
File xfa/fwl/core/ifwl_listbox.cpp (right):

https://codereview.chromium.org/2520413002/diff/20001/xfa/fwl/core/ifwl_listb...
xfa/fwl/core/ifwl_listbox.cpp:218: void
IFWL_ListBox::GetDataProviderItemText(CFWL_ListItem* pItem,
same

https://codereview.chromium.org/2520413002/diff/20001/xfa/fxfa/app/xfa_ffchoi...
File xfa/fxfa/app/xfa_ffchoicelist.cpp (right):

https://codereview.chromium.org/2520413002/diff/20001/xfa/fxfa/app/xfa_ffchoi...
xfa/fxfa/app/xfa_ffchoicelist.cpp:186:
((CFWL_ListBox*)m_pNormalWidget)->GetItem(nullptr, nIndex));
nit: static_cast. (two places plus previous lines).  Local might help.

[dsinclair, 2016-11-22 17:24:37]

https://codereview.chromium.org/2520413002/diff/20001/xfa/fwl/core/cfwl_listb...
File xfa/fwl/core/cfwl_listbox.cpp (right):

https://codereview.chromium.org/2520413002/diff/20001/xfa/fwl/core/cfwl_listb...
xfa/fwl/core/cfwl_listbox.cpp:85: void CFWL_ListBox::GetItemText(IFWL_Widget*
pWidget,
On 2016/11/22 17:10:24, Tom Sepez wrote:
> can we return it instead of out params?

Getting there .... I want to merge the IFWL and CFWL classes first so I only
have to change things in one place instead of 2.

https://codereview.chromium.org/2520413002/diff/20001/xfa/fwl/core/ifwl_listb...
File xfa/fwl/core/ifwl_listbox.cpp (right):

https://codereview.chromium.org/2520413002/diff/20001/xfa/fwl/core/ifwl_listb...
xfa/fwl/core/ifwl_listbox.cpp:218: void
IFWL_ListBox::GetDataProviderItemText(CFWL_ListItem* pItem,
On 2016/11/22 17:10:24, Tom Sepez wrote:
> same

Acknowledged.

https://codereview.chromium.org/2520413002/diff/20001/xfa/fxfa/app/xfa_ffchoi...
File xfa/fxfa/app/xfa_ffchoicelist.cpp (right):

https://codereview.chromium.org/2520413002/diff/20001/xfa/fxfa/app/xfa_ffchoi...
xfa/fxfa/app/xfa_ffchoicelist.cpp:186:
((CFWL_ListBox*)m_pNormalWidget)->GetItem(nullptr, nIndex));
On 2016/11/22 17:10:24, Tom Sepez wrote:
> nit: static_cast. (two places plus previous lines).  Local might help.

Done.

[dsinclair, 2016-11-22 17:24:37]

https://codereview.chromium.org/2520413002/diff/20001/xfa/fwl/core/cfwl_listb...
File xfa/fwl/core/cfwl_listbox.cpp (right):

https://codereview.chromium.org/2520413002/diff/20001/xfa/fwl/core/cfwl_listb...
xfa/fwl/core/cfwl_listbox.cpp:85: void CFWL_ListBox::GetItemText(IFWL_Widget*
pWidget,
On 2016/11/22 17:10:24, Tom Sepez wrote:
> can we return it instead of out params?

Getting there .... I want to merge the IFWL and CFWL classes first so I only
have to change things in one place instead of 2.

https://codereview.chromium.org/2520413002/diff/20001/xfa/fwl/core/ifwl_listb...
File xfa/fwl/core/ifwl_listbox.cpp (right):

https://codereview.chromium.org/2520413002/diff/20001/xfa/fwl/core/ifwl_listb...
xfa/fwl/core/ifwl_listbox.cpp:218: void
IFWL_ListBox::GetDataProviderItemText(CFWL_ListItem* pItem,
On 2016/11/22 17:10:24, Tom Sepez wrote:
> same

Acknowledged.

https://codereview.chromium.org/2520413002/diff/20001/xfa/fxfa/app/xfa_ffchoi...
File xfa/fxfa/app/xfa_ffchoicelist.cpp (right):

https://codereview.chromium.org/2520413002/diff/20001/xfa/fxfa/app/xfa_ffchoi...
xfa/fxfa/app/xfa_ffchoicelist.cpp:186:
((CFWL_ListBox*)m_pNormalWidget)->GetItem(nullptr, nIndex));
On 2016/11/22 17:10:24, Tom Sepez wrote:
> nit: static_cast. (two places plus previous lines).  Local might help.

Done.

[dsinclair, 2016-11-22 17:24:53]

The CQ bit was checked by dsinclair@chromium.org

[dsinclair, 2016-11-22 17:24:53]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
npm@chromium.org
Link to the patchset: https://codereview.chromium.org/2520413002/#ps40001
(title: "Review cleanup")

[commit-bot: I haz the power, 2016-11-22 17:25:01]

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-22 17:34:45]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1479835493384420,
"parent_rev": "4572c8a20beee5cbab2eb1a90cb6cadcc5fa512c", "commit_rev":
"e50184c087ee6f90568ad1166a55f5d25be4d09e"}

[commit-bot: I haz the power, 2016-11-22 17:34:48]

Description was changed from

==========
Rename common methods between ifwl and cfwl.

This CL renames some methods in IFWL which have the same name as the CFWL
methods but the CFWL methods are not proxy methods.
==========

to

==========
Rename common methods between ifwl and cfwl.

This CL renames some methods in IFWL which have the same name as the CFWL
methods but the CFWL methods are not proxy methods.

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

[commit-bot: I haz the power, 2016-11-22 17:34:49]

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