Cleaning up nits in fwl/core files.

Cleaning up nits in fwl/core files.

This Cl cleans up the nits in the first set of cfwl_* files. The unused Event
and Message classes are removed.

Committed: https://pdfium.googlesource.com/pdfium/+/72f957a0bafaf7c9628980e404c471b54f42de8f

[dsinclair, 2016-11-15 20:49:38]

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

[dsinclair, 2016-11-15 20:49:38]

PTAL.

The events and messages removed are never instantiated.

[dsinclair, 2016-11-15 20:49:42]

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

[commit-bot: I haz the power, 2016-11-15 20:49:55]

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-15 21:03:27]

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

[commit-bot: I haz the power, 2016-11-15 21:03:28]

Dry run: This issue passed the CQ dry run.

[npm, 2016-11-15 21:38:30]

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

https://codereview.chromium.org/2505703003/diff/1/xfa/fwl/core/cfwl_combobox....
xfa/fwl/core/cfwl_combobox.cpp:61: void CFWL_ComboBox::GetTextByIndex(int32_t
iIndex,
Is this the point where you change the Get methods to return? :)

https://codereview.chromium.org/2505703003/diff/1/xfa/fwl/core/cfwl_combobox....
xfa/fwl/core/cfwl_combobox.cpp:183: uint32_t
CFWL_ComboBox::GetItemStyles(IFWL_Widget* pWidget,
Is this some inherited method, or can you remove pWidget? Similarly
in several more methods in this class.

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

https://codereview.chromium.org/2505703003/diff/1/xfa/fwl/core/cfwl_listbox.c...
xfa/fwl/core/cfwl_listbox.cpp:52: if (iSel >= iCount) {
nit: remove iCount, directly call CountItems(m_pIface.get()) here.

https://codereview.chromium.org/2505703003/diff/1/xfa/fwl/core/cfwl_listbox.c...
xfa/fwl/core/cfwl_listbox.cpp:104: void CFWL_ListBox::GetCaption(IFWL_Widget*
pWidget, CFX_WideString& wsCaption) {
Similarly here, pWidget is not used in most of the methods.

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

https://codereview.chromium.org/2505703003/diff/1/xfa/fwl/core/cfwl_message.h...
xfa/fwl/core/cfwl_message.h:89: #define FWL_MESSAGE_DEF(classname, msgType, ...)
                          \
Is this really the best way to do this?

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

https://codereview.chromium.org/2505703003/diff/1/xfa/fwl/core/ifwl_form.cpp#...
xfa/fwl/core/ifwl_form.cpp:669: void IFWL_Form::OnProcessMessage(CFWL_Message*
pMessage) {}
Can this method now be virtual or something like that?

[dsinclair, 2016-11-15 21:44:06]

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

https://codereview.chromium.org/2505703003/diff/1/xfa/fwl/core/cfwl_combobox....
xfa/fwl/core/cfwl_combobox.cpp:61: void CFWL_ComboBox::GetTextByIndex(int32_t
iIndex,
On 2016/11/15 21:38:30, npm wrote:
> Is this the point where you change the Get methods to return? :)

After I merge the classes.

https://codereview.chromium.org/2505703003/diff/1/xfa/fwl/core/cfwl_combobox....
xfa/fwl/core/cfwl_combobox.cpp:183: uint32_t
CFWL_ComboBox::GetItemStyles(IFWL_Widget* pWidget,
On 2016/11/15 21:38:30, npm wrote:
> Is this some inherited method, or can you remove pWidget? Similarly
> in several more methods in this class.


They're inherited from the DataProvider override. It's possible it could be
removed but I'd have to go through all the things that use that provider. I'll
add it to my list of things to do after the merge.

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

https://codereview.chromium.org/2505703003/diff/1/xfa/fwl/core/cfwl_listbox.c...
xfa/fwl/core/cfwl_listbox.cpp:52: if (iSel >= iCount) {
On 2016/11/15 21:38:30, npm wrote:
> nit: remove iCount, directly call CountItems(m_pIface.get()) here.

Followup.

https://codereview.chromium.org/2505703003/diff/1/xfa/fwl/core/cfwl_listbox.c...
xfa/fwl/core/cfwl_listbox.cpp:104: void CFWL_ListBox::GetCaption(IFWL_Widget*
pWidget, CFX_WideString& wsCaption) {
On 2016/11/15 21:38:30, npm wrote:
> Similarly here, pWidget is not used in most of the methods.

Acknowledged.

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

https://codereview.chromium.org/2505703003/diff/1/xfa/fwl/core/cfwl_message.h...
xfa/fwl/core/cfwl_message.h:89: #define FWL_MESSAGE_DEF(classname, msgType, ...)
                          \
On 2016/11/15 21:38:30, npm wrote:
> Is this really the best way to do this?

Probably not, but it's the way it's currently done. We could create actual
classes for the messages and events now that they're trimmed down, but would be
better in a followup if that's what we decide to do.

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

https://codereview.chromium.org/2505703003/diff/1/xfa/fwl/core/ifwl_form.cpp#...
xfa/fwl/core/ifwl_form.cpp:669: void IFWL_Form::OnProcessMessage(CFWL_Message*
pMessage) {}
On 2016/11/15 21:38:30, npm wrote:
> Can this method now be virtual or something like that?

It is virtual. We want the implementation here that does nothing to make sure
the parent implementation doesn't get called.

[npm, 2016-11-15 22:18:55]

lgtm
I take your word on the followups :)

[Tom Sepez, 2016-11-15 22:41:47]

lgtm

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

https://codereview.chromium.org/2505703003/diff/1/xfa/fwl/core/cfwl_message.h...
xfa/fwl/core/cfwl_message.h:89: #define FWL_MESSAGE_DEF(classname, msgType, ...)
                          \
I'd suggest just writing it out "longhand" in the follow up now that there are
only 5 instantiations of it.

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

The CQ bit was checked by dsinclair@chromium.org

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

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

Description was changed from

==========
Cleaning up nits in fwl/core files.

This Cl cleans up the nits in the first set of cfwl_* files. The unused Event
and Message classes are removed.
==========

to

==========
Cleaning up nits in fwl/core files.

This Cl cleans up the nits in the first set of cfwl_* files. The unused Event
and Message classes are removed.

Committed:
https://pdfium.googlesource.com/pdfium/+/72f957a0bafaf7c9628980e404c471b54f42...
==========

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

Committed patchset #1 (id:1) as
https://pdfium.googlesource.com/pdfium/+/72f957a0bafaf7c9628980e404c471b54f42...