Make code compile with clang_use_chrome_plugin (part I)

Make code compile with clang_use_chrome_plugin (part I)

This change contains files in core/fpdfapi 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/+/868150bd69f0f1f9472affc20deea0a9341bf22b

[Wei Li, 2016-06-13 17:51:09]

Description was changed from

==========
Make code compile with clang_use_chrome_plugin=true (part I)

This change contains files in core/fpdfapi 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 I)

This change contains files in core/fpdfapi 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
==========

[Wei Li, 2016-06-13 18:26:01]

Patchset #1 (id:1) has been deleted

[Wei Li, 2016-06-13 18:26:11]

Patchset #1 (id:20001) has been deleted

[Wei Li, 2016-06-13 19:26:44]

Patchset #1 (id:40001) has been deleted

[Wei Li, 2016-06-13 19:31:00]

Patchset #1 (id:60001) has been deleted

[Wei Li, 2016-06-13 20:11:32]

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

[Wei Li, 2016-06-13 20:11:32]

pls review, thanks

[dsinclair, 2016-06-13 20:24:08]

lgtm w/ nits

https://codereview.chromium.org/2060973002/diff/80001/core/fpdfapi/fpdf_font/...
File core/fpdfapi/fpdf_font/ttgsubtable.cpp (right):

https://codereview.chromium.org/2060973002/diff/80001/core/fpdfapi/fpdf_font/...
core/fpdfapi/fpdf_font/ttgsubtable.cpp:73: bool CFX_CTTGSUBTable::IsOk(void)
const {
nit: blank line above.

https://codereview.chromium.org/2060973002/diff/80001/core/fpdfapi/fpdf_font/...
File core/fpdfapi/fpdf_font/ttgsubtable.h (right):

https://codereview.chromium.org/2060973002/diff/80001/core/fpdfapi/fpdf_font/...
core/fpdfapi/fpdf_font/ttgsubtable.h:32: CFX_CTTGSUBTable(void);
nit: void not needed.

https://codereview.chromium.org/2060973002/diff/80001/core/fpdfapi/fpdf_font/...
core/fpdfapi/fpdf_font/ttgsubtable.h:33: CFX_CTTGSUBTable(FT_Bytes gsub);
nit: explicit

https://codereview.chromium.org/2060973002/diff/80001/core/fpdfapi/fpdf_font/...
core/fpdfapi/fpdf_font/ttgsubtable.h:76: ~TScript() { delete[] LangSysRecord; }
Can we add a todo to go through and update these to smart-pointers at some point
in the future?

https://codereview.chromium.org/2060973002/diff/80001/core/fpdfapi/fpdf_rende...
File core/fpdfapi/fpdf_render/render_int.h (right):

https://codereview.chromium.org/2060973002/diff/80001/core/fpdfapi/fpdf_rende...
core/fpdfapi/fpdf_render/render_int.h:610: CPDF_DIBTransferFunc(const
CPDF_TransferFunc* pTransferFunc);
nit: explicit

[Lei Zhang, 2016-06-13 20:41:37]

Description was changed from

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

This change contains files in core/fpdfapi 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 I)

This change contains files in core/fpdfapi 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
==========

[Lei Zhang, 2016-06-13 20:44:28]

https://codereview.chromium.org/2060973002/diff/80001/core/fpdfapi/fpdf_font/...
File core/fpdfapi/fpdf_font/font_int.h (left):

https://codereview.chromium.org/2060973002/diff/80001/core/fpdfapi/fpdf_font/...
core/fpdfapi/fpdf_font/font_int.h:143: FX_BOOL IsLoaded() const { return
m_bLoaded; }
Curious why these simple getters can't be inlined.

https://codereview.chromium.org/2060973002/diff/80001/core/fpdfapi/fpdf_font/...
File core/fpdfapi/fpdf_font/ttgsubtable.h (right):

https://codereview.chromium.org/2060973002/diff/80001/core/fpdfapi/fpdf_font/...
core/fpdfapi/fpdf_font/ttgsubtable.h:32: CFX_CTTGSUBTable(void);
On 2016/06/13 20:24:08, dsinclair wrote:
> nit: void not needed.

We probably should do a sweep for these separately.

https://codereview.chromium.org/2060973002/diff/80001/core/fpdfapi/fpdf_font/...
core/fpdfapi/fpdf_font/ttgsubtable.h:34: virtual ~CFX_CTTGSUBTable();
Does this need to be virtual?

[Lei Zhang, 2016-06-13 20:48:43]

lgtm

https://codereview.chromium.org/2060973002/diff/80001/core/fpdfapi/fpdf_page/...
File core/fpdfapi/fpdf_page/cpdf_shadingpattern.cpp (right):

