Remove IWFL_WidgetMgr in favor of CFWL_WidgetMgr

Remove IWFL_WidgetMgr in favor of CFWL_WidgetMgr.

The pure virtual class isn't buying us anything, and is costing
us an otherwise unneeded vtable.

Fix pre-existing presubmit warning in fwl_noteimp.cpp:88:
  If statement had no body and no else clause ...

Remove switch on enum() and make separate GetWidget() methods.

Committed: https://pdfium.googlesource.com/pdfium/+/1529756d0d62638ea0fc7965416bef717ff47413

[Tom Sepez, 2016-05-23 21:52:58]

Description was changed from

==========
Tidy IWFL_WidgetMgr

Remove unused method.
Make creation a static method of the class rather than a
standalone function.
==========

to

==========
Tidy IWFL_WidgetMgr

Remove unused method. Make creation a static method of the
class rather than a standalone function.
==========

[Tom Sepez, 2016-05-23 21:52:59]

tsepez@chromium.org changed reviewers:
+ thestig@chromium.org

[Tom Sepez, 2016-05-23 21:53:16]

Lei, for review.

[Lei Zhang, 2016-05-23 21:57:20]

lgtm

https://codereview.chromium.org/2004213002/diff/1/xfa/fwl/lightwidget/cfwl_wi...
File xfa/fwl/lightwidget/cfwl_widget.cpp (right):

https://codereview.chromium.org/2004213002/diff/1/xfa/fwl/lightwidget/cfwl_wi...
xfa/fwl/lightwidget/cfwl_widget.cpp:220: m_pWidgetMgr =
static_cast<CFWL_WidgetMgr*>(IFWL_WidgetMgr::GetInstance());
Do we want to get rid of IFWL_WidgetMgr altogether in favor of CFWL_WidgetMgr?

[Tom Sepez, 2016-05-23 22:02:55]

https://codereview.chromium.org/2004213002/diff/1/xfa/fwl/lightwidget/cfwl_wi...
File xfa/fwl/lightwidget/cfwl_widget.cpp (right):

https://codereview.chromium.org/2004213002/diff/1/xfa/fwl/lightwidget/cfwl_wi...
xfa/fwl/lightwidget/cfwl_widget.cpp:220: m_pWidgetMgr =
static_cast<CFWL_WidgetMgr*>(IFWL_WidgetMgr::GetInstance());
On 2016/05/23 21:57:20, Lei Zhang wrote:
> Do we want to get rid of IFWL_WidgetMgr altogether in favor of CFWL_WidgetMgr?

Yes, I think so.

[Tom Sepez, 2016-05-23 22:27:39]

Description was changed from

==========
Tidy IWFL_WidgetMgr

Remove unused method. Make creation a static method of the
class rather than a standalone function.
==========

to

==========
Remove IWFL_WidgetMgr in favor of CFWL_WidgetMgr.

The pure virtual class isn't buying us anything, and is costing
us an otherwise unneeded vtable.
==========

[Tom Sepez, 2016-05-23 23:28:53]

Description was changed from

==========
Remove IWFL_WidgetMgr in favor of CFWL_WidgetMgr.

The pure virtual class isn't buying us anything, and is costing
us an otherwise unneeded vtable.
==========

to

==========
Remove IWFL_WidgetMgr in favor of CFWL_WidgetMgr.

The pure virtual class isn't buying us anything, and is costing
us an otherwise unneeded vtable.

Fix pre-existing presubmit warning in fwl_noteimp.cpp:88:
  If statement had no body and no else clause ...

Remove switch on enum() and make separate GetWidget() methods.
==========

[Tom Sepez, 2016-05-24 00:22:07]

Lei, PTAL.

[Lei Zhang, 2016-05-24 00:36:11]

Do you want to rename fwl_widgetmgrimp.{cpp,h} to cfwl_widgetmgr{cpp,h}?

https://codereview.chromium.org/2004213002/diff/120001/xfa/fwl/core/fwl_widge...
File xfa/fwl/core/fwl_widgetmgrimp.h (right):

https://codereview.chromium.org/2004213002/diff/120001/xfa/fwl/core/fwl_widge...
xfa/fwl/core/fwl_widgetmgrimp.h:64: CFWL_WidgetMgr(CXFA_FFApp* pAdapterNative);
explicit

