Segv when PDF-side JS object property getter invoked from XFA.

Segv when PDF-side JS object property getter invoked from XFA.

The PDF-side native objects require that the current v8 context
has been set-up to point at the state (via CJS_Runtime) for the
getters, setters, and methods to operate against. XFA doesn't
supply a context with that state, so at the first opportunity for
a PDF-side object to be leaked to XFA, set up the context to mimic
the PDF side.

Changed FXJS_GetRuntimeFromIsolate() to FXJS_GetRuntimeFromV8Context()
for consistency with the newly added method.

BUG=pdfium:266
R=jochen@chromium.org

Committed: https://pdfium.googlesource.com/pdfium/+/4f4603cc1b498bca3b1619006137e50ce80088c1

[Tom Sepez, 2015-11-05 21:51:27]

Description was changed from

==========
Segv when PDF-side JS object property getter invoked from XFA.

The PDF-side methods require that the current v8 context has been
set-up to point at the state (via CJS_Runtime) that the objects
getters, setters, and methods are to operate against. XFA doen't
supply a context with that state, so at the first opportunity for
a PDF-side object to returned to XFA, set up the context to mimic
the PDF side.

BUG=pdfium:266
==========

to

==========
Segv when PDF-side JS object property getter invoked from XFA.

The PDF-side methods require that the current v8 context has been
set-up to point at the state (via CJS_Runtime) that the objects
getters, setters, and methods are to operate against. XFA doen't
supply a context with that state, so at the first opportunity for
a PDF-side object to returned to XFA, set up the context to mimic
the PDF side.

Changed FXJS_GetRuntimeFromIsolate() to FXJS_GetRuntimeFromV8Context() for
consistency with the
newly added method.
BUG=pdfium:266
==========

[Tom Sepez, 2015-11-05 21:51:44]

Description was changed from

==========
Segv when PDF-side JS object property getter invoked from XFA.

The PDF-side methods require that the current v8 context has been
set-up to point at the state (via CJS_Runtime) that the objects
getters, setters, and methods are to operate against. XFA doen't
supply a context with that state, so at the first opportunity for
a PDF-side object to returned to XFA, set up the context to mimic
the PDF side.

Changed FXJS_GetRuntimeFromIsolate() to FXJS_GetRuntimeFromV8Context() for
consistency with the
newly added method.
BUG=pdfium:266
==========

to

==========
Segv when PDF-side JS object property getter invoked from XFA.

The PDF-side methods require that the current v8 context has been
set-up to point at the state (via CJS_Runtime) that the objects
getters, setters, and methods are to operate against. XFA doen't
supply a context with that state, so at the first opportunity for
a PDF-side object to returned to XFA, set up the context to mimic
the PDF side.

Changed FXJS_GetRuntimeFromIsolate() to FXJS_GetRuntimeFromV8Context()
for consistency with the newly added method.

BUG=pdfium:266
==========

[Tom Sepez, 2015-11-05 21:58:15]

Description was changed from

==========
Segv when PDF-side JS object property getter invoked from XFA.

The PDF-side methods require that the current v8 context has been
set-up to point at the state (via CJS_Runtime) that the objects
getters, setters, and methods are to operate against. XFA doen't
supply a context with that state, so at the first opportunity for
a PDF-side object to returned to XFA, set up the context to mimic
the PDF side.

Changed FXJS_GetRuntimeFromIsolate() to FXJS_GetRuntimeFromV8Context()
for consistency with the newly added method.

BUG=pdfium:266
==========

to

==========
Segv when PDF-side JS object property getter invoked from XFA.

The PDF-side native objects require that the current v8 context has 
been set-up to point at the state (via CJS_Runtime)for the 
getters, setters, and methods are to operate against. XFA doen't
supply a context with that state, so at the first opportunity for
a PDF-side object to returned to XFA, set up the context to mimic
the PDF side.

Changed FXJS_GetRuntimeFromIsolate() to FXJS_GetRuntimeFromV8Context()
for consistency with the newly added method.