https://codereview.chromium.org/2060973002/diff/80001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/cpdf_shadingpattern.cpp:52: CPDF_TilingPattern*
CPDF_ShadingPattern::AsTilingPattern() {
need blank line above

[Wei Li, 2016-06-13 21:38:04]

thanks

https://codereview.chromium.org/2060973002/diff/80001/core/fpdfapi/fpdf_font/...
File core/fpdfapi/fpdf_font/font_int.h (left):

https://codereview.chromium.org/2060973002/diff/80001/core/fpdfapi/fpdf_font/...
core/fpdfapi/fpdf_font/font_int.h:143: FX_BOOL IsLoaded() const { return
m_bLoaded; }
On 2016/06/13 20:44:28, Lei Zhang wrote:
> Curious why these simple getters can't be inlined.

These are not needed by clang plugins, but to make things
consistent as they are the only two in the file used inline
functions although some constructors/distructors and
CPDF_CID2UnicodeMap::IsLoaded() are small enough also.

https://codereview.chromium.org/2060973002/diff/80001/core/fpdfapi/fpdf_font/...
File core/fpdfapi/fpdf_font/ttgsubtable.cpp (right):

https://codereview.chromium.org/2060973002/diff/80001/core/fpdfapi/fpdf_font/...
core/fpdfapi/fpdf_font/ttgsubtable.cpp:73: bool CFX_CTTGSUBTable::IsOk(void)
const {
On 2016/06/13 20:24:07, dsinclair wrote:
> nit: blank line above.

Done.

https://codereview.chromium.org/2060973002/diff/80001/core/fpdfapi/fpdf_font/...
File core/fpdfapi/fpdf_font/ttgsubtable.h (right):

https://codereview.chromium.org/2060973002/diff/80001/core/fpdfapi/fpdf_font/...
core/fpdfapi/fpdf_font/ttgsubtable.h:32: CFX_CTTGSUBTable(void);
On 2016/06/13 20:24:08, dsinclair wrote:
> nit: void not needed.

Done.

https://codereview.chromium.org/2060973002/diff/80001/core/fpdfapi/fpdf_font/...
core/fpdfapi/fpdf_font/ttgsubtable.h:32: CFX_CTTGSUBTable(void);
On 2016/06/13 20:44:28, Lei Zhang wrote:
> On 2016/06/13 20:24:08, dsinclair wrote:
> > nit: void not needed.
> 
> We probably should do a sweep for these separately.

Created bug 519 to track the efforts.

https://codereview.chromium.org/2060973002/diff/80001/core/fpdfapi/fpdf_font/...
core/fpdfapi/fpdf_font/ttgsubtable.h:33: CFX_CTTGSUBTable(FT_Bytes gsub);
On 2016/06/13 20:24:08, dsinclair wrote:
> nit: explicit

Done.

https://codereview.chromium.org/2060973002/diff/80001/core/fpdfapi/fpdf_font/...
core/fpdfapi/fpdf_font/ttgsubtable.h:34: virtual ~CFX_CTTGSUBTable();
On 2016/06/13 20:44:28, Lei Zhang wrote:
> Does this need to be virtual?

Probably not needed since this class is not inherited. But no harm either so
leave it for now.

https://codereview.chromium.org/2060973002/diff/80001/core/fpdfapi/fpdf_font/...
core/fpdfapi/fpdf_font/ttgsubtable.h:76: ~TScript() { delete[] LangSysRecord; }
On 2016/06/13 20:24:08, dsinclair wrote:
> Can we add a todo to go through and update these to smart-pointers at some
point
> in the future?

Done.

https://codereview.chromium.org/2060973002/diff/80001/core/fpdfapi/fpdf_page/...
File core/fpdfapi/fpdf_page/cpdf_shadingpattern.cpp (right):

https://codereview.chromium.org/2060973002/diff/80001/core/fpdfapi/fpdf_page/...
core/fpdfapi/fpdf_page/cpdf_shadingpattern.cpp:52: CPDF_TilingPattern*
CPDF_ShadingPattern::AsTilingPattern() {
On 2016/06/13 20:48:41, Lei Zhang wrote:
> need blank line above

Done.

https://codereview.chromium.org/2060973002/diff/80001/core/fpdfapi/fpdf_rende...
File core/fpdfapi/fpdf_render/render_int.h (right):

https://codereview.chromium.org/2060973002/diff/80001/core/fpdfapi/fpdf_rende...
core/fpdfapi/fpdf_render/render_int.h:610: CPDF_DIBTransferFunc(const
CPDF_TransferFunc* pTransferFunc);
On 2016/06/13 20:24:08, dsinclair wrote:
> nit: explicit

Done.

[Wei Li, 2016-06-13 21:56:55]

The CQ bit was checked by weili@chromium.org

[Wei Li, 2016-06-13 21:56:55]

The patchset sent to the CQ was uploaded after l-g-t-m from
thestig@chromium.org, dsinclair@chromium.org
Link to the patchset: https://codereview.chromium.org/2060973002/#ps100001
(title: "address comments")

[commit-bot: I haz the power, 2016-06-13 21:57:02]

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

[commit-bot: I haz the power, 2016-06-13 21:57:35]

Description was changed from

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

This change contains files in core/fpdfapi 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 I)

This change contains files in core/fpdfapi 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/+/868150bd69f0f1f9472affc20deea0a9341b...
==========

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

Committed patchset #2 (id:100001) as
https://pdfium.googlesource.com/pdfium/+/868150bd69f0f1f9472affc20deea0a9341b...