Continue fwl/core nit cleanup.

Continue fwl/core nit cleanup.

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

[dsinclair, 2016-11-15 22:05:57]

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

[dsinclair, 2016-11-15 22:05:57]

PTAL.

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

https://codereview.chromium.org/2501033005/diff/1/xfa/fwl/core/ifwl_checkbox....
xfa/fwl/core/ifwl_checkbox.cpp:161: FXSYS_round(m_pProperties->m_rtWidget.width
+ 0.5f);
tsepez@, these are equivalent, yea?

[dsinclair, 2016-11-15 22:06:01]

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

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

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

[npm, 2016-11-15 22:42:44]

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

https://codereview.chromium.org/2501033005/diff/1/xfa/fwl/core/ifwl_checkbox....
xfa/fwl/core/ifwl_checkbox.cpp:105: CFWL_ThemeBackground param;
We could have a helper method to fill up a CFWL_ThemePart.

https://codereview.chromium.org/2501033005/diff/1/xfa/fwl/core/ifwl_checkbox....
xfa/fwl/core/ifwl_checkbox.cpp:418: void
IFWL_CheckBox::OnFocusChanged(CFWL_Message* pMsg, bool bSet) {
Unused pMsg

https://codereview.chromium.org/2501033005/diff/1/xfa/fwl/core/ifwl_checkbox....
xfa/fwl/core/ifwl_checkbox.cpp:427: void
IFWL_CheckBox::OnLButtonDown(CFWL_MsgMouse* pMsg) {
Ditto

https://codereview.chromium.org/2501033005/diff/1/xfa/fwl/core/ifwl_checkbox....
xfa/fwl/core/ifwl_checkbox.cpp:489: void
IFWL_CheckBox::OnMouseLeave(CFWL_MsgMouse* pMsg) {
Ditto

[Tom Sepez, 2016-11-15 22:49:32]

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

https://codereview.chromium.org/2501033005/diff/1/xfa/fwl/core/ifwl_caret.cpp...
xfa/fwl/core/ifwl_caret.cpp:72: if (!(m_pProperties->m_dwStates &
FWL_STATE_CAT_HightLight))
can this move to line 68?

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

https://codereview.chromium.org/2501033005/diff/1/xfa/fwl/core/ifwl_checkbox....
xfa/fwl/core/ifwl_checkbox.cpp:161: FXSYS_round(m_pProperties->m_rtWidget.width
+ 0.5f);
On 2016/11/15 22:05:57, dsinclair wrote:
> tsepez@, these are equivalent, yea?

No, I think round adds the 0.5 for you.

https://codereview.chromium.org/2501033005/diff/1/xfa/fwl/core/ifwl_checkbox....
xfa/fwl/core/ifwl_checkbox.cpp:424: Repaint(&(m_rtClient));
nit: overparenthesized.  Also below.

[commit-bot: I haz the power, 2016-11-15 22:58:33]

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

[commit-bot: I haz the power, 2016-11-15 22:58:35]

Dry run: This issue passed the CQ dry run.

[dsinclair, 2016-11-16 15:18:33]

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

https://codereview.chromium.org/2501033005/diff/1/xfa/fwl/core/ifwl_caret.cpp...
xfa/fwl/core/ifwl_caret.cpp:72: if (!(m_pProperties->m_dwStates &
FWL_STATE_CAT_HightLight))
On 2016/11/15 22:49:32, Tom Sepez wrote:
> can this move to line 68?

Done.

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

https://codereview.chromium.org/2501033005/diff/1/xfa/fwl/core/ifwl_checkbox....
xfa/fwl/core/ifwl_checkbox.cpp:105: CFWL_ThemeBackground param;
On 2016/11/15 22:42:44, npm wrote:
> We could have a helper method to fill up a CFWL_ThemePart.

We could, but I'm not sure how much better it would look. There are a bunch of
params in ThemePart and more in things like ThemeBackground. We'd probably end
up with individual setters that would look pretty much the same.

I think we should take a look at the Theme code in general and figure out if it
all needs to be in separate files or if it can be merged back up to parent
classes which may solve some of this.

https://codereview.chromium.org/2501033005/diff/1/xfa/fwl/core/ifwl_checkbox....
xfa/fwl/core/ifwl_checkbox.cpp:161: FXSYS_round(m_pProperties->m_rtWidget.width
+ 0.5f);
On 2016/11/15 22:49:32, Tom Sepez wrote:
> On 2016/11/15 22:05:57, dsinclair wrote:
> > tsepez@, these are equivalent, yea?
> 
> No, I think round adds the 0.5 for you.

Done.

https://codereview.chromium.org/2501033005/diff/1/xfa/fwl/core/ifwl_checkbox....
xfa/fwl/core/ifwl_checkbox.cpp:418: void
IFWL_CheckBox::OnFocusChanged(CFWL_Message* pMsg, bool bSet) {
On 2016/11/15 22:42:44, npm wrote:
> Unused pMsg

Done.

https://codereview.chromium.org/2501033005/diff/1/xfa/fwl/core/ifwl_checkbox....
xfa/fwl/core/ifwl_checkbox.cpp:424: Repaint(&(m_rtClient));
On 2016/11/15 22:49:32, Tom Sepez wrote:
> nit: overparenthesized.  Also below.

Done.

https://codereview.chromium.org/2501033005/diff/1/xfa/fwl/core/ifwl_checkbox....
xfa/fwl/core/ifwl_checkbox.cpp:427: void
IFWL_CheckBox::OnLButtonDown(CFWL_MsgMouse* pMsg) {
On 2016/11/15 22:42:44, npm wrote:
> Ditto

Done.

https://codereview.chromium.org/2501033005/diff/1/xfa/fwl/core/ifwl_checkbox....
xfa/fwl/core/ifwl_checkbox.cpp:489: void
IFWL_CheckBox::OnMouseLeave(CFWL_MsgMouse* pMsg) {
On 2016/11/15 22:42:44, npm wrote:
> Ditto

Done.

[npm, 2016-11-16 16:25:48]

lgtm

[dsinclair, 2016-11-16 16:32:32]

The CQ bit was checked by dsinclair@chromium.org

[commit-bot: I haz the power, 2016-11-16 16:32: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-16 16:44:33]

Description was changed from

==========
Continue fwl/core nit cleanup.
==========

to

==========
Continue fwl/core nit cleanup.

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

[commit-bot: I haz the power, 2016-11-16 16:44:35]

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