Make imageSkia:::extractSubset multi-image aware.

Make imageSkia:::extractSubset multi-image aware.

BUG=127791
.
TEST=Manually.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=141215

[kevers, 2012-06-05 17:02:15]

Please take a look at this CL.

The addition of multi-resolution support within extractSubset  fixes extraction
of high DPI network icons for WiFi and cellular, which are stored as multi-frame
images.

[Robert Sesek, 2012-06-05 17:52:09]

http://codereview.chromium.org/10532009/diff/1/ui/gfx/image/image_skia.cc
File ui/gfx/image/image_skia.cc (right):

http://codereview.chromium.org/10532009/diff/1/ui/gfx/image/image_skia.cc#new...
ui/gfx/image/image_skia.cc:168: it != bitmaps.end(); ++it) {
nit: align under s

http://codereview.chromium.org/10532009/diff/1/ui/gfx/image/image_skia.cc#new...
ui/gfx/image/image_skia.cc:171: float dip_scale = (float) px_width / (float)
dip_width;
C-style casts aren't allowed by the style guide.

http://codereview.chromium.org/10532009/diff/1/ui/gfx/image/image_skia.cc#new...
ui/gfx/image/image_skia.cc:181: *dst = image;
Should you overwrite dst ven if return_value==false?

[kevers, 2012-06-05 20:29:32]

http://codereview.chromium.org/10532009/diff/1/ui/gfx/image/image_skia.cc
File ui/gfx/image/image_skia.cc (right):

http://codereview.chromium.org/10532009/diff/1/ui/gfx/image/image_skia.cc#new...
ui/gfx/image/image_skia.cc:168: it != bitmaps.end(); ++it) {
On 2012/06/05 17:52:09, rsesek wrote:
> nit: align under s

Done.

http://codereview.chromium.org/10532009/diff/1/ui/gfx/image/image_skia.cc#new...
ui/gfx/image/image_skia.cc:171: float dip_scale = (float) px_width / (float)
dip_width;
On 2012/06/05 17:52:09, rsesek wrote:
> C-style casts aren't allowed by the style guide.

Changed to static_cast.

http://codereview.chromium.org/10532009/diff/1/ui/gfx/image/image_skia.cc#new...
ui/gfx/image/image_skia.cc:181: *dst = image;
On 2012/06/05 17:52:09, rsesek wrote:
> Should you overwrite dst ven if return_value==false?

Changed to store only successfully extracted subsets and to skip updating dst if
the resulting image is empty.

[Robert Sesek, 2012-06-06 19:44:39]

LGTM w/ 2 nits

http://codereview.chromium.org/10532009/diff/5002/ui/gfx/image/image_skia.cc
File ui/gfx/image/image_skia.cc (right):

http://codereview.chromium.org/10532009/diff/5002/ui/gfx/image/image_skia.cc#...
ui/gfx/image/image_skia.cc:166: for (std::vector<SkBitmap> ::const_iterator it =
bitmaps.begin();
nit: no space before :

http://codereview.chromium.org/10532009/diff/5002/ui/gfx/image/image_skia.cc#...
ui/gfx/image/image_skia.cc:170: float dip_scale = (float) px_width / (float)
dip_width;
C-style cast

[kevers, 2012-06-07 12:58:03]

http://codereview.chromium.org/10532009/diff/5002/ui/gfx/image/image_skia.cc
File ui/gfx/image/image_skia.cc (right):

http://codereview.chromium.org/10532009/diff/5002/ui/gfx/image/image_skia.cc#...
ui/gfx/image/image_skia.cc:166: for (std::vector<SkBitmap> ::const_iterator it =
bitmaps.begin();
On 2012/06/06 19:44:39, rsesek wrote:
> nit: no space before :

Done.

http://codereview.chromium.org/10532009/diff/5002/ui/gfx/image/image_skia.cc#...
ui/gfx/image/image_skia.cc:170: float dip_scale = (float) px_width / (float)
dip_width;
On 2012/06/06 19:44:39, rsesek wrote:
> C-style cast
Fixed.

[sky, 2012-06-07 17:42:19]

http://codereview.chromium.org/10532009/diff/6005/ui/gfx/image/image_skia.cc
File ui/gfx/image/image_skia.cc (right):

http://codereview.chromium.org/10532009/diff/6005/ui/gfx/image/image_skia.cc#...
ui/gfx/image/image_skia.cc:165: std::vector<SkBitmap> bitmaps =
storage_->bitmaps();
const std::vector<SkBitmap>&

http://codereview.chromium.org/10532009/diff/6005/ui/gfx/image/image_skia.cc#...
ui/gfx/image/image_skia.cc:168: SkBitmap bitmap = *it;
const SkBitmap&

http://codereview.chromium.org/10532009/diff/6005/ui/gfx/image/image_skia.cc#...
ui/gfx/image/image_skia.cc:172: int x = static_cast<int>(subset.left() *
dip_scale + 0.5);
Add a comment as to why the + .5

[kevers, 2012-06-07 18:12:00]

http://codereview.chromium.org/10532009/diff/6005/ui/gfx/image/image_skia.cc
File ui/gfx/image/image_skia.cc (right):

http://codereview.chromium.org/10532009/diff/6005/ui/gfx/image/image_skia.cc#...
ui/gfx/image/image_skia.cc:165: std::vector<SkBitmap> bitmaps =
storage_->bitmaps();
On 2012/06/07 17:42:19, sky wrote:
> const std::vector<SkBitmap>&

Done.

http://codereview.chromium.org/10532009/diff/6005/ui/gfx/image/image_skia.cc#...
ui/gfx/image/image_skia.cc:168: SkBitmap bitmap = *it;
On 2012/06/07 17:42:19, sky wrote:
> const SkBitmap&

Done.

http://codereview.chromium.org/10532009/diff/6005/ui/gfx/image/image_skia.cc#...
ui/gfx/image/image_skia.cc:172: int x = static_cast<int>(subset.left() *
dip_scale + 0.5);
On 2012/06/07 17:42:19, sky wrote:
> Add a comment as to why the + .5

Done.

[sky, 2012-06-07 19:59:14]

This patch looks the same as the last.

[kevers, 2012-06-07 20:46:23]

On 2012/06/07 19:59:14, sky wrote:
> This patch looks the same as the last.

Oops... fixed.

[sky, 2012-06-07 22:15:56]

LGTM

[commit-bot: I haz the power, 2012-06-08 13:40:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kevers@chromium.org/10532009/10002

[commit-bot: I haz the power, 2012-06-08 15:59:06]

Change committed as 141215