Change class member variables in raw pointer type into unique_ptr

Change class member variables in raw pointer type into unique_ptr

Also did some cleanups such as removing an unused member variables and some unused structs.

BUG=pdfium:518
Committed: https://pdfium.googlesource.com/pdfium/+/44105d862bfcaf9fce0ee0dfe283337bf5980337

[Wei Li, 2016-07-02 01:00:23]

Description was changed from

==========
Change class member variables in raw pointer type into unique_ptr

Also did some cleanups such as removing an unused member variables.

BUG=pdfium:518
==========

to

==========
Change class member variables in raw pointer type into unique_ptr

Also did some cleanups such as removing an unused member variables and some
unused structs.

BUG=pdfium:518
==========

[Wei Li, 2016-07-02 01:01:23]

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

[Wei Li, 2016-07-02 01:01:23]

for review, thanks

[Lei Zhang, 2016-07-02 02:05:34]

Still got a couple of red bots.

[Lei Zhang, 2016-07-02 02:07:53]

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

https://codereview.chromium.org/2119013002/diff/40001/core/fpdfapi/fpdf_font/...
core/fpdfapi/fpdf_font/ttgsubtable.h:55: std::unique_ptr<uint16_t[]>
FeatureIndex;
std::vector?

[Wei Li, 2016-07-06 22:48:08]

Fixed red bots, PTAL, thanks

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

https://codereview.chromium.org/2119013002/diff/40001/core/fpdfapi/fpdf_font/...
core/fpdfapi/fpdf_font/ttgsubtable.h:55: std::unique_ptr<uint16_t[]>
FeatureIndex;
On 2016/07/02 02:07:53, Lei Zhang wrote:
> std::vector?

Both would do. Since these are simple dynamic arrays which don't need
size change or other operations, unique_ptrs to arrays would suffice.

[Lei Zhang, 2016-07-06 23:19:56]

lgtm, maybe we can do std::vector some other time.

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

https://codereview.chromium.org/2119013002/diff/40001/core/fpdfapi/fpdf_font/...
core/fpdfapi/fpdf_font/ttgsubtable.h:55: std::unique_ptr<uint16_t[]>
FeatureIndex;
On 2016/07/06 22:48:07, Wei Li wrote:
> On 2016/07/02 02:07:53, Lei Zhang wrote:
> > std::vector?
> 
> Both would do. Since these are simple dynamic arrays which don't need
> size change or other operations, unique_ptrs to arrays would suffice.

std::vector has some small advantages:
- Linux debug builds check array indexing at runtime. (Sure, we have ASAN)
- One can initialize in the std::vector ctor, instead of using a separate
memset() call.
- You can get rid of the FeatureCount member.
- May be able to use more easily use range-based for loops.

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

https://codereview.chromium.org/2119013002/diff/80001/core/fpdfapi/fpdf_font/...
core/fpdfapi/fpdf_font/ttgsubtable.cpp:105: .LangSys.FeatureIndex[k];
If we have a reference variable at each level of the for-loop, this will look
better.

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

https://codereview.chromium.org/2119013002/diff/80001/core/fpdfapi/fpdf_font/...
core/fpdfapi/fpdf_font/ttgsubtable.h:207: virtual ~TSubTableBase() {}
BTW, does this need to be virtual? There are subclasses, but no virtual
functions. Also ~TCoverageFormatBase above.

[Lei Zhang, 2016-07-06 23:20:27]

On 2016/07/06 23:19:56, Lei Zhang wrote:
> - May be able to use more easily use range-based for loops.

"May be able to more easily use ..."

[Wei Li, 2016-07-07 00:03:35]

thanks

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

https://codereview.chromium.org/2119013002/diff/40001/core/fpdfapi/fpdf_font/...
core/fpdfapi/fpdf_font/ttgsubtable.h:55: std::unique_ptr<uint16_t[]>
FeatureIndex;
On 2016/07/06 23:19:56, Lei Zhang wrote:
> On 2016/07/06 22:48:07, Wei Li wrote:
> > On 2016/07/02 02:07:53, Lei Zhang wrote:
> > > std::vector?
> > 
> > Both would do. Since these are simple dynamic arrays which don't need
> > size change or other operations, unique_ptrs to arrays would suffice.
> 
> std::vector has some small advantages:
> - Linux debug builds check array indexing at runtime. (Sure, we have ASAN)
> - One can initialize in the std::vector ctor, instead of using a separate
> memset() call.
> - You can get rid of the FeatureCount member.
> - May be able to use more easily use range-based for loops.

In this case, I think getting rid of the count would be one benefit. But it may
cost slightly more memory. Let's defer this later.

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

https://codereview.chromium.org/2119013002/diff/80001/core/fpdfapi/fpdf_font/...
core/fpdfapi/fpdf_font/ttgsubtable.cpp:105: .LangSys.FeatureIndex[k];
On 2016/07/06 23:19:56, Lei Zhang wrote:
> If we have a reference variable at each level of the for-loop, this will look
> better.

Done.

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

https://codereview.chromium.org/2119013002/diff/80001/core/fpdfapi/fpdf_font/...
core/fpdfapi/fpdf_font/ttgsubtable.h:207: virtual ~TSubTableBase() {}
On 2016/07/06 23:19:56, Lei Zhang wrote:
> BTW, does this need to be virtual? There are subclasses, but no virtual
> functions. Also ~TCoverageFormatBase above.

yes since the destructors of subclasses are virtual functions too. This is to
ensure proper destructor is called.

[Wei Li, 2016-07-07 00:47:57]

The CQ bit was checked by weili@chromium.org

[Wei Li, 2016-07-07 00:47:58]

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/2119013002/#ps100001
(title: "address comments")

[commit-bot: I haz the power, 2016-07-07 00:48:07]

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-07-07 01:01:41]

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

[commit-bot: I haz the power, 2016-07-07 01:01:42]

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

[Lei Zhang, 2016-07-07 01:07:06]

The CQ bit was checked by thestig@chromium.org

[commit-bot: I haz the power, 2016-07-07 01:07:11]

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-07-07 01:10:39]

Description was changed from

==========
Change class member variables in raw pointer type into unique_ptr

Also did some cleanups such as removing an unused member variables and some
unused structs.

BUG=pdfium:518
==========

to

==========
Change class member variables in raw pointer type into unique_ptr

Also did some cleanups such as removing an unused member variables and some
unused structs.

BUG=pdfium:518

Committed:
https://pdfium.googlesource.com/pdfium/+/44105d862bfcaf9fce0ee0dfe283337bf598...
==========

[commit-bot: I haz the power, 2016-07-07 01:10:40]

Committed patchset #6 (id:100001) as
https://pdfium.googlesource.com/pdfium/+/44105d862bfcaf9fce0ee0dfe283337bf598...