Fixed Windows system tray icon.

chrome/browser/background/background_mode_manager.cc

 // Copyright (c) 2012 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/background/background_mode_manager.h"
 
 #include <algorithm>
 #include <string>
 #include <vector>
 
@@ skipping 31 matching lines @@
 #include "chrome/browser/ui/extensions/app_launch_params.h"
 #include "chrome/browser/ui/extensions/application_launch.h"
 #include "chrome/browser/ui/host_desktop.h"
 #include "chrome/browser/ui/user_manager.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/grit/chromium_strings.h"
 #include "chrome/grit/generated_resources.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/user_metrics.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/common/constants.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/manifest_handlers/options_page_info.h"
 #include "extensions/common/one_shot_event.h"
 #include "extensions/common/permissions/permission_set.h"
 #include "grit/chrome_unscaled_resources.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/resource/resource_bundle.h"
 
+#if defined(OS_WIN)
+#include "chrome/browser/app_icon_win.h"
+#endif
+
 using base::UserMetricsAction;
 using extensions::Extension;
 
 namespace {
 
 // Records histogram about which auto-launch pattern (if any) was used to launch
 // the current process based on |command_line|.
 void RecordAutoLaunchState(const base::CommandLine& command_line) {
   enum AutoLaunchState {
     AUTO_LAUNCH_NONE = 0,
@@ skipping 707 matching lines @@
   background_mode_suspended_ = false;
   UpdateKeepAliveAndTrayIcon();
 }
 
 void BackgroundModeManager::UpdateKeepAliveAndTrayIcon() {
   if (in_background_mode_ && !background_mode_suspended_) {
     if (!keeping_alive_) {
       keeping_alive_ = true;
       chrome::IncrementKeepAliveCount();
     }
-    CreateStatusTrayIcon();
+
+    // Creating the status tray icon can take time (as it has to load and
+    // potentially scale the icon resource), so this task should not block
+    // startup time.
+    content::BrowserThread::PostAfterStartupTask(
+        FROM_HERE, content::BrowserThread::GetMessageLoopProxyForThread(
+                       content::BrowserThread::UI),
+        base::Bind(&BackgroundModeManager::CreateStatusTrayIcon,
+                   weak_factory_.GetWeakPtr()));
     return;
   }
 
   RemoveStatusTrayIcon();
   if (keeping_alive_) {
     keeping_alive_ = false;
     chrome::DecrementKeepAliveCount();
   }
 }
 
@@ skipping 81 matching lines @@
 
   // Ensure we have a tray icon (needed so we can display the app-installed
   // notification below).
   EnableBackgroundMode();
   ResumeBackgroundMode();
 
   // Notify the user that a background client has been installed.
   DisplayClientInstalledNotification(name);
 }
 
+// Gets the image for the status tray icon, at the correct size for the current
+// platform and display settings.
+gfx::ImageSkia GetStatusTrayIcon() {
+#if defined(OS_WIN)
+  // On Windows, use GetSmallAppIconSize to get the correct image size. The
+  // user's "text size" setting in Windows determines how large the system tray
+  // icon should be.
+  gfx::Size size = GetSmallAppIconSize();
+
+  // This loads all of the icon images, which is a bit wasteful because we're
+  // going to pick one and throw the rest away, but that is the price of using
+  // the ImageFamily abstraction. Note: We could just use the LoadImage function
+  // from the Windows API, but that does a *terrible* job scaling images.
+  // Therefore, we fetch the images and do our own high-quality scaling.
+  scoped_ptr<gfx::ImageFamily> family = GetAppIconImageFamily();
+  if (!family)

[msw, 2015/11/12 00:43:29]

Maybe invert the check to conditionally return the scaled image, and then change
the #else to #endif to let the code fall through and return the static image on
errors. (then you'll want to keep the IDR_STATUS_TRAY_ICON definition on Win)

[Matt Giuca, 2015/11/12 04:44:39]

There should never be an error here (in fact, I'll add a DCHECK here).

Falling back to a secondary (and buggy) icon which is otherwise never needed on
Windows is a bad idea. You wouldn't suggest adding this code and resource if it
didn't already exist (on other platforms), so why add it as a fallback?

[msw, 2015/11/12 18:27:23]

Fair enough... Where is that image used? Can it be removed? nit: ternary here
  return family ? family->CreateExact(size).AsImageSkia() : gfx::ImageSkia();

[Matt Giuca, 2015/11/13 07:21:49]

It is used for the status tray on Mac and Linux (because they don't use Windows
ICO loading code). It can't be removed there. But it is being removed (in this
CL) on Windows, so the resource no longer exists.
I prefer not to use that style because "if (!family)" is an error case.

I think it's preferable to write:

if (error happened)
  report error and exit
normal code

Instead of

return (!error happened) ? normal code : error code

+    return gfx::ImageSkia();
+
+  return family->CreateExact(size).AsImageSkia();
+#else
+  // On other platforms, just get a static resource image.
+  return *ui::ResourceBundle::GetSharedInstance().GetImageSkiaNamed(
+      IDR_STATUS_TRAY_ICON);
+#endif
+}
+
 void BackgroundModeManager::CreateStatusTrayIcon() {
   // Only need status icons on windows/linux. ChromeOS doesn't allow exiting
   // Chrome and Mac can use the dock icon instead.
 
   // Since there are multiple profiles which share the status tray, we now
   // use the browser process to keep track of it.
 #if !defined(OS_MACOSX) && !defined(OS_CHROMEOS)
   if (!status_tray_)
     status_tray_ = g_browser_process->status_tray();
 #endif
 
   // If the platform doesn't support status icons, or we've already created
   // our status icon, just return.
   if (!status_tray_ || status_icon_)
     return;
 
-  gfx::ImageSkia* image_skia = ui::ResourceBundle::GetSharedInstance().
-      GetImageSkiaNamed(IDR_STATUS_TRAY_ICON);
-
   status_icon_ = status_tray_->CreateStatusIcon(
-      StatusTray::BACKGROUND_MODE_ICON,
-      *image_skia,
+      StatusTray::BACKGROUND_MODE_ICON, GetStatusTrayIcon(),
       l10n_util::GetStringUTF16(IDS_PRODUCT_NAME));
   if (!status_icon_)
     return;
   UpdateStatusTrayIconContextMenu();
 }
 
 void BackgroundModeManager::UpdateStatusTrayIconContextMenu() {
   // Ensure we have a tray icon if appropriate.
   UpdateKeepAliveAndTrayIcon();
 
@@ skipping 100 matching lines @@
     }
   }
   return profile_it;
 }
 
 bool BackgroundModeManager::IsBackgroundModePrefEnabled() const {
   PrefService* service = g_browser_process->local_state();
   DCHECK(service);
   return service->GetBoolean(prefs::kBackgroundModeEnabled);
 }