Issue 1610 in pdfium: Consider removing CFXJSE_Value

37 views
Skip to first unread message

tse… via monorail

unread,
Nov 2, 2020, 1:08:55 PM11/2/20
to pdfiu...@googlegroups.com
Status: Assigned
Owner: tse...@chromium.org
Labels: Type-Defect Priority-Medium

New issue 1610 by tse...@chromium.org: Consider removing CFXJSE_Value
https://bugs.chromium.org/p/pdfium/issues/detail?id=1610

It is essentially a "malloc'd" wrapper around a v8::Global used in places where a v8::Local would suffice on its own without any additional allocation required.

There are a couple of preliminary steps that can be taken:

1. Remove the pointer to the v8::Isolate that is also contained in the object, instead passing it in as required. V8 supplies this to us in all the places where it is required with a little bit of additional plumbing.

2. Provide equivalents for the To*() routines that first check for empty values before applying the test.

--
You received this message because:
1. The project was configured to send all issue notifications to this address

You may adjust your notification preferences at:
https://bugs.chromium.org/hosting/settings

tse… via monorail

unread,
Nov 2, 2020, 1:09:15 PM11/2/20
to pdfiu...@googlegroups.com
Updates:
Status: Started

Comment #1 on issue 1610 by tse...@chromium.org: Consider removing CFXJSE_Value
https://bugs.chromium.org/p/pdfium/issues/detail?id=1610#c1

(No comment was entered for this change.)

bugdroid via monorail

unread,
Nov 2, 2020, 3:14:17 PM11/2/20
to pdfiu...@googlegroups.com

Comment #2 on issue 1610 by bugdroid: Consider removing CFXJSE_Value
https://bugs.chromium.org/p/pdfium/issues/detail?id=1610#c2

The following revision refers to this bug:
https://pdfium.googlesource.com/pdfium/+/af6371f3276419d16bcf1f97f8e1b88041812f9f

commit af6371f3276419d16bcf1f97f8e1b88041812f9f
Author: Tom Sepez <tse...@chromium.org>
Date: Mon Nov 02 20:13:01 2020

Consolidate some tests in new fxv8:: functions.

Task #2 from the referenced bug.

Bug: pdfium:1610
Change-Id: I2422b3f8ffe4254388d7b705985b3d3667ecf9f8
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/75891
Reviewed-by: Daniel Hosseinian <dh...@chromium.org>
Commit-Queue: Tom Sepez <tse...@chromium.org>

[modify] https://pdfium.googlesource.com/pdfium/+/af6371f3276419d16bcf1f97f8e1b88041812f9f/fxjs/xfa/cfxjse_engine.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/af6371f3276419d16bcf1f97f8e1b88041812f9f/fxjs/xfa/cfxjse_context.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/af6371f3276419d16bcf1f97f8e1b88041812f9f/fxjs/xfa/fxjse.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/af6371f3276419d16bcf1f97f8e1b88041812f9f/fxjs/xfa/cfxjse_value.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/af6371f3276419d16bcf1f97f8e1b88041812f9f/fxjs/cjs_field.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/af6371f3276419d16bcf1f97f8e1b88041812f9f/fxjs/cjs_util.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/af6371f3276419d16bcf1f97f8e1b88041812f9f/fxjs/cjs_color.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/af6371f3276419d16bcf1f97f8e1b88041812f9f/fxjs/fxv8.h
[modify] https://pdfium.googlesource.com/pdfium/+/af6371f3276419d16bcf1f97f8e1b88041812f9f/fxjs/fxv8.cpp

bugdroid via monorail

unread,
Nov 3, 2020, 4:50:55 PM11/3/20
to pdfiu...@googlegroups.com

Comment #3 on issue 1610 by bugdroid: Consider removing CFXJSE_Value
https://bugs.chromium.org/p/pdfium/issues/detail?id=1610#c3


