Merge to master: Take Jun's UTF16LE patches.

Merge to master: Take Jun's UTF16LE patches.

(cherry picked from commit 8d89e65897d8b6cf7899e7a82d9d381c3ad327cb)
(cherry picked from commit c70b19aad245fb1ed39bf8c264d991555f4c5a58)

BUG=pdfium:275
TBR=jun_fang@foxitsoftware.com

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

[Tom Sepez, 2015-11-25 23:37:45]

TBR, trivial conflicts.

[Tom Sepez, 2015-11-25 23:37:59]

Description was changed from

==========
Merge to master: Take Jun's UTF16LE patches.

(cherry picked from commit 8d89e65897d8b6cf7899e7a82d9d381c3ad327cb)
(cherry picked from commit c70b19aad245fb1ed39bf8c264d991555f4c5a58)

BUG=pdfium:275
TBR=jun_fang@foxitsoftware.com
==========

to

==========
Merge to master: Take Jun's UTF16LE patches.

(cherry picked from commit 8d89e65897d8b6cf7899e7a82d9d381c3ad327cb)
(cherry picked from commit c70b19aad245fb1ed39bf8c264d991555f4c5a58)

BUG=pdfium:275
TBR=jun_fang@foxitsoftware.com

Committed:
https://pdfium.googlesource.com/pdfium/+/c3e4ae5fe5067723b58a2029a95c6411c92b...
==========

[Tom Sepez, 2015-11-25 23:38:00]

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

[Oliver Chang, 2015-12-08 00:21:49]

On 2015/11/25 at 23:38:00, tsepez wrote:
> Committed patchset #1 (id:1) manually as
c3e4ae5fe5067723b58a2029a95c6411c92bed15 (presubmit successful).

Looks like this broke browser_tests:

e.g.

 Error in testHasCorrectTitle
Actual: PDF title tes
Expected: PDF title test


Test Error in testHasCorrectBookmarks
Actual: First Sectio
Expected: First Section

[Oliver Chang, 2015-12-08 00:35:23]

On 2015/12/08 at 00:21:49, Oliver Chang wrote:
> On 2015/11/25 at 23:38:00, tsepez wrote:
> > Committed patchset #1 (id:1) manually as
c3e4ae5fe5067723b58a2029a95c6411c92bed15 (presubmit successful).
> 
> Looks like this broke browser_tests:
> 
> e.g.
> 
>  Error in testHasCorrectTitle
> Actual: PDF title tes
> Expected: PDF title test
> 
> 
> Test Error in testHasCorrectBookmarks
> Actual: First Sectio
> Expected: First Section

Looks like this is the problem, since the size returned doesn't include the null
terminator now.

template <class StringType>
void PDFiumAPIStringBufferAdapter<StringType>::Close(size_t actual_size) {
  DCHECK(!is_closed_);
  is_closed_ = true;

  if (check_expected_size_)
    DCHECK_EQ(expected_size_, actual_size);

  if (actual_size > 0) {
    DCHECK((*str_)[actual_size - 1] == 0);
    str_->resize(actual_size - 1);
  } else {
    str_->clear();
  }
}

Tom, I'm not familiar with the plugin/Chrome bindings side. Are there any other
places that could break?

[Oliver Chang, 2015-12-08 00:36:20]