BUG=pdfium:266
==========

[Tom Sepez, 2015-11-05 21:58:35]

Description was changed from

==========
Segv when PDF-side JS object property getter invoked from XFA.

The PDF-side native objects require that the current v8 context has 
been set-up to point at the state (via CJS_Runtime)for the 
getters, setters, and methods are to operate against. XFA doen't
supply a context with that state, so at the first opportunity for
a PDF-side object to returned to XFA, set up the context to mimic
the PDF side.

Changed FXJS_GetRuntimeFromIsolate() to FXJS_GetRuntimeFromV8Context()
for consistency with the newly added method.

BUG=pdfium:266
==========

to

==========
Segv when PDF-side JS object property getter invoked from XFA.

The PDF-side native objects require that the current v8 context
has been set-up to point at the state (via CJS_Runtime)for the 
getters, setters, and methods are to operate against. XFA doen't
supply a context with that state, so at the first opportunity for
a PDF-side object to returned to XFA, set up the context to mimic
the PDF side.

Changed FXJS_GetRuntimeFromIsolate() to FXJS_GetRuntimeFromV8Context()
for consistency with the newly added method.

BUG=pdfium:266
==========

[Tom Sepez, 2015-11-05 21:58:51]

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

[Tom Sepez, 2015-11-05 21:59:07]

Lei, for review.  This is probably blocking testing.

[Tom Sepez, 2015-11-05 21:59:49]

Description was changed from

==========
Segv when PDF-side JS object property getter invoked from XFA.

The PDF-side native objects require that the current v8 context
has been set-up to point at the state (via CJS_Runtime)for the 
getters, setters, and methods are to operate against. XFA doen't
supply a context with that state, so at the first opportunity for
a PDF-side object to returned to XFA, set up the context to mimic
the PDF side.

Changed FXJS_GetRuntimeFromIsolate() to FXJS_GetRuntimeFromV8Context()
for consistency with the newly added method.

BUG=pdfium:266
==========

to

==========
Segv when PDF-side JS object property getter invoked from XFA.

The PDF-side native objects require that the current v8 context
has been set-up to point at the state (via CJS_Runtime )for the 
getters, setters, and methods to operate against. XFA doesn't
supply a context with that state, so at the first opportunity for
a PDF-side object to be leaked to XFA, set up the context to mimic
the PDF side.

Changed FXJS_GetRuntimeFromIsolate() to FXJS_GetRuntimeFromV8Context()
for consistency with the newly added method.

BUG=pdfium:266
==========

[Lei Zhang, 2015-11-05 22:14:57]

Description was changed from

==========
Segv when PDF-side JS object property getter invoked from XFA.

The PDF-side native objects require that the current v8 context
has been set-up to point at the state (via CJS_Runtime )for the 
getters, setters, and methods to operate against. XFA doesn't
supply a context with that state, so at the first opportunity for
a PDF-side object to be leaked to XFA, set up the context to mimic
the PDF side.

Changed FXJS_GetRuntimeFromIsolate() to FXJS_GetRuntimeFromV8Context()
for consistency with the newly added method.

BUG=pdfium:266
==========

to

==========
Segv when PDF-side JS object property getter invoked from XFA.

The PDF-side native objects require that the current v8 context
has been set-up to point at the state (via CJS_Runtime) for the 
getters, setters, and methods to operate against. XFA doesn't
supply a context with that state, so at the first opportunity for
a PDF-side object to be leaked to XFA, set up the context to mimic
the PDF side.

Changed FXJS_GetRuntimeFromIsolate() to FXJS_GetRuntimeFromV8Context()
for consistency with the newly added method.

BUG=pdfium:266
==========

[Lei Zhang, 2015-11-05 22:16:13]

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

[Lei Zhang, 2015-11-05 22:16:14]

+jochen

[Tom Sepez, 2015-11-06 17:08:19]

On 2015/11/05 22:16:14, Lei Zhang wrote:
> +jochen

