Add new public APIs to find the z-order for links and widgets.

Add new public APIs to find the z-order for links and widgets.

- Implement FPDFLink_GetLinkZOrderAtPoint().
- Implement FPDFPage_FormFieldZOrderAtPoint().
- Mark FPDPage_HasFormFieldAtPoint() as deprecated.
- Modify CPDF_LinkList and CPDF_InterForm to support new APIs.
- Clean up dead code in CPDF_LinkList and CPDF_InterForm.

BUG=chromium:515837
R=jun_fang@foxitsoftware.com, tsepez@chromium.org

Committed: https://pdfium.googlesource.com/pdfium/+/24fbf134d43a7ec4226de3db601f0a617bbe428b

[Lei Zhang, 2015-08-07 07:47:27]

thestig@chromium.org changed reviewers:
+ jun_fang@foxitsoftware.com

[Lei Zhang, 2015-08-07 07:47:27]

Note: previous discussion on https://codereview.chromium.org/958423005/

This CL will be used by https://codereview.chromium.org/1274413002/

[Tom Sepez, 2015-08-07 18:14:06]

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

[Tom Sepez, 2015-08-07 18:14:07]

Nothing but nits and one illegal cast.  LGTM after fixing.

https://codereview.chromium.org/1278053004/diff/1/core/src/fpdfdoc/doc_form.cpp
File core/src/fpdfdoc/doc_form.cpp (right):

https://codereview.chromium.org/1278053004/diff/1/core/src/fpdfdoc/doc_form.c...
core/src/fpdfdoc/doc_form.cpp:878: for (FX_DWORD i = pAnnotList->GetCount(); i >
0; i--) {
nit: --i.

https://codereview.chromium.org/1278053004/diff/1/core/src/fpdfdoc/doc_form.c...
core/src/fpdfdoc/doc_form.cpp:885: if (!m_ControlMap.Lookup(pAnnot,
(void*&)pControl))
Illegal cast.  Needs to be &of an actual void*, and then cast to
CPDF_FormControl* later on.

https://codereview.chromium.org/1278053004/diff/1/core/src/fpdfdoc/doc_link.cpp
File core/src/fpdfdoc/doc_link.cpp (right):

https://codereview.chromium.org/1278053004/diff/1/core/src/fpdfdoc/doc_link.c...
core/src/fpdfdoc/doc_link.cpp:43: int size = pPageLinkList->GetSize();
Nit: wish this were consistent with the index types and tricks used in the
previous file to do the same thing.

https://codereview.chromium.org/1278053004/diff/1/fpdfsdk/src/fpdfdoc.cpp
File fpdfsdk/src/fpdfdoc.cpp (right):

https://codereview.chromium.org/1278053004/diff/1/fpdfsdk/src/fpdfdoc.cpp#new...
fpdfsdk/src/fpdfdoc.cpp:195: CPDF_LinkList* pLinkList =
(CPDF_LinkList*)pDoc->GetPrivateData(&THISMODULE);
nit: Is this the only place we do "get or create"? Otherwise consolidate
duplicate code?

https://codereview.chromium.org/1278053004/diff/1/fpdfsdk/src/fpdfdoc.cpp#new...
fpdfsdk/src/fpdfdoc.cpp:201: pLinkList->GetLinkAtPoint(pPage, (FX_FLOAT)x,
(FX_FLOAT)y, &z_order);
nit: maybe (void) to indicate result not used?

https://codereview.chromium.org/1278053004/diff/1/fpdfsdk/src/fpdfformfill.cpp
File fpdfsdk/src/fpdfformfill.cpp (right):

https://codereview.chromium.org/1278053004/diff/1/fpdfsdk/src/fpdfformfill.cp...
fpdfsdk/src/fpdfformfill.cpp:58: DLLEXPORT int STDCALL
FPDPage_FormFieldZOrderAtPoint(FPDF_FORMHANDLE hHandle,
nit: I never noticed these entry points were named FPDPage, not FPDFPage. I
guess we're stuck with it ...

https://codereview.chromium.org/1278053004/diff/1/public/fpdf_doc.h
File public/fpdf_doc.h (right):

https://codereview.chromium.org/1278053004/diff/1/public/fpdf_doc.h#newcode207
public/fpdf_doc.h:207: //     Z-order of the link, or -1 if no link found at
that point.
nit: just to be clear, higher numbers are closer to front (RHS)?

[Tom Sepez, 2015-08-07 22:13:31]

https://codereview.chromium.org/1278053004/diff/20001/public/fpdf_doc.h
File public/fpdf_doc.h (right):

https://codereview.chromium.org/1278053004/diff/20001/public/fpdf_doc.h#newco...
public/fpdf_doc.h:208: //     Higher numbers is closer to the front.
nit: are

https://codereview.chromium.org/1278053004/diff/20001/public/fpdf_formfill.h
File public/fpdf_formfill.h (right):

https://codereview.chromium.org/1278053004/diff/20001/public/fpdf_formfill.h#...
public/fpdf_formfill.h:1002: *     Higher numbers is closer to the front.
nit: are

[Lei Zhang, 2015-08-07 23:03:41]

https://codereview.chromium.org/1278053004/diff/1/core/src/fpdfdoc/doc_form.cpp
File core/src/fpdfdoc/doc_form.cpp (right):

https://codereview.chromium.org/1278053004/diff/1/core/src/fpdfdoc/doc_form.c...
core/src/fpdfdoc/doc_form.cpp:878: for (FX_DWORD i = pAnnotList->GetCount(); i >
0; i--) {
On 2015/08/07 18:14:06, Tom Sepez wrote:
> nit: --i.

Done.

https://codereview.chromium.org/1278053004/diff/1/core/src/fpdfdoc/doc_form.c...
core/src/fpdfdoc/doc_form.cpp:885: if (!m_ControlMap.Lookup(pAnnot,
(void*&)pControl))
On 2015/08/07 18:14:06, Tom Sepez wrote:
> Illegal cast.  Needs to be &of an actual void*, and then cast to
> CPDF_FormControl* later on.

I converted m_ControlMap to a std::map.

https://codereview.chromium.org/1278053004/diff/1/core/src/fpdfdoc/doc_link.cpp
File core/src/fpdfdoc/doc_link.cpp (right):

https://codereview.chromium.org/1278053004/diff/1/core/src/fpdfdoc/doc_link.c...
core/src/fpdfdoc/doc_link.cpp:43: int size = pPageLinkList->GetSize();
On 2015/08/07 18:14:06, Tom Sepez wrote:
> Nit: wish this were consistent with the index types and tricks used in the
> previous file to do the same thing.

Converting |pPageLinkList| from a CFX_PtrArray to a
std::vector<CPDF_Dictionary*>.

https://codereview.chromium.org/1278053004/diff/1/fpdfsdk/src/fpdfdoc.cpp
File fpdfsdk/src/fpdfdoc.cpp (right):

https://codereview.chromium.org/1278053004/diff/1/fpdfsdk/src/fpdfdoc.cpp#new...
fpdfsdk/src/fpdfdoc.cpp:195: CPDF_LinkList* pLinkList =
(CPDF_LinkList*)pDoc->GetPrivateData(&THISMODULE);
On 2015/08/07 18:14:06, Tom Sepez wrote:
> nit: Is this the only place we do "get or create"? Otherwise consolidate
> duplicate code?

I consolidated.

https://codereview.chromium.org/1278053004/diff/1/fpdfsdk/src/fpdfdoc.cpp#new...
fpdfsdk/src/fpdfdoc.cpp:201: pLinkList->GetLinkAtPoint(pPage, (FX_FLOAT)x,
(FX_FLOAT)y, &z_order);
On 2015/08/07 18:14:06, Tom Sepez wrote:
> nit: maybe (void) to indicate result not used?

Done.

https://codereview.chromium.org/1278053004/diff/1/fpdfsdk/src/fpdfformfill.cpp
File fpdfsdk/src/fpdfformfill.cpp (right):

https://codereview.chromium.org/1278053004/diff/1/fpdfsdk/src/fpdfformfill.cp...
fpdfsdk/src/fpdfformfill.cpp:58: DLLEXPORT int STDCALL
FPDPage_FormFieldZOrderAtPoint(FPDF_FORMHANDLE hHandle,
On 2015/08/07 18:14:06, Tom Sepez wrote:
> nit: I never noticed these entry points were named FPDPage, not FPDFPage. I
> guess we're stuck with it ... 

I'll fix it but retain the old one.

https://codereview.chromium.org/1278053004/diff/1/public/fpdf_doc.h
File public/fpdf_doc.h (right):

https://codereview.chromium.org/1278053004/diff/1/public/fpdf_doc.h#newcode207
public/fpdf_doc.h:207: //     Z-order of the link, or -1 if no link found at
that point.
On 2015/08/07 18:14:06, Tom Sepez wrote:
> nit: just to be clear, higher numbers are closer to front (RHS)?

Yep. Added comment here and in fpdf_formfill.h.

https://codereview.chromium.org/1278053004/diff/20001/public/fpdf_doc.h
File public/fpdf_doc.h (right):

https://codereview.chromium.org/1278053004/diff/20001/public/fpdf_doc.h#newco...
public/fpdf_doc.h:208: //     Higher numbers is closer to the front.
On 2015/08/07 22:13:31, Tom Sepez wrote:
> nit: are

Done.

https://codereview.chromium.org/1278053004/diff/20001/public/fpdf_formfill.h
File public/fpdf_formfill.h (right):

https://codereview.chromium.org/1278053004/diff/20001/public/fpdf_formfill.h#...
public/fpdf_formfill.h:1002: *     Higher numbers is closer to the front.
On 2015/08/07 22:13:31, Tom Sepez wrote:
> nit: are

Done.

[Lei Zhang, 2015-08-07 23:43:16]

Jun, I would like your input since we are adding a new API.

Tom, PTAL at patch set 3, since it changed quite a bit from patch set 1.

[jun_fang, 2015-08-08 01:02:38]

On 2015/08/07 23:43:16, Lei Zhang wrote:
> Jun, I would like your input since we are adding a new API.
> 
> Tom, PTAL at patch set 3, since it changed quite a bit from patch set 1.

Hi Lei,

I am interested in how applications utilize z-order to choose links or widgets.
In this new API, it always returns the largest index matching the condition.

[Lei Zhang, 2015-08-08 01:20:36]

On 2015/08/08 01:02:38, jun_fang wrote:
> On 2015/08/07 23:43:16, Lei Zhang wrote:
> > Jun, I would like your input since we are adding a new API.
> > 
> > Tom, PTAL at patch set 3, since it changed quite a bit from patch set 1.
> 
> Hi Lei,
> 
> I am interested in how applications utilize z-order to choose links or
widgets.
> In this new API, it always returns the largest index matching the condition.

See https://codereview.chromium.org/1274413002/

The idea is the top most link at coordinate (X, Y) has z-order index M, and the
top most widget at coordinate (X, Y) has z-order index N. Then we check M vs N
to see which is the top-most.

[jun_fang, 2015-08-10 23:50:53]

On 2015/08/08 01:20:36, Lei Zhang wrote:
> On 2015/08/08 01:02:38, jun_fang wrote:
> > On 2015/08/07 23:43:16, Lei Zhang wrote:
> > > Jun, I would like your input since we are adding a new API.
> > > 
> > > Tom, PTAL at patch set 3, since it changed quite a bit from patch set 1.
> > 
> > Hi Lei,
> > 
> > I am interested in how applications utilize z-order to choose links or
> widgets.
> > In this new API, it always returns the largest index matching the condition.
> 
> See https://codereview.chromium.org/1274413002/
> 
> The idea is the top most link at coordinate (X, Y) has z-order index M, and
the
> top most widget at coordinate (X, Y) has z-order index N. Then we check M vs N
> to see which is the top-most.

Hi Lei,

Thanks for the clarification. I have a comment on how to implement applications
when there are links and widgets at the same point. Sometimes, applications
should choose links or widgets to handle the event based on the type of event
rather than z-order. Some events are only applied for links or widgets. 

For this new API, I don't have more comments except two nits.

[jun_fang, 2015-08-10 23:51:00]

https://codereview.chromium.org/1278053004/diff/40001/fpdfsdk/src/fpdfformfil...
File fpdfsdk/src/fpdfformfill.cpp (right):

https://codereview.chromium.org/1278053004/diff/40001/fpdfsdk/src/fpdfformfil...
fpdfsdk/src/fpdfformfill.cpp:45: CPDF_InterForm pInterForm(pPage->m_pDocument,
FALSE);
nit: p as a prefix represents a pointer. However, it's an object here.

https://codereview.chromium.org/1278053004/diff/40001/fpdfsdk/src/fpdfformfil...
fpdfsdk/src/fpdfformfill.cpp:73: CPDF_InterForm pInterForm(pPage->m_pDocument,
FALSE);
The same as above.

[Lei Zhang, 2015-08-11 16:02:14]

On 2015/08/10 23:50:53, jun_fang wrote:
> Thanks for the clarification. I have a comment on how to implement
applications
> when there are links and widgets at the same point. Sometimes, applications
> should choose links or widgets to handle the event based on the type of event
> rather than z-order. Some events are only applied for links or widgets. 

Interesting, can you give an example?

[Tom Sepez, 2015-08-11 17:25:01]

Still LGTM

https://codereview.chromium.org/1278053004/diff/40001/core/src/fpdfdoc/doc_fo...
File core/src/fpdfdoc/doc_form.cpp (right):

https://codereview.chromium.org/1278053004/diff/40001/core/src/fpdfdoc/doc_fo...
core/src/fpdfdoc/doc_form.cpp:265: delete m_pFieldTree;
nit: maybe unique_ptr to tree itself?

https://codereview.chromium.org/1278053004/diff/40001/core/src/fpdfdoc/doc_fo...
core/src/fpdfdoc/doc_form.cpp:1125: (CPDF_FormField*)pField,
(CPDF_Dictionary*)pWidgetDict);
nit: not your fault, but maybe these should be const_casts<> or fix the args
themselves.

https://codereview.chromium.org/1278053004/diff/40001/fpdfsdk/src/fpdfdoc.cpp
File fpdfsdk/src/fpdfdoc.cpp (right):

https://codereview.chromium.org/1278053004/diff/40001/fpdfsdk/src/fpdfdoc.cpp...
fpdfsdk/src/fpdfdoc.cpp:12: int THISMODULE = 0;
nit: g_ThisModule so long as you're mucking with it so that it doesn't look like
an all-caps #define.

[jun_fang, 2015-08-11 17:46:43]

On 2015/08/11 16:02:14, Lei Zhang wrote:
> On 2015/08/10 23:50:53, jun_fang wrote:
> > Thanks for the clarification. I have a comment on how to implement
> applications
> > when there are links and widgets at the same point. Sometimes, applications
> > should choose links or widgets to handle the event based on the type of
event
> > rather than z-order. Some events are only applied for links or widgets. 
> 
> Interesting, can you give an example?

Here is an example:
There are one link and form field. They overlap in some area. For the overlapped
area,
let's assume the link's z-order is 5 and the form field's z-order is 4. If
applications
choose links or form fields only based on their z-order, the form field is never
chosen
for this case even if an user try to fill this form field. So in this overlapped
area, 
applications may ask users to use "Ctrl + Click" to replace "Click" on the
links.

[jun_fang, 2015-08-11 17:47:03]

On 2015/08/11 17:46:43, jun_fang wrote:
> On 2015/08/11 16:02:14, Lei Zhang wrote:
> > On 2015/08/10 23:50:53, jun_fang wrote:
> > > Thanks for the clarification. I have a comment on how to implement
> > applications
> > > when there are links and widgets at the same point. Sometimes,
applications
> > > should choose links or widgets to handle the event based on the type of
> event
> > > rather than z-order. Some events are only applied for links or widgets. 
> > 
> > Interesting, can you give an example?
> 
> Here is an example:
> There are one link and form field. They overlap in some area. For the
overlapped
> area,
> let's assume the link's z-order is 5 and the form field's z-order is 4. If
> applications
> choose links or form fields only based on their z-order, the form field is
never
> chosen
> for this case even if an user try to fill this form field. So in this
overlapped
> area, 
> applications may ask users to use "Ctrl + Click" to replace "Click" on the
> links.

LGTM.

[Lei Zhang, 2015-08-15 02:00:27]

On 2015/08/11 17:46:43, jun_fang wrote:
> Here is an example:
> There are one link and form field. They overlap in some area. For the
overlapped
> area,
> let's assume the link's z-order is 5 and the form field's z-order is 4. If
> applications
> choose links or form fields only based on their z-order, the form field is
never
> chosen
> for this case even if an user try to fill this form field. So in this
overlapped
> area, 
> applications may ask users to use "Ctrl + Click" to replace "Click" on the
> links.

Thanks. I'll have to make a PDF to test this tricky example and see how
different PDF viewers handle it.

[Lei Zhang, 2015-08-15 02:02:28]

https://codereview.chromium.org/1278053004/diff/40001/fpdfsdk/src/fpdfformfil...
File fpdfsdk/src/fpdfformfill.cpp (right):

https://codereview.chromium.org/1278053004/diff/40001/fpdfsdk/src/fpdfformfil...
fpdfsdk/src/fpdfformfill.cpp:45: CPDF_InterForm pInterForm(pPage->m_pDocument,
FALSE);
On 2015/08/10 23:51:00, jun_fang wrote:
> nit: p as a prefix represents a pointer. However, it's an object here.

Done.

https://codereview.chromium.org/1278053004/diff/40001/fpdfsdk/src/fpdfformfil...
fpdfsdk/src/fpdfformfill.cpp:73: CPDF_InterForm pInterForm(pPage->m_pDocument,
FALSE);
On 2015/08/10 23:51:00, jun_fang wrote:
> The same as above.

Done.

[Lei Zhang, 2015-08-15 02:02:35]

Committed patchset #5 (id:80001) manually as
24fbf134d43a7ec4226de3db601f0a617bbe428b (presubmit successful).