Add APIs for limited use of document tagged code.

Add APIs for limited use of document tagged code.

BUG=pdfium:568
Committed: https://pdfium.googlesource.com/pdfium/+/9067fd683ebf8d6467f8cc5aa7daf5e1f950f846

[Lei Zhang, 2016-11-22 09:22:28]

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

[commit-bot: I haz the power, 2016-11-22 09:22:36]

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

[Lei Zhang, 2016-11-22 09:24:10]

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

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

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

[Lei Zhang, 2016-11-22 09:24:24]

Patchset #1 (id:1) has been deleted

[Lei Zhang, 2016-11-22 23:29:52]

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

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

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

[Lei Zhang, 2016-11-22 23:30:41]

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

[Lei Zhang, 2016-11-22 23:30:42]

https://codereview.chromium.org/2519343002/diff/60001/fpdfsdk/fpdf_structtree...
File fpdfsdk/fpdf_structtree_embeddertest.cpp (right):

https://codereview.chromium.org/2519343002/diff/60001/fpdfsdk/fpdf_structtree...
fpdfsdk/fpdf_structtree_embeddertest.cpp:12:
ASSERT_TRUE(OpenDocument("tagged_alt_text.pdf"));
Made tagged_alt_text.pdf in OpenOffice. I set the alternate text property on the
image in the doc, and the OpenOffice PDF exporter has a "tagged pdf" option.

https://codereview.chromium.org/2519343002/diff/60001/fpdfsdk/fpdf_structtree...
fpdfsdk/fpdf_structtree_embeddertest.cpp:52: "\xFE\xFF\00B\00l\00a\00c\00k\00
\00I\00m\00a\00g\00e";
Is there a better way to do this?

[commit-bot: I haz the power, 2016-11-22 23:38:52]

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

[commit-bot: I haz the power, 2016-11-22 23:38:53]

Dry run: This issue passed the CQ dry run.

[Tom Sepez, 2016-11-22 23:45:36]

lgtm

https://codereview.chromium.org/2519343002/diff/60001/fpdfsdk/fpdf_structtree...
File fpdfsdk/fpdf_structtree.cpp (right):

https://codereview.chromium.org/2519343002/diff/60001/fpdfsdk/fpdf_structtree...
fpdfsdk/fpdf_structtree.cpp:5: // Original code copyright 2014 Foxit Software
Inc. http://www.foxitsoftware.com
I'm not sure if we need this line since we cooked this up ourselves.

https://codereview.chromium.org/2519343002/diff/60001/fpdfsdk/fpdf_structtree...
File fpdfsdk/fpdf_structtree_embeddertest.cpp (right):

https://codereview.chromium.org/2519343002/diff/60001/fpdfsdk/fpdf_structtree...
fpdfsdk/fpdf_structtree_embeddertest.cpp:12:
ASSERT_TRUE(OpenDocument("tagged_alt_text.pdf"));
On 2016/11/22 23:30:42, Lei Zhang (Mostly OOO) wrote:
> Made tagged_alt_text.pdf in OpenOffice. I set the alternate text property on
the
> image in the doc, and the OpenOffice PDF exporter has a "tagged pdf" option.

That should be fine.  I pulled the patch and it looks like this is only 2KB or
so.

https://codereview.chromium.org/2519343002/diff/60001/fpdfsdk/fpdf_structtree...
fpdfsdk/fpdf_structtree_embeddertest.cpp:52: "\xFE\xFF\00B\00l\00a\00c\00k\00
\00I\00m\00a\00g\00e";
On 2016/11/22 23:30:42, Lei Zhang (Mostly OOO) wrote:
> Is there a better way to do this?

