Convert IJS_Runtime to CPDFSDK_FormFillEnvironment

Convert IJS_Runtime to CPDFSDK_FormFillEnvironment

This CL changes the use of CPDFSDK_Document to CPDFSDK_FormFillEnvironment.

Committed: https://pdfium.googlesource.com/pdfium/+/82e1767b757d96e6b81d3e42b24048a9bea387a8

[dsinclair, 2016-10-06 20:38:22]

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

[dsinclair, 2016-10-06 20:38:23]

PTAL.

https://codereview.chromium.org/2394313002/diff/1/fpdfsdk/cpdfsdk_interform.cpp
File fpdfsdk/cpdfsdk_interform.cpp (left):

https://codereview.chromium.org/2394313002/diff/1/fpdfsdk/cpdfsdk_interform.c...
fpdfsdk/cpdfsdk_interform.cpp:239:
pRuntime->SetReaderDocument(m_pFormFillEnv->GetSDKDocument());
We create the JSRuntime and pass 'this' as the form fill environment. So, this
would just set ourselves back as the environment.

https://codereview.chromium.org/2394313002/diff/1/fpdfsdk/fpdfxfa/cpdfxfa_doc...
File fpdfsdk/fpdfxfa/cpdfxfa_docenvironment.cpp (left):

https://codereview.chromium.org/2394313002/diff/1/fpdfsdk/fpdfxfa/cpdfxfa_doc...
fpdfsdk/fpdfxfa/cpdfxfa_docenvironment.cpp:1017: m_pDocument->GetSDKDoc());
We get the runtime from the env, which will have the env set internally, we then
set that env back into the runtime, so they should be the same.

https://codereview.chromium.org/2394313002/diff/1/fpdfsdk/fsdk_actionhandler.cpp
File fpdfsdk/fsdk_actionhandler.cpp (left):

https://codereview.chromium.org/2394313002/diff/1/fpdfsdk/fsdk_actionhandler....
fpdfsdk/fsdk_actionhandler.cpp:158: pRuntime->SetReaderDocument(pDocument);
We got the env from the Document, then get the runtime for that env, so the env
should be the same in both cases.

https://codereview.chromium.org/2394313002/diff/1/fpdfsdk/javascript/app.cpp
File fpdfsdk/javascript/app.cpp (left):

https://codereview.chromium.org/2394313002/diff/1/fpdfsdk/javascript/app.cpp#...
fpdfsdk/javascript/app.cpp:226: if (pDoc == pCurDoc) {
As far as I can tell, these should always be equal.  The context gets its
environment from the runtime. So, those two documents should be the same.

[dsinclair, 2016-10-11 14:06:29]

ping.

[Tom Sepez, 2016-10-11 16:17:13]

https://codereview.chromium.org/2394313002/diff/20001/fpdfsdk/cpdfsdk_interfo...
File fpdfsdk/cpdfsdk_interform.cpp (left):

https://codereview.chromium.org/2394313002/diff/20001/fpdfsdk/cpdfsdk_interfo...
fpdfsdk/cpdfsdk_interform.cpp:239:
pRuntime->SetReaderDocument(m_pFormFillEnv->GetSDKDocument());
Remind me again why we think documents are 1:1 with IJS_Runtimes.

[dsinclair, 2016-10-11 17:37:45]

Moved my various comments from PS1 to PS2. PTAL.

https://codereview.chromium.org/2394313002/diff/20001/fpdfsdk/cpdfsdk_interfo...
File fpdfsdk/cpdfsdk_interform.cpp (left):

https://codereview.chromium.org/2394313002/diff/20001/fpdfsdk/cpdfsdk_interfo...
fpdfsdk/cpdfsdk_interform.cpp:239:
pRuntime->SetReaderDocument(m_pFormFillEnv->GetSDKDocument());
On 2016/10/11 16:17:13, Tom Sepez wrote:
> Remind me again why we think documents are 1:1 with IJS_Runtimes.


The GetJSRuntime method, if it doesn't already exist, creates the JSRuntime and
passes |this| as the environment for the runtime. The |CPDFSDK_Document| is only
created in the env, so they have to be 1:1. So, getting the sdk document out of
the environment and putting it into the |SetReaderDocument| should be the same
as just getting it from the environment.

Does that make sense? (It's possible I'm off base here ....)

https://codereview.chromium.org/2394313002/diff/20001/fpdfsdk/cpdfsdk_interfo...
fpdfsdk/cpdfsdk_interform.cpp:289:
pRuntime->SetReaderDocument(m_pFormFillEnv->GetSDKDocument());
Same reasoning as above.

https://codereview.chromium.org/2394313002/diff/20001/fpdfsdk/fpdfxfa/cpdfxfa...
File fpdfsdk/fpdfxfa/cpdfxfa_docenvironment.cpp (left):

https://codereview.chromium.org/2394313002/diff/20001/fpdfsdk/fpdfxfa/cpdfxfa...
fpdfsdk/fpdfxfa/cpdfxfa_docenvironment.cpp:1017: m_pDocument->GetSDKDoc());
We get the runtime from the env, which will have the env set internally, we then
set that env back into the runtime, so they should be the same.

https://codereview.chromium.org/2394313002/diff/20001/fpdfsdk/fsdk_actionhand...
File fpdfsdk/fsdk_actionhandler.cpp (left):

https://codereview.chromium.org/2394313002/diff/20001/fpdfsdk/fsdk_actionhand...
fpdfsdk/fsdk_actionhandler.cpp:158: pRuntime->SetReaderDocument(pDocument);
We got the env from the Document, then get the runtime for that env, so the env
should be the same in both cases.

https://codereview.chromium.org/2394313002/diff/20001/fpdfsdk/javascript/app.cpp
File fpdfsdk/javascript/app.cpp (left):

https://codereview.chromium.org/2394313002/diff/20001/fpdfsdk/javascript/app....
fpdfsdk/javascript/app.cpp:226: if (pDoc == pCurDoc) {
As far as I can tell, these should always be equal.  The context gets its
environment from the runtime. So, those two documents should be the same.

[Tom Sepez, 2016-10-11 18:36:38]

lgtm

[dsinclair, 2016-10-11 19:25:31]

The CQ bit was checked by dsinclair@chromium.org

[commit-bot: I haz the power, 2016-10-11 19:25:41]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-11 19:38:04]

Description was changed from

==========
Convert IJS_Runtime to CPDFSDK_FormFillEnvironment

This CL changes the use of CPDFSDK_Document to CPDFSDK_FormFillEnvironment.
==========

to

==========
Convert IJS_Runtime to CPDFSDK_FormFillEnvironment

This CL changes the use of CPDFSDK_Document to CPDFSDK_FormFillEnvironment.

Committed:
https://pdfium.googlesource.com/pdfium/+/82e1767b757d96e6b81d3e42b24048a9bea3...
==========

[commit-bot: I haz the power, 2016-10-11 19:38:05]

Committed patchset #2 (id:20001) as
https://pdfium.googlesource.com/pdfium/+/82e1767b757d96e6b81d3e42b24048a9bea3...