Add fpdfppo_embeddertest.cpp.

Add fpdfppo_embeddertest.cpp.

The lack of coverage of the fpdfppo APIs was noticed while
trying to diagnose another issue.  Adding basic calls to
these APIs then kicked out an assert in XFA, where duplicate
global CFXA_TimeZoneProviders were not expected.  These are
cheap to create except for the global C RTL tzset() call, so
keep track of that and make these on demand.

Committed: https://pdfium.googlesource.com/pdfium/+/211d4edbe2f71ca62c76f36ce25090342c58e43c

[Tom Sepez, 2016-11-11 22:13:58]

The CQ bit was checked by tsepez@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-11 22:14:05]

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

[Tom Sepez, 2016-11-11 22:15:41]

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

[Tom Sepez, 2016-11-11 22:15:42]

Lei, ready for review.  Presumably a consequence of some of
the XFA refactoring, but Dan's out and this is blocking work
on another test.

[Tom Sepez, 2016-11-11 22:19:50]

https://codereview.chromium.org/2488403004/diff/1/xfa/fxfa/parser/xfa_localem...
File xfa/fxfa/parser/xfa_localemgr.cpp (left):

https://codereview.chromium.org/2488403004/diff/1/xfa/fxfa/parser/xfa_localem...
xfa/fxfa/parser/xfa_localemgr.cpp:1252: ASSERT(!g_pProvider);
This is the assert that trips when trying to create a second doc under XFA.

[commit-bot: I haz the power, 2016-11-11 22:30:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-11 22:30:06]

Dry run: This issue passed the CQ dry run.

[Tom Sepez, 2016-11-11 22:44:16]

The CQ bit was checked by tsepez@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-11 22:44:24]

Dry run: 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-11-11 22:58:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-11 22:58:14]

Dry run: This issue passed the CQ dry run.

[Lei Zhang, 2016-11-11 23:56:48]

lgtm with a bunch of suggestions.

https://codereview.chromium.org/2488403004/diff/1/fpdfsdk/fpdfppo_embeddertes...
File fpdfsdk/fpdfppo_embeddertest.cpp (right):

https://codereview.chromium.org/2488403004/diff/1/fpdfsdk/fpdfppo_embeddertes...
fpdfsdk/fpdfppo_embeddertest.cpp:30: FPDF_ImportPages(output_doc, document(),
"0", 0);
Check return result?

https://codereview.chromium.org/2488403004/diff/1/fpdfsdk/fpdfppo_embeddertes...
fpdfsdk/fpdfppo_embeddertest.cpp:30: FPDF_ImportPages(output_doc, document(),
"0", 0);
Check FPDF_GetPageCount(output_doc) returns 1?

https://codereview.chromium.org/2488403004/diff/1/xfa/fxfa/fm2js/xfa_fm2jscon...
File xfa/fxfa/fm2js/xfa_fm2jscontext.cpp (right):

https://codereview.chromium.org/2488403004/diff/1/xfa/fxfa/fm2js/xfa_fm2jscon...
xfa/fxfa/fm2js/xfa_fm2jscontext.cpp:1613: while (mins > 1440)
mins %= 1440?

https://codereview.chromium.org/2488403004/diff/1/xfa/fxfa/fm2js/xfa_fm2jscon...
xfa/fxfa/fm2js/xfa_fm2jscontext.cpp:1616: while (mins < 0)
And then just if (mins < 0) ?

https://codereview.chromium.org/2488403004/diff/1/xfa/fxfa/parser/xfa_localem...
File xfa/fxfa/parser/xfa_localemgr.h (right):

https://codereview.chromium.org/2488403004/diff/1/xfa/fxfa/parser/xfa_localem...
xfa/fxfa/parser/xfa_localemgr.h:65: void GetTimeZone(FX_TIMEZONE& tz) const;
Change this to take a pointer?

[Tom Sepez, 2016-11-12 00:35:46]

Ok, ready for another quick look, having implemented suggestions plus a bunch of
my own.

https://codereview.chromium.org/2488403004/diff/1/fpdfsdk/fpdfppo_embeddertes...
File fpdfsdk/fpdfppo_embeddertest.cpp (right):

https://codereview.chromium.org/2488403004/diff/1/fpdfsdk/fpdfppo_embeddertes...
fpdfsdk/fpdfppo_embeddertest.cpp:30: FPDF_ImportPages(output_doc, document(),
"0", 0);
On 2016/11/11 23:56:48, Lei Zhang (slow) wrote:
> Check FPDF_GetPageCount(output_doc) returns 1?

Done.

https://codereview.chromium.org/2488403004/diff/1/fpdfsdk/fpdfppo_embeddertes...
fpdfsdk/fpdfppo_embeddertest.cpp:30: FPDF_ImportPages(output_doc, document(),
"0", 0);
On 2016/11/11 23:56:48, Lei Zhang (slow) wrote:
> Check FPDF_GetPageCount(output_doc) returns 1?

Done.

https://codereview.chromium.org/2488403004/diff/1/xfa/fxfa/fm2js/xfa_fm2jscon...
File xfa/fxfa/fm2js/xfa_fm2jscontext.cpp (right):

https://codereview.chromium.org/2488403004/diff/1/xfa/fxfa/fm2js/xfa_fm2jscon...
xfa/fxfa/fm2js/xfa_fm2jscontext.cpp:1613: while (mins > 1440)
On 2016/11/11 23:56:48, Lei Zhang (slow) wrote:
> mins %= 1440?

leave for now, no functional changes til another cl.

