Refactor some common code for bounding boxes.

Refactor some common code for bounding boxes.

R=tsepez@chromium.org

Committed: https://pdfium.googlesource.com/pdfium/+/721f003bcb6a3631571bd8f046bc90bcb6def13a

[Lei Zhang, 2015-09-29 20:30:37]

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

[Lei Zhang, 2015-09-29 20:30:37]

Sadly actually added a few lines of code.

[Tom Sepez, 2015-09-29 20:39:48]

https://codereview.chromium.org/1373333002/diff/1/fpdfsdk/src/fpdf_transformp...
File fpdfsdk/src/fpdf_transformpage.cpp (right):

https://codereview.chromium.org/1373333002/diff/1/fpdfsdk/src/fpdf_transformp...
fpdfsdk/src/fpdf_transformpage.cpp:18: if (!page)
For reasons I don't entirely understand myself, I'd be happier if the helper
method didn't have to check this.  There are too many places where we check for
null deep down, paths that often can't actually hit.  These can, since it
originates from the embedder, but maybe the if(!page) sanity check belongs in
the public FPDF routine  ...

[Tom Sepez, 2015-09-29 21:20:47]

lgtm

[Tom Sepez, 2015-09-29 21:22:35]

https://codereview.chromium.org/1373333002/diff/20001/fpdfsdk/src/fpdf_transf...
File fpdfsdk/src/fpdf_transformpage.cpp (right):

https://codereview.chromium.org/1373333002/diff/20001/fpdfsdk/src/fpdf_transf...
fpdfsdk/src/fpdf_transformpage.cpp:91: return
GetBoundingBox(static_cast<CPDF_Page*>(page), "CropBox", left, bottom,
nit: the really cool kids might have written

return page && GetBoundingBox(...);

[Lei Zhang, 2015-09-29 21:34:12]

https://codereview.chromium.org/1373333002/diff/1/fpdfsdk/src/fpdf_transformp...
File fpdfsdk/src/fpdf_transformpage.cpp (right):

https://codereview.chromium.org/1373333002/diff/1/fpdfsdk/src/fpdf_transformp...
fpdfsdk/src/fpdf_transformpage.cpp:18: if (!page)
On 2015/09/29 20:39:48, Tom Sepez wrote:
> For reasons I don't entirely understand myself, I'd be happier if the helper
> method didn't have to check this.  There are too many places where we check
for
> null deep down, paths that often can't actually hit.  These can, since it
> originates from the embedder, but maybe the if(!page) sanity check belongs in
> the public FPDF routine  ...

Done.

https://codereview.chromium.org/1373333002/diff/20001/fpdfsdk/src/fpdf_transf...
File fpdfsdk/src/fpdf_transformpage.cpp (right):

https://codereview.chromium.org/1373333002/diff/20001/fpdfsdk/src/fpdf_transf...
fpdfsdk/src/fpdf_transformpage.cpp:91: return
GetBoundingBox(static_cast<CPDF_Page*>(page), "CropBox", left, bottom,
On 2015/09/29 21:22:35, Tom Sepez wrote:
> nit: the really cool kids might have written
> 
> return page && GetBoundingBox(...);

sure.

[Lei Zhang, 2015-09-29 21:34:26]

Committed patchset #3 (id:40001) manually as
721f003bcb6a3631571bd8f046bc90bcb6def13a (presubmit successful).