arc: Support non-standard display scale factors.

chrome/browser/ui/app_list/arc/arc_app_icon.cc

 // Copyright 2015 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/app_list/arc/arc_app_icon.h"
 
 #include <algorithm>
 #include <map>
 #include <memory>
 #include <utility>
@@ skipping 195 matching lines @@
   if (bitmap.width() != expected_dim || bitmap.height() != expected_dim) {
     VLOG(2) << "Decoded ARC icon has unexpected dimension "
             << bitmap.width() << "x" << bitmap.height() << ". Expected "
             << expected_dim << "x" << ".";
 
     host_->MaybeRequestIcon(scale_factor_);
     host_->DiscardDecodeRequest(this);
     return;
   }
 
-  gfx::ImageSkia image_skia;
-  image_skia.AddRepresentation(gfx::ImageSkiaRep(
-      bitmap,
-      ui::GetScaleForScaleFactor(scale_factor_)));
-  host_->Update(&image_skia);
+  host_->Update(scale_factor_, bitmap);
   host_->DiscardDecodeRequest(this);
 }
 
 void ArcAppIcon::DecodeRequest::OnDecodeImageFailed() {
   VLOG(2) << "Failed to decode ARC icon.";
 
   if (!host_)
     return;
 
   host_->MaybeRequestIcon(scale_factor_);
@@ skipping 96 matching lines @@
 
   return base::MakeUnique<ArcAppIcon::ReadResult>(
       false, request_to_install, scale_factor, unsafe_icon_data);
 }
 
 void ArcAppIcon::OnIconRead(
     std::unique_ptr<ArcAppIcon::ReadResult> read_result) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   if (read_result->request_to_install)
     MaybeRequestIcon(read_result->scale_factor);

[oshima, 2017/05/26 16:03:09]

How is this scale_factor obtained?

[khmel, 2017/05/26 16:10:54]

There is a long chain of calls and finally this scale factor is obtained from
device_scale_factor.
https://cs.chromium.org/chromium/src/ui/compositor/layer.h?l=389

Note, there is similar issue for extension-based apps. In that case we don't use
IconImage but use IconImageLoader and force loading image via loader of 100P. I
think we can do better, similar I do for ARC apps. I plan in one of the next CL
to do similar think for extension-based apps. To use ChromeAppIcon and map image
to non-standard reps.
In last case I think it is better to move functionality to update non-supported
reps to ImageSkia. But this is not for this CL.

 
   if (!read_result->unsafe_icon_data.empty()) {
     decode_requests_.push_back(base::MakeUnique<DecodeRequest>(
         weak_ptr_factory_.GetWeakPtr(), resource_size_in_dip_,
         read_result->scale_factor));
     if (disable_safe_decoding_for_testing) {
       SkBitmap bitmap;
       if (!read_result->unsafe_icon_data.empty() &&
           gfx::PNGCodec::Decode(
               reinterpret_cast<const unsigned char*>(
                   &read_result->unsafe_icon_data.front()),
               read_result->unsafe_icon_data.length(),
               &bitmap)) {
         decode_requests_.back()->OnImageDecoded(bitmap);
       } else {
         decode_requests_.back()->OnDecodeImageFailed();
       }
     } else {
       ImageDecoder::Start(decode_requests_.back().get(),
                           read_result->unsafe_icon_data);
     }
   }
 }
 
-void ArcAppIcon::Update(const gfx::ImageSkia* image) {
+void ArcAppIcon::Update(ui::ScaleFactor scale_factor, const SkBitmap& bitmap) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  CHECK(image && !image->isNull());
 
-  std::vector<gfx::ImageSkiaRep> reps = image->image_reps();
-  for (const auto& image_rep : reps) {
-    if (ui::IsSupportedScale(image_rep.scale())) {
-      image_skia_.RemoveRepresentation(image_rep.scale());
-      image_skia_.AddRepresentation(image_rep);

[oshima, 2017/05/26 18:13:23]

I looked into a bit more, and the problem seems to be that it has a cache
created from default app icon.
You can clear the cache for unsupported scales like this:

  for (const gfx::ImageSkiaRep& image_rep_to_test : image_skia_.image_reps()) {
    if (!ui::IsSupportedScale(image_rep_to_test.scale()))
      image_skia_.RemoveRepresentation(image_rep_to_test.scale());
  }

I tested on caroline + 1.25dsf and this did fix  both issues.

Maybe ImageSkia should have a method to clear the cache for unsupported scales,
so you can add it to ImageSkia.
I'm fine with either way.

[khmel, 2017/05/26 19:31:20]

Thanks for offline chat! As discussed I moved invalidation code to ImageSkia.

PTAL

+  gfx::ImageSkiaRep image_rep(bitmap, ui::GetScaleForScaleFactor(scale_factor));
+  DCHECK(ui::IsSupportedScale(image_rep.scale()));
+
+  image_skia_.RemoveRepresentation(image_rep.scale());
+
+  // Regenerate existing non-supported image representations that depend on the
+  // rep of |scale_factor|.
+  std::vector<float> scales_to_regenerate;
+  for (const gfx::ImageSkiaRep& image_rep_to_test : image_skia_.image_reps()) {
+    if (gfx::ImageSkia::MapToSupportedScale(image_rep_to_test.scale()) ==
+        image_rep.scale()) {
+      DCHECK(!ui::IsSupportedScale(image_rep_to_test.scale()));
+      scales_to_regenerate.push_back(image_rep_to_test.scale());
     }
   }
 
+  image_skia_.AddRepresentation(image_rep);
+
+  for (float scale_to_regenerate : scales_to_regenerate) {
+    image_skia_.RemoveRepresentation(scale_to_regenerate);
+    image_skia_.GetRepresentation(scale_to_regenerate);
+  }
+
   observer_->OnIconUpdated(this);
 }
 
 void ArcAppIcon::DiscardDecodeRequest(DecodeRequest* request) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   auto it = std::find_if(decode_requests_.begin(), decode_requests_.end(),
                          [request](const std::unique_ptr<DecodeRequest>& ptr) {
                            return ptr.get() == request;
                          });
   CHECK(it != decode_requests_.end());
   decode_requests_.erase(it);
 }