Make JS app.setTimeOut() work again.

Make JS app.setTimeOut() work again.

This regressed in commit 794c9b6.

BUG=551248
R=tsepez@chromium.org

Committed: https://pdfium.googlesource.com/pdfium/+/8cadf995e9a0fec8da19f69edac9d10fccca7eed

[Lei Zhang, 2015-11-04 08:24:08]

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

[Lei Zhang, 2015-11-04 08:24:09]

https://codereview.chromium.org/1424743006/diff/1/fpdfsdk/src/javascript/JS_O...
File fpdfsdk/src/javascript/JS_Object.cpp (right):

https://codereview.chromium.org/1424743006/diff/1/fpdfsdk/src/javascript/JS_O...
fpdfsdk/src/javascript/JS_Object.cpp:111: m_swJScript(script),
This is the fix.

[dsinclair, 2015-11-04 14:15:07]

dsinclair@chromium.org changed reviewers:
+ dsinclair@chromium.org

[dsinclair, 2015-11-04 14:15:07]

https://codereview.chromium.org/1424743006/diff/20001/fpdfsdk/src/fpdfformfil...
File fpdfsdk/src/fpdfformfill_embeddertest.cpp (right):

https://codereview.chromium.org/1424743006/diff/20001/fpdfsdk/src/fpdfformfil...
fpdfsdk/src/fpdfformfill_embeddertest.cpp:82: wchar_t* message =
GetWideString(alerts[0].message);
FX_WCHAR?

https://codereview.chromium.org/1424743006/diff/20001/fpdfsdk/src/fpdfformfil...
fpdfsdk/src/fpdfformfill_embeddertest.cpp:83: wchar_t* title =
GetWideString(alerts[0].title);
Can these go in a unique_ptr so they just get cleaned at the end?

https://codereview.chromium.org/1424743006/diff/20001/samples/pdfium_test.cc
File samples/pdfium_test.cc (right):

https://codereview.chromium.org/1424743006/diff/20001/samples/pdfium_test.cc#...
samples/pdfium_test.cc:199: // Deal with differences between UTF16LE and wchar_t
on this platform.
Does this comment still apply?

https://codereview.chromium.org/1424743006/diff/20001/samples/pdfium_test.cc#...
samples/pdfium_test.cc:200: wchar_t* platform_string = GetWideString(msg);
unique_ptr?

https://codereview.chromium.org/1424743006/diff/20001/testing/test_support.h
File testing/test_support.h (right):

https://codereview.chromium.org/1424743006/diff/20001/testing/test_support.h#...
testing/test_support.h:22: wchar_t* GetWideString(FPDF_WIDESTRING wstr);
FX_WCHAR?

https://codereview.chromium.org/1424743006/diff/20001/testing/test_support.h#...
testing/test_support.h:22: wchar_t* GetWideString(FPDF_WIDESTRING wstr);
const FPDF_WIDESTRING wstr

https://codereview.chromium.org/1424743006/diff/20001/testing/test_support.h#...
testing/test_support.h:22: wchar_t* GetWideString(FPDF_WIDESTRING wstr);
Can this return a unique_ptr?

[Tom Sepez, 2015-11-04 18:02:33]

lgtm

Nothing but nits from me.

https://codereview.chromium.org/1424743006/diff/20001/fpdfsdk/src/fpdfformfil...
File fpdfsdk/src/fpdfformfill_embeddertest.cpp (right):

https://codereview.chromium.org/1424743006/diff/20001/fpdfsdk/src/fpdfformfil...
fpdfsdk/src/fpdfformfill_embeddertest.cpp:84: EXPECT_STREQ(L"hello chamal",
message);
nit: we might want to sanitize chamal's identity.

https://codereview.chromium.org/1424743006/diff/20001/testing/test_support.cpp
File testing/test_support.cpp (right):

https://codereview.chromium.org/1424743006/diff/20001/testing/test_support.cp...
testing/test_support.cpp:102: if (!wstr)
nit: move the comment about utf16le from the other file here.

[Lei Zhang, 2015-11-04 22:11:30]

https://codereview.chromium.org/1424743006/diff/20001/fpdfsdk/src/fpdfformfil...
File fpdfsdk/src/fpdfformfill_embeddertest.cpp (right):