The following revision refers to this bug:
https://pdfium.googlesource.com/pdfium/+/0d76dab2254fdc3e39f29ac3ee1ae4fd0750542a

commit 0d76dab2254fdc3e39f29ac3ee1ae4fd0750542a
Author: Tom Sepez <tse...@chromium.org>
Date: Tue Nov 03 21:49:31 2020

Move some more binding logic out of CFXJSE_Value.

Pass locals where possible rather than references to globals,
which means plumbing an isolate into a few place so we can
make a local from the global.

-- de-duplicates some code in CFXJSE_Engine in the process.
-- rename NewXFAObject() and remove arg that is always the same.
-- replace friendship with name testing method.

Bug: pdfium:1610
Change-Id: Iddf429bb7ee388090100ba60871c56503c292f07
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/75931

Reviewed-by: Daniel Hosseinian <dh...@chromium.org>
Commit-Queue: Tom Sepez <tse...@chromium.org>

[modify] https://pdfium.googlesource.com/pdfium/+/0d76dab2254fdc3e39f29ac3ee1ae4fd0750542a/fxjs/xfa/cfxjse_engine.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/0d76dab2254fdc3e39f29ac3ee1ae4fd0750542a/fxjs/xfa/cfxjse_context.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/0d76dab2254fdc3e39f29ac3ee1ae4fd0750542a/fxjs/xfa/cfxjse_class.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/0d76dab2254fdc3e39f29ac3ee1ae4fd0750542a/fxjs/xfa/cjx_container.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/0d76dab2254fdc3e39f29ac3ee1ae4fd0750542a/fxjs/xfa/cfxjse_class.h
[modify] https://pdfium.googlesource.com/pdfium/+/0d76dab2254fdc3e39f29ac3ee1ae4fd0750542a/fxjs/xfa/fxjse.h
[modify] https://pdfium.googlesource.com/pdfium/+/0d76dab2254fdc3e39f29ac3ee1ae4fd0750542a/fxjs/xfa/fxjse.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/0d76dab2254fdc3e39f29ac3ee1ae4fd0750542a/fxjs/xfa/cjx_layoutpseudomodel.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/0d76dab2254fdc3e39f29ac3ee1ae4fd0750542a/fxjs/xfa/cfxjse_engine.h
[modify] https://pdfium.googlesource.com/pdfium/+/0d76dab2254fdc3e39f29ac3ee1ae4fd0750542a/fxjs/xfa/cfxjse_value.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/0d76dab2254fdc3e39f29ac3ee1ae4fd0750542a/fxjs/xfa/cfxjse_value.h
[modify] https://pdfium.googlesource.com/pdfium/+/0d76dab2254fdc3e39f29ac3ee1ae4fd0750542a/fxjs/xfa/cjx_list.cpp

tse… via monorail

unread,
Nov 3, 2020, 5:43:43 PM11/3/20
to pdfiu...@googlegroups.com

Comment #4 on issue 1610 by tse...@chromium.org: Consider removing CFXJSE_Value
https://bugs.chromium.org/p/pdfium/issues/detail?id=1610#c4

Task #3 is splitting apart the Setters and Getters, so that we don't have to pass pointers to locals as out parameters (nice to have).

bugdroid via monorail

unread,
Nov 3, 2020, 6:06:23 PM11/3/20
to pdfiu...@googlegroups.com

Comment #5 on issue 1610 by bugdroid: Consider removing CFXJSE_Value
https://bugs.chromium.org/p/pdfium/issues/detail?id=1610#c5


The following revision refers to this bug:
https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a

commit 33001e2632db0713dc9cf0a3440b239385c1f25a
Author: Tom Sepez <tse...@chromium.org>
Date: Tue Nov 03 23:04:52 2020

Remove pointer to isolate from CFXJSE_Value

Task #1 from the referenced bug.

