Merge https://codereview.chromium.org/897973002/ and https://codereview.chromium.org/902753002/ to …

Merge https://codereview.chromium.org/897973002/ and https://codereview.chromium.org/902753002/ to XFA branch

R=jochen@chromium.org

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

[jam, 2015-02-05 01:41:19]

jam@chromium.org changed reviewers:
+ jochen@chromium.org, jun_fang@foxitsoftware.com

[jam, 2015-02-05 01:41:19]

https://codereview.chromium.org/887073005/diff/1/xfa/src/fxjse/src/runtime.cpp
File xfa/src/fxjse/src/runtime.cpp (left):

https://codereview.chromium.org/887073005/diff/1/xfa/src/fxjse/src/runtime.cp...
xfa/src/fxjse/src/runtime.cpp:30: v8::V8::SetFlagsFromString(szCmdFlags,
FXSYS_strlen(szCmdFlags));
I don't know if this works anymore

[jochen (gone - plz use gerrit), 2015-02-06 10:27:12]

https://codereview.chromium.org/887073005/diff/1/xfa/src/fxjse/src/runtime.cpp
File xfa/src/fxjse/src/runtime.cpp (right):

https://codereview.chromium.org/887073005/diff/1/xfa/src/fxjse/src/runtime.cp...
xfa/src/fxjse/src/runtime.cpp:30: v8::V8::SetFlagsFromString(szCmdFlags,
FXSYS_strlen(szCmdFlags));
On 2015/02/05 at 01:41:19, jam wrote:
> I don't know if this works anymore

this works in the sense that it sets the flag.

It doesn't work in the sense that the resulting behavior of V8 is broken, but
that was already the case before. We must not ship this code as long as it
depends on these flags.

[jam, 2015-02-06 23:52:48]

On 2015/02/06 10:27:12, jochen (slow) wrote:
> https://codereview.chromium.org/887073005/diff/1/xfa/src/fxjse/src/runtime.cpp
> File xfa/src/fxjse/src/runtime.cpp (right):
> 
>
https://codereview.chromium.org/887073005/diff/1/xfa/src/fxjse/src/runtime.cp...
> xfa/src/fxjse/src/runtime.cpp:30: v8::V8::SetFlagsFromString(szCmdFlags,
> FXSYS_strlen(szCmdFlags));
> On 2015/02/05 at 01:41:19, jam wrote:
> > I don't know if this works anymore
> 
> this works in the sense that it sets the flag.
> 
> It doesn't work in the sense that the resulting behavior of V8 is broken, but
> that was already the case before. We must not ship this code as long as it
> depends on these flags.

sure, as long as i'm not introducing more breakage, I'm just trying to merge
changes to the branch from trunk.

if this is good as a merge, let me know

[jochen (gone - plz use gerrit), 2015-02-09 14:19:19]

lgtm to merge then

[jun_fang, 2015-02-09 23:44:32]

https://codereview.chromium.org/887073005/diff/1/xfa/src/fxjse/src/runtime.cpp
File xfa/src/fxjse/src/runtime.cpp (right):

https://codereview.chromium.org/887073005/diff/1/xfa/src/fxjse/src/runtime.cp...
xfa/src/fxjse/src/runtime.cpp:30: v8::V8::SetFlagsFromString(szCmdFlags,
FXSYS_strlen(szCmdFlags));
On 2015/02/06 10:27:12, jochen (slow) wrote:
> On 2015/02/05 at 01:41:19, jam wrote:
> > I don't know if this works anymore
> 
> this works in the sense that it sets the flag.
> 
> It doesn't work in the sense that the resulting behavior of V8 is broken, but
> that was already the case before. We must not ship this code as long as it
> depends on these flags.


Feedback from our developers, according to current design,
v8::V8::SetFlagsFromString(szCmdFlags, FXSYS_strlen(szCmdFlags)) shall run
before v8::V8::Initialize(). Otherwise, it doesn't take effect. We plan to
enhance the current design. Before that, can we keep the current implementation?

[jam, 2015-02-10 17:15:20]

Committed patchset #1 (id:1) manually as
b045ed21ec797b5d05a6cef819451b0e8bfdee47 (presubmit successful).

[jam, 2015-02-10 17:15:48]

On 2015/02/09 23:44:32, jun_fang wrote:
> https://codereview.chromium.org/887073005/diff/1/xfa/src/fxjse/src/runtime.cpp
> File xfa/src/fxjse/src/runtime.cpp (right):
> 
>
https://codereview.chromium.org/887073005/diff/1/xfa/src/fxjse/src/runtime.cp...
> xfa/src/fxjse/src/runtime.cpp:30: v8::V8::SetFlagsFromString(szCmdFlags,
> FXSYS_strlen(szCmdFlags));
> On 2015/02/06 10:27:12, jochen (slow) wrote:
> > On 2015/02/05 at 01:41:19, jam wrote:
> > > I don't know if this works anymore
> > 
> > this works in the sense that it sets the flag.
> > 
> > It doesn't work in the sense that the resulting behavior of V8 is broken,
but
> > that was already the case before. We must not ship this code as long as it
> > depends on these flags.
> 
> 
> Feedback from our developers, according to current design,
> v8::V8::SetFlagsFromString(szCmdFlags, FXSYS_strlen(szCmdFlags)) shall run
> before v8::V8::Initialize(). Otherwise, it doesn't take effect. We plan to
> enhance the current design. Before that, can we keep the current
implementation?

please let's keep this conversation on the email thread, so it's easier to have
Jochen comment on the plan