https://codereview.chromium.org/2488403004/diff/1/xfa/fxfa/fm2js/xfa_fm2jscon...
xfa/fxfa/fm2js/xfa_fm2jscontext.cpp:1616: while (mins < 0)
On 2016/11/11 23:56:48, Lei Zhang (slow) wrote:
> And then just if (mins < 0) ?
ditto

https://codereview.chromium.org/2488403004/diff/1/xfa/fxfa/parser/xfa_localem...
File xfa/fxfa/parser/xfa_localemgr.h (right):

https://codereview.chromium.org/2488403004/diff/1/xfa/fxfa/parser/xfa_localem...
xfa/fxfa/parser/xfa_localemgr.h:65: void GetTimeZone(FX_TIMEZONE& tz) const;
On 2016/11/11 23:56:48, Lei Zhang (slow) wrote:
> Change this to take a pointer?

Done.

[Lei Zhang, 2016-11-12 00:44:07]

still lgtm

https://codereview.chromium.org/2488403004/diff/40001/fpdfsdk/fpdfppo_embedde...
File fpdfsdk/fpdfppo_embeddertest.cpp (right):

https://codereview.chromium.org/2488403004/diff/40001/fpdfsdk/fpdfppo_embedde...
fpdfsdk/fpdfppo_embeddertest.cpp:78:
EXPECT_TRUE(OpenDocument("viewer_ref.pdf"));
Maybe add another test with a multi-page PDF so you can test 1-2 or whatever
else.

https://codereview.chromium.org/2488403004/diff/40001/fpdfsdk/fpdfppo_embedde...
fpdfsdk/fpdfppo_embeddertest.cpp:86: EXPECT_TRUE(FPDF_ImportPages(output_doc,
document(), "1,1,1,1", 0));
Test FPDF_GetPageCount().

[Tom Sepez, 2016-11-12 00:50:10]

https://codereview.chromium.org/2488403004/diff/40001/fpdfsdk/fpdfppo_embedde...
File fpdfsdk/fpdfppo_embeddertest.cpp (right):

https://codereview.chromium.org/2488403004/diff/40001/fpdfsdk/fpdfppo_embedde...
fpdfsdk/fpdfppo_embeddertest.cpp:78:
EXPECT_TRUE(OpenDocument("viewer_ref.pdf"));
On 2016/11/12 00:44:07, Lei Zhang (slow) wrote:
> Maybe add another test with a multi-page PDF so you can test 1-2 or whatever
> else.

Ok, lots of follow-ups.   Theres a more important one blocked on this, though.

https://codereview.chromium.org/2488403004/diff/40001/fpdfsdk/fpdfppo_embedde...
fpdfsdk/fpdfppo_embeddertest.cpp:86: EXPECT_TRUE(FPDF_ImportPages(output_doc,
document(), "1,1,1,1", 0));
On 2016/11/12 00:44:07, Lei Zhang (slow) wrote:
> Test FPDF_GetPageCount().

Done.

Fascinating.  Doc now has 5 copies of page 1.  We can deal with whether that
makes sense later.

[Tom Sepez, 2016-11-12 00:50:46]

The CQ bit was checked by tsepez@chromium.org

[Tom Sepez, 2016-11-12 00:50:46]

The patchset sent to the CQ was uploaded after l-g-t-m from thestig@chromium.org
Link to the patchset: https://codereview.chromium.org/2488403004/#ps60001
(title: "One last suggestion")

[commit-bot: I haz the power, 2016-11-12 00:50:54]

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

[Lei Zhang, 2016-11-12 00:57:45]

https://codereview.chromium.org/2488403004/diff/40001/fpdfsdk/fpdfppo_embedde...
File fpdfsdk/fpdfppo_embeddertest.cpp (right):

https://codereview.chromium.org/2488403004/diff/40001/fpdfsdk/fpdfppo_embedde...
fpdfsdk/fpdfppo_embeddertest.cpp:86: EXPECT_TRUE(FPDF_ImportPages(output_doc,
document(), "1,1,1,1", 0));
On 2016/11/12 00:50:09, Tom Sepez wrote:
> On 2016/11/12 00:44:07, Lei Zhang (slow) wrote:
> > Test FPDF_GetPageCount().
> 
> Done.
> 
> Fascinating.  Doc now has 5 copies of page 1.  We can deal with whether that
> makes sense later.

Yes, it does to me. If you have imported page 1 4x here and 1x on the next line.
FPDF_ImportPages() is rather flexible and for a 7 page PDF, can do things like
"import pages 5,3,1-4,7,7,2 in that order."

[commit-bot: I haz the power, 2016-11-12 01:23:51]

Description was changed from

==========
Add fpdfppo_embeddertest.cpp.

The lack of coverage of the fpdfppo APIs was noticed while
trying to diagnose another issue.  Adding basic calls to
these APIs then kicked out an assert in XFA, where duplicate
global CFXA_TimeZoneProviders were not expected.  These are
cheap to create except for the global C RTL tzset() call, so
keep track of that and make these on demand.
==========

to

==========
Add fpdfppo_embeddertest.cpp.

The lack of coverage of the fpdfppo APIs was noticed while
trying to diagnose another issue.  Adding basic calls to
these APIs then kicked out an assert in XFA, where duplicate
global CFXA_TimeZoneProviders were not expected.  These are
cheap to create except for the global C RTL tzset() call, so
keep track of that and make these on demand.

Committed:
https://pdfium.googlesource.com/pdfium/+/211d4edbe2f71ca62c76f36ce25090342c58...
==========

[commit-bot: I haz the power, 2016-11-12 01:23:52]

Committed patchset #4 (id:60001) as
https://pdfium.googlesource.com/pdfium/+/211d4edbe2f71ca62c76f36ce25090342c58...