Sanitize values in chrome_pdf::PDFiumPage::PageToScreen().

Sanitize values in chrome_pdf::PDFiumPage::PageToScreen().

BUG=650167
Committed: https://crrev.com/7253c367c1fdf94da3b299f13c6140b815b72da3
Cr-Commit-Position: refs/heads/master@{#421370}

[Lei Zhang, 2016-09-27 00:15:25]

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

[commit-bot: I haz the power, 2016-09-27 00:15:51]

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-09-27 01:28:27]

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

[commit-bot: I haz the power, 2016-09-27 02:14:46]

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

[commit-bot: I haz the power, 2016-09-27 02:14:47]

Dry run: This issue passed the CQ dry run.

[Tom Sepez, 2016-09-27 15:57:17]

https://codereview.chromium.org/2374643002/diff/1/pdf/pdfium/pdfium_page.cc
File pdf/pdfium/pdfium_page.cc (right):

https://codereview.chromium.org/2374643002/diff/1/pdf/pdfium/pdfium_page.cc#n...
pdf/pdfium/pdfium_page.cc:465: base::CheckedNumeric<double> start_x = rect_.x();
can we get away with just using doubles here?  I'm not sure how they would
overflow.

https://codereview.chromium.org/2374643002/diff/1/pdf/pdfium/pdfium_page.cc#n...
pdf/pdfium/pdfium_page.cc:509: if
(!base::IsValueInRangeForNumericType<int32_t>(new_left) ||
new_left is an int (presumably == int32_t for practical purposes), so this would
always be in range?

[Lei Zhang, 2016-09-27 16:44:41]

https://codereview.chromium.org/2374643002/diff/1/pdf/pdfium/pdfium_page.cc
File pdf/pdfium/pdfium_page.cc (right):

https://codereview.chromium.org/2374643002/diff/1/pdf/pdfium/pdfium_page.cc#n...
pdf/pdfium/pdfium_page.cc:465: base::CheckedNumeric<double> start_x = rect_.x();
On 2016/09/27 15:57:17, Tom Sepez wrote:
> can we get away with just using doubles here?  I'm not sure how they would
> overflow.

Can multiplying by |zoom| overflow? Am I being too paranoid?

[Tom Sepez, 2016-09-27 16:46:42]

https://codereview.chromium.org/2374643002/diff/1/pdf/pdfium/pdfium_page.cc
File pdf/pdfium/pdfium_page.cc (right):

https://codereview.chromium.org/2374643002/diff/1/pdf/pdfium/pdfium_page.cc#n...
pdf/pdfium/pdfium_page.cc:465: base::CheckedNumeric<double> start_x = rect_.x();
On 2016/09/27 16:44:40, Lei Zhang wrote:
> On 2016/09/27 15:57:17, Tom Sepez wrote:
> > can we get away with just using doubles here?  I'm not sure how they would
> > overflow.
> 
> Can multiplying by |zoom| overflow? Am I being too paranoid?

Probably can, but I wouldn't worry about it.  There are too many places where
doubles would overflow throughout the code ...

[Lei Zhang, 2016-09-27 17:49:53]

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

[commit-bot: I haz the power, 2016-09-27 17:50:14]

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-09-27 17:50:44]

https://codereview.chromium.org/2374643002/diff/1/pdf/pdfium/pdfium_page.cc
File pdf/pdfium/pdfium_page.cc (right):

https://codereview.chromium.org/2374643002/diff/1/pdf/pdfium/pdfium_page.cc#n...
pdf/pdfium/pdfium_page.cc:465: base::CheckedNumeric<double> start_x = rect_.x();
On 2016/09/27 16:46:42, Tom Sepez wrote:
> On 2016/09/27 16:44:40, Lei Zhang wrote:
> > On 2016/09/27 15:57:17, Tom Sepez wrote:
> > > can we get away with just using doubles here?  I'm not sure how they would
> > > overflow.
> > 
> > Can multiplying by |zoom| overflow? Am I being too paranoid?
> 
> Probably can, but I wouldn't worry about it.  There are too many places where
> doubles would overflow throughout the code ...

Done.

[commit-bot: I haz the power, 2016-09-27 18:41:28]

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

[commit-bot: I haz the power, 2016-09-27 18:41:29]

Dry run: This issue passed the CQ dry run.

[Tom Sepez, 2016-09-27 20:00:42]

LGTM otherwise.

https://codereview.chromium.org/2374643002/diff/20001/pdf/pdfium/pdfium_page.cc
File pdf/pdfium/pdfium_page.cc (right):

https://codereview.chromium.org/2374643002/diff/20001/pdf/pdfium/pdfium_page....
pdf/pdfium/pdfium_page.cc:497: if
(!base::IsValueInRangeForNumericType<int32_t>(new_left) ||
Is this overkill per previous comment?

[Lei Zhang, 2016-09-27 22:07:53]

https://codereview.chromium.org/2374643002/diff/20001/pdf/pdfium/pdfium_page.cc
File pdf/pdfium/pdfium_page.cc (right):

https://codereview.chromium.org/2374643002/diff/20001/pdf/pdfium/pdfium_page....
pdf/pdfium/pdfium_page.cc:497: if
(!base::IsValueInRangeForNumericType<int32_t>(new_left) ||
On 2016/09/27 20:00:42, Tom Sepez wrote:
> Is this overkill per previous comment?

Done.

[Lei Zhang, 2016-09-27 22:07:57]

The CQ bit was checked by thestig@chromium.org

[Lei Zhang, 2016-09-27 22:07:59]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org
Link to the patchset: https://codereview.chromium.org/2374643002/#ps40001
(title: "less")

[commit-bot: I haz the power, 2016-09-27 22:08:30]

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-09-27 22:47:43]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-09-27 22:50:08]

Description was changed from

==========
Sanitize values in chrome_pdf::PDFiumPage::PageToScreen().

BUG=650167
==========

to

==========
Sanitize values in chrome_pdf::PDFiumPage::PageToScreen().

BUG=650167

Committed: https://crrev.com/7253c367c1fdf94da3b299f13c6140b815b72da3
Cr-Commit-Position: refs/heads/master@{#421370}
==========

[commit-bot: I haz the power, 2016-09-27 22:50:09]

Patchset 3 (id:??) landed as
https://crrev.com/7253c367c1fdf94da3b299f13c6140b815b72da3
Cr-Commit-Position: refs/heads/master@{#421370}