Plumb in an externally created v8::Isolate

Plumb in an externally created v8::Isolate

In Chrome, all Isolates must be created by gin

R=tsepez@chromium.org, ulan@chromium.org
BUG=

Committed: https://pdfium.googlesource.com/pdfium/+/1c79928b4ade95806d0351a21b4d4727d6c8ed38

[ulan, 2015-07-14 14:35:06]

lgtm

[Tom Sepez, 2015-07-14 16:22:53]

Thanks immensely.

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

https://codereview.chromium.org/1234053003/diff/1/fpdfsdk/src/javascript/JS_R...
fpdfsdk/src/javascript/JS_Runtime.cpp:115: if
(m_pApp->GetFormFillInfo()->m_pJsPlatform->version == 2) {
I think we want >= 2 for future compatibility. Also, how about allowing callers
to pass a null m_isolate to get the current behaviour?

[Tom Sepez, 2015-07-14 16:29:27]

https://codereview.chromium.org/1234053003/diff/1/public/fpdf_formfill.h
File public/fpdf_formfill.h (right):

https://codereview.chromium.org/1234053003/diff/1/public/fpdf_formfill.h#newc...
public/fpdf_formfill.h:26: * Version number of the interface. Currently must be
1 or 2 (in the latter case
nit: probably just say "Currently must be 2."

https://codereview.chromium.org/1234053003/diff/1/public/fpdf_formfill.h#newc...
public/fpdf_formfill.h:31: /**
nit: Comment here like 
   /* Version 1. */
before the comments for the set of members present in V1.

https://codereview.chromium.org/1234053003/diff/1/public/fpdf_formfill.h#newc...
public/fpdf_formfill.h:230: 
nit: comment like
   /* Version 2. */

https://codereview.chromium.org/1234053003/diff/1/public/fpdf_formfill.h#newc...
public/fpdf_formfill.h:232: *   pointer to the v8::Isolate to use.
nit: , or NULL to force PDFIUm to create one.

[tsepez (do not use), 2015-07-14 17:02:29]

tsepez@google.com changed reviewers:
+ tsepez@google.com

[tsepez (do not use), 2015-07-14 17:02:29]

https://codereview.chromium.org/1234053003/diff/1/public/fpdf_formfill.h
File public/fpdf_formfill.h (right):

https://codereview.chromium.org/1234053003/diff/1/public/fpdf_formfill.h#newc...
public/fpdf_formfill.h:14: namespace v8 {
Actually, these public files are expected to compile under C or C++, so you
don't get to use namespaces and classes.  Sadly, your isolate will need to be a
void* (I can hear the sound of revulsion coming across the atlantic ...)

[jochen (gone - plz use gerrit), 2015-07-14 18:18:06]

ptal

[Tom Sepez, 2015-07-14 19:32:35]

lgtm

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

https://codereview.chromium.org/1234053003/diff/20001/fpdfsdk/src/javascript/...
fpdfsdk/src/javascript/JS_Runtime.cpp:119: if (m_isolate == NULL) {
nit: prefer if (!m_isolate) {

https://codereview.chromium.org/1234053003/diff/20001/public/fpdf_formfill.h
File public/fpdf_formfill.h (right):

https://codereview.chromium.org/1234053003/diff/20001/public/fpdf_formfill.h#...
public/fpdf_formfill.h:26: 
nit: still would like a /* Version 1. */ comment here..

https://codereview.chromium.org/1234053003/diff/20001/public/fpdf_formfill.h#...
public/fpdf_formfill.h:226: 
nit: still would like a /* Version 2. */ comment here, so we can tell that all
that comes after this was added in V2.

[Tom Sepez, 2015-07-14 19:34:17]

https://codereview.chromium.org/1234053003/diff/20001/public/fpdf_formfill.h
File public/fpdf_formfill.h (right):

https://codereview.chromium.org/1234053003/diff/20001/public/fpdf_formfill.h#...
public/fpdf_formfill.h:23: * m_isolate must be set).
nit: stray line.

[jochen (gone - plz use gerrit), 2015-07-15 12:31:34]

Committed patchset #3 (id:40001) manually as
1c79928b4ade95806d0351a21b4d4727d6c8ed38 (presubmit successful).