Moves calling TaskbarList::SetOverlayIcon to a separate thread. It

turns out calling SetOverlayIcon() runs a nested message loop, which
can result in all sorts of problems (and crashes). Since we don't care
about the result I'm moving the call to a worker thread.

BUG=174768
TEST=none
R=ben@chromium.org


Review URL: https://chromiumcodereview.appspot.com/12212053

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@182145 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/ui/views/avatar_menu_button.cc

@@ -20,73 +20,10 @@
 #include "ui/gfx/canvas.h"
 #include "ui/views/widget/widget.h"
 
-
-#if defined(OS_WIN)
-#include <shobjidl.h>
-#include "base/win/scoped_comptr.h"
-#include "base/win/windows_version.h"
-#include "skia/ext/image_operations.h"
-#include "ui/gfx/icon_util.h"
-#endif
-
 static inline int Round(double x) {
   return static_cast<int>(x + 0.5);
 }
 
-// The Windows 7 taskbar supports dynamic overlays and effects, we use this
-// to ovelay the avatar icon there. The overlay only applies if the taskbar
-// is in "default large icon mode". This function is a best effort deal so
-// we bail out silently at any error condition.
-// See http://msdn.microsoft.com/en-us/library/dd391696(VS.85).aspx for
-// more information.
-void DrawTaskBarDecoration(gfx::NativeWindow window, const gfx::Image* image) {
-#if defined(OS_WIN) && !defined(USE_AURA)
-  if (base::win::GetVersion() < base::win::VERSION_WIN7)
-    return;
-
-  // SetOverlayIcon does nothing if the window is not visible so testing
-  // here avoids all the wasted effort of the image resizing.
-  if (!::IsWindowVisible(window))
-    return;
-
-  base::win::ScopedComPtr<ITaskbarList3> taskbar;
-  HRESULT result = taskbar.CreateInstance(CLSID_TaskbarList, NULL,
-                                          CLSCTX_INPROC_SERVER);
-  if (FAILED(result) || FAILED(taskbar->HrInit()))
-    return;
-  HICON icon = NULL;
-  if (image) {
-    const SkBitmap* bitmap = image->ToSkBitmap();
-    const SkBitmap* source_bitmap = NULL;
-    SkBitmap squarer_bitmap;
-    if ((bitmap->width() == profiles::kAvatarIconWidth) &&
-        (bitmap->height() == profiles::kAvatarIconHeight)) {
-      // Shave a couple of columns so the bitmap is more square. So when
-      // resized to a square aspect ratio it looks pretty.
-      int x = 2;
-      bitmap->extractSubset(&squarer_bitmap, SkIRect::MakeXYWH(x, 0,
-          profiles::kAvatarIconWidth - x * 2, profiles::kAvatarIconHeight));
-      source_bitmap = &squarer_bitmap;
-    } else {
-      // The image's size has changed. Resize what we have.
-      source_bitmap = bitmap;
-    }
-    // Since the target size is so small, we use our best resizer. Never pass
-    // windows a different size because it will badly hammer it to 16x16.
-    SkBitmap sk_icon = skia::ImageOperations::Resize(
-        *source_bitmap,
-        skia::ImageOperations::RESIZE_LANCZOS3,
-        16, 16);
-    icon = IconUtil::CreateHICONFromSkBitmap(sk_icon);
-    if (!icon)
-      return;
-  }
-  taskbar->SetOverlayIcon(window, icon, L"");
-  if (icon)
-    DestroyIcon(icon);
-#endif
-}
-
 AvatarMenuButton::AvatarMenuButton(Browser* browser, bool incognito)
     : MenuButton(NULL, string16(), this, false),
       browser_(browser),

chrome/browser/ui/views/avatar_menu_button.h

@@ -18,11 +18,6 @@ class Image;
 }
 class Browser;
 
-// Draws a scaled version of the avatar in |image| on the taskbar button
-// associated with top level, visible |window|. Currently only implemented
-// for Windows 7 and above.
-void DrawTaskBarDecoration(gfx::NativeWindow window, const gfx::Image* image);
-
 // AvatarMenuButton
 //
 // A button used to show either the incognito avatar or the profile avatar.