const uint8_t kBlackImageText[25] = {
  0xfe, 0xff, 0x00 ...

[Lei Zhang, 2016-11-23 00:03:17]

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

[commit-bot: I haz the power, 2016-11-23 00:03:20]

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

[Lei Zhang, 2016-11-23 00:03:26]

https://codereview.chromium.org/2519343002/diff/60001/fpdfsdk/fpdf_structtree...
File fpdfsdk/fpdf_structtree.cpp (right):

https://codereview.chromium.org/2519343002/diff/60001/fpdfsdk/fpdf_structtree...
fpdfsdk/fpdf_structtree.cpp:5: // Original code copyright 2014 Foxit Software
Inc. http://www.foxitsoftware.com
On 2016/11/22 23:45:36, Tom Sepez wrote:
> I'm not sure if we need this line since we cooked this up ourselves.

Done.

[Lei Zhang, 2016-11-23 00:09:08]

https://codereview.chromium.org/2519343002/diff/80001/fpdfsdk/fpdf_structtree...
File fpdfsdk/fpdf_structtree_embeddertest.cpp (right):

https://codereview.chromium.org/2519343002/diff/80001/fpdfsdk/fpdf_structtree...
fpdfsdk/fpdf_structtree_embeddertest.cpp:51: const char kBlackImageText[25] =
{0xfe, 0xff, 0x00, 'B',  0x00, 'l',  0x00,
I'll try CFX_WideString::FromUTF16LE() and see if that's better.

[commit-bot: I haz the power, 2016-11-23 00:10:09]

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

[commit-bot: I haz the power, 2016-11-23 00:10:10]

Dry run: Try jobs failed on following builders:
  win_xfa_32 on master.tryserver.client.pdfium (JOB_FAILED,
https://build.chromium.org/p/tryserver.client.pdfium/builders/win_xfa_32/buil...)

[Lei Zhang, 2016-11-23 00:56:21]

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

[commit-bot: I haz the power, 2016-11-23 00:56:23]

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

[Lei Zhang, 2016-11-23 01:00:40]

Please take another look.

https://codereview.chromium.org/2519343002/diff/100001/fpdfsdk/fpdf_structtre...
File fpdfsdk/fpdf_structtree.cpp (right):

https://codereview.chromium.org/2519343002/diff/100001/fpdfsdk/fpdf_structtre...
fpdfsdk/fpdf_structtree.cpp:50: CFX_WideString str =
elem->GetDict()->GetUnicodeTextFor("Alt");
Made this code, the test, and the comments in the public header consistent with
FPDFBookmark_GetTitle().

https://codereview.chromium.org/2519343002/diff/100001/fpdfsdk/fpdf_structtre...
fpdfsdk/fpdf_structtree.cpp:55: const unsigned long len =
encodedStr.GetLength();
The length of the UTF16LE_Encoded string includes the \0\0 terminator, so no
need to add 1 or 2.

[Lei Zhang, 2016-11-23 01:01:36]

https://codereview.chromium.org/2519343002/diff/100001/public/fpdf_structtree.h
File public/fpdf_structtree.h (right):

https://codereview.chromium.org/2519343002/diff/100001/public/fpdf_structtree...
public/fpdf_structtree.h:67: //          Regardless of the platform, the
|buffer| is always in UTF-16LE
I should add this is no longer what the client's PDF viewer expects, but it's
somewhat sane and they can adapt.

[commit-bot: I haz the power, 2016-11-23 01:05:24]

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

[commit-bot: I haz the power, 2016-11-23 01:05:25]

Dry run: This issue passed the CQ dry run.

[dsinclair, 2016-11-23 14:04:19]

https://codereview.chromium.org/2519343002/diff/100001/fpdfsdk/fpdf_structtre...
File fpdfsdk/fpdf_structtree.cpp (right):

https://codereview.chromium.org/2519343002/diff/100001/fpdfsdk/fpdf_structtre...
fpdfsdk/fpdf_structtree.cpp:16: return
IPDF_StructTree::LoadPage(pPage->m_pDocument, pPage->m_pFormDict);
return page ? IPDF_StructTree::LoadPage(pPage->m_pDocument, pPage->m_pFormDict)
: nullptr?

https://codereview.chromium.org/2519343002/diff/100001/fpdfsdk/fpdf_structtre...
fpdfsdk/fpdf_structtree.cpp:21: }
Add a anonymous ToStructTree helper and remove the casts?

https://codereview.chromium.org/2519343002/diff/100001/fpdfsdk/fpdf_structtre...
fpdfsdk/fpdf_structtree.cpp:56: if (buffer || buflen >= len)
This should be && yea?

https://codereview.chromium.org/2519343002/diff/100001/fpdfsdk/fpdf_structtre...
fpdfsdk/fpdf_structtree.cpp:64:
reinterpret_cast<IPDF_StructElement*>(struct_element);
ToStructElement helper?

https://codereview.chromium.org/2519343002/diff/100001/fpdfsdk/fpdf_structtre...
File fpdfsdk/fpdf_structtree_embeddertest.cpp (right):

https://codereview.chromium.org/2519343002/diff/100001/fpdfsdk/fpdf_structtre...
fpdfsdk/fpdf_structtree_embeddertest.cpp:54: CFX_WideString::FromUTF16LE(buffer,
FXSYS_len(kExpected)));
Add a test where buffer is provided by buflen is less then the text length.

https://codereview.chromium.org/2519343002/diff/100001/public/fpdf_structtree.h
File public/fpdf_structtree.h (right):

https://codereview.chromium.org/2519343002/diff/100001/public/fpdf_structtree...
public/fpdf_structtree.h:23: //          A handle to the structure tree.
null on failure?

https://codereview.chromium.org/2519343002/diff/100001/public/fpdf_structtree...
public/fpdf_structtree.h:52: //          The child at the n-th index.
null on failure?

https://codereview.chromium.org/2519343002/diff/100001/public/fpdf_structtree...
public/fpdf_structtree.h:67: //          Regardless of the platform, the
|buffer| is always in UTF-16LE
On 2016/11/23 01:01:36, Lei Zhang (Mostly OOO) wrote:
> I should add this is no longer what the client's PDF viewer expects, but it's
> somewhat sane and they can adapt.

For curiousity, what do they expect utf-8?

https://codereview.chromium.org/2519343002/diff/100001/public/fpdf_structtree...
public/fpdf_structtree.h:91: //          The child at the n-th index.
null on failure?

[npm, 2016-11-23 16:23:01]

https://codereview.chromium.org/2519343002/diff/100001/fpdfsdk/fpdf_structtre...
File fpdfsdk/fpdf_structtree_embeddertest.cpp (right):

https://codereview.chromium.org/2519343002/diff/100001/fpdfsdk/fpdf_structtre...
fpdfsdk/fpdf_structtree_embeddertest.cpp:56: FPDF_StructTree_Close(struct_tree);
Test that struct_tree is null now?

[Lei Zhang, 2016-11-23 19:48:55]

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

[commit-bot: I haz the power, 2016-11-23 19:49:01]

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

[Lei Zhang, 2016-11-23 19:49:55]

https://codereview.chromium.org/2519343002/diff/100001/fpdfsdk/fpdf_structtre...
File fpdfsdk/fpdf_structtree.cpp (right):

https://codereview.chromium.org/2519343002/diff/100001/fpdfsdk/fpdf_structtre...
fpdfsdk/fpdf_structtree.cpp:16: return
IPDF_StructTree::LoadPage(pPage->m_pDocument, pPage->m_pFormDict);
On 2016/11/23 14:04:19, dsinclair wrote:
> return page ? IPDF_StructTree::LoadPage(pPage->m_pDocument,
pPage->m_pFormDict)
> : nullptr?

It's kind of ugly:
  return pPage
             ? IPDF_StructTree::LoadPage(pPage->m_pDocument, pPage->m_pFormDict)
             : nullptr;

https://codereview.chromium.org/2519343002/diff/100001/fpdfsdk/fpdf_structtre...
fpdfsdk/fpdf_structtree.cpp:21: }
On 2016/11/23 14:04:19, dsinclair wrote:
> Add a anonymous ToStructTree helper and remove the casts?

