Bring back support for 38x31 avatars.

chrome/browser/ui/views/frame/taskbar_decorator_win.cc

Index: chrome/browser/ui/views/frame/taskbar_decorator_win.cc
diff --git a/chrome/browser/ui/views/frame/taskbar_decorator_win.cc b/chrome/browser/ui/views/frame/taskbar_decorator_win.cc
index e9a5ad883b99f1bd435042a06d3c3824fb16805b..672b2d01663c4e1150c5246a2acd7c0ba157bcc6 100644
--- a/chrome/browser/ui/views/frame/taskbar_decorator_win.cc
+++ b/chrome/browser/ui/views/frame/taskbar_decorator_win.cc
@@ -16,7 +16,6 @@
 #include "content/public/browser/browser_thread.h"
 #include "skia/ext/image_operations.h"
 #include "skia/ext/platform_canvas.h"
-#include "third_party/skia/include/core/SkRect.h"
 #include "ui/gfx/icon_util.h"
 #include "ui/gfx/image/image.h"
 #include "ui/views/win/hwnd_util.h"
@@ -41,15 +40,17 @@ void SetOverlayIcon(HWND hwnd, scoped_ptr<SkBitmap> bitmap) {
 
   base::win::ScopedGDIObject<HICON> icon;
   if (bitmap.get()) {
+    // Maintain aspect ratio on resize. It is assumed that the image is wider

[sky, 2014/05/09 20:52:19]

assumed->DCHECK

[Mike Lerman, 2014/05/12 15:35:47]

Done.

+    // than it is tall.
     const size_t kOverlayIconSize = 16;
-    const SkBitmap* source_bitmap = bitmap.get();
-
-    // Maintain aspect ratio on resize. Image is assumed to be square.
+    size_t resized_height =

[sky, 2014/05/09 20:52:19]

Why the size_t? Resize takes ints.

[Mike Lerman, 2014/05/12 15:35:47]

Done.

+        bitmap.get()->height() * kOverlayIconSize / bitmap.get()->width();
+    DCHECK_GE(kOverlayIconSize, resized_height);
     // Since the target size is so small, we use our best resizer.
     SkBitmap sk_icon = skia::ImageOperations::Resize(
-        *source_bitmap,
+        *bitmap.get(),
         skia::ImageOperations::RESIZE_LANCZOS3,
-        kOverlayIconSize, kOverlayIconSize);
+        kOverlayIconSize, resized_height);
 
     // Paint the resized icon onto a 16x16 canvas otherwise Windows will badly
     // hammer it to 16x16.
@@ -57,8 +58,7 @@ void SetOverlayIcon(HWND hwnd, scoped_ptr<SkBitmap> bitmap) {
     offscreen_bitmap.allocN32Pixels(kOverlayIconSize, kOverlayIconSize);
     SkCanvas offscreen_canvas(offscreen_bitmap);
     offscreen_canvas.clear(SK_ColorTRANSPARENT);
-    offscreen_canvas.drawBitmap(sk_icon, 0, 0);
-
+    offscreen_canvas.drawBitmap(sk_icon, 0, kOverlayIconSize - resized_height);
     icon.Set(IconUtil::CreateHICONFromSkBitmap(offscreen_bitmap));
     if (!icon.Get())
       return;
@@ -84,8 +84,12 @@ void DrawTaskbarDecoration(gfx::NativeWindow window, const gfx::Image* image) {
 
   // Copy the image since we're going to use it on a separate thread and
   // gfx::Image isn't thread safe.
-  scoped_ptr<SkBitmap> bitmap(
-      image ? new SkBitmap(*image->ToSkBitmap()) : NULL);
+  scoped_ptr<SkBitmap> bitmap;
+  if (image) {
+    SkBitmap* square_bitmap = new SkBitmap();

[sky, 2014/05/09 20:52:19]

Make this a scoped_ptr. But if you go with the other issue I mentioned for
GetAvatarIconAsSquare (make it return a SkBitmap), then this won't be an issue.

[Mike Lerman, 2014/05/12 15:35:47]

I have I think addressed this issue, but I'll admit I am not 100% clear about
this comment. Do you want me to completely remove the scoped_ptr semantics from
this method and the signature to SetOverlayIcon, and pass the SkBitmap directly
into that method, invoking its copy semantics? 
I think you wanted me to keep a scoped_ptr, which I have kept as the variable
bitmap. I've introduced a new SkBitmap() call because I'm concerned that the
SkBitmap object returned from GetAvatarIconAsSquare will go out of scope once
the method return has completed.

[sky, 2014/05/12 16:29:42]

You got what I was after in your latest patch.
You originally had:

SkBitmap* square_bitmap = new SkBitmap();

Which is error prone and easy to leak.

+    profiles::GetAvatarIconAsSquare(*image->ToSkBitmap(), 1, *square_bitmap);
+    bitmap.reset(square_bitmap);
+  }
   content::BrowserThread::GetBlockingPool()->PostWorkerTaskWithShutdownBehavior(
       FROM_HERE, base::Bind(&SetOverlayIcon, hwnd, Passed(&bitmap)),
       base::SequencedWorkerPool::CONTINUE_ON_SHUTDOWN);