@@ -41,8 +36,7 @@ class AvatarMenuButton : public views::MenuButton,
   virtual void OnPaint(gfx::Canvas* canvas) OVERRIDE;
   virtual bool HitTestRect(const gfx::Rect& rect) const OVERRIDE;
 
-  virtual void SetAvatarIcon(const gfx::Image& icon,
-                             bool is_gaia_picture);
+  virtual void SetAvatarIcon(const gfx::Image& icon, bool is_gaia_picture);
 
   void ShowAvatarBubble();
 

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

@@ -12,6 +12,7 @@
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/views/avatar_menu_button.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
+#include "chrome/browser/ui/views/frame/taskbar_decorator.h"
 #include "grit/theme_resources.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/image/image.h"
@@ -71,9 +72,9 @@ void BrowserNonClientFrameView::UpdateAvatarInfo() {
   // need to draw the taskbar decoration.
   if (AvatarMenuModel::ShouldShowAvatarMenu() ||
       ManagedMode::IsInManagedMode()) {
-    DrawTaskBarDecoration(frame_->GetNativeWindow(), &avatar);
+    chrome::DrawTaskbarDecoration(frame_->GetNativeWindow(), &avatar);
   } else {
-    DrawTaskBarDecoration(frame_->GetNativeWindow(), NULL);
+    chrome::DrawTaskbarDecoration(frame_->GetNativeWindow(), NULL);
   }
 }
 

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

+// Copyright (c) 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/ui/views/frame/taskbar_decorator.h"
+
+namespace chrome {
+
+void DrawTaskbarDecoration(gfx::NativeWindow window, const gfx::Image* image) {
+}
+
+}  // namespace chrome

chrome/browser/ui/views/frame/taskbar_decorator.h

+// Copyright (c) 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_UI_VIEWS_FRAME_TASKBAR_DECORATOR_H_
+#define CHROME_BROWSER_UI_VIEWS_FRAME_TASKBAR_DECORATOR_H_
+
+#include "ui/gfx/native_widget_types.h"
+
+namespace gfx {
+class Image;
+}
+
+namespace chrome {
+
+// Draws a scaled version of the avatar in |image| on the taskbar button
+// associated with top level, visible |window|. Currently only implemented
+// for Windows 7 and above.
+void DrawTaskbarDecoration(gfx::NativeWindow window, const gfx::Image* image);
+
+}
+
+#endif  // CHROME_BROWSER_UI_VIEWS_FRAME_TASKBAR_DECORATOR_H_

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