https://codereview.chromium.org/2004213002/diff/120001/xfa/fwl/core/fwl_widge...
xfa/fwl/core/fwl_widgetmgrimp.h:77: FX_BOOL SetWidgetIndex(IFWL_Widget* pWidget,
int32_t nIndex);
BTW, nobody checks the return value.

[Lei Zhang, 2016-05-24 00:36:51]

https://codereview.chromium.org/2004213002/diff/120001/xfa/fwl/core/fwl_widge...
File xfa/fwl/core/fwl_widgetmgrimp.h (right):

https://codereview.chromium.org/2004213002/diff/120001/xfa/fwl/core/fwl_widge...
xfa/fwl/core/fwl_widgetmgrimp.h:77: FX_BOOL SetWidgetIndex(IFWL_Widget* pWidget,
int32_t nIndex);
On 2016/05/24 00:36:10, Lei Zhang wrote:
> BTW, nobody checks the return value.

I'm fine if we leave it alone for now.

[Lei Zhang, 2016-05-24 00:40:00]

https://codereview.chromium.org/2004213002/diff/120001/xfa/fwl/core/fwl_widge...
File xfa/fwl/core/fwl_widgetmgrimp.cpp (right):

https://codereview.chromium.org/2004213002/diff/120001/xfa/fwl/core/fwl_widge...
xfa/fwl/core/fwl_widgetmgrimp.cpp:421: ++iRet;
Umm, right.

[Tom Sepez, 2016-05-24 16:27:15]

https://codereview.chromium.org/2004213002/diff/120001/xfa/fwl/core/fwl_widge...
File xfa/fwl/core/fwl_widgetmgrimp.h (right):

https://codereview.chromium.org/2004213002/diff/120001/xfa/fwl/core/fwl_widge...
xfa/fwl/core/fwl_widgetmgrimp.h:64: CFWL_WidgetMgr(CXFA_FFApp* pAdapterNative);
On 2016/05/24 00:36:10, Lei Zhang wrote:
> explicit

Done.

https://codereview.chromium.org/2004213002/diff/120001/xfa/fwl/core/fwl_widge...
xfa/fwl/core/fwl_widgetmgrimp.h:77: FX_BOOL SetWidgetIndex(IFWL_Widget* pWidget,
int32_t nIndex);
On 2016/05/24 00:36:10, Lei Zhang wrote:
> BTW, nobody checks the return value.

Left alone for now.

[Tom Sepez, 2016-05-24 16:28:22]

On 2016/05/24 00:36:11, Lei Zhang wrote:
> Do you want to rename fwl_widgetmgrimp.{cpp,h} to cfwl_widgetmgr{cpp,h}?
> 
Sounds like a good follow-on.  We can also make cfwl_widgetmgrItem be a nested
class of cfwl_widgetMgr while we're at it so that only cfwl_widgetmgr class is
in that file.

[Tom Sepez, 2016-05-24 16:28:32]

The CQ bit was checked by tsepez@chromium.org

[Tom Sepez, 2016-05-24 16:28:33]

The patchset sent to the CQ was uploaded after l-g-t-m from thestig@chromium.org
Link to the patchset: https://codereview.chromium.org/2004213002/#ps140001
(title: "Explicit ctor")

[commit-bot: I haz the power, 2016-05-24 16:28:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2004213002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2004213002/140001

[commit-bot: I haz the power, 2016-05-24 16:46:06]

Description was changed from

==========
Remove IWFL_WidgetMgr in favor of CFWL_WidgetMgr.

The pure virtual class isn't buying us anything, and is costing
us an otherwise unneeded vtable.

Fix pre-existing presubmit warning in fwl_noteimp.cpp:88:
  If statement had no body and no else clause ...

Remove switch on enum() and make separate GetWidget() methods.
==========

to

==========
Remove IWFL_WidgetMgr in favor of CFWL_WidgetMgr.

The pure virtual class isn't buying us anything, and is costing
us an otherwise unneeded vtable.

Fix pre-existing presubmit warning in fwl_noteimp.cpp:88:
  If statement had no body and no else clause ...

Remove switch on enum() and make separate GetWidget() methods.

Committed:
https://pdfium.googlesource.com/pdfium/+/1529756d0d62638ea0fc7965416bef717ff4...
==========

[commit-bot: I haz the power, 2016-05-24 16:46:07]

Committed patchset #8 (id:140001) as
https://pdfium.googlesource.com/pdfium/+/1529756d0d62638ea0fc7965416bef717ff4...