Fix Jumplist favicons to have high resolution in HDPI Windows displays

chrome/browser/win/jumplist.cc

 // Copyright 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/win/jumplist.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/files/file_util.h"
@@ skipping 23 matching lines @@
 #include "components/history/core/browser/page_usage_data.h"
 #include "components/history/core/browser/top_sites.h"
 #include "components/prefs/pref_change_registrar.h"
 #include "components/sessions/core/session_types.h"
 #include "components/sessions/core/tab_restore_service.h"
 #include "components/strings/grit/components_strings.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_registrar.h"
 #include "content/public/browser/notification_source.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "ui/display/display.h"
+#include "ui/display/screen.h"
 #include "ui/gfx/codec/png_codec.h"
 #include "ui/gfx/favicon_size.h"
 #include "ui/gfx/icon_util.h"
 #include "ui/gfx/image/image_family.h"
+#include "ui/gfx/image/image_skia.h"
 #include "url/gurl.h"
 
 using content::BrowserThread;
 using JumpListData = JumpList::JumpListData;
 
 namespace {
 
 // Delay jumplist updates to allow collapsing of redundant update requests.
 const int kDelayForJumplistUpdateInMS = 3500;
 
@@ skipping 476 matching lines @@
 
   // If there is currently a favicon request in progress, it is now outdated,
   // as we have received another, so nullify the handle from the old request.
   task_id_ = base::CancelableTaskTracker::kBadTaskId;
   // Lock the list to set icon data and pop the url.
   {
     JumpListData* data = &jumplist_data_->data;
     base::AutoLock auto_lock(data->list_lock_);
     // Attach the received data to the ShellLinkItem object.
     // This data will be decoded by the RunUpdateOnFileThread method.
-    if (!image_result.image.IsEmpty()) {
-      if (!data->icon_urls_.empty() && data->icon_urls_.front().second.get())
-        data->icon_urls_.front().second->set_icon_data(
-            image_result.image.AsBitmap());
+    if (!image_result.image.IsEmpty() && !data->icon_urls_.empty() &&
+        data->icon_urls_.front().second.get()) {
+      const gfx::ImageSkia* imageskia = image_result.image.ToImageSkia();

[sky, 2017/02/04 00:01:59]

The check you want is
if (!image_result.image.IsEmpty()) {
}

The docs describe this.

[chengx, 2017/02/04 00:18:59]

This check is already used in Line 547 above. Or do you mean Line 550? Although
imageskia should not be null as image_result.image is not empty now, but I still
feel like adding a null pointer check in Line 550 makes things safe. 

[sky, 2017/02/06 16:50:46]

Because of the IsEmpty() check you shouldn't need the null check. We generally
don't add superflous checks, if you feel strongly use a DCHECK.

+      if (imageskia) {
+        float scale = 1.0;
+        display::Screen* screen = display::Screen::GetScreen();
+        if (screen) {
+          const std::vector<display::Display> displays(
+              screen->GetAllDisplays());
+          for (auto& display : displays)
+            scale = std::max(scale, display.device_scale_factor());
+        }
+        gfx::ImageSkiaRep imageSkiaRep = imageskia->GetRepresentation(scale);
+        if (!imageSkiaRep.is_null()) {
+          data->icon_urls_.front().second->set_icon_data(
+              imageSkiaRep.sk_bitmap());
+        }
+      }
     }
 
     if (!data->icon_urls_.empty())
       data->icon_urls_.pop_front();
   }
   // Check whether we need to load more favicons.
   StartLoadingFavicon();
 }
 
 void JumpList::OnIncognitoAvailabilityChanged() {
@@ skipping 50 matching lines @@
 void JumpList::TopSitesLoaded(history::TopSites* top_sites) {
 }
 
 void JumpList::TopSitesChanged(history::TopSites* top_sites,
                                ChangeReason change_reason) {
   top_sites->GetMostVisitedURLs(
       base::Bind(&JumpList::OnMostVisitedURLsAvailable,
                  weak_ptr_factory_.GetWeakPtr()),
       false);
 }