XFA: contention between FXJSE and FXJS over isolate data slots

XFA: contention between FXJSE and FXJS over isolate data slots

This probably broke at 06b60021e when the FXJS slot moved to 0
from 1 unless explicitly overriden by the embedder, which conflicted
with the FXJSE_ usage of slot 0.

Also simplify some logic used to track global intialization of the
underling JS.

TEST=run_javascript_tests.py on XFA branch doesn't segv.
R=jochen@chromium.org

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

[Tom Sepez, 2015-09-17 21:58:38]

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

[Tom Sepez, 2015-09-17 21:58:39]

Lei, Jochen, for review.  A temporary fix until we sort this out.

https://codereview.chromium.org/1351173002/diff/1/fpdfsdk/src/fpdfxfa/fpdfxfa...
File fpdfsdk/src/fpdfxfa/fpdfxfa_app.cpp (left):

https://codereview.chromium.org/1351173002/diff/1/fpdfsdk/src/fpdfxfa/fpdfxfa...
fpdfsdk/src/fpdfxfa/fpdfxfa_app.cpp:115:
JS_ReleaseRuntime((v8::Isolate*)m_hJSERuntime, context);
Different segv, can't pass NULL ctx here. Already taken care of elsewhere.

[Lei Zhang, 2015-09-17 22:42:15]

I defer to jochen.

[jochen (gone - plz use gerrit), 2015-09-18 08:31:57]

it would be preferable to introduce a new struct PerIsolateData that held all
pointers required by PDFium, so PDFium would use only one slot

[Tom Sepez, 2015-09-21 21:28:36]

Ok, added FXJS_PerIsolateData class.

[jochen (gone - plz use gerrit), 2015-09-22 15:13:56]

lgtm

[Tom Sepez, 2015-09-22 15:36:22]

Committed patchset #5 (id:80001) manually as
ed7b2b50aa1744e0bc5a60bef12c61fa91d863b7 (presubmit successful).

[jun_fang, 2015-10-28 16:19:25]

jun_fang@foxitsoftware.com changed reviewers:
+ jun_fang@foxitsoftware.com

[jun_fang, 2015-10-28 16:19:26]

Hi Tom, 

Please take a look at my comment. Thanks!

https://codereview.chromium.org/1351173002/diff/80001/xfa/src/fxjse/src/runti...
File xfa/src/fxjse/src/runtime.cpp (left):

https://codereview.chromium.org/1351173002/diff/80001/xfa/src/fxjse/src/runti...
xfa/src/fxjse/src/runtime.cpp:91: if (!pRuntimeData) {
When pRuntimeData is nullptr, it calls CFXJSE_RuntimeData::Create to create a
new one. In the right side, we didn't check whether pData is nullptr. In fact,
we found that it could be nullptr when we investigate one issue reported by
Foxit QA. If pData is nullptr, it causes a crashier.

[Tom Sepez, 2015-10-28 16:55:29]

https://codereview.chromium.org/1351173002/diff/80001/xfa/src/fxjse/src/runti...
File xfa/src/fxjse/src/runtime.cpp (left):

https://codereview.chromium.org/1351173002/diff/80001/xfa/src/fxjse/src/runti...
xfa/src/fxjse/src/runtime.cpp:91: if (!pRuntimeData) {
On 2015/10/28 16:19:26, jun_fang wrote:
> When pRuntimeData is nullptr, it calls CFXJSE_RuntimeData::Create to create a
> new one. In the right side, we didn't check whether pData is nullptr. In fact,
> we found that it could be nullptr when we investigate one issue reported by
> Foxit QA. If pData is nullptr, it causes a crashier.  

Ok, please file a bug for this in the issue tracker.

[jun_fang, 2015-10-30 12:41:23]

On 2015/10/28 16:55:29, Tom Sepez wrote:
>
https://codereview.chromium.org/1351173002/diff/80001/xfa/src/fxjse/src/runti...
> File xfa/src/fxjse/src/runtime.cpp (left):
> 
>
https://codereview.chromium.org/1351173002/diff/80001/xfa/src/fxjse/src/runti...
> xfa/src/fxjse/src/runtime.cpp:91: if (!pRuntimeData) {
> On 2015/10/28 16:19:26, jun_fang wrote:
> > When pRuntimeData is nullptr, it calls CFXJSE_RuntimeData::Create to create
a
> > new one. In the right side, we didn't check whether pData is nullptr. In
fact,
> > we found that it could be nullptr when we investigate one issue reported by
> > Foxit QA. If pData is nullptr, it causes a crashier.  
> 
> Ok, please file a bug for this in the issue tracker.

Hi Tom,

Our QA raised an issue in
https://bugs.chromium.org/p/pdfium/issues/detail?id=259.
Currently, XFA testing is blocked by this issue. Can you help to check it ASAP?
Thanks!