Supply default icon to extension icon image.

chrome/browser/extensions/extension_icon_image.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/extensions/extension_icon_image.h"
 
 #include <vector>
 
+#include "base/message_loop_proxy.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/extensions/extension.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_service.h"
 #include "ui/gfx/image/image.h"
 #include "ui/gfx/image/image_skia_source.h"
 #include "ui/gfx/size.h"
 
+using content::BrowserThread;
+
 namespace {
 
 const int kMatchBiggerTreshold = 32;
 
 ExtensionResource GetExtensionIconResource(
     const extensions::Extension* extension,
     const ExtensionIconSet& icons,
     int size,
     ExtensionIconSet::MatchType match_type) {
   std::string path = icons.Get(size, match_type);
   if (path.empty())
     return ExtensionResource();
 
   return extension->GetResource(path);
 }
 
 }  // namespace
 
 namespace extensions {
 
 ////////////////////////////////////////////////////////////////////////////////
 // ExtensionIconImage::Source
 
 class IconImage::Source : public gfx::ImageSkiaSource {
  public:
-  explicit Source(IconImage* host);
+  explicit Source(IconImage* host, const gfx::ImageSkia& default_image);
   virtual ~Source();
 
   void ResetHost();
 
  private:
   // gfx::ImageSkiaSource overrides:
   virtual gfx::ImageSkiaRep GetImageForScale(
       ui::ScaleFactor scale_factor) OVERRIDE;
 
   IconImage* host_;
 
+  // Image whose representation GetImageForScale will return if |host_| supplies
+  // a null representation.
+  gfx::ImageSkia default_image_;
+
   DISALLOW_COPY_AND_ASSIGN(Source);
 };
 
-IconImage::Source::Source(IconImage* host) : host_(host) {
+IconImage::Source::Source(IconImage* host, const gfx::ImageSkia& default_image)
+    : host_(host), default_image_(default_image) {
 }
 
 IconImage::Source::~Source() {
 }
 
 void IconImage::Source::ResetHost() {
   host_ = NULL;
 }
 
 gfx::ImageSkiaRep IconImage::Source::GetImageForScale(
     ui::ScaleFactor scale_factor) {
+  gfx::ImageSkiaRep representation;
   if (host_)
-    host_->LoadImageForScaleFactor(scale_factor);
-  return gfx::ImageSkiaRep();
+    representation = host_->LoadImageForScaleFactor(scale_factor);
+
+  if (!representation.is_null())
+    return representation;
+
+  representation = default_image_.GetRepresentation(scale_factor);
+  return gfx::ImageSkiaRep(representation.sk_bitmap(), scale_factor);

[tbarzic, 2012/08/22 02:39:10]

pkotwicz: is it safe to assume that e.g.
ui::ResourceBundle::GetSharedInstance().GetImageSkiaNamed(IDR_EXTENSIONS_FAVICON)
will have representations for all the scales we support?

Do you think we should do resize operation on representation if
representation/scale_factor doesn't match scale_factor, or just expect that
default_icon has all the representations we need?

[pkotwicz, 2012/08/22 23:17:55]

If the default icon comes from the resource bundle, it should be safe to assume
that it has all the scale factors we need.
Note that it MAY not right now but that's something which should be fixed.

 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // ExtensionIconImage
 
 IconImage::IconImage(
     const Extension* extension,
     const ExtensionIconSet& icon_set,
     int resource_size_in_dip,
+    const gfx::ImageSkia& default_icon,
     Observer* observer)
     : extension_(extension),
       icon_set_(icon_set),
       resource_size_in_dip_(resource_size_in_dip),
       desired_size_in_dip_(resource_size_in_dip, resource_size_in_dip),
       observer_(observer),
       source_(NULL),
-      ALLOW_THIS_IN_INITIALIZER_LIST(tracker_(this)) {
-  source_ = new Source(this);
+      ALLOW_THIS_IN_INITIALIZER_LIST(tracker_(this)),
+      ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)) {
+  source_ = new Source(this, default_icon);
   image_skia_ = gfx::ImageSkia(source_, desired_size_in_dip_);
 
   registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_UNLOADED,
                  content::NotificationService::AllSources());
 }
 
 IconImage::~IconImage() {
   // |source_| could be NULL if resource does not exist.
   if (source_)
     source_->ResetHost();
 }
 
-void IconImage::LoadImageForScaleFactor(ui::ScaleFactor scale_factor) {
+gfx::ImageSkiaRep IconImage::LoadImageForScaleFactor(
+    ui::ScaleFactor scale_factor) {
   // Do nothing if extension is unloaded.
   if (!extension_)
-    return;
+    return gfx::ImageSkiaRep();
 
   const float scale = ui::GetScaleFactorScale(scale_factor);
   const int resource_size_in_pixel =
       static_cast<int>(resource_size_in_dip_ * scale);
 
   ExtensionResource resource;
   // We try loading bigger image only if resource size is >= 32.
   if (resource_size_in_pixel >= kMatchBiggerTreshold) {
     resource = GetExtensionIconResource(extension_, icon_set_,
         resource_size_in_pixel, ExtensionIconSet::MATCH_BIGGER);
   }
 
   // If resource is not found by now, try matching smaller one.
   if (resource.empty()) {
     resource = GetExtensionIconResource(extension_, icon_set_,
         resource_size_in_pixel, ExtensionIconSet::MATCH_SMALLER);
   }
 
   // If there is no resource found, bail out and notify observer of failure.
   if (resource.empty()) {
-    if (observer_)
-      observer_->OnIconImageLoadFailed(this, scale_factor);
-    return;
+    base::MessageLoopProxy::current()->PostTask(FROM_HERE,
+        base::Bind(&IconImage::NotifyFail, weak_ptr_factory_.GetWeakPtr(),
+            scale_factor));
+    return gfx::ImageSkiaRep();
   }
 
   int id = tracker_.next_id();
-  load_map_[id] = scale_factor;
+  load_map_[id].scale_factor = scale_factor;
+  load_map_[id].is_async = false;
 
   std::vector<ImageLoadingTracker::ImageRepresentation> info_list;
   info_list.push_back(ImageLoadingTracker::ImageRepresentation(
       resource,
       ImageLoadingTracker::ImageRepresentation::RESIZE_WHEN_LARGER,
       desired_size_in_dip_.Scale(scale),
       scale_factor));
   tracker_.LoadImages(extension_, info_list, ImageLoadingTracker::DONT_CACHE);
+
+  // If requested image has been cached, we may already have the wanted
+  // representation.

[pkotwicz, 2012/08/22 23:17:55]

Nit: return representation if we have cached it a different scale factor.

[tbarzic, 2012/08/23 00:06:09]

are you sure, we could paint two different images in small time span (which is
just what Aaron had against returning default icon reps).

[pkotwicz, 2012/08/23 13:59:09]

I should have been clearer. My nit was about changing the wording of the
comment.

[pkotwicz, 2012/08/23 14:09:45]

Ok, I see what you are doing. Sorry about that. Change the comment to "If
LoadImages returned synchronously and the requested image rep is cached in the
extension return the cached image rep."

[tbarzic, 2012/08/23 17:41:53]

Done.

+  if (image_skia_.HasRepresentation(scale_factor))
+    return image_skia_.GetRepresentation(scale_factor);
+
+  // If we have not received |OnImageLoaded|, note that iamge load request is
+  // asynchronous.

[pkotwicz, 2012/08/22 23:17:55]

Nit: I think it might be a bit more understandable if you set the is_async flag
for both cases here.

[tbarzic, 2012/08/23 00:06:09]

Done.

+  if (load_map_.find(id) != load_map_.end())
+    load_map_[id].is_async = true;
+
+  return gfx::ImageSkiaRep();
 }
 
 void IconImage::OnImageLoaded(const gfx::Image& image,
                               const std::string& extension_id,
                               int index) {
   LoadMap::iterator load_map_it = load_map_.find(index);
   DCHECK(load_map_it != load_map_.end());
 
-  ui::ScaleFactor scale_factor = load_map_it->second;
+  ui::ScaleFactor scale_factor = load_map_it->second.scale_factor;
+  bool is_async = load_map_it->second.is_async;
 
   load_map_.erase(load_map_it);
-
   if (image.IsEmpty()) {
-    // There waas an error loading the image.
-    if (observer_)
-      observer_->OnIconImageLoadFailed(this, scale_factor);
+    base::MessageLoopProxy::current()->PostTask(FROM_HERE,
+        base::Bind(&IconImage::NotifyFail, weak_ptr_factory_.GetWeakPtr(),
+            scale_factor));
     return;
   }
 
   DCHECK(image.ToImageSkia()->HasRepresentation(scale_factor));
   gfx::ImageSkiaRep rep = image.ToImageSkia()->GetRepresentation(scale_factor);
   DCHECK(!rep.is_null());
+
+  // Remove old representation if there is one.
+  image_skia_.RemoveRepresentation(rep.scale_factor());
   image_skia_.AddRepresentation(rep);
 
+  // If |tracker_| called us synchronously the image did not really change from
+  // the observers perspective, since we still haven't returned the initial
+  // image representation.
+  if (is_async && observer_)
+    observer_->OnExtensionIconImageChanged(this);
+}
+
+void IconImage::NotifyFail(ui::ScaleFactor scale_factor) {
   if (observer_)
-    observer_->OnExtensionIconImageChanged(this);
+    observer_->OnIconImageLoadFailed(this, scale_factor);
 }
 
 void IconImage::Observe(int type,
                         const content::NotificationSource& source,
                         const content::NotificationDetails& details) {
   DCHECK_EQ(type, chrome::NOTIFICATION_EXTENSION_UNLOADED);
 
   const Extension* extension =
       content::Details<extensions::UnloadedExtensionInfo>(details)->extension;
 
   if (extension_ == extension)
     extension_ = NULL;
 }
 
 }  // namespace extensions