+// Copyright (c) 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/ui/views/frame/taskbar_decorator.h"
+
+#include <shobjidl.h>
+
+#include "base/bind.h"
+#include "base/location.h"
+#include "base/threading/worker_pool.h"
+#include "base/win/scoped_com_initializer.h"
+#include "base/win/scoped_comptr.h"
+#include "base/win/scoped_gdi_object.h"
+#include "base/win/windows_version.h"
+#include "chrome/browser/profiles/profile_info_util.h"
+#include "chrome/browser/ui/host_desktop.h"
+#include "chrome/browser/ui/views/hwnd_util.h"
+#include "skia/ext/image_operations.h"
+#include "third_party/skia/include/core/SkRect.h"
+#include "ui/gfx/icon_util.h"
+#include "ui/gfx/image/image.h"
+
+namespace chrome {
+
+namespace {
+
+// Responsible for invoking TaskbarList::SetOverlayIcon(). The call to
+// TaskbarList::SetOverlayIcon() runs a nested message loop that proves
+// problematic when called on the UI thread. Additionally it seems the call may
+// take a while to complete. For this reason we call it on a worker thread.
+//
+// Docs for TaskbarList::SetOverlayIcon() say it does nothing if the HWND is not
+// valid.
+void SetOverlayIcon(HWND hwnd, scoped_ptr<SkBitmap> bitmap) {
+  base::win::ScopedCOMInitializer com_initializer;
+  base::win::ScopedComPtr<ITaskbarList3> taskbar;
+  HRESULT result = taskbar.CreateInstance(CLSID_TaskbarList, NULL,
+                                          CLSCTX_INPROC_SERVER);
+  if (FAILED(result) || FAILED(taskbar->HrInit()))
+    return;
+
+  base::win::ScopedGDIObject<HICON> icon;
+  if (bitmap.get()) {
+    const SkBitmap* source_bitmap = NULL;
+    SkBitmap squarer_bitmap;
+    if ((bitmap->width() == profiles::kAvatarIconWidth) &&
+        (bitmap->height() == profiles::kAvatarIconHeight)) {
+      // Shave a couple of columns so the bitmap is more square. So when
+      // resized to a square aspect ratio it looks pretty.
+      int x = 2;
+      bitmap->extractSubset(&squarer_bitmap, SkIRect::MakeXYWH(x, 0,
+          profiles::kAvatarIconWidth - x * 2, profiles::kAvatarIconHeight));
+      source_bitmap = &squarer_bitmap;
+    } else {
+      // The image's size has changed. Resize what we have.
+      source_bitmap = bitmap.get();
+    }
+    // Since the target size is so small, we use our best resizer. Never pass
+    // windows a different size because it will badly hammer it to 16x16.
+    SkBitmap sk_icon = skia::ImageOperations::Resize(
+        *source_bitmap,
+        skia::ImageOperations::RESIZE_LANCZOS3,
+        16, 16);
+    icon.Set(IconUtil::CreateHICONFromSkBitmap(sk_icon));
+    if (!icon.Get())
+      return;
+  }
+  taskbar->SetOverlayIcon(hwnd, icon, L"");
+}
+
+}  // namespace
+
+void DrawTaskbarDecoration(gfx::NativeWindow window, const gfx::Image* image) {
+  // HOST_DESKTOP_TYPE_ASH doesn't use the taskbar.
+  if (base::win::GetVersion() < base::win::VERSION_WIN7 ||
+      chrome::GetHostDesktopTypeForNativeWindow(window) !=
+      chrome::HOST_DESKTOP_TYPE_NATIVE)
+    return;
+
+  HWND hwnd = chrome::HWNDForNativeWindow(window);
+
+  // SetOverlayIcon() does nothing if the window is not visible so testing here
+  // avoids all the wasted effort of the image resizing.
+  if (!::IsWindowVisible(hwnd))
+    return;
+
+  // 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);
+  // TaskbarList::SetOverlayIcon() may take a while, so we use slow here.
+  base::WorkerPool::PostTask(
+      FROM_HERE, base::Bind(&SetOverlayIcon, hwnd, Passed(&bitmap)), true);
+}
+
+}  // namespace chrome

chrome/chrome_browser_ui.gypi

@@ -1564,6 +1564,9 @@
         'browser/ui/views/frame/system_menu_model_builder.h',
         'browser/ui/views/frame/system_menu_model_delegate.cc',
         'browser/ui/views/frame/system_menu_model_delegate.h',
+        'browser/ui/views/frame/taskbar_decorator.cc',
+        'browser/ui/views/frame/taskbar_decorator.h',
+        'browser/ui/views/frame/taskbar_decorator_win.cc',
         'browser/ui/views/fullscreen_exit_bubble_views.cc',
         'browser/ui/views/fullscreen_exit_bubble_views.h',
         'browser/ui/views/global_error_bubble_view.cc',
@@ -2706,6 +2709,7 @@
             'browser/ui/crypto_module_password_dialog.cc',
             'browser/ui/crypto_module_password_dialog_nss.cc',
             'browser/ui/startup/autolaunch_prompt.cc',
+            'browser/ui/views/frame/taskbar_decorator.cc',
             'browser/ui/views/simple_message_box_views.cc',
             'browser/ui/webui/help/version_updater_basic.cc',
             'browser/ui/webui/help/version_updater_basic.h',