OOP PDF - Update zoom factor to fall within range

OOP PDF - Update zoom factor to fall within range

Currently setZoom() doesn't check whether zoom factor provided
falls within range or not. It is possible to pass big zoom level
through 'zoom' open pdf parameter which can make browser unresponsive.

This patch updates the zoom factor provided in setZoom() to fall
within range.

BUG=303491
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=291387

[Nikhil, 2014-08-21 10:39:48]

PTAL. Thanks!

[raymes, 2014-08-22 01:02:23]

https://codereview.chromium.org/475933004/diff/1/chrome/browser/resources/pdf...
File chrome/browser/resources/pdf/viewport.js (right):

https://codereview.chromium.org/475933004/diff/1/chrome/browser/resources/pdf...
chrome/browser/resources/pdf/viewport.js:82: Viewport.ZOOM_FACTOR_RANGE = {min:
0.1, max: 10.0};
could we set min to Viewport.ZOOM_FACTORS[0] and max to
Viewport.ZOOM_FACTORS[Viewport.ZOOM_FACTORS.length - 1] ?

[Nikhil, 2014-08-22 03:07:42]

https://codereview.chromium.org/475933004/diff/1/chrome/browser/resources/pdf...
File chrome/browser/resources/pdf/viewport.js (right):

https://codereview.chromium.org/475933004/diff/1/chrome/browser/resources/pdf...
chrome/browser/resources/pdf/viewport.js:82: Viewport.ZOOM_FACTOR_RANGE = {min:
0.1, max: 10.0};
On 2014/08/22 01:02:23, raymes wrote:
> could we set min to Viewport.ZOOM_FACTORS[0] and max to
> Viewport.ZOOM_FACTORS[Viewport.ZOOM_FACTORS.length - 1] ?

Hmm, I've set min to 0.1 and max to 10.0 based on values which current
in-process plugin uses. 

If you think we should set min to 0.25 and max to 5 then please let me know and
I'll update it to use Viewport.ZOOM_FACTORS[0] and
Viewport.ZOOM_FACTORS[Viewport.ZOOM_FACTORS.length - 1].

[raymes, 2014-08-22 03:54:03]

> Hmm, I've set min to 0.1 and max to 10.0 based on values which current
> in-process plugin uses. 
> 
> If you think we should set min to 0.25 and max to 5 then please let me know
and
> I'll update it to use Viewport.ZOOM_FACTORS[0] and
> Viewport.ZOOM_FACTORS[Viewport.ZOOM_FACTORS.length - 1].

I see. I think we should just use Viewport.ZOOM_FACTORS[0] and
> Viewport.ZOOM_FACTORS[Viewport.ZOOM_FACTORS.length - 1]. I don't think it
makes sense to allow something to zoom outside of that if that is the
restriction we are placing on the user.

[raymes, 2014-08-22 03:54:05]

> Hmm, I've set min to 0.1 and max to 10.0 based on values which current
> in-process plugin uses. 
> 
> If you think we should set min to 0.25 and max to 5 then please let me know
and
> I'll update it to use Viewport.ZOOM_FACTORS[0] and
> Viewport.ZOOM_FACTORS[Viewport.ZOOM_FACTORS.length - 1].

I see. I think we should just use Viewport.ZOOM_FACTORS[0] and
> Viewport.ZOOM_FACTORS[Viewport.ZOOM_FACTORS.length - 1]. I don't think it
makes sense to allow something to zoom outside of that if that is the
restriction we are placing on the user.

[Nikhil, 2014-08-22 04:59:04]

Updated to use ZOOM_FACTORS to set the range.

PTAL. Thanks!

https://codereview.chromium.org/475933004/diff/1/chrome/browser/resources/pdf...
File chrome/browser/resources/pdf/viewport.js (right):

https://codereview.chromium.org/475933004/diff/1/chrome/browser/resources/pdf...
chrome/browser/resources/pdf/viewport.js:82: Viewport.ZOOM_FACTOR_RANGE = {min:
0.1, max: 10.0};
On 2014/08/22 01:02:23, raymes wrote:
> could we set min to Viewport.ZOOM_FACTORS[0] and max to
> Viewport.ZOOM_FACTORS[Viewport.ZOOM_FACTORS.length - 1] ?

Done.

[raymes, 2014-08-22 05:14:32]

https://codereview.chromium.org/475933004/diff/20001/chrome/browser/resources...
File chrome/browser/resources/pdf/viewport.js (right):

https://codereview.chromium.org/475933004/diff/20001/chrome/browser/resources...
chrome/browser/resources/pdf/viewport.js:84: Viewport.ZOOM_FACTORS.length - 1]};
Perhaps indent this like:

Viewport.ZOOM_FACTOR_RANGE = {
  min: Viewport.ZOOM_FACTORS[0],
  max: Viewport.ZOOM_FACTORS[Viewport.ZOOM_FACTORS.length - 1]
};

[Nikhil, 2014-08-22 05:20:51]

Looks so much better now. Thanks :)

Missing "git cl format" already.

https://codereview.chromium.org/475933004/diff/20001/chrome/browser/resources...
File chrome/browser/resources/pdf/viewport.js (right):

https://codereview.chromium.org/475933004/diff/20001/chrome/browser/resources...
chrome/browser/resources/pdf/viewport.js:84: Viewport.ZOOM_FACTORS.length - 1]};
On 2014/08/22 05:14:32, raymes wrote:
> Perhaps indent this like:
> 
> Viewport.ZOOM_FACTOR_RANGE = {
>   min: Viewport.ZOOM_FACTORS[0],
>   max: Viewport.ZOOM_FACTORS[Viewport.ZOOM_FACTORS.length - 1]
> };

Done.

[raymes, 2014-08-22 06:54:45]

lgtm

[Nikhil, 2014-08-22 07:04:42]

thestig@ - OWNERS review please.

On 2014/08/22 06:54:45, raymes wrote:
> lgtm

Thanks for reviewing :)

[Lei Zhang, 2014-08-22 07:08:29]

lgtm

[Nikhil, 2014-08-22 07:10:37]

Thanks raymes@ and thestig@ for quick reviews!

Going to submit this now.

[Nikhil, 2014-08-22 07:10:45]

The CQ bit was checked by n.bansal@samsung.com

[commit-bot: I haz the power, 2014-08-22 07:12:11]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/n.bansal@samsung.com/475933004/40001

[commit-bot: I haz the power, 2014-08-22 08:05:18]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)

[commit-bot: I haz the power, 2014-08-22 08:10:10]

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

[commit-bot: I haz the power, 2014-08-22 08:10:11]

Try jobs failed on following builders:
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)

[Nikhil, 2014-08-22 09:50:21]

The CQ bit was checked by n.bansal@samsung.com

[commit-bot: I haz the power, 2014-08-22 09:50:37]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/n.bansal@samsung.com/475933004/40001

[commit-bot: I haz the power, 2014-08-22 09:56:00]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)

[commit-bot: I haz the power, 2014-08-22 10:04:29]

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

[commit-bot: I haz the power, 2014-08-22 10:04:31]

Try jobs failed on following builders:
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)

[Nikhil, 2014-08-22 11:21:34]

Rebase

[Nikhil, 2014-08-22 11:21:52]

The CQ bit was checked by n.bansal@samsung.com

[commit-bot: I haz the power, 2014-08-22 11:22:05]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/n.bansal@samsung.com/475933004/60001

[commit-bot: I haz the power, 2014-08-22 12:19:07]

Committed patchset #4 (60001) as 291387