Make code compile with clang_use_chrome_plugin (part VI)

Make code compile with clang_use_chrome_plugin (part VI)

This change mainly contains files in xfa/fwl directory.
This is part of the efforts to make PDFium code compilable
by Clang chromium style plugins.

The changes are mainly the following:
-- move inline constructor/destructor of complex class/struct out-of-line;
-- add constructor/destructor of complex class/struct if not
 explicitly defined;
-- add explicit out-of-line copy constructor when needed;
-- move inline virtual functions out-of-line;
-- Properly mark virtual functions with 'override';
-- some minor cleanups;

BUG=pdfium:469
Committed: https://pdfium.googlesource.com/pdfium/+/4ce94e118d66064715de5baebeb4b2b580dcac66

[Wei Li, 2016-06-16 17:03:45]

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

[Wei Li, 2016-06-16 17:03:46]

pls review, thanks

[dsinclair, 2016-06-16 17:35:20]

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

https://codereview.chromium.org/2070583003/diff/20001/xfa/fwl/core/cfwl_event...
xfa/fwl/core/cfwl_event.h:112: __VA_ARGS__                                      
              \
That's frighteningly awesome.

https://codereview.chromium.org/2070583003/diff/20001/xfa/fwl/core/cfwl_event...
xfa/fwl/core/cfwl_event.h:113: };  // NOLINT
Does this need a nolint? It was there before because the ; was on its own line.

https://codereview.chromium.org/2070583003/diff/20001/xfa/fwl/core/cfwl_event...
xfa/fwl/core/cfwl_event.h:115: #define FWL_EVENT_FUNCTION_DEF(classname,
eventType)                        \
Do we ever doe the above without doing this macro? If not, can we just combine
them and have one call?

https://codereview.chromium.org/2070583003/diff/20001/xfa/fwl/core/cfwl_messa...
File xfa/fwl/core/cfwl_message.h (right):

https://codereview.chromium.org/2070583003/diff/20001/xfa/fwl/core/cfwl_messa...
xfa/fwl/core/cfwl_message.h:118: #define FWL_MESSAGE_FUNCTION_DEF(classname,
msgType, ...)                   \
ditto, can they be combined?

[Wei Li, 2016-06-16 18:16:33]

thanks

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

https://codereview.chromium.org/2070583003/diff/20001/xfa/fwl/core/cfwl_event...
xfa/fwl/core/cfwl_event.h:112: __VA_ARGS__                                      
              \
On 2016/06/16 17:35:19, dsinclair wrote:
> That's frighteningly awesome.

thanks :)

https://codereview.chromium.org/2070583003/diff/20001/xfa/fwl/core/cfwl_event...
xfa/fwl/core/cfwl_event.h:113: };  // NOLINT
On 2016/06/16 17:35:20, dsinclair wrote:
> Does this need a nolint? It was there before because the ; was on its own
line.

Done.

https://codereview.chromium.org/2070583003/diff/20001/xfa/fwl/core/cfwl_event...
xfa/fwl/core/cfwl_event.h:115: #define FWL_EVENT_FUNCTION_DEF(classname,
eventType)                        \
On 2016/06/16 17:35:19, dsinclair wrote:
> Do we ever doe the above without doing this macro? If not, can we just combine
> them and have one call?

This is the price we have to pay for using clang_use_chrome_plugin. The plugins
complain when
we inline those virtual functions in the class.
However, the plugins makes an exception for explicitly inlined functions.

I have not found a better way to work around this.

https://codereview.chromium.org/2070583003/diff/20001/xfa/fwl/core/cfwl_messa...
File xfa/fwl/core/cfwl_message.h (right):

https://codereview.chromium.org/2070583003/diff/20001/xfa/fwl/core/cfwl_messa...
xfa/fwl/core/cfwl_message.h:118: #define FWL_MESSAGE_FUNCTION_DEF(classname,
msgType, ...)                   \
On 2016/06/16 17:35:20, dsinclair wrote:
> ditto, can they be combined?

Maybe not, this is to give Macro user the flexibility to add code in both class
definition and function definition. See line 180 for a use case.

[dsinclair, 2016-06-16 18:21:13]

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

https://codereview.chromium.org/2070583003/diff/20001/xfa/fwl/core/cfwl_event...
xfa/fwl/core/cfwl_event.h:115: #define FWL_EVENT_FUNCTION_DEF(classname,
eventType)                        \
On 2016/06/16 18:16:33, Wei Li wrote:
> On 2016/06/16 17:35:19, dsinclair wrote:
> > Do we ever doe the above without doing this macro? If not, can we just
combine
> > them and have one call?
> 
> This is the price we have to pay for using clang_use_chrome_plugin. The
plugins
> complain when
> we inline those virtual functions in the class.
> However, the plugins makes an exception for explicitly inlined functions.
> 
> I have not found a better way to work around this.

Right, but could the macro be:

#define FWL_EVENT_DEF(classname, eventType, ...)\
  class classname : public CFWL_Event { \
   public: \
    classname(); \
    ... \
  }; \ 
  inline classname::classname() {} \
  inline classname::~classname() {} \
  ...

