gfx::Image: Made Image objects shallow-copy their representations.

ui/gfx/image/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 "ui/gfx/image/image.h"
 
 #include <algorithm>
 #include <set>
 #include <utility>
 
@@ skipping 163 matching lines @@
 // the memory that the ImageRep holds. When an ImageRep is created, it expects
 // to take ownership of the image, without having to retain it or increase its
 // reference count.
 class ImageRep {
  public:
   explicit ImageRep(Image::RepresentationType rep) : type_(rep) {}
 
   // Deletes the associated pixels of an ImageRep.
   virtual ~ImageRep() {}
 
+  virtual std::unique_ptr<ImageRep> Clone() = 0;
+
   // Cast helpers ("fake RTTI").
   ImageRepPNG* AsImageRepPNG() {
     CHECK_EQ(type_, Image::kImageRepPNG);
     return reinterpret_cast<ImageRepPNG*>(this);
   }
 
   ImageRepSkia* AsImageRepSkia() {
     CHECK_EQ(type_, Image::kImageRepSkia);
     return reinterpret_cast<ImageRepSkia*>(this);
   }
@@ skipping 23 matching lines @@
 class ImageRepPNG : public ImageRep {
  public:
   ImageRepPNG() : ImageRep(Image::kImageRepPNG) {
   }
 
   ImageRepPNG(const std::vector<ImagePNGRep>& image_png_reps)
       : ImageRep(Image::kImageRepPNG),
         image_png_reps_(image_png_reps) {
   }
 
+  ImageRepPNG(const ImageRepPNG& other)
+      : ImageRep(Image::kImageRepPNG),
+        image_png_reps_(other.image_png_reps_),
+        size_cache_(other.size_cache_ ? new gfx::Size(*other.size_cache_)
+                                      : nullptr) {}
+
+  std::unique_ptr<ImageRep> Clone() override {
+    return base::WrapUnique(new ImageRepPNG(*this));
+  }
+
   ~ImageRepPNG() override {}
 
   int Width() const override { return Size().width(); }
 
   int Height() const override { return Size().height(); }
 
   gfx::Size Size() const override {
     // Read the PNG data to get the image size, caching it.
     if (!size_cache_) {
       for (std::vector<ImagePNGRep>::const_iterator it = image_reps().begin();
            it != image_reps().end(); ++it) {
         if (it->scale == 1.0f) {
           size_cache_.reset(new gfx::Size(it->Size()));
           return *size_cache_;
         }
       }
       size_cache_.reset(new gfx::Size);
     }
 
     return *size_cache_;
   }
 
   const std::vector<ImagePNGRep>& image_reps() const { return image_png_reps_; }
 
  private:
   std::vector<ImagePNGRep> image_png_reps_;
 
   // Cached to avoid having to parse the raw data multiple times.
   mutable std::unique_ptr<gfx::Size> size_cache_;
-
-  DISALLOW_COPY_AND_ASSIGN(ImageRepPNG);
 };
 
 class ImageRepSkia : public ImageRep {
  public:
   // Takes ownership of |image|.
   explicit ImageRepSkia(ImageSkia* image)
       : ImageRep(Image::kImageRepSkia),
         image_(image) {
   }
 
-  ~ImageRepSkia() override {}
+  ImageRepSkia(const ImageRepSkia& other)
+      : ImageRep(Image::kImageRepSkia), image_(new ImageSkia(*other.image_)) {}
+
+  std::unique_ptr<ImageRep> Clone() override {
+    return base::WrapUnique(new ImageRepSkia(*this));
+  }
+
+  ~ImageRepSkia() override {
+    // TODO(mgiuca): Obviously bad, but we just leak the pointer because there
+    // is a lot of code depending on this outliving the Image.
+    image_.release();
+  }
 
   int Width() const override { return image_->width(); }
 
   int Height() const override { return image_->height(); }
 
   gfx::Size Size() const override { return image_->size(); }
 
+  // So... this no longer returns a stable pointer because the Image owns the
+  // ImageSkia and if the local image is destroyed, so is the ImageSkia. Major
+  // problem with this approach: any call to ToImageSkia will now require the
+  // immediate Image object to outlive the ImageSkia*. See
+  // ThemeService::GetImageSkiaNamed, which very deeply relies on the ImageSkia*
+  // outliving the immediate Image object there.
   ImageSkia* image() { return image_.get(); }
 
  private:
   std::unique_ptr<ImageSkia> image_;
-
-  DISALLOW_COPY_AND_ASSIGN(ImageRepSkia);
 };
 
 #if defined(OS_IOS)
 class ImageRepCocoaTouch : public ImageRep {
  public:
   explicit ImageRepCocoaTouch(UIImage* image)
       : ImageRep(Image::kImageRepCocoaTouch),
         image_(image) {
     CHECK(image);
   }
 
+  // TODO(mgiuca): |image_| double-free!!! DO NOT SUBMIT.
+  ImageRepCocoaTouch(const ImageRepCocoaTouch& other) = default;
+
+  std::unique_ptr<ImageRep> Clone() override {
+    return base::WrapUnique(new ImageRepCocoaTouch(*this));
+  }
+
   ~ImageRepCocoaTouch() override {
     base::mac::NSObjectRelease(image_);
     image_ = nil;
   }
 
   int Width() const override { return Size().width(); }
 
   int Height() const override { return Size().height(); }
 
   gfx::Size Size() const override { return internal::UIImageSize(image_); }
 
   UIImage* image() const { return image_; }
 
  private:
   UIImage* image_;
-
-  DISALLOW_COPY_AND_ASSIGN(ImageRepCocoaTouch);
 };
 #elif defined(OS_MACOSX)
 class ImageRepCocoa : public ImageRep {
  public:
   explicit ImageRepCocoa(NSImage* image)
       : ImageRep(Image::kImageRepCocoa),
         image_(image) {
     CHECK(image);
   }
 
+  // TODO(mgiuca): |image_| double-free!!! DO NOT SUBMIT.
+  ImageRepCocoa(const ImageRepCocoa& other) = default;
+
+  std::unique_ptr<ImageRep> Clone() override {
+    return base::WrapUnique(new ImageRepCocoa(*this));
+  }
+
   ~ImageRepCocoa() override {
     base::mac::NSObjectRelease(image_);
     image_ = nil;
   }
 
   int Width() const override { return Size().width(); }
 
   int Height() const override { return Size().height(); }
 
   gfx::Size Size() const override { return internal::NSImageSize(image_); }
 
   NSImage* image() const { return image_; }
 
  private:
   NSImage* image_;
-
-  DISALLOW_COPY_AND_ASSIGN(ImageRepCocoa);
 };
 #endif  // defined(OS_MACOSX)
 
 // The Storage class acts similarly to the pixels in a SkBitmap: the Image
 // class holds a refptr instance of Storage, which in turn holds all the
 // ImageReps. This way, the Image can be cheaply copied.
-class ImageStorage : public base::RefCounted<ImageStorage> {
+class ImageStorage {
  public:
   ImageStorage(Image::RepresentationType default_type)
       : default_representation_type_(default_type)
 #if defined(OS_MACOSX) && !defined(OS_IOS)
         ,
         default_representation_color_space_(
             base::mac::GetGenericRGBColorSpace())
 #endif  // defined(OS_MACOSX) && !defined(OS_IOS)
   {
   }
 
+  ImageStorage(const ImageStorage& other)
+      : default_representation_type_(other.default_representation_type_)
+#if defined(OS_MACOSX) && !defined(OS_IOS)
+        ,
+        default_representation_color_space_(
+            other.default_representation_color_space_)
+#endif  // defined(OS_MACOSX) && !defined(OS_IOS)
+  {
+    for (const auto& item : other.representations_)
+      representations_.insert(std::make_pair(item.first, item.second->Clone()));
+  }
+
+  ~ImageStorage() {}
+
   Image::RepresentationType default_representation_type() {
     return default_representation_type_;
   }
   Image::RepresentationMap& representations() { return representations_; }
 
 #if defined(OS_MACOSX) && !defined(OS_IOS)
   void set_default_representation_color_space(CGColorSpaceRef color_space) {
     default_representation_color_space_ = color_space;
   }
   CGColorSpaceRef default_representation_color_space() {
     return default_representation_color_space_;
   }
 #endif  // defined(OS_MACOSX) && !defined(OS_IOS)
 
  private:
   friend class base::RefCounted<ImageStorage>;
 
-  ~ImageStorage() {}
-
   // The type of image that was passed to the constructor. This key will always
   // exist in the |representations_| map.
   Image::RepresentationType default_representation_type_;
 
 #if defined(OS_MACOSX) && !defined(OS_IOS)
   // The default representation's colorspace. This is used for converting to
   // NSImage. This field exists to compensate for PNGCodec not writing or
   // reading colorspace ancillary chunks. (sRGB, iCCP).
   // Not owned.
   CGColorSpaceRef default_representation_color_space_;
 #endif  // defined(OS_MACOSX) && !defined(OS_IOS)
 
   // All the representations of an Image. Size will always be at least one, with
   // more for any converted representations.
   Image::RepresentationMap representations_;
-
-  DISALLOW_COPY_AND_ASSIGN(ImageStorage);
 };
 
 }  // namespace internal
 
 Image::Image() {
   // |storage_| is NULL for empty Images.
 }
 
 Image::Image(const std::vector<ImagePNGRep>& image_reps) {
   // Do not store obviously invalid ImagePNGReps.
   std::vector<ImagePNGRep> filtered;
   for (size_t i = 0; i < image_reps.size(); ++i) {
     if (image_reps[i].raw_data.get() && image_reps[i].raw_data->size())
       filtered.push_back(image_reps[i]);
   }
 
   if (filtered.empty())
     return;
 
-  storage_ = new internal::ImageStorage(Image::kImageRepPNG);
+  storage_.reset(new internal::ImageStorage(Image::kImageRepPNG));
   AddRepresentation(base::WrapUnique(new internal::ImageRepPNG(filtered)));
 }
 
 Image::Image(const ImageSkia& image) {
   if (!image.isNull()) {
-    storage_ = new internal::ImageStorage(Image::kImageRepSkia);
+    storage_.reset(new internal::ImageStorage(Image::kImageRepSkia));
     AddRepresentation(
         base::WrapUnique(new internal::ImageRepSkia(new ImageSkia(image))));
   }
 }
 
 #if defined(OS_IOS)
 Image::Image(UIImage* image)
     : storage_(new internal::ImageStorage(Image::kImageRepCocoaTouch)) {
   if (image)
     AddRepresentation(
         base::WrapUnique(new internal::ImageRepCocoaTouch(image)));
 }
 #elif defined(OS_MACOSX)
 Image::Image(NSImage* image) {
   if (image) {
-    storage_ = new internal::ImageStorage(Image::kImageRepCocoa);
+    storage_.reset(new internal::ImageStorage(Image::kImageRepCocoa));
     AddRepresentation(base::WrapUnique(new internal::ImageRepCocoa(image)));
   }
 }
 #endif
 
-Image::Image(const Image& other) : storage_(other.storage_) {
-}
+Image::Image(const Image& other)
+    : storage_(other.storage_ ? new internal::ImageStorage(*other.storage_)
+                              : nullptr) {}
 
 Image& Image::operator=(const Image& other) {
-  storage_ = other.storage_;
+  storage_.reset(other.storage_ ? new internal::ImageStorage(*other.storage_)
+                                : nullptr);
   return *this;
 }
 
 Image::~Image() {
 }
 
 // static
 Image Image::CreateFrom1xBitmap(const SkBitmap& bitmap) {
   return Image(ImageSkia::CreateFrom1xBitmap(bitmap));
 }
@@ skipping 304 matching lines @@
       storage_->representations().insert(std::make_pair(type, std::move(rep)));
 
   // insert should not fail (implies that there was already a representation of
   // that type in the map).
   CHECK(result.second) << "type was already in map.";
 
   return result.first->second.get();
 }
 
 }  // namespace gfx