Use std::unique_ptr in CXFA_FMParse.

Use std::unique_ptr in CXFA_FMParse.

R=dsinclair@chromium.org

Committed: https://pdfium.googlesource.com/pdfium/+/5870b76eab54b30491e1dc35ce09a308462af29a

[Lei Zhang, 2016-02-25 18:49:12]

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

[dsinclair, 2016-02-25 19:36:58]

https://codereview.chromium.org/1740643002/diff/1/xfa/src/fxfa/src/fm2js/xfa_...
File xfa/src/fxfa/src/fm2js/xfa_fmparse.cpp (left):

https://codereview.chromium.org/1740643002/diff/1/xfa/src/fxfa/src/fm2js/xfa_...
xfa/src/fxfa/src/fm2js/xfa_fmparse.cpp:146: pExpressions->RemoveAll();
Is this needed? Does the pExpressions destructor clear un-removed elements or
anything?

https://codereview.chromium.org/1740643002/diff/1/xfa/src/fxfa/src/fm2js/xfa_...
xfa/src/fxfa/src/fm2js/xfa_fmparse.cpp:892: expression->RemoveAll();
RemoveAll() needed?

https://codereview.chromium.org/1740643002/diff/1/xfa/src/fxfa/src/fm2js/xfa_...
File xfa/src/fxfa/src/fm2js/xfa_fmparse.cpp (right):

https://codereview.chromium.org/1740643002/diff/1/xfa/src/fxfa/src/fm2js/xfa_...
xfa/src/fxfa/src/fm2js/xfa_fmparse.cpp:660: return nullptr;
Should this do e = nullptr; (and below) since it's passed in? Need to reset it
for the caller.

https://codereview.chromium.org/1740643002/diff/1/xfa/src/fxfa/src/fm2js/xfa_...
xfa/src/fxfa/src/fm2js/xfa_fmparse.cpp:915: e.reset();
e is never set, so if you do:

if (m_pErrorInfo->message.IsEmpty()) {
  e.reset(new CXFA_FMWhileExpression(line, pCondition.release(),
                                          pExpression.release()));
}
return e.release();

It should be fine.

[Lei Zhang, 2016-02-25 20:22:54]

https://codereview.chromium.org/1740643002/diff/1/xfa/src/fxfa/src/fm2js/xfa_...
File xfa/src/fxfa/src/fm2js/xfa_fmparse.cpp (left):

https://codereview.chromium.org/1740643002/diff/1/xfa/src/fxfa/src/fm2js/xfa_...
xfa/src/fxfa/src/fm2js/xfa_fmparse.cpp:146: pExpressions->RemoveAll();
On 2016/02/25 19:36:57, dsinclair wrote:
> Is this needed? Does the pExpressions destructor clear un-removed elements or
> anything?

We just deleted all int eh pointers in the array above. This just clears the
array, but we'll do that anyway in the dtor.

https://codereview.chromium.org/1740643002/diff/1/xfa/src/fxfa/src/fm2js/xfa_...
xfa/src/fxfa/src/fm2js/xfa_fmparse.cpp:892: expression->RemoveAll();
On 2016/02/25 19:36:57, dsinclair wrote:
> RemoveAll() needed?

No, same reason as above.

https://codereview.chromium.org/1740643002/diff/1/xfa/src/fxfa/src/fm2js/xfa_...
File xfa/src/fxfa/src/fm2js/xfa_fmparse.cpp (right):

https://codereview.chromium.org/1740643002/diff/1/xfa/src/fxfa/src/fm2js/xfa_...
xfa/src/fxfa/src/fm2js/xfa_fmparse.cpp:660: return nullptr;
On 2016/02/25 19:36:57, dsinclair wrote:
> Should this do e = nullptr; (and below) since it's passed in? Need to reset it
> for the caller.

It's not passed in by ref, so it has no effect on the caller, right?

https://codereview.chromium.org/1740643002/diff/1/xfa/src/fxfa/src/fm2js/xfa_...
xfa/src/fxfa/src/fm2js/xfa_fmparse.cpp:915: e.reset();
On 2016/02/25 19:36:57, dsinclair wrote:
> e is never set, so if you do:
> 
> if (m_pErrorInfo->message.IsEmpty()) {
>   e.reset(new CXFA_FMWhileExpression(line, pCondition.release(),
>                                           pExpression.release()));
> }
> return e.release();
> 
> It should be fine.

Done.

[dsinclair, 2016-02-25 20:25:26]

lgtm

[Lei Zhang, 2016-02-25 20:40:25]

Description was changed from

==========
Use std::unique_ptr in CXFA_FMParse.
==========

to

==========
Use std::unique_ptr in CXFA_FMParse.

R=dsinclair@chromium.org

Committed:
https://pdfium.googlesource.com/pdfium/+/5870b76eab54b30491e1dc35ce09a308462a...
==========

[Lei Zhang, 2016-02-25 20:40:27]

Committed patchset #2 (id:20001) manually as
5870b76eab54b30491e1dc35ce09a308462af29a (presubmit successful).