Initialize V8 external data (XFA version).

Initialize V8 external data (XFA version).

This is a quick patch to fix the segv which occurs in pdfium_test when
the v8 external data is not provided.  Fixing the full-up initialization
is left as a follow-on exercise.

Path manipulations remain a nuisance since we don't want to depend on a
particular /base library.


BUG=439793
R=thestig@chromium.org

Committed: https://pdfium.googlesource.com/pdfium/+/5ee12d778eb90ad865085680d49cd9f0391f3edd

[Tom Sepez, 2014-12-17 20:45:12]

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

[Tom Sepez, 2014-12-17 20:45:48]

Lei, please review

[Lei Zhang, 2014-12-17 22:35:51]

https://codereview.chromium.org/810793004/diff/60001/samples/pdfium_test.cc
File samples/pdfium_test.cc (right):

https://codereview.chromium.org/810793004/diff/60001/samples/pdfium_test.cc#n...
samples/pdfium_test.cc:25: #define SEPARATOR '\\'
Maybe PATH_SEPARATOR?

https://codereview.chromium.org/810793004/diff/60001/samples/pdfium_test.cc#n...
samples/pdfium_test.cc:45: static char* GetFileContents(const char* filename,
size_t* retlen) {
It would be nice to document the functions.

https://codereview.chromium.org/810793004/diff/60001/samples/pdfium_test.cc#n...
samples/pdfium_test.cc:52: size_t file_length = ftell(file);
Can we get 0 here? If so, don't bother mallocing?

https://codereview.chromium.org/810793004/diff/60001/samples/pdfium_test.cc#n...
samples/pdfium_test.cc:72: char* output_buffer,
|output_buffer| and |output_buffer_size| -> a single std::string* and don't
worry about |space_remaining| ? If the output string is too long, the caller can
deal with it.

https://codereview.chromium.org/810793004/diff/60001/samples/pdfium_test.cc#n...
samples/pdfium_test.cc:118: const char *bin_filename,
nit: char*, ditto below.

https://codereview.chromium.org/810793004/diff/60001/samples/pdfium_test.cc#n...
samples/pdfium_test.cc:120: char full_path[4096];
4096 -> PATH_MAX ?

https://codereview.chromium.org/810793004/diff/60001/samples/pdfium_test.cc#n...
samples/pdfium_test.cc:304: options->output_format = OUTPUT_NONE;
Options probably should have a constructor to do this.

[Tom Sepez, 2014-12-17 23:53:00]

Please re-review.

https://codereview.chromium.org/810793004/diff/60001/samples/pdfium_test.cc
File samples/pdfium_test.cc (right):

https://codereview.chromium.org/810793004/diff/60001/samples/pdfium_test.cc#n...
samples/pdfium_test.cc:25: #define SEPARATOR '\\'
On 2014/12/17 22:35:50, Lei Zhang wrote:
> Maybe PATH_SEPARATOR?

Done.

https://codereview.chromium.org/810793004/diff/60001/samples/pdfium_test.cc#n...
samples/pdfium_test.cc:45: static char* GetFileContents(const char* filename,
size_t* retlen) {
On 2014/12/17 22:35:51, Lei Zhang wrote:
> It would be nice to document the functions.

Done.

https://codereview.chromium.org/810793004/diff/60001/samples/pdfium_test.cc#n...
samples/pdfium_test.cc:52: size_t file_length = ftell(file);
On 2014/12/17 22:35:50, Lei Zhang wrote:
> Can we get 0 here? If so, don't bother mallocing?

Done.

https://codereview.chromium.org/810793004/diff/60001/samples/pdfium_test.cc#n...
samples/pdfium_test.cc:72: char* output_buffer,
On 2014/12/17 22:35:51, Lei Zhang wrote:
> |output_buffer| and |output_buffer_size| -> a single std::string* and don't
> worry about |space_remaining| ? If the output string is too long, the caller
can
> deal with it.

I think I'm going to replace this whole mess with std::strings.

https://codereview.chromium.org/810793004/diff/60001/samples/pdfium_test.cc#n...
samples/pdfium_test.cc:120: char full_path[4096];
On 2014/12/17 22:35:51, Lei Zhang wrote:
> 4096 -> PATH_MAX ?
Present on windows?

https://codereview.chromium.org/810793004/diff/60001/samples/pdfium_test.cc#n...
samples/pdfium_test.cc:304: options->output_format = OUTPUT_NONE;
On 2014/12/17 22:35:50, Lei Zhang wrote:
> Options probably should have a constructor to do this.

D'oh. Done.

[Lei Zhang, 2014-12-18 00:07:01]

lgtm with some minor changes:

https://codereview.chromium.org/810793004/diff/60001/samples/pdfium_test.cc
File samples/pdfium_test.cc (right):

https://codereview.chromium.org/810793004/diff/60001/samples/pdfium_test.cc#n...
samples/pdfium_test.cc:72: char* output_buffer,
On 2014/12/17 23:52:59, Tom Sepez wrote:
> On 2014/12/17 22:35:51, Lei Zhang wrote:
> > |output_buffer| and |output_buffer_size| -> a single std::string* and don't
> > worry about |space_remaining| ? If the output string is too long, the caller
> can
> > deal with it.
> 
> I think I'm going to replace this whole mess with std::strings.

Oh, so much nicer. :)

https://codereview.chromium.org/810793004/diff/100001/samples/pdfium_test.cc
File samples/pdfium_test.cc (right):

https://codereview.chromium.org/810793004/diff/100001/samples/pdfium_test.cc#...
samples/pdfium_test.cc:104: char *data_buffer =
GetFileContents(full_path.c_str(), &data_length);
Still got a couple "char *"s in this file.

https://codereview.chromium.org/810793004/diff/100001/samples/pdfium_test.cc#...
samples/pdfium_test.cc:281: if (!args.size()) {
!args.size() -> args.empty()

https://codereview.chromium.org/810793004/diff/100001/samples/pdfium_test.cc#...
samples/pdfium_test.cc:317: options->bin_directory = cur_arg.substr(10);
If |cur_arg| is "--bin-dir=" then this goes out of range. I'd check
cur_arg.size() on line 312 and then do cur_arg.compare().

[Tom Sepez, 2014-12-18 00:15:59]

Thanks.

https://codereview.chromium.org/810793004/diff/100001/samples/pdfium_test.cc
File samples/pdfium_test.cc (right):

https://codereview.chromium.org/810793004/diff/100001/samples/pdfium_test.cc#...
samples/pdfium_test.cc:104: char *data_buffer =
GetFileContents(full_path.c_str(), &data_length);
On 2014/12/18 00:07:01, Lei Zhang wrote:
> Still got a couple "char *"s in this file.

Done.

https://codereview.chromium.org/810793004/diff/100001/samples/pdfium_test.cc#...
samples/pdfium_test.cc:281: if (!args.size()) {
On 2014/12/18 00:07:01, Lei Zhang wrote:
> !args.size() -> args.empty()

Done.

[Lei Zhang, 2014-12-18 00:23:07]

lgtm++

[Tom Sepez, 2014-12-18 00:24:08]

Committed patchset #7 (id:120001) manually as
5ee12d778eb90ad865085680d49cd9f0391f3edd (presubmit successful).