Prevent webp from producing 0 dimensional images

Prevent webp from producing 0 dimensional images

BUG=skia:

Committed: https://skia.googlesource.com/skia/+/a0c414d7b4d88ed5a1784e4f9fb6ed90905f5247

[msarett, 2015-06-19 13:25:17]

msarett@google.com changed reviewers:
+ emmaleer@google.com, scroggo@google.com

[msarett, 2015-06-19 13:25:18]

This is cool that webp supports arbitrary scaling!

I'm not sure we want to allow it to scale to 0x0 though.  This came up because
there are dm failures when we ask to decode a 0x0 image.

On the other hand, maybe we do want to produce a 0x0 image if the client
requests too much scaling?  Let me know what you think.

[scroggo, 2015-06-19 14:19:51]

> Scaling webps produces 0 dimensional images

nit: please make the change description a description of the change you're
making: e.g.

Prevent webp from producing 0 dimensional images.

On 2015/06/19 13:25:18, msarett wrote:
> This is cool that webp supports arbitrary scaling!
> 
> I'm not sure we want to allow it to scale to 0x0 though.  This came up because
> there are dm failures when we ask to decode a 0x0 image.
> 
> On the other hand, maybe we do want to produce a 0x0 image if the client
> requests too much scaling?  Let me know what you think.

As discussed in person, I *think* this is the right change to make, and I
support making it until someone/something convinces us otherwise...

lgtm

https://codereview.chromium.org/1196643002/diff/1/src/codec/SkWebpCodec.cpp
File src/codec/SkWebpCodec.cpp (right):

https://codereview.chromium.org/1196643002/diff/1/src/codec/SkWebpCodec.cpp#n...
src/codec/SkWebpCodec.cpp:106: dim.fWidth = SkTMax(1,
SkScalarRoundToInt(desiredScale * dim.fWidth));
Maybe add a comment explaining our decision (on the other hand, maybe it's
obvious).

[msarett, 2015-06-19 14:28:45]

https://codereview.chromium.org/1196643002/diff/1/src/codec/SkWebpCodec.cpp
File src/codec/SkWebpCodec.cpp (right):

https://codereview.chromium.org/1196643002/diff/1/src/codec/SkWebpCodec.cpp#n...
src/codec/SkWebpCodec.cpp:106: dim.fWidth = SkTMax(1,
SkScalarRoundToInt(desiredScale * dim.fWidth));
On 2015/06/19 14:19:51, scroggo wrote:
> Maybe add a comment explaining our decision (on the other hand, maybe it's
> obvious).

Done.

[msarett, 2015-06-19 14:28:49]

The CQ bit was checked by msarett@google.com

[msarett, 2015-06-19 14:28:49]

The patchset sent to the CQ was uploaded after l-g-t-m from scroggo@google.com
Link to the patchset: https://codereview.chromium.org/1196643002/#ps20001
(title: "Added comment")

[commit-bot: I haz the power, 2015-06-19 14:29:04]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1196643002/20001

[commit-bot: I haz the power, 2015-06-19 14:34:38]

Committed patchset #2 (id:20001) as
https://skia.googlesource.com/skia/+/a0c414d7b4d88ed5a1784e4f9fb6ed90905f5247