Cleanup ownership in CXFA_ScriptContext.

Cleanup ownership in CXFA_ScriptContext.

This Cl substitutes some unique_ptrs for various bits of manual memory
management in CXFA_ScriptContext.

Committed: https://pdfium.googlesource.com/pdfium/+/ee2abec93f22bd10522181dc0362f24d389fc66b

[dsinclair, 2016-06-14 15:55:09]

Patchset #1 (id:1) has been deleted

[dsinclair, 2016-06-14 19:01:34]

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

[dsinclair, 2016-06-14 19:01:35]

PTAL.

https://codereview.chromium.org/2062113002/diff/20001/xfa/fxfa/parser/xfa_scr...
File xfa/fxfa/parser/xfa_script_imp.cpp (right):

https://codereview.chromium.org/2062113002/diff/20001/xfa/fxfa/parser/xfa_scr...
xfa/fxfa/parser/xfa_script_imp.cpp:102: delete
ToThisProxy(pVariableContext->GetGlobalObject().get(), nullptr);
Don't think we can easily convert this to std::map because the delete deletes an
object that we get from the context, not the context itself.

[Lei Zhang, 2016-06-14 19:10:17]

Description was changed from

==========
Cleanup ownership in CXFA_ScriptContext.

This Cl substitues some unique_ptrs for various bits of manual memory management
in CXFA_ScriptContext.
==========

to

==========
Cleanup ownership in CXFA_ScriptContext.

This Cl substitutes some unique_ptrs for various bits of manual memory
management in CXFA_ScriptContext.
==========

[Lei Zhang, 2016-06-14 19:29:13]

https://codereview.chromium.org/2062113002/diff/20001/xfa/fxfa/parser/xfa_scr...
File xfa/fxfa/parser/xfa_script_imp.cpp (right):

https://codereview.chromium.org/2062113002/diff/20001/xfa/fxfa/parser/xfa_scr...
xfa/fxfa/parser/xfa_script_imp.cpp:84: m_JsContext(nullptr),
No need to initialize an unique_ptr. Ditto below.

https://codereview.chromium.org/2062113002/diff/20001/xfa/fxfa/parser/xfa_scr...
xfa/fxfa/parser/xfa_script_imp.cpp:102: delete
ToThisProxy(pVariableContext->GetGlobalObject().get(), nullptr);
On 2016/06/14 19:01:35, dsinclair wrote:
> Don't think we can easily convert this to std::map because the delete deletes
an
> object that we get from the context, not the context itself.

Are you referring to |m_mapVariableToContext|? Did you mean it's hard to convert
to a std::map that holds unique_ptrs?

https://codereview.chromium.org/2062113002/diff/20001/xfa/fxfa/parser/xfa_scr...
File xfa/fxfa/parser/xfa_script_imp.h (right):

https://codereview.chromium.org/2062113002/diff/20001/xfa/fxfa/parser/xfa_scr...
xfa/fxfa/parser/xfa_script_imp.h:42: // This takes ownership of |pList|
No need, already implied by unique_ptr.

https://codereview.chromium.org/2062113002/diff/20001/xfa/fxfa/parser/xfa_scr...
xfa/fxfa/parser/xfa_script_imp.h:44: m_CacheList.push_back(std::move(pList));
The Clang plugin may complain this is complex and belong in the .cpp file.

[dsinclair, 2016-06-14 20:19:43]

https://codereview.chromium.org/2062113002/diff/20001/xfa/fxfa/parser/xfa_scr...
File xfa/fxfa/parser/xfa_script_imp.cpp (right):

https://codereview.chromium.org/2062113002/diff/20001/xfa/fxfa/parser/xfa_scr...
xfa/fxfa/parser/xfa_script_imp.cpp:84: m_JsContext(nullptr),
On 2016/06/14 19:29:12, Lei Zhang wrote:
> No need to initialize an unique_ptr. Ditto below.

Done.

https://codereview.chromium.org/2062113002/diff/20001/xfa/fxfa/parser/xfa_scr...
xfa/fxfa/parser/xfa_script_imp.cpp:102: delete
ToThisProxy(pVariableContext->GetGlobalObject().get(), nullptr);
On 2016/06/14 19:29:12, Lei Zhang wrote:
> On 2016/06/14 19:01:35, dsinclair wrote:
> > Don't think we can easily convert this to std::map because the delete
deletes
> an
> > object that we get from the context, not the context itself.
> 
> Are you referring to |m_mapVariableToContext|? Did you mean it's hard to
convert
> to a std::map that holds unique_ptrs?

Sorry, yea, converting m_mapVariableToContext to a std::map that holds
std::unique_ptrs is difficult is what I'm trying to say.

https://codereview.chromium.org/2062113002/diff/20001/xfa/fxfa/parser/xfa_scr...
File xfa/fxfa/parser/xfa_script_imp.h (right):

https://codereview.chromium.org/2062113002/diff/20001/xfa/fxfa/parser/xfa_scr...
xfa/fxfa/parser/xfa_script_imp.h:42: // This takes ownership of |pList|
On 2016/06/14 19:29:12, Lei Zhang wrote:
> No need, already implied by unique_ptr.

Done.

https://codereview.chromium.org/2062113002/diff/20001/xfa/fxfa/parser/xfa_scr...
xfa/fxfa/parser/xfa_script_imp.h:44: m_CacheList.push_back(std::move(pList));
On 2016/06/14 19:29:12, Lei Zhang wrote:
> The Clang plugin may complain this is complex and belong in the .cpp file.

Moved.

[Lei Zhang, 2016-06-14 20:24:41]

lgtm

[dsinclair, 2016-06-14 20:26:44]

The CQ bit was checked by dsinclair@chromium.org

[commit-bot: I haz the power, 2016-06-14 20:26:53]

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

[dsinclair, 2016-06-14 21:07:21]

The CQ bit was unchecked by dsinclair@chromium.org

[dsinclair, 2016-06-14 21:07:24]

The CQ bit was checked by dsinclair@chromium.org

[commit-bot: I haz the power, 2016-06-14 21:07:26]

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

[commit-bot: I haz the power, 2016-06-14 21:36:55]

Description was changed from

==========
Cleanup ownership in CXFA_ScriptContext.

This Cl substitutes some unique_ptrs for various bits of manual memory
management in CXFA_ScriptContext.
==========

to

==========
Cleanup ownership in CXFA_ScriptContext.

This Cl substitutes some unique_ptrs for various bits of manual memory
management in CXFA_ScriptContext.

Committed:
https://pdfium.googlesource.com/pdfium/+/ee2abec93f22bd10522181dc0362f24d389f...
==========

[commit-bot: I haz the power, 2016-06-14 21:36:56]

Committed patchset #2 (id:40001) as
https://pdfium.googlesource.com/pdfium/+/ee2abec93f22bd10522181dc0362f24d389f...