Done.

https://codereview.chromium.org/2519343002/diff/100001/fpdfsdk/fpdf_structtre...
fpdfsdk/fpdf_structtree.cpp:56: if (buffer || buflen >= len)
On 2016/11/23 14:04:19, dsinclair wrote:
> This should be && yea?

Ya....

https://codereview.chromium.org/2519343002/diff/100001/fpdfsdk/fpdf_structtre...
fpdfsdk/fpdf_structtree.cpp:64:
reinterpret_cast<IPDF_StructElement*>(struct_element);
On 2016/11/23 14:04:18, dsinclair wrote:
> ToStructElement helper?

Done.

https://codereview.chromium.org/2519343002/diff/100001/fpdfsdk/fpdf_structtre...
File fpdfsdk/fpdf_structtree_embeddertest.cpp (right):

https://codereview.chromium.org/2519343002/diff/100001/fpdfsdk/fpdf_structtre...
fpdfsdk/fpdf_structtree_embeddertest.cpp:54: CFX_WideString::FromUTF16LE(buffer,
FXSYS_len(kExpected)));
On 2016/11/23 14:04:19, dsinclair wrote:
> Add a test where buffer is provided by buflen is less then the text length.

Sure.

https://codereview.chromium.org/2519343002/diff/100001/fpdfsdk/fpdf_structtre...
fpdfsdk/fpdf_structtree_embeddertest.cpp:56: FPDF_StructTree_Close(struct_tree);
On 2016/11/23 16:23:01, npm wrote:
> Test that struct_tree is null now?

Calling "delete ptr" doesn't set |ptr| to nullptr...

https://codereview.chromium.org/2519343002/diff/100001/public/fpdf_structtree.h
File public/fpdf_structtree.h (right):