Bug: pdfium:1610
Change-Id: I44a6347e9384216f08baeaa879fd774587816f15
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/75870
Commit-Queue: Tom Sepez <tse...@chromium.org>
Reviewed-by: Daniel Hosseinian <dh...@chromium.org>

[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/cfxjse_formcalc_context_embeddertest.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/xfa/fxfa/parser/xfa_basic_data.h
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/cfxjse_class.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/cfxjse_value_embeddertest.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/cjx_eventpseudomodel.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/fxjse.h
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/testing/xfa_js_embedder_test.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/cjx_hostpseudomodel.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/cjx_handler.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/cjx_tree.h
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/cjx_boolean.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/cjx_source.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/cjx_form.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/cjx_packet.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/cjx_draw.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/cjx_extras.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/cjx_exclgroup.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/cjx_node.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/cjx_instancemanager.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/cfxjse_engine.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/cfxjse_context.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/cfxjse_formcalc_context.h
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/cfxjse_formcalc_context.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/cfxjse_resolveprocessor.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/cjx_datawindow.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/cfxjse_resolveprocessor.h
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/cfxjse_engine.h
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/cjx_model.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/cjx_object.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/cfxjse_value.h
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/cjx_script.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/cjx_delta.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/cjx_tree.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/cjx_layoutpseudomodel.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/cjx_encrypt.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/cfxjse_value.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/jse_define.h
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/cjx_object.h
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/xfa/fxfa/parser/cxfa_node.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/cjx_occur.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/cjx_xfa.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/cjx_list.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/cjx_subform.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/cjx_eventpseudomodel.h
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/cjx_textnode.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/33001e2632db0713dc9cf0a3440b239385c1f25a/fxjs/xfa/cjx_field.cpp

bugdroid via monorail

unread,
Nov 4, 2020, 1:49:26 PM11/4/20
to pdfiu...@googlegroups.com

Comment #6 on issue 1610 by bugdroid: Consider removing CFXJSE_Value
https://bugs.chromium.org/p/pdfium/issues/detail?id=1610#c6


The following revision refers to this bug:
https://pdfium.googlesource.com/pdfium/+/acea960c16e7e903ed3c81b9e1c7e444dc3eba41

commit acea960c16e7e903ed3c81b9e1c7e444dc3eba41
Author: Tom Sepez <tse...@chromium.org>
Date: Wed Nov 04 18:47:46 2020

Remove arg from CFXJSE_Value's constructor.

It is no longer used as of the previous change[1], but do this
in a separate step to keep the diff smaller.

-- Also remove some NDEBUG sections with ASSERTS() that are
easily hit based on document contents (avoids unused
variable warnings).

1. https://pdfium-review.googlesource.com/c/pdfium/+/75870

Bug: pdfium:1610
Change-Id: Ie41aa0707db391496ae64090d474dfb19dffb56e
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/75890
Reviewed-by: Tom Sepez <tse...@chromium.org>
Reviewed-by: Daniel Hosseinian <dh...@chromium.org>
Commit-Queue: Tom Sepez <tse...@chromium.org>

[modify] https://pdfium.googlesource.com/pdfium/+/acea960c16e7e903ed3c81b9e1c7e444dc3eba41/fxjs/xfa/cjx_tree.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/acea960c16e7e903ed3c81b9e1c7e444dc3eba41/fxjs/xfa/cfxjse_engine.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/acea960c16e7e903ed3c81b9e1c7e444dc3eba41/fxjs/xfa/cfxjse_context.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/acea960c16e7e903ed3c81b9e1c7e444dc3eba41/fxjs/xfa/cfxjse_class.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/acea960c16e7e903ed3c81b9e1c7e444dc3eba41/fxjs/xfa/cfxjse_value_embeddertest.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/acea960c16e7e903ed3c81b9e1c7e444dc3eba41/fxjs/xfa/cfxjse_formcalc_context.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/acea960c16e7e903ed3c81b9e1c7e444dc3eba41/fxjs/xfa/cfxjse_resolveprocessor.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/acea960c16e7e903ed3c81b9e1c7e444dc3eba41/testing/xfa_js_embedder_test.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/acea960c16e7e903ed3c81b9e1c7e444dc3eba41/testing/fuzzers/pdf_formcalc_context_fuzzer.cc
[modify] https://pdfium.googlesource.com/pdfium/+/acea960c16e7e903ed3c81b9e1c7e444dc3eba41/xfa/fxfa/parser/cxfa_node.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/acea960c16e7e903ed3c81b9e1c7e444dc3eba41/fxjs/xfa/cfxjse_value.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/acea960c16e7e903ed3c81b9e1c7e444dc3eba41/fxjs/xfa/cfxjse_value.h

bugdroid via monorail

unread,
Nov 5, 2020, 12:36:31 PM11/5/20
to pdfiu...@googlegroups.com

Comment #7 on issue 1610 by bugdroid: Consider removing CFXJSE_Value
https://bugs.chromium.org/p/pdfium/issues/detail?id=1610#c7


The following revision refers to this bug:
https://pdfium.googlesource.com/pdfium/+/0b6978453ebc756874105349a3926c87eae26e9f

commit 0b6978453ebc756874105349a3926c87eae26e9f
Author: Tom Sepez <tse...@chromium.org>
Date: Thu Nov 05 17:34:58 2020

Remove use of CFXJSE_Value in CFXJSE_Engine::m_mapObjectToValue.

Just use v8::Global<v8::Object> directly, and avoid dynamic
allocation of another object.

-- Remove NDEBUG section that can reach an ASSERT() against
document-controlled values that can't be constrained.

-- Return locals rather than reference to global, which required
introducing a new scope in one place, since you can't have a
local without a scope already in effect.

Bug: pdfium:1610
Change-Id: I8fb9982eb65ed2c01a1ac844f04eb722120b6331
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/75933
Commit-Queue: Tom Sepez <tse...@chromium.org>
Reviewed-by: Daniel Hosseinian <dh...@chromium.org>

[modify] https://pdfium.googlesource.com/pdfium/+/0b6978453ebc756874105349a3926c87eae26e9f/fxjs/xfa/cjx_tree.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/0b6978453ebc756874105349a3926c87eae26e9f/fxjs/xfa/cfxjse_engine.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/0b6978453ebc756874105349a3926c87eae26e9f/fxjs/xfa/cfxjse_context.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/0b6978453ebc756874105349a3926c87eae26e9f/fxjs/xfa/cjx_xfa.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/0b6978453ebc756874105349a3926c87eae26e9f/fxjs/xfa/cfxjse_isolatetracker.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/0b6978453ebc756874105349a3926c87eae26e9f/fxjs/xfa/cjx_object.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/0b6978453ebc756874105349a3926c87eae26e9f/fxjs/xfa/cfxjse_formcalc_context.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/0b6978453ebc756874105349a3926c87eae26e9f/fxjs/xfa/cjx_instancemanager.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/0b6978453ebc756874105349a3926c87eae26e9f/fxjs/xfa/cfxjse_context.h
[modify] https://pdfium.googlesource.com/pdfium/+/0b6978453ebc756874105349a3926c87eae26e9f/fxjs/xfa/cjx_hostpseudomodel.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/0b6978453ebc756874105349a3926c87eae26e9f/fxjs/xfa/cfxjse_isolatetracker.h
[modify] https://pdfium.googlesource.com/pdfium/+/0b6978453ebc756874105349a3926c87eae26e9f/fxjs/xfa/cfxjse_engine.h
[modify] https://pdfium.googlesource.com/pdfium/+/0b6978453ebc756874105349a3926c87eae26e9f/fxjs/xfa/cjx_treelist.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/0b6978453ebc756874105349a3926c87eae26e9f/fxjs/xfa/cfxjse_value.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/0b6978453ebc756874105349a3926c87eae26e9f/fxjs/xfa/cjx_exclgroup.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/0b6978453ebc756874105349a3926c87eae26e9f/fxjs/xfa/cfxjse_value.h
[modify] https://pdfium.googlesource.com/pdfium/+/0b6978453ebc756874105349a3926c87eae26e9f/fxjs/xfa/cjx_subform.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/0b6978453ebc756874105349a3926c87eae26e9f/fxjs/xfa/cjx_model.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/0b6978453ebc756874105349a3926c87eae26e9f/fxjs/xfa/cjx_node.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/0b6978453ebc756874105349a3926c87eae26e9f/fxjs/xfa/cjx_form.cpp

bugdroid via monorail

unread,
Nov 6, 2020, 1:28:31 PM11/6/20
to pdfiu...@googlegroups.com

Comment #8 on issue 1610 by bugdroid: Consider removing CFXJSE_Value
https://bugs.chromium.org/p/pdfium/issues/detail?id=1610#c8


The following revision refers to this bug:
https://pdfium.googlesource.com/pdfium/+/d564b0d2baa3fe03087780d7b03a55ab3e171d41

commit d564b0d2baa3fe03087780d7b03a55ab3e171d41
Author: Tom Sepez <tse...@chromium.org>
Date: Fri Nov 06 18:28:10 2020

Use v8::Global<> directly in CFXJSE_FormCalcContext and tests.

This is complicated by the need to have additional handle scopes
to permit the use of v8::Local<> in several places.

Bug: pdfium:1610
Change-Id: I60eebcc1eecd397efb1bd60bf2d62f1aa91fb869
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/75950

Commit-Queue: Tom Sepez <tse...@chromium.org>
Reviewed-by: Daniel Hosseinian <dh...@chromium.org>

[modify] https://pdfium.googlesource.com/pdfium/+/d564b0d2baa3fe03087780d7b03a55ab3e171d41/fxjs/xfa/cfxjse_formcalc_context_embeddertest.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/d564b0d2baa3fe03087780d7b03a55ab3e171d41/fxjs/xfa/cfxjse_formcalc_context.h
[modify] https://pdfium.googlesource.com/pdfium/+/d564b0d2baa3fe03087780d7b03a55ab3e171d41/testing/xfa_js_embedder_test.h
[modify] https://pdfium.googlesource.com/pdfium/+/d564b0d2baa3fe03087780d7b03a55ab3e171d41/fxjs/xfa/cfxjse_formcalc_context.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/d564b0d2baa3fe03087780d7b03a55ab3e171d41/testing/xfa_js_embedder_test.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/d564b0d2baa3fe03087780d7b03a55ab3e171d41/fxjs/fxv8.h
[modify] https://pdfium.googlesource.com/pdfium/+/d564b0d2baa3fe03087780d7b03a55ab3e171d41/fxjs/fxv8.cpp

bugdroid via monorail

unread,
Nov 9, 2020, 2:28:25 PM11/9/20
to pdfiu...@googlegroups.com

Comment #9 on issue 1610 by bugdroid: Consider removing CFXJSE_Value
https://bugs.chromium.org/p/pdfium/issues/detail?id=1610#c9


The following revision refers to this bug:
https://pdfium.googlesource.com/pdfium/+/4643cf6518895ebfcc5c1e4ca7249da7879dc19b

commit 4643cf6518895ebfcc5c1e4ca7249da7879dc19b
Author: Tom Sepez <tse...@chromium.org>
Date: Mon Nov 09 19:27:05 2020

Avoid allocating CFXJSE_Values for |this| in poperty callbacks

Let it flow further down the code as a v8::Local<>. The only real
consequence is that we need to expose a delete wrapper for locals
that was formerly part of the CFXJSE_Value code.

Bug: pdfium:1610
Change-Id: I95dd709c4ff3716e3e37e6824b7ee4d0a00d5375
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/76011

Reviewed-by: Daniel Hosseinian <dh...@chromium.org>
Commit-Queue: Tom Sepez <tse...@chromium.org>

[modify] https://pdfium.googlesource.com/pdfium/+/4643cf6518895ebfcc5c1e4ca7249da7879dc19b/fxjs/xfa/cfxjse_engine.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/4643cf6518895ebfcc5c1e4ca7249da7879dc19b/fxjs/xfa/cfxjse_value.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/4643cf6518895ebfcc5c1e4ca7249da7879dc19b/fxjs/xfa/cfxjse_class.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/4643cf6518895ebfcc5c1e4ca7249da7879dc19b/fxjs/xfa/fxjse.h
[modify] https://pdfium.googlesource.com/pdfium/+/4643cf6518895ebfcc5c1e4ca7249da7879dc19b/fxjs/xfa/cfxjse_engine.h
[modify] https://pdfium.googlesource.com/pdfium/+/4643cf6518895ebfcc5c1e4ca7249da7879dc19b/fxjs/xfa/cfxjse_value.h
[modify] https://pdfium.googlesource.com/pdfium/+/4643cf6518895ebfcc5c1e4ca7249da7879dc19b/fxjs/fxv8.h
[modify] https://pdfium.googlesource.com/pdfium/+/4643cf6518895ebfcc5c1e4ca7249da7879dc19b/fxjs/fxv8.cpp

bugdroid via monorail

unread,
Nov 9, 2020, 4:14:33 PM11/9/20
to pdfiu...@googlegroups.com

Comment #10 on issue 1610 by bugdroid: Consider removing CFXJSE_Value
https://bugs.chromium.org/p/pdfium/issues/detail?id=1610#c10


The following revision refers to this bug:
https://pdfium.googlesource.com/pdfium/+/f4874458f5c070253fd02a53879521a11d9418b2

commit f4874458f5c070253fd02a53879521a11d9418b2
Author: Tom Sepez <tse...@chromium.org>
Date: Mon Nov 09 21:14:06 2020

Return v8::Local<> from CFXJSE_Context::GetGlobalContext()

No reason to cobble together a CFXJSE_Value when a local would do.

-- requires moving some functionality into fxv8 that was formerly
part of CFXJSE_Value.

-- introduce a new scope in one place, which made a method no longer
be const (probably never was logically const to begin with).

Bug: pdfium:1610
Change-Id: Id7064cf802f47b52abc3bbb9a83ff0b9624cf447
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/76012

Commit-Queue: Tom Sepez <tse...@chromium.org>
Reviewed-by: Daniel Hosseinian <dh...@chromium.org>

[modify] https://pdfium.googlesource.com/pdfium/+/f4874458f5c070253fd02a53879521a11d9418b2/fxjs/xfa/cfxjse_engine.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/f4874458f5c070253fd02a53879521a11d9418b2/fxjs/xfa/cfxjse_context.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/f4874458f5c070253fd02a53879521a11d9418b2/fxjs/xfa/cfxjse_context.h
[modify] https://pdfium.googlesource.com/pdfium/+/f4874458f5c070253fd02a53879521a11d9418b2/fxjs/xfa/cfxjse_engine.h
[modify] https://pdfium.googlesource.com/pdfium/+/f4874458f5c070253fd02a53879521a11d9418b2/fxjs/xfa/cfxjse_value.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/f4874458f5c070253fd02a53879521a11d9418b2/fxjs/xfa/cfxjse_value.h
[modify] https://pdfium.googlesource.com/pdfium/+/f4874458f5c070253fd02a53879521a11d9418b2/fxjs/fxv8.h
[modify] https://pdfium.googlesource.com/pdfium/+/f4874458f5c070253fd02a53879521a11d9418b2/fxjs/fxv8.cpp

bugdroid via monorail

unread,
Nov 12, 2020, 8:55:59 PM11/12/20
to pdfiu...@googlegroups.com

Comment #11 on issue 1610 by bugdroid: Consider removing CFXJSE_Value
https://bugs.chromium.org/p/pdfium/issues/detail?id=1610#c11


The following revision refers to this bug:
https://pdfium.googlesource.com/pdfium/+/1c8311278ae8bfbb4b4f86895899cabf48f67542

commit 1c8311278ae8bfbb4b4f86895899cabf48f67542
Author: Tom Sepez <tse...@chromium.org>
Date: Fri Nov 13 01:53:55 2020

Remove most CFXJSE_Value usage from CFXJSE_FormCalcContext.

Try to create the smallest possible change that is self-consistent,
though it is not very small. The remainder of the CFXJSE_Value
usage can then be removed in smaller chunks.

Bug: pdfium:1610
Change-Id: I9a77ad88b6b13b4b646234d5dbfbdfbbf8fa0a7b
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/76190
Commit-Queue: Thomas Sepez <tse...@google.com>
Reviewed-by: Daniel Hosseinian <dh...@chromium.org>

[modify] https://pdfium.googlesource.com/pdfium/+/1c8311278ae8bfbb4b4f86895899cabf48f67542/fxjs/xfa/cfxjse_formcalc_context.cpp

bugdroid via monorail

unread,
Nov 16, 2020, 4:31:46 PM11/16/20
to pdfiu...@googlegroups.com

Comment #12 on issue 1610 by bugdroid: Consider removing CFXJSE_Value
https://bugs.chromium.org/p/pdfium/issues/detail?id=1610#c12


The following revision refers to this bug:
https://pdfium.googlesource.com/pdfium/+/b38e42b4d74b72c13d5f463fbc66ff6827d04970

commit b38e42b4d74b72c13d5f463fbc66ff6827d04970
Author: Tom Sepez <tse...@chromium.org>
Date: Mon Nov 16 21:27:31 2020

Remove CFXJSE_Value from CFXJSE_FormCalcContext::ApplyToExpansion().

Also ApplyToArray() and ApplyToObject().

-- combine two loops despite introducing a (presumably well-
predicted) branch into the loop body.

Bug: pdfium:1610
Change-Id: I72395db33523feb7780654a96ba1494d217763b1
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/76293

Reviewed-by: Daniel Hosseinian <dh...@chromium.org>
Commit-Queue: Tom Sepez <tse...@chromium.org>

bugdroid via monorail

unread,
Nov 17, 2020, 7:50:37 PM11/17/20
to pdfiu...@googlegroups.com

Comment #13 on issue 1610 by bugdroid: Consider removing CFXJSE_Value
https://bugs.chromium.org/p/pdfium/issues/detail?id=1610#c13


The following revision refers to this bug:
https://pdfium.googlesource.com/pdfium/+/145e22d4d49f698d686280a4801acf78409c083c

commit 145e22d4d49f698d686280a4801acf78409c083c
Author: Tom Sepez <tse...@chromium.org>
Date: Wed Nov 18 00:49:06 2020

Remove CFXJSE_Value from CFXJSE_FormCalcContext::DotAccessorCommon().

Bug: pdfium:1610
Change-Id: I211383cd45dfa54d2ffe22e95cf7e9d47dbfb735
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/76422
Commit-Queue: Tom Sepez <tse...@chromium.org>
Reviewed-by: Daniel Hosseinian <dh...@chromium.org>

[modify] https://pdfium.googlesource.com/pdfium/+/145e22d4d49f698d686280a4801acf78409c083c/fxjs/xfa/cfxjse_formcalc_context.cpp
[modify] https://pdfium.googlesource.com/pdfium/+/145e22d4d49f698d686280a4801acf78409c083c/fxjs/fxv8.h
[modify] https://pdfium.googlesource.com/pdfium/+/145e22d4d49f698d686280a4801acf78409c083c/fxjs/fxv8.cpp
Reply all
Reply to author
Forward
0 new messages