On 2015/12/08 at 00:35:23, Oliver Chang wrote:
> On 2015/12/08 at 00:21:49, Oliver Chang wrote:
> > On 2015/11/25 at 23:38:00, tsepez wrote:
> > > Committed patchset #1 (id:1) manually as
c3e4ae5fe5067723b58a2029a95c6411c92bed15 (presubmit successful).
> > 
> > Looks like this broke browser_tests:
> > 
> > e.g.
> > 
> >  Error in testHasCorrectTitle
> > Actual: PDF title tes
> > Expected: PDF title test
> > 
> > 
> > Test Error in testHasCorrectBookmarks
> > Actual: First Sectio
> > Expected: First Section
> 
> Looks like this is the problem, since the size returned doesn't include the
null terminator now.
> 
> template <class StringType>
> void PDFiumAPIStringBufferAdapter<StringType>::Close(size_t actual_size) {
>   DCHECK(!is_closed_);
>   is_closed_ = true;
> 
>   if (check_expected_size_)
>     DCHECK_EQ(expected_size_, actual_size);
> 
>   if (actual_size > 0) {
>     DCHECK((*str_)[actual_size - 1] == 0);
>     str_->resize(actual_size - 1);
>   } else {
>     str_->clear();
>   }
> }
> 
> Tom, I'm not familiar with the plugin/Chrome bindings side. Are there any
other places that could break?

+Lei as fyi.

[Lei Zhang, 2015-12-08 01:19:03]

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

[Lei Zhang, 2015-12-08 01:19:03]

Looks like we have some changes that affects the public API? We should update
the API docs, and fix the affected sites in Chromium's pdf/ in the CL that does
the DEPS roll? We should also mention this on various pdfium mailing lists.

[jun_fang, 2015-12-08 02:58:04]

On 2015/12/08 01:19:03, Lei Zhang wrote:
> Looks like we have some changes that affects the public API? We should update
> the API docs, and fix the affected sites in Chromium's pdf/ in the CL that
does
> the DEPS roll? We should also mention this on various pdfium mailing lists.

Hi Lei,

You are right. FPDFBookmark_GetTitle returns the number of bytes after 
my UTF16LE patches. Before these patches, it returned including trailing zeros.
We should go through all places where UTF16LE_Encode is called.

// Function: FPDFBookmark_GetTitle
//          Get title of a bookmark.
// Parameters:
//          bookmark    -   Handle to the bookmark.
//          buffer      -   Buffer for the title. Can be NULL.
//          buflen      -   The length of the buffer in bytes. Can be 0.
// Return value:
//          Number of bytes the title consumes, including trailing zeros.

[jun_fang, 2015-12-08 03:00:26]

On 2015/12/08 02:58:04, jun_fang wrote:
> On 2015/12/08 01:19:03, Lei Zhang wrote:
> > Looks like we have some changes that affects the public API? We should
update
> > the API docs, and fix the affected sites in Chromium's pdf/ in the CL that
> does
> > the DEPS roll? We should also mention this on various pdfium mailing lists.
> 
> Hi Lei,
> 
> You are right. FPDFBookmark_GetTitle returns the number of bytes after 
> my UTF16LE patches. Before these patches, it returned including trailing
zeros.
> We should go through all places where UTF16LE_Encode is called.
> 
> // Function: FPDFBookmark_GetTitle
> //          Get title of a bookmark.
> // Parameters:
> //          bookmark    -   Handle to the bookmark.
> //          buffer      -   Buffer for the title. Can be NULL.
> //          buflen      -   The length of the buffer in bytes. Can be 0.
> // Return value:
> //          Number of bytes the title consumes, including trailing zeros.

In Pdfium_engine.cc, FPDFBookmark_GetTitle's return is treated as buffer size.
After my patches, it should be the number of bytes without trailing zeros.

pp::VarDictionary TraverseBookmarks(FPDF_DOCUMENT doc,
                                    FPDF_BOOKMARK bookmark,
                                    unsigned int depth) {
  pp::VarDictionary dict;
  base::string16 title;
  unsigned long buffer_size = FPDFBookmark_GetTitle(bookmark, NULL, 0);
  if (buffer_size > 0) {
    PDFiumAPIStringBufferSizeInBytesAdapter<base::string16> api_string_adapter(
        &title, buffer_size, true);
    api_string_adapter.Close(FPDFBookmark_GetTitle(
        bookmark, api_string_adapter.GetData(), buffer_size));
  }
  dict.Set(pp::Var("title"), pp::Var(base::UTF16ToUTF8(title)));