https://codereview.chromium.org/2519343002/diff/100001/public/fpdf_structtree...
public/fpdf_structtree.h:23: //          A handle to the structure tree.
On 2016/11/23 14:04:19, dsinclair wrote:
> null on failure?

Done.

https://codereview.chromium.org/2519343002/diff/100001/public/fpdf_structtree...
public/fpdf_structtree.h:52: //          The child at the n-th index.
On 2016/11/23 14:04:19, dsinclair wrote:
> null on failure?

Done.

https://codereview.chromium.org/2519343002/diff/100001/public/fpdf_structtree...
public/fpdf_structtree.h:67: //          Regardless of the platform, the
|buffer| is always in UTF-16LE
On 2016/11/23 14:04:19, dsinclair wrote:
> On 2016/11/23 01:01:36, Lei Zhang (Mostly OOO) wrote:
> > I should add this is no longer what the client's PDF viewer expects, but
it's
> > somewhat sane and they can adapt.
> 
> For curiousity, what do they expect utf-8?

I believe that's the case.

https://codereview.chromium.org/2519343002/diff/100001/public/fpdf_structtree...
public/fpdf_structtree.h:91: //          The child at the n-th index.
On 2016/11/23 14:04:19, dsinclair wrote:
> null on failure?

Added a more detailed comment section.

[commit-bot: I haz the power, 2016-11-23 20:06:56]

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

[commit-bot: I haz the power, 2016-11-23 20:06:57]

Dry run: This issue passed the CQ dry run.

[Tom Sepez, 2016-11-23 20:28:33]

https://codereview.chromium.org/2519343002/diff/100001/fpdfsdk/fpdf_structtre...
File fpdfsdk/fpdf_structtree.cpp (right):

https://codereview.chromium.org/2519343002/diff/100001/fpdfsdk/fpdf_structtre...
fpdfsdk/fpdf_structtree.cpp:56: if (buffer || buflen >= len)
On 2016/11/23 19:49:55, Lei Zhang (Mostly OOO) wrote:
> On 2016/11/23 14:04:19, dsinclair wrote:
> > This should be && yea?
> 
> Ya....

Also len < buflen may read better, in that < comparisions are usually the safe
case and >= comparisions are usually out-of-bounds cases.

[Lei Zhang, 2016-11-23 21:38:56]

https://codereview.chromium.org/2519343002/diff/100001/fpdfsdk/fpdf_structtre...
File fpdfsdk/fpdf_structtree.cpp (right):

https://codereview.chromium.org/2519343002/diff/100001/fpdfsdk/fpdf_structtre...
fpdfsdk/fpdf_structtree.cpp:56: if (buffer || buflen >= len)
On 2016/11/23 20:28:33, Tom Sepez wrote:
> On 2016/11/23 19:49:55, Lei Zhang (Mostly OOO) wrote:
> > On 2016/11/23 14:04:19, dsinclair wrote:
> > > This should be && yea?
> > 
> > Ya....
> 
> Also len < buflen may read better, in that < comparisions are usually the safe
> case and >= comparisions are usually out-of-bounds cases.

It's copy pasta, so I flipped all the obvious ones around.

[Lei Zhang, 2016-11-23 21:40:57]

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

[commit-bot: I haz the power, 2016-11-23 21:41:06]

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-23 21:48:23]

lgtm

[dsinclair, 2016-11-23 21:48:39]

lgtm

[commit-bot: I haz the power, 2016-11-23 22:01:18]

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

[commit-bot: I haz the power, 2016-11-23 22:01:19]

Dry run: This issue passed the CQ dry run.

[Lei Zhang, 2016-11-23 22:09:36]

The CQ bit was checked by thestig@chromium.org

[commit-bot: I haz the power, 2016-11-23 22:09:49]

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-23 22:10:07]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1479938976964320,
"parent_rev": "841a85b19cc10d719505ea2c7e0e49bae843db6e", "commit_rev":
"9067fd683ebf8d6467f8cc5aa7daf5e1f950f846"}

[commit-bot: I haz the power, 2016-11-23 22:10:09]

Description was changed from

==========
Add APIs for limited use of document tagged code.

BUG=pdfium:568
==========

to

==========
Add APIs for limited use of document tagged code.

BUG=pdfium:568

Committed:
https://pdfium.googlesource.com/pdfium/+/9067fd683ebf8d6467f8cc5aa7daf5e1f950...
==========

[commit-bot: I haz the power, 2016-11-23 22:10:12]

Committed patchset #8 (id:160001) as
https://pdfium.googlesource.com/pdfium/+/9067fd683ebf8d6467f8cc5aa7daf5e1f950...