Remove dubious cast in CXFA_ScriptContext::AddJSBuiltinObject()

Remove dubious cast in CXFA_ScriptContext::AddJSBuiltinObject()

BUG=pdfium:354
R=tsepez@chromium.org

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

[jun_fang, 2016-01-12 16:07:06]

jun_fang@foxitsoftware.com changed reviewers:
+ thestig@chromium.org, tsepez@chromium.org

[jun_fang, 2016-01-12 16:07:07]

Hi Tom and Lei,

Please review this CL. Thanks!

[jun_fang, 2016-01-12 16:23:24]

https://codereview.chromium.org/1580773002/diff/60001/xfa/src/fxfa/src/parser...
File xfa/src/fxfa/src/parser/xfa_script_imp.cpp (left):

https://codereview.chromium.org/1580773002/diff/60001/xfa/src/fxfa/src/parser...
xfa/src/fxfa/src/parser/xfa_script_imp.cpp:116:
m_JSBuiltInObjects.GetNextAssoc(ps, bsKey, pValue);
Here, we got risky pValue. Luckily, we didn't use it.

https://codereview.chromium.org/1580773002/diff/60001/xfa/src/fxfa/src/parser...
xfa/src/fxfa/src/parser/xfa_script_imp.cpp:118: if
(FXJSE_Value_GetObjectProp(hObject, bsKey, hProp)) {
If we can find this object on JSE, we replace the risky pValue with hProp at
line 119. Otherwise, we remove bsKey and pValue from m_JSBuiltInObjects at line
123.

https://codereview.chromium.org/1580773002/diff/60001/xfa/src/fxfa/src/parser...
xfa/src/fxfa/src/parser/xfa_script_imp.cpp:247: if
(lpScriptContext->QueryBuiltinHValue(szPropName, hValue)) {
If there is a built-in object, we stop handling it here. In this CL, we only
removed built-in objects from contexts after they were created.

https://codereview.chromium.org/1580773002/diff/60001/xfa/src/fxfa/src/parser...
xfa/src/fxfa/src/parser/xfa_script_imp.cpp:600:
m_JSBuiltInObjects.SetAt(pBuitinObject->pName, (void*)pBuitinObject);
It may not cause a real problem. It's a tricky at line 116.

[Tom Sepez, 2016-01-12 18:33:20]

lgtm

[jun_fang, 2016-01-13 16:17:45]

On 2016/01/12 18:33:20, Tom Sepez wrote:
> lgtm

I am working on a test case for this CL like a similar one requested in
https://codereview.chromium.org/1539193003/.

[jun_fang, 2016-01-15 13:47:40]

On 2016/01/13 16:17:45, jun_fang wrote:
> On 2016/01/12 18:33:20, Tom Sepez wrote:
> > lgtm
> 
> I am working on a test case for this CL like a similar one requested in
> https://codereview.chromium.org/1539193003/.

I created a xfa file and 4 corpus test cases in
https://codereview.chromium.org/1589293003/.
That XFA file can only be rendered on XFA enabled pdfium. This corpus test will
fail on
XFA disabled pdfium. Let's discuss how to design corpus tests for XFA in 
https://codereview.chromium.org/1589293003/.

[Tom Sepez, 2016-01-19 23:27:52]

On 2016/01/15 13:47:40, jun_fang wrote:
> On 2016/01/13 16:17:45, jun_fang wrote:
> > On 2016/01/12 18:33:20, Tom Sepez wrote:
> > > lgtm
> > 
> > I am working on a test case for this CL like a similar one requested in
> > https://codereview.chromium.org/1539193003/.
> 
> I created a xfa file and 4 corpus test cases in
> https://codereview.chromium.org/1589293003/.
> That XFA file can only be rendered on XFA enabled pdfium. This corpus test
will
> fail on
> XFA disabled pdfium. Let's discuss how to design corpus tests for XFA in 
> https://codereview.chromium.org/1589293003/.

The support for XFA on non-xfa has landed. See the testing/SUPRRESSION file for
details on the syntax.  For this test, you'd add a line to it that say to
suppress for "noxfa"

[jun_fang, 2016-01-20 02:41:10]

Description was changed from

==========
Remove dubious cast in CXFA_ScriptContext::AddJSBuiltinObject()

BUG=pdfium:354
==========

to

==========
Remove dubious cast in CXFA_ScriptContext::AddJSBuiltinObject()

BUG=pdfium:354
R=tsepez@chromium.org

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

[jun_fang, 2016-01-20 02:41:14]

Committed patchset #6 (id:100001) manually as
c4fd5c183142aef5ae93b918b65a0bf01d2a0a1e (presubmit successful).