This won't work for the message macros because you use VA_ARGS in both macros,
but should work with this one, no?

https://codereview.chromium.org/2070583003/diff/20001/xfa/fwl/core/cfwl_messa...
File xfa/fwl/core/cfwl_message.h (right):

https://codereview.chromium.org/2070583003/diff/20001/xfa/fwl/core/cfwl_messa...
xfa/fwl/core/cfwl_message.h:118: #define FWL_MESSAGE_FUNCTION_DEF(classname,
msgType, ...)                   \
On 2016/06/16 18:16:33, Wei Li wrote:
> On 2016/06/16 17:35:20, dsinclair wrote:
> > ditto, can they be combined?
> 
> Maybe not, this is to give Macro user the flexibility to add code in both
class
> definition and function definition. See line 180 for a use case.


Doh, missed the VA_ARGS used in both macros.

[Wei Li, 2016-06-17 18:11:18]

ptal, thanks

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

https://codereview.chromium.org/2070583003/diff/20001/xfa/fwl/core/cfwl_event...
xfa/fwl/core/cfwl_event.h:115: #define FWL_EVENT_FUNCTION_DEF(classname,
eventType)                        \
On 2016/06/16 18:21:13, dsinclair wrote:
> On 2016/06/16 18:16:33, Wei Li wrote:
> > On 2016/06/16 17:35:19, dsinclair wrote:
> > > Do we ever doe the above without doing this macro? If not, can we just
> combine
> > > them and have one call?
> > 
> > This is the price we have to pay for using clang_use_chrome_plugin. The
> plugins
> > complain when
> > we inline those virtual functions in the class.
> > However, the plugins makes an exception for explicitly inlined functions.
> > 
> > I have not found a better way to work around this.
> 
> Right, but could the macro be:
> 
> #define FWL_EVENT_DEF(classname, eventType, ...)\
>   class classname : public CFWL_Event { \
>    public: \
>     classname(); \
>     ... \
>   }; \ 
>   inline classname::classname() {} \
>   inline classname::~classname() {} \
>   ...
> 
> This won't work for the message macros because you use VA_ARGS in both macros,
> but should work with this one, no?
> 

oh, since customized function implementation is not needed for this macro, I can
directly merge them.

https://codereview.chromium.org/2070583003/diff/20001/xfa/fwl/core/cfwl_messa...
File xfa/fwl/core/cfwl_message.h (right):

https://codereview.chromium.org/2070583003/diff/20001/xfa/fwl/core/cfwl_messa...
xfa/fwl/core/cfwl_message.h:118: #define FWL_MESSAGE_FUNCTION_DEF(classname,
msgType, ...)                   \
On 2016/06/16 18:21:13, dsinclair wrote:
> On 2016/06/16 18:16:33, Wei Li wrote:
> > On 2016/06/16 17:35:20, dsinclair wrote:
> > > ditto, can they be combined?
> > 
> > Maybe not, this is to give Macro user the flexibility to add code in both
> class
> > definition and function definition. See line 180 for a use case.
> 
> 
> Doh, missed the VA_ARGS used in both macros.

Found a way to make this better. :) thanks.

[dsinclair, 2016-06-18 13:21:29]

The CQ bit was checked by dsinclair@chromium.org

[dsinclair, 2016-06-18 13:21:30]

lgtm

Awesome.

[commit-bot: I haz the power, 2016-06-18 13:21:36]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2070583003/80001

[commit-bot: I haz the power, 2016-06-18 13:22:04]

Description was changed from

==========
Make code compile with clang_use_chrome_plugin (part VI)

This change mainly contains files in xfa/fwl directory.
This is part of the efforts to make PDFium code compilable
by Clang chromium style plugins.

The changes are mainly the following:
-- move inline constructor/destructor of complex class/struct out-of-line;
-- add constructor/destructor of complex class/struct if not
 explicitly defined;
-- add explicit out-of-line copy constructor when needed;
-- move inline virtual functions out-of-line;
-- Properly mark virtual functions with 'override';
-- some minor cleanups;

BUG=pdfium:469
==========

to

==========
Make code compile with clang_use_chrome_plugin (part VI)

This change mainly contains files in xfa/fwl directory.
This is part of the efforts to make PDFium code compilable
by Clang chromium style plugins.

The changes are mainly the following:
-- move inline constructor/destructor of complex class/struct out-of-line;
-- add constructor/destructor of complex class/struct if not
 explicitly defined;
-- add explicit out-of-line copy constructor when needed;
-- move inline virtual functions out-of-line;
-- Properly mark virtual functions with 'override';
-- some minor cleanups;

BUG=pdfium:469

Committed:
https://pdfium.googlesource.com/pdfium/+/4ce94e118d66064715de5baebeb4b2b580dc...
==========

[commit-bot: I haz the power, 2016-06-18 13:22:05]

Committed patchset #5 (id:80001) as
https://pdfium.googlesource.com/pdfium/+/4ce94e118d66064715de5baebeb4b2b580dc...