Cleanup IFWL_Adapter interfaces.

Cleanup IFWL_Adapter interfaces.

This CL removes:
  * IFWL_AdapterNative
  * IFWL_Adapter{Widget|Thread}Mgr
  * IFWL_WidgetMgrDelegate
  * CFWL_SDAdapter{Widget|Thread}Mgr

Methods which just returned have also been removed.

Committed: https://pdfium.googlesource.com/pdfium/+/55fa356c8ce72c597e75fc591fc67597b1c019d5

[dsinclair, 2016-04-28 15:57:44]

Patchset #1 (id:1) has been deleted

[dsinclair, 2016-04-28 16:01:17]

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

[dsinclair, 2016-04-28 16:01:17]

PTAL.

[Tom Sepez, 2016-04-28 16:42:12]

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

https://codereview.chromium.org/1928963004/diff/20001/xfa/fwl/core/fwl_appimp...
xfa/fwl/core/fwl_appimp.cpp:85: FWL_ERR
CFWL_AppImp::SetThemeProvider(IFWL_ThemeProvider* pThemeProvider) {
This should probably be a void throughout.  I doubt they check the return
anyways???

https://codereview.chromium.org/1928963004/diff/20001/xfa/fwl/core/fwl_appimp...
xfa/fwl/core/fwl_appimp.cpp:107: static IFWL_App* _theApp = NULL;
nit: how about g_theApp?  nullpr.

https://codereview.chromium.org/1928963004/diff/20001/xfa/fwl/core/fwl_appimp.h
File xfa/fwl/core/fwl_appimp.h (right):

https://codereview.chromium.org/1928963004/diff/20001/xfa/fwl/core/fwl_appimp...
xfa/fwl/core/fwl_appimp.h:37: protected:
Are there subclasses of this?  If not, this can be private.

https://codereview.chromium.org/1928963004/diff/20001/xfa/fwl/core/fwl_formim...
File xfa/fwl/core/fwl_formimp.cpp (right):

https://codereview.chromium.org/1928963004/diff/20001/xfa/fwl/core/fwl_formim...
xfa/fwl/core/fwl_formimp.cpp:793: if (pBigIcon && pBigIcon != m_pBigIcon)
The second part of the && seems redundant, do we care about self-reassignment?

https://codereview.chromium.org/1928963004/diff/20001/xfa/fwl/core/fwl_widget...
File xfa/fwl/core/fwl_widgetimp.cpp (right):

https://codereview.chromium.org/1928963004/diff/20001/xfa/fwl/core/fwl_widget...
xfa/fwl/core/fwl_widgetimp.cpp:304: FWL_ERR CFWL_WidgetImp::SetStates(uint32_t
dwStates, FX_BOOL bSet) {
nit: always success?  can this be void?

https://codereview.chromium.org/1928963004/diff/20001/xfa/fwl/core/fwl_widget...
File xfa/fwl/core/fwl_widgetmgrimp.cpp (right):

https://codereview.chromium.org/1928963004/diff/20001/xfa/fwl/core/fwl_widget...
xfa/fwl/core/fwl_widgetmgrimp.cpp:37: #if (_FX_OS_ == _FX_WIN32_DESKTOP_) ||
(_FX_OS_ == _FX_WIN64_)
Curious. I didn't think _FX_WIN64_ was a legitimate _FX_OS_VALUE_, maybe a
follow-up to check this out.

https://codereview.chromium.org/1928963004/diff/20001/xfa/fwl/core/fwl_widget...
File xfa/fwl/core/fwl_widgetmgrimp.h (right):

https://codereview.chromium.org/1928963004/diff/20001/xfa/fwl/core/fwl_widget...
xfa/fwl/core/fwl_widgetmgrimp.h:97: CXFA_FWLAdapterWidgetMgr*
GetAdapterWidgetMgr() { return m_pAdapter; }
nit: const method

https://codereview.chromium.org/1928963004/diff/20001/xfa/fwl/core/fwl_widget...
xfa/fwl/core/fwl_widgetmgrimp.h:98: CFWL_WidgetMgrDelegate* GetDelegate() {
return m_pDelegate; }
nit: const method

https://codereview.chromium.org/1928963004/diff/20001/xfa/fwl/core/ifwl_app.h
File xfa/fwl/core/ifwl_app.h (right):

https://codereview.chromium.org/1928963004/diff/20001/xfa/fwl/core/ifwl_app.h...
xfa/fwl/core/ifwl_app.h:10: // The FWL thread/app code contains three parallel
classes, which reference each
nit: did we obliterate the word "thread"?

[dsinclair, 2016-04-28 17:53:46]

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

https://codereview.chromium.org/1928963004/diff/20001/xfa/fwl/core/fwl_appimp...
xfa/fwl/core/fwl_appimp.cpp:85: FWL_ERR
CFWL_AppImp::SetThemeProvider(IFWL_ThemeProvider* pThemeProvider) {
On 2016/04/28 16:42:11, Tom Sepez wrote:
> This should probably be a void throughout.  I doubt they check the return
> anyways???

Done.

https://codereview.chromium.org/1928963004/diff/20001/xfa/fwl/core/fwl_appimp...
xfa/fwl/core/fwl_appimp.cpp:107: static IFWL_App* _theApp = NULL;
On 2016/04/28 16:42:11, Tom Sepez wrote:
> nit: how about g_theApp?  nullpr.

Done.

https://codereview.chromium.org/1928963004/diff/20001/xfa/fwl/core/fwl_appimp.h
File xfa/fwl/core/fwl_appimp.h (right):

https://codereview.chromium.org/1928963004/diff/20001/xfa/fwl/core/fwl_appimp...
xfa/fwl/core/fwl_appimp.h:37: protected:
On 2016/04/28 16:42:11, Tom Sepez wrote:
> Are there subclasses of this?  If not, this can be private.

Done.

https://codereview.chromium.org/1928963004/diff/20001/xfa/fwl/core/fwl_formim...
File xfa/fwl/core/fwl_formimp.cpp (right):

https://codereview.chromium.org/1928963004/diff/20001/xfa/fwl/core/fwl_formim...
xfa/fwl/core/fwl_formimp.cpp:793: if (pBigIcon && pBigIcon != m_pBigIcon)
On 2016/04/28 16:42:11, Tom Sepez wrote:
> The second part of the && seems redundant, do we care about self-reassignment?


I guess it was guarding the _Native calls. Removed.

https://codereview.chromium.org/1928963004/diff/20001/xfa/fwl/core/fwl_widget...
File xfa/fwl/core/fwl_widgetimp.cpp (right):

https://codereview.chromium.org/1928963004/diff/20001/xfa/fwl/core/fwl_widget...
xfa/fwl/core/fwl_widgetimp.cpp:304: FWL_ERR CFWL_WidgetImp::SetStates(uint32_t
dwStates, FX_BOOL bSet) {
On 2016/04/28 16:42:12, Tom Sepez wrote:
> nit: always success?  can this be void?

Done.

https://codereview.chromium.org/1928963004/diff/20001/xfa/fwl/core/fwl_widget...
File xfa/fwl/core/fwl_widgetmgrimp.cpp (right):

https://codereview.chromium.org/1928963004/diff/20001/xfa/fwl/core/fwl_widget...
xfa/fwl/core/fwl_widgetmgrimp.cpp:37: #if (_FX_OS_ == _FX_WIN32_DESKTOP_) ||
(_FX_OS_ == _FX_WIN64_)
On 2016/04/28 16:42:12, Tom Sepez wrote:
> Curious. I didn't think _FX_WIN64_ was a legitimate _FX_OS_VALUE_, maybe a
> follow-up to check this out.

I think it should be WIN64_DESKTOP, will do as a followup.

https://codereview.chromium.org/1928963004/diff/20001/xfa/fwl/core/fwl_widget...
File xfa/fwl/core/fwl_widgetmgrimp.h (right):

https://codereview.chromium.org/1928963004/diff/20001/xfa/fwl/core/fwl_widget...
xfa/fwl/core/fwl_widgetmgrimp.h:97: CXFA_FWLAdapterWidgetMgr*
GetAdapterWidgetMgr() { return m_pAdapter; }
On 2016/04/28 16:42:12, Tom Sepez wrote:
> nit: const method

Done.

https://codereview.chromium.org/1928963004/diff/20001/xfa/fwl/core/fwl_widget...
xfa/fwl/core/fwl_widgetmgrimp.h:98: CFWL_WidgetMgrDelegate* GetDelegate() {
return m_pDelegate; }
On 2016/04/28 16:42:12, Tom Sepez wrote:
> nit: const method

Done.

https://codereview.chromium.org/1928963004/diff/20001/xfa/fwl/core/ifwl_app.h
File xfa/fwl/core/ifwl_app.h (right):

https://codereview.chromium.org/1928963004/diff/20001/xfa/fwl/core/ifwl_app.h...
xfa/fwl/core/ifwl_app.h:10: // The FWL thread/app code contains three parallel
classes, which reference each
On 2016/04/28 16:42:12, Tom Sepez wrote:
> nit: did we obliterate the word "thread"?

Yup.

[Tom Sepez, 2016-04-28 23:40:32]

lgtm

https://codereview.chromium.org/1928963004/diff/60001/xfa/fwl/lightwidget/cfw...
File xfa/fwl/lightwidget/cfwl_widget.cpp (right):

https://codereview.chromium.org/1928963004/diff/60001/xfa/fwl/lightwidget/cfw...
xfa/fwl/lightwidget/cfwl_widget.cpp:130: if (!m_pIface)
nit: maybe just
 if (m_pIface)
  m_pIface->SetStates(...);

[dsinclair, 2016-05-02 13:43:30]

https://codereview.chromium.org/1928963004/diff/60001/xfa/fwl/lightwidget/cfw...
File xfa/fwl/lightwidget/cfwl_widget.cpp (right):

https://codereview.chromium.org/1928963004/diff/60001/xfa/fwl/lightwidget/cfw...
xfa/fwl/lightwidget/cfwl_widget.cpp:130: if (!m_pIface)
On 2016/04/28 23:40:32, Tom Sepez wrote:
> nit: maybe just
>  if (m_pIface)
>   m_pIface->SetStates(...);

Done.

[dsinclair, 2016-05-02 13:50:57]

The CQ bit was checked by dsinclair@chromium.org

[dsinclair, 2016-05-02 13:50:57]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org
Link to the patchset: https://codereview.chromium.org/1928963004/#ps100001
(title: " ")

[commit-bot: I haz the power, 2016-05-02 13:51:06]

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

[commit-bot: I haz the power, 2016-05-02 14:01:26]

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

[commit-bot: I haz the power, 2016-05-02 14:01:27]

Try jobs failed on following builders:
  mac_xfa on master.tryserver.client.pdfium (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.pdfium/builders/mac_xfa/builds/461)

[dsinclair, 2016-05-02 14:15:06]

The CQ bit was checked by dsinclair@chromium.org

[dsinclair, 2016-05-02 14:15:06]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org
Link to the patchset: https://codereview.chromium.org/1928963004/#ps120001
(title: " ")

[commit-bot: I haz the power, 2016-05-02 14:15:11]

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

[dsinclair, 2016-05-02 14:28:24]

The CQ bit was unchecked by dsinclair@chromium.org

[dsinclair, 2016-05-02 14:41:20]

PTAL.

Had to make some changes to fix win and mac builds.

[dsinclair, 2016-05-03 13:34:38]

On 2016/05/02 14:41:20, dsinclair wrote:
> PTAL.
> 
> Had to make some changes to fix win and mac builds.


ping.

[dsinclair, 2016-05-04 14:04:03]

On 2016/05/03 13:34:38, dsinclair wrote:
> On 2016/05/02 14:41:20, dsinclair wrote:
> > PTAL.
> > 
> > Had to make some changes to fix win and mac builds.
> 
> 
> ping.

ping

[Tom Sepez, 2016-05-04 20:45:32]

lgtm

[dsinclair, 2016-05-04 20:46:51]

The CQ bit was checked by dsinclair@chromium.org

[commit-bot: I haz the power, 2016-05-04 20:47:01]

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

[commit-bot: I haz the power, 2016-05-04 20:54:51]

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

[commit-bot: I haz the power, 2016-05-04 20:54:52]

Try jobs failed on following builders:
  linux on master.tryserver.client.pdfium (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.pdfium/builders/linux/builds/498)

[dsinclair, 2016-05-04 21:02:01]

The CQ bit was checked by dsinclair@chromium.org

[dsinclair, 2016-05-04 21:02:02]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org
Link to the patchset: https://codereview.chromium.org/1928963004/#ps180001
(title: "Rebase to master")

[commit-bot: I haz the power, 2016-05-04 21:02:11]

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

[commit-bot: I haz the power, 2016-05-04 21:24:55]

Description was changed from

==========
Cleanup IFWL_Adapter interfaces.

This CL removes:
  * IFWL_AdapterNative
  * IFWL_Adapter{Widget|Thread}Mgr
  * IFWL_WidgetMgrDelegate
  * CFWL_SDAdapter{Widget|Thread}Mgr

Methods which just returned have also been removed.
==========

to

==========
Cleanup IFWL_Adapter interfaces.

This CL removes:
  * IFWL_AdapterNative
  * IFWL_Adapter{Widget|Thread}Mgr
  * IFWL_WidgetMgrDelegate
  * CFWL_SDAdapter{Widget|Thread}Mgr

Methods which just returned have also been removed.

Committed:
https://pdfium.googlesource.com/pdfium/+/55fa356c8ce72c597e75fc591fc67597b1c0...
==========

[commit-bot: I haz the power, 2016-05-04 21:24:57]

Committed patchset #9 (id:180001) as
https://pdfium.googlesource.com/pdfium/+/55fa356c8ce72c597e75fc591fc67597b1c0...