Continue cleaning IFWL classes

Continue cleaning IFWL classes

More work cleaning up visiblity, unused methods and return values.

Committed: https://pdfium.googlesource.com/pdfium/+/12f72bfc06198fcddf86f366adbbdeb152163b0e

[dsinclair, 2016-11-09 17:05:22]

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

[dsinclair, 2016-11-09 17:05:23]

PTAL.

[dsinclair, 2016-11-09 17:05:26]

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

[commit-bot: I haz the power, 2016-11-09 17:05: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-11-09 17:46:02]

lgtm

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

https://codereview.chromium.org/2489013002/diff/1/xfa/fwl/core/ifwl_scrollbar...
xfa/fwl/core/ifwl_scrollbar.cpp:131: return OnScroll(dwCode, fPos);
nit: replace line 127 with this.

https://codereview.chromium.org/2489013002/diff/1/xfa/fwl/core/ifwl_scrollbar.h
File xfa/fwl/core/ifwl_scrollbar.h (right):

https://codereview.chromium.org/2489013002/diff/1/xfa/fwl/core/ifwl_scrollbar...
xfa/fwl/core/ifwl_scrollbar.h:80: return m_pProperties->m_dwStyleExes &
FWL_STYLEEXT_SCB_Vert;
nit: should be 
  return !!(m_pProperties->m_dwStyleExes & FWL_STYLEEXT_SCB_Vert);
to avoid an int->bool warning.

https://codereview.chromium.org/2489013002/diff/1/xfa/fwl/core/ifwl_spinbutton.h
File xfa/fwl/core/ifwl_spinbutton.h (right):

https://codereview.chromium.org/2489013002/diff/1/xfa/fwl/core/ifwl_spinbutto...
xfa/fwl/core/ifwl_spinbutton.h:51: bool IsButtonEnable(bool bUp = true);
nit: IsButtonEnabled while we're at it.

[commit-bot: I haz the power, 2016-11-09 18:23:48]

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

[commit-bot: I haz the power, 2016-11-09 18:23:49]

Dry run: This issue passed the CQ dry run.

[dsinclair, 2016-11-09 18:25:57]

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

https://codereview.chromium.org/2489013002/diff/1/xfa/fwl/core/ifwl_scrollbar...
xfa/fwl/core/ifwl_scrollbar.cpp:131: return OnScroll(dwCode, fPos);
On 2016/11/09 17:46:02, Tom Sepez wrote:
> nit: replace line 127 with this.

Fixed in followup CL.

https://codereview.chromium.org/2489013002/diff/1/xfa/fwl/core/ifwl_scrollbar.h
File xfa/fwl/core/ifwl_scrollbar.h (right):

https://codereview.chromium.org/2489013002/diff/1/xfa/fwl/core/ifwl_scrollbar...
xfa/fwl/core/ifwl_scrollbar.h:80: return m_pProperties->m_dwStyleExes &
FWL_STYLEEXT_SCB_Vert;
On 2016/11/09 17:46:02, Tom Sepez wrote:
> nit: should be 
>   return !!(m_pProperties->m_dwStyleExes & FWL_STYLEEXT_SCB_Vert);
> to avoid an int->bool warning.

Done.

https://codereview.chromium.org/2489013002/diff/1/xfa/fwl/core/ifwl_spinbutton.h
File xfa/fwl/core/ifwl_spinbutton.h (right):

https://codereview.chromium.org/2489013002/diff/1/xfa/fwl/core/ifwl_spinbutto...
xfa/fwl/core/ifwl_spinbutton.h:51: bool IsButtonEnable(bool bUp = true);
On 2016/11/09 17:46:02, Tom Sepez wrote:
> nit: IsButtonEnabled while we're at it.

Done.

https://codereview.chromium.org/2489013002/diff/20001/xfa/fwl/core/ifwl_scrol...
File xfa/fwl/core/ifwl_scrollbar.h (right):