https://codereview.chromium.org/1424743006/diff/20001/fpdfsdk/src/fpdfformfil...
fpdfsdk/src/fpdfformfill_embeddertest.cpp:82: wchar_t* message =
GetWideString(alerts[0].message);
On 2015/11/04 14:15:07, dsinclair wrote:
> FX_WCHAR?

Nope, see same comment in other file.

https://codereview.chromium.org/1424743006/diff/20001/fpdfsdk/src/fpdfformfil...
fpdfsdk/src/fpdfformfill_embeddertest.cpp:83: wchar_t* title =
GetWideString(alerts[0].title);
On 2015/11/04 14:15:07, dsinclair wrote:
> Can these go in a unique_ptr so they just get cleaned at the end?

Went with std::wstring.

https://codereview.chromium.org/1424743006/diff/20001/fpdfsdk/src/fpdfformfil...
fpdfsdk/src/fpdfformfill_embeddertest.cpp:84: EXPECT_STREQ(L"hello chamal",
message);
On 2015/11/04 18:02:33, Tom Sepez wrote:
> nit: we might want to sanitize chamal's identity.

But hello world is so boring. The bug is public, *shrug*.

https://codereview.chromium.org/1424743006/diff/20001/samples/pdfium_test.cc
File samples/pdfium_test.cc (right):

https://codereview.chromium.org/1424743006/diff/20001/samples/pdfium_test.cc#...
samples/pdfium_test.cc:199: // Deal with differences between UTF16LE and wchar_t
on this platform.
On 2015/11/04 14:15:07, dsinclair wrote:
> Does this comment still apply?

Moved.

https://codereview.chromium.org/1424743006/diff/20001/samples/pdfium_test.cc#...
samples/pdfium_test.cc:200: wchar_t* platform_string = GetWideString(msg);
On 2015/11/04 14:15:07, dsinclair wrote:
> unique_ptr?

Went with std::wstring.

https://codereview.chromium.org/1424743006/diff/20001/testing/test_support.cpp
File testing/test_support.cpp (right):

https://codereview.chromium.org/1424743006/diff/20001/testing/test_support.cp...
testing/test_support.cpp:102: if (!wstr)
On 2015/11/04 18:02:33, Tom Sepez wrote:
> nit: move the comment about utf16le from the other file here.

I put it in the header.

https://codereview.chromium.org/1424743006/diff/20001/testing/test_support.h
File testing/test_support.h (right):

https://codereview.chromium.org/1424743006/diff/20001/testing/test_support.h#...
testing/test_support.h:22: wchar_t* GetWideString(FPDF_WIDESTRING wstr);
On 2015/11/04 14:15:07, dsinclair wrote:
> const FPDF_WIDESTRING wstr

typedef const unsigned short* FPDF_WIDESTRING;

-> const const unsigned short* ?? @_@

https://codereview.chromium.org/1424743006/diff/20001/testing/test_support.h#...
testing/test_support.h:22: wchar_t* GetWideString(FPDF_WIDESTRING wstr);
On 2015/11/04 14:15:07, dsinclair wrote:
> FX_WCHAR?

But then we'll need to pull in core/ #includes. We want to move away from FX_
types anyway.

https://codereview.chromium.org/1424743006/diff/20001/testing/test_support.h#...
testing/test_support.h:22: wchar_t* GetWideString(FPDF_WIDESTRING wstr);
On 2015/11/04 14:15:07, dsinclair wrote:
> Can this return a unique_ptr?

Went with std::wstring.

[Tom Sepez, 2015-11-04 22:16:03]

lgtm

[Lei Zhang, 2015-11-04 22:40:09]

Committed patchset #4 (id:60001) manually as
8cadf995e9a0fec8da19f69edac9d10fccca7eed (presubmit successful).

[Tom Sepez, 2015-11-04 23:01:11]

Hey Lei, try this:

https://codereview.chromium.org/1424743006/diff/60001/testing/embedder_test_t...
File testing/embedder_test_timer_handling_delegate.h (right):

https://codereview.chromium.org/1424743006/diff/60001/testing/embedder_test_t...
testing/embedder_test_timer_handling_delegate.h:23: FPDF_WIDESTRING message;
maybe make these the std::wstrings() so the copy happens here. Not sure how long
the memory is supposed to live after you return from the handler.