Jun, if testing is blocked, you might try pulling this patch locally while
Jochen reviews it.

[jochen (gone - plz use gerrit), 2015-11-07 05:56:19]

https://codereview.chromium.org/1412103010/diff/1/fpdfsdk/src/javascript/JS_R...
File fpdfsdk/src/javascript/JS_Runtime.cpp (right):

https://codereview.chromium.org/1412103010/diff/1/fpdfsdk/src/javascript/JS_R...
fpdfsdk/src/javascript/JS_Runtime.cpp:256: // with it at this point.
i don't understand this comment?

https://codereview.chromium.org/1412103010/diff/1/fpdfsdk/src/javascript/JS_R...
fpdfsdk/src/javascript/JS_Runtime.cpp:257: // TODO(tsepez): redesign PDF-side
objects to not rely on context.
nor this

[Tom Sepez, 2015-11-09 22:15:05]

https://codereview.chromium.org/1412103010/diff/1/fpdfsdk/src/javascript/JS_R...
File fpdfsdk/src/javascript/JS_Runtime.cpp (right):

https://codereview.chromium.org/1412103010/diff/1/fpdfsdk/src/javascript/JS_R...
fpdfsdk/src/javascript/JS_Runtime.cpp:256: // with it at this point.
On 2015/11/07 05:56:18, jochen (slow - traveling) wrote:
> i don't understand this comment?

Because the new XFA code and the old PDF code don't co-operate with each other
when invoking javascript, there are two (or more?) possible v8::Contexts that
are present -- one for each.  PDFium wants to access data from the current
contex'ts embedder data slot in order to figure out which document to operate
on.  This was set up by the PDF code.  The XFA code has no such pointer, so
whenever this code is invoked on top of a XFA-side v8::context, we store the
embedder data as if it were created by pdf.

https://codereview.chromium.org/1412103010/diff/1/fpdfsdk/src/javascript/JS_R...
fpdfsdk/src/javascript/JS_Runtime.cpp:257: // TODO(tsepez): redesign PDF-side
objects to not rely on context.
On 2015/11/07 05:56:18, jochen (slow - traveling) wrote:
> nor this
Changed.  Better?

[Tom Sepez, 2015-11-10 00:20:23]

Jun, please pull this patch and try it in your testing.

[Tom Sepez, 2015-11-10 00:20:25]

Jun, please pull this patch and try it in your testing.

[jun_fang, 2015-11-10 00:36:21]

On 2015/11/10 00:20:25, Tom Sepez wrote:
> Jun, please pull this patch and try it in your testing.

OK. Thanks!

[jochen (gone - plz use gerrit), 2015-11-10 20:00:32]

lgtm with nit

https://codereview.chromium.org/1412103010/diff/20001/fpdfsdk/src/javascript/...
File fpdfsdk/src/javascript/JS_Runtime.cpp (right):

https://codereview.chromium.org/1412103010/diff/20001/fpdfsdk/src/javascript/...
fpdfsdk/src/javascript/JS_Runtime.cpp:259: // embedder data slots.
hum, I think it's ok to rely on this - the better solution is to always use the
right context

[Tom Sepez, 2015-11-10 23:02:42]

https://codereview.chromium.org/1412103010/diff/20001/fpdfsdk/src/javascript/...
File fpdfsdk/src/javascript/JS_Runtime.cpp (right):

https://codereview.chromium.org/1412103010/diff/20001/fpdfsdk/src/javascript/...
fpdfsdk/src/javascript/JS_Runtime.cpp:259: // embedder data slots.
On 2015/11/10 20:00:31, jochen (slow - traveling) wrote:
> hum, I think it's ok to rely on this - the better solution is to always use
the
> right context
Added that suggestion to the comment.

[Tom Sepez, 2015-11-10 23:03:17]

Committed patchset #3 (id:40001) manually as
4f4603cc1b498bca3b1619006137e50ce80088c1 (presubmit successful).