https://codereview.chromium.org/2489013002/diff/20001/xfa/fwl/core/ifwl_scrol...
xfa/fwl/core/ifwl_scrollbar.h:53: void GetRange(FX_FLOAT* fMin, FX_FLOAT* fMax)
const {
Updated this to *'s as well.

[dsinclair, 2016-11-09 18:26:17]

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

[commit-bot: I haz the power, 2016-11-09 18:26:27]

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

[npm_g, 2016-11-09 18:35:22]

npm@google.com changed reviewers:
+ npm@google.com

[npm_g, 2016-11-09 18:35:23]

lgtm

https://codereview.chromium.org/2489013002/diff/20001/xfa/fwl/core/ifwl_spinb...
File xfa/fwl/core/ifwl_spinbutton.h (right):

https://codereview.chromium.org/2489013002/diff/20001/xfa/fwl/core/ifwl_spinb...
xfa/fwl/core/ifwl_spinbutton.h:51: bool IsButtonEnabled(bool bUp = true);
Followup nit: bool bUp

[Tom Sepez, 2016-11-09 18:46:08]

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

https://codereview.chromium.org/2489013002/diff/20001/xfa/fwl/core/ifwl_edit....
xfa/fwl/core/ifwl_edit.cpp:1934: pScrollBar->GetRange(&(fs.x), &(fs.y));
nit: overparenthesized.

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

https://codereview.chromium.org/2489013002/diff/20001/xfa/fwl/core/ifwl_listb...
xfa/fwl/core/ifwl_listbox.cpp:1095: pScrollBar->GetRange(&(fs.x), &(fs.y));
nitto

https://codereview.chromium.org/2489013002/diff/20001/xfa/fwl/core/ifwl_scrol...
File xfa/fwl/core/ifwl_scrollbar.h (right):

https://codereview.chromium.org/2489013002/diff/20001/xfa/fwl/core/ifwl_scrol...
xfa/fwl/core/ifwl_scrollbar.h:53: void GetRange(FX_FLOAT* fMin, FX_FLOAT* fMax)
const {
On 2016/11/09 18:25:57, dsinclair wrote:
> Updated this to *'s as well.

Are these ever nullptrs?  can we lose the checks?

[commit-bot: I haz the power, 2016-11-09 19:35:18]

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

[commit-bot: I haz the power, 2016-11-09 19:35:19]

Dry run: This issue passed the CQ dry run.

[dsinclair, 2016-11-10 17:56:04]

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

https://codereview.chromium.org/2489013002/diff/20001/xfa/fwl/core/ifwl_edit....
xfa/fwl/core/ifwl_edit.cpp:1934: pScrollBar->GetRange(&(fs.x), &(fs.y));
On 2016/11/09 18:46:08, Tom Sepez wrote:
> nit: overparenthesized.

Done.

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

https://codereview.chromium.org/2489013002/diff/20001/xfa/fwl/core/ifwl_listb...
xfa/fwl/core/ifwl_listbox.cpp:1095: pScrollBar->GetRange(&(fs.x), &(fs.y));
On 2016/11/09 18:46:08, Tom Sepez wrote:
> nitto

Done.

https://codereview.chromium.org/2489013002/diff/20001/xfa/fwl/core/ifwl_scrol...
File xfa/fwl/core/ifwl_scrollbar.h (right):

https://codereview.chromium.org/2489013002/diff/20001/xfa/fwl/core/ifwl_scrol...
xfa/fwl/core/ifwl_scrollbar.h:53: void GetRange(FX_FLOAT* fMin, FX_FLOAT* fMax)
const {
On 2016/11/09 18:46:08, Tom Sepez wrote:
> On 2016/11/09 18:25:57, dsinclair wrote:
> > Updated this to *'s as well.
> 
> Are these ever nullptrs?  can we lose the checks?

Made them ASSERTs.

https://codereview.chromium.org/2489013002/diff/20001/xfa/fwl/core/ifwl_spinb...
File xfa/fwl/core/ifwl_spinbutton.h (right):

https://codereview.chromium.org/2489013002/diff/20001/xfa/fwl/core/ifwl_spinb...
xfa/fwl/core/ifwl_spinbutton.h:51: bool IsButtonEnabled(bool bUp = true);
On 2016/11/09 18:35:23, npm_g wrote:
> Followup nit: bool bUp

Acknowledged.

[dsinclair, 2016-11-10 17:56:26]

The CQ bit was checked by dsinclair@chromium.org

[dsinclair, 2016-11-10 17:56:26]

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

[commit-bot: I haz the power, 2016-11-10 17:56:37]

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-10 18:25:50]

Description was changed from

==========
Continue cleaning IFWL classes

More work cleaning up visiblity, unused methods and return values.
==========

to

==========
Continue cleaning IFWL classes

More work cleaning up visiblity, unused methods and return values.

Committed:
https://pdfium.googlesource.com/pdfium/+/12f72bfc06198fcddf86f366adbbdeb15216...
==========

[commit-bot: I haz the power, 2016-11-10 18:25:51]

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