Add FPDFDest_GetLocationInPage API

Add FPDFDest_GetLocationInPage API

Add an API to get the value of the /XYZ destination parameter.

This CL was originally from https://codereview.chromium.org/1960193003/ by
halcanary@.

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

[dsinclair, 2016-11-07 16:13:03]

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

[dsinclair, 2016-11-07 16:13:04]

PTAL.

[dsinclair, 2016-11-07 16:13:15]

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

[commit-bot: I haz the power, 2016-11-07 16:13:23]

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-07 16:29:34]

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

[commit-bot: I haz the power, 2016-11-07 16:29:35]

Dry run: This issue passed the CQ dry run.

[Lei Zhang, 2016-11-07 20:19:20]

- Add a check in fpdfsdk/fpdfview_c_api_test.c
- Maybe test with named_dests.pdf in fpdfsdk/fpdfdoc_embeddertest.cpp?

https://codereview.chromium.org/2481743004/diff/1/core/fpdfdoc/cpdf_dest.h
File core/fpdfdoc/cpdf_dest.h (right):

https://codereview.chromium.org/2481743004/diff/1/core/fpdfdoc/cpdf_dest.h#ne...
core/fpdfdoc/cpdf_dest.h:28: bool HasXYZ() const;
Why not just let GetXYZ() return a bool?

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

https://codereview.chromium.org/2481743004/diff/1/public/fpdf_doc.h#newcode181
public/fpdf_doc.h:181: // Returns TRUE on success.
Better define "success" ?

https://codereview.chromium.org/2481743004/diff/1/public/fpdf_doc.h#newcode183
public/fpdf_doc.h:183: FPDF_BOOL* hasX,
Document.

[Lei Zhang, 2016-11-07 20:27:16]

https://codereview.chromium.org/2481743004/diff/1/core/fpdfdoc/cpdf_dest.cpp
File core/fpdfdoc/cpdf_dest.cpp (right):

https://codereview.chromium.org/2481743004/diff/1/core/fpdfdoc/cpdf_dest.cpp#...
core/fpdfdoc/cpdf_dest.cpp:60: CFX_ByteString mode = pObj->GetString();
(For later) Should this also check and make sure |pObj| is of type name?

https://codereview.chromium.org/2481743004/diff/1/core/fpdfdoc/cpdf_dest.cpp#...
core/fpdfdoc/cpdf_dest.cpp:115: if (numZoom->GetNumber() == 0.0)
Call GetNumber() just once?

https://codereview.chromium.org/2481743004/diff/1/core/fpdfdoc/cpdf_dest.h
File core/fpdfdoc/cpdf_dest.h (right):

https://codereview.chromium.org/2481743004/diff/1/core/fpdfdoc/cpdf_dest.h#ne...
core/fpdfdoc/cpdf_dest.h:29: void GetXYZ(bool* hasX,
Do you want to name the variabled bHasX and so on?

[dsinclair, 2016-11-07 21:49:18]

https://codereview.chromium.org/2481743004/diff/1/core/fpdfdoc/cpdf_dest.cpp
File core/fpdfdoc/cpdf_dest.cpp (right):

https://codereview.chromium.org/2481743004/diff/1/core/fpdfdoc/cpdf_dest.cpp#...
core/fpdfdoc/cpdf_dest.cpp:60: CFX_ByteString mode = pObj->GetString();
On 2016/11/07 20:27:16, Lei Zhang wrote:
> (For later) Should this also check and make sure |pObj| is of type name?

Acknowledged.

https://codereview.chromium.org/2481743004/diff/1/core/fpdfdoc/cpdf_dest.cpp#...
core/fpdfdoc/cpdf_dest.cpp:115: if (numZoom->GetNumber() == 0.0)
On 2016/11/07 20:27:16, Lei Zhang wrote:
> Call GetNumber() just once?

Done.

https://codereview.chromium.org/2481743004/diff/1/core/fpdfdoc/cpdf_dest.h
File core/fpdfdoc/cpdf_dest.h (right):

https://codereview.chromium.org/2481743004/diff/1/core/fpdfdoc/cpdf_dest.h#ne...
core/fpdfdoc/cpdf_dest.h:28: bool HasXYZ() const;
On 2016/11/07 20:19:20, Lei Zhang wrote:
> Why not just let GetXYZ() return a bool?

Done.

https://codereview.chromium.org/2481743004/diff/1/core/fpdfdoc/cpdf_dest.h#ne...
core/fpdfdoc/cpdf_dest.h:29: void GetXYZ(bool* hasX,
On 2016/11/07 20:27:16, Lei Zhang wrote:
> Do you want to name the variabled bHasX and so on?

Sigh. Done. pHas I believe as they're pointers.

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

https://codereview.chromium.org/2481743004/diff/1/public/fpdf_doc.h#newcode181
public/fpdf_doc.h:181: // Returns TRUE on success.
On 2016/11/07 20:19:20, Lei Zhang wrote:
> Better define "success" ?

Done.

https://codereview.chromium.org/2481743004/diff/1/public/fpdf_doc.h#newcode183
public/fpdf_doc.h:183: FPDF_BOOL* hasX,
On 2016/11/07 20:19:20, Lei Zhang wrote:
> Document.

Done.

[dsinclair, 2016-11-07 21:49:24]

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

[commit-bot: I haz the power, 2016-11-07 21:49:35]

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-07 22:04:17]

lgtm

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

https://codereview.chromium.org/2481743004/diff/20001/public/fpdf_doc.h#newco...
public/fpdf_doc.h:178: //   hasX    - out parameter; true if the x value is not
null
Still a little ambiguous, as "x" can refer to either the x in [page /XYZ x y
zoom], or the x parameter. Maybe rename the parameters to avoid confusion?

I would also mention that the float out parameters are only written to if the
corresponding boolean is set to true.

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

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

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

Dry run: This issue passed the CQ dry run.

[dsinclair, 2016-11-08 14:04:46]

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

https://codereview.chromium.org/2481743004/diff/20001/public/fpdf_doc.h#newco...
public/fpdf_doc.h:178: //   hasX    - out parameter; true if the x value is not
null
On 2016/11/07 22:04:17, Lei Zhang (slow) wrote:
> Still a little ambiguous, as "x" can refer to either the x in [page /XYZ x y
> zoom], or the x parameter. Maybe rename the parameters to avoid confusion?
> 
> I would also mention that the float out parameters are only written to if the
> corresponding boolean is set to true.

Done.

[dsinclair, 2016-11-08 14:04:49]

The CQ bit was checked by dsinclair@chromium.org

[dsinclair, 2016-11-08 14:04:50]

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/2481743004/#ps40001
(title: "Review feedback")

[commit-bot: I haz the power, 2016-11-08 14:04:53]

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-08 14:55:43]

Description was changed from

==========
Add FPDFDest_GetLocationInPage API

Add an API to get the value of the /XYZ destination parameter.

This CL was originally from https://codereview.chromium.org/1960193003/ by
halcanary@.
==========

to

==========
Add FPDFDest_GetLocationInPage API

Add an API to get the value of the /XYZ destination parameter.

This CL was originally from https://codereview.chromium.org/1960193003/ by
halcanary@.

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

[commit-bot: I haz the power, 2016-11-08 14:55:45]

Committed patchset #3 (id:40001) as
https://pdfium.googlesource.com/pdfium/+/c59fa8895fa6fa8428b9b278eee6f05478ab...