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..6b34e4404026b3df787c9b14767a5cccf26d6ac6 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,20 @@ void SetOverlayIcon(HWND hwnd, scoped_ptr<SkBitmap> bitmap) {
 
   base::win::ScopedGDIObject<HICON> icon;
   if (bitmap.get()) {
-    const size_t kOverlayIconSize = 16;
-    const SkBitmap* source_bitmap = bitmap.get();
+    const SkBitmap source_bitmap =

[sky, 2014/05/08 16:04:36]

Can you pass this bitmap into the function? That way you're minimizing what is
called on a separate thread.

[Mike Lerman, 2014/05/08 18:46:39]

Done.

[sky, 2014/05/08 19:09:55]

Sorry if I wasn't being clear. Can you move this call to
content::BrowserThread::GetBlockingPool()->PostWorkerTaskWithShutdownB so that
this function is not calling profiles::GetAvatarIconAsSquare? Again, the reason
for this is to avoid calling out to profile functions on random threads.

+        profiles::GetAvatarIconAsSquare(*bitmap.get(), 1);

[Mike Lerman, 2014/05/08 20:07:32]

I'm unsure about the threading issues you're referring to.

This method used to contain SkBitmap altering code, until a CL
(https://codereview.chromium.org/212603011) removed it. I need to partially
revert the CL (the resized avatars, not the refactoring) and return the
squaring-code, but am trying to refactor the SkBitmap manipulating code into a
common location from this class and from profile_shortcut_manager_win (since
they code that performed the same function). This is being refactored into
profile_avatar_icon_util, because it contains avatar icon manipulating functions
in the profiles namespace, but nothing is thread-specific AFAIK. But I'm still
new here :)

I assume ui/views/frame takes place on the UI-thread. Is the concern that the
profiles:: namespace should not be called directly from the UI thread, but
rather from the browser thread? If that's the case, I'd rather move the new
profiles::GetAvatarIconAsSquare function to somewhere accessible to the UI
thread. The new locations that call it are on the UI thread, and it's just
supposed to be a refactored utility function.

[sky, 2014/05/08 20:23:51]

This function, SetOverlayIcon, is executed on a background thread. I prefer to
avoid calling out to random functions on background threads where possible.

AFAICT you should be able to do the resizing that you want in
DrawTaskbarDecoration, which is executed on the main thread (main thread and UI
thread are used interchangeably and mean the same thing). In other words move
this code to DrawTaskbarDecoration and I'm happy.

[Mike Lerman, 2014/05/09 19:32:06]

Done.

 
-    // Maintain aspect ratio on resize. Image is assumed to be square.
+    // Maintain aspect ratio on resize. It is assumed that the image is wider
+    // than it is tall.
+    const size_t kOverlayIconSize = 16;
+    size_t resized_height =
+        source_bitmap.height() * kOverlayIconSize / source_bitmap.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,
+        source_bitmap,
         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 +61,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;