Allow CefImage usage from any thread

include/capi/cef_image_capi.h

@@ -33,7 +33,7 @@
 // by hand. See the translator.README.txt file in the tools directory for
 // more information.
 //
-// $hash=2dce975084deacbed701faebcb978ab5bb21e98c$
+// $hash=80cf165be85863b46b49d32772253a0763b59d67$
 //
 
 #ifndef CEF_INCLUDE_CAPI_CEF_IMAGE_CAPI_H_
@@ -53,7 +53,7 @@ extern "C" {
 // (DIP) units. For example, if the image at scale factor 1.0 is 100x100 pixels
 // then the image at scale factor 2.0 should be 200x200 pixels -- both images
 // will display with a DIP size of 100x100 units. The functions of this
-// structure must be called on the browser process UI thread.
+// structure can be called on any browser process thread.
 ///
 typedef struct _cef_image_t {
   ///

include/cef_image.h

@@ -46,8 +46,8 @@
 // image representations should be the same size in density independent pixel
 // (DIP) units. For example, if the image at scale factor 1.0 is 100x100 pixels
 // then the image at scale factor 2.0 should be 200x200 pixels -- both images
-// will display with a DIP size of 100x100 units. The methods of this class must
+// will display with a DIP size of 100x100 units. The methods of this class can
-// be called on the browser process UI thread.
+// be called on any browser process thread.
 ///
 /*--cef(source=library)--*/
 class CefImage : public virtual CefBaseRefCounted {

libcef/browser/image_impl.cc

@@ -69,30 +69,18 @@ bool JPEGMethod(int quality,
 
 // static
 CefRefPtr<CefImage> CefImage::CreateImage() {
-  CEF_REQUIRE_UIT_RETURN(nullptr);
   return new CefImageImpl();
 }
 
-CefImageImpl::CefImageImpl() {
-  CEF_REQUIRE_UIT();
-}
-
 CefImageImpl::CefImageImpl(const gfx::ImageSkia& image_skia)
-    : image_(image_skia) {
+    : image_(image_skia) {}
-  CEF_REQUIRE_UIT();
-}
-
-CefImageImpl::~CefImageImpl() {
-  CEF_REQUIRE_UIT();
-}
 
 bool CefImageImpl::IsEmpty() {
-  CEF_REQUIRE_UIT_RETURN(false);
+  base::AutoLock lock_scope(lock_);
   return image_.IsEmpty();
 }
 
 bool CefImageImpl::IsSame(CefRefPtr<CefImage> that) {
-  CEF_REQUIRE_UIT_RETURN(false);
   CefImageImpl* that_impl = static_cast<CefImageImpl*>(that.get());
   if (!that_impl)
     return false;
@@ -101,6 +89,7 @@ bool CefImageImpl::IsSame(CefRefPtr<CefImage> that) {
   if (this == that_impl)
     return true;
 
+  base::AutoLock lock_scope(lock_);
   return image_.AsImageSkia().BackedBySameObjectAs(
       that_impl->image_.AsImageSkia());
 }
@@ -112,7 +101,6 @@ bool CefImageImpl::AddBitmap(float scale_factor,
                              cef_alpha_type_t alpha_type,
                              const void* pixel_data,
                              size_t pixel_data_size) {
-  CEF_REQUIRE_UIT_RETURN(false);
   const SkColorType ct = GetSkColorType(color_type);
   const SkAlphaType at = GetSkAlphaType(alpha_type);
 
@@ -137,8 +125,6 @@ bool CefImageImpl::AddBitmap(float scale_factor,
 bool CefImageImpl::AddPNG(float scale_factor,
                           const void* png_data,
                           size_t png_data_size) {
-  CEF_REQUIRE_UIT_RETURN(false);
-
   SkBitmap bitmap;
   if (!gfx::PNGCodec::Decode(static_cast<const unsigned char*>(png_data),
                              png_data_size, &bitmap)) {
@@ -151,8 +137,6 @@ bool CefImageImpl::AddPNG(float scale_factor,
 bool CefImageImpl::AddJPEG(float scale_factor,
                            const void* jpeg_data,
                            size_t jpeg_data_size) {
-  CEF_REQUIRE_UIT_RETURN(false);
-
   std::unique_ptr<SkBitmap> bitmap(gfx::JPEGCodec::Decode(
       static_cast<const unsigned char*>(jpeg_data), jpeg_data_size));
   if (!bitmap.get())
@@ -162,22 +146,22 @@ bool CefImageImpl::AddJPEG(float scale_factor,
 }
 
 size_t CefImageImpl::GetWidth() {
-  CEF_REQUIRE_UIT_RETURN(false);
+  base::AutoLock lock_scope(lock_);
   return image_.Width();
 }
 
 size_t CefImageImpl::GetHeight() {
-  CEF_REQUIRE_UIT_RETURN(false);
+  base::AutoLock lock_scope(lock_);
   return image_.Height();
 }
 
 bool CefImageImpl::HasRepresentation(float scale_factor) {
-  CEF_REQUIRE_UIT_RETURN(false);
+  base::AutoLock lock_scope(lock_);
   return image_.AsImageSkia().HasRepresentation(scale_factor);
 }
 
 bool CefImageImpl::RemoveRepresentation(float scale_factor) {
-  CEF_REQUIRE_UIT_RETURN(false);
+  base::AutoLock lock_scope(lock_);
   gfx::ImageSkia image_skia = image_.AsImageSkia();
   if (image_skia.HasRepresentation(scale_factor)) {
     image_skia.RemoveRepresentation(scale_factor);
@@ -190,7 +174,7 @@ bool CefImageImpl::GetRepresentationInfo(float scale_factor,
                                          float& actual_scale_factor,
                                          int& pixel_width,
                                          int& pixel_height) {
-  CEF_REQUIRE_UIT_RETURN(false);
+  base::AutoLock lock_scope(lock_);
   gfx::ImageSkia image_skia = image_.AsImageSkia();
   if (image_skia.isNull())
     return false;
@@ -210,11 +194,10 @@ CefRefPtr<CefBinaryValue> CefImageImpl::GetAsBitmap(float scale_factor,
                                                     cef_alpha_type_t alpha_type,
                                                     int& pixel_width,
                                                     int& pixel_height) {
-  CEF_REQUIRE_UIT_RETURN(nullptr);
-
   const SkColorType desired_ct = GetSkColorType(color_type);
   const SkAlphaType desired_at = GetSkAlphaType(alpha_type);
 
+  base::AutoLock lock_scope(lock_);
   const SkBitmap* bitmap = GetBitmap(scale_factor);
   if (!bitmap)
     return nullptr;
@@ -242,7 +225,7 @@ CefRefPtr<CefBinaryValue> CefImageImpl::GetAsPNG(float scale_factor,
                                                  bool with_transparency,
                                                  int& pixel_width,
                                                  int& pixel_height) {
-  CEF_REQUIRE_UIT_RETURN(nullptr);
+  base::AutoLock lock_scope(lock_);
   const SkBitmap* bitmap = GetBitmap(scale_factor);
   if (!bitmap)
     return nullptr;
@@ -261,7 +244,7 @@ CefRefPtr<CefBinaryValue> CefImageImpl::GetAsJPEG(float scale_factor,
                                                   int quality,
                                                   int& pixel_width,
                                                   int& pixel_height) {
-  CEF_REQUIRE_UIT_RETURN(nullptr);
+  base::AutoLock lock_scope(lock_);
   const SkBitmap* bitmap = GetBitmap(scale_factor);
   if (!bitmap)
     return nullptr;
@@ -299,6 +282,7 @@ void CefImageImpl::AddBitmaps(int32_t scale_1x_size,
 
 gfx::ImageSkia CefImageImpl::GetForced1xScaleRepresentation(
     float scale_factor) const {
+  base::AutoLock lock_scope(lock_);
   if (scale_factor == 1.0f) {
     // We can use the existing image without modification.
     return image_.AsImageSkia();
@@ -311,6 +295,11 @@ gfx::ImageSkia CefImageImpl::GetForced1xScaleRepresentation(
   return image_skia;
 }
 
+gfx::ImageSkia CefImageImpl::AsImageSkia() const {
+  base::AutoLock lock_scope(lock_);
+  return image_.AsImageSkia();
+}
+
 bool CefImageImpl::AddBitmap(float scale_factor, const SkBitmap& bitmap) {
 #if DCHECK_IS_ON()
   DCHECK(bitmap.readyToDraw());
@@ -319,6 +308,7 @@ bool CefImageImpl::AddBitmap(float scale_factor, const SkBitmap& bitmap) {
          bitmap.colorType() == kRGBA_8888_SkColorType);
 
   gfx::ImageSkiaRep skia_rep(bitmap, scale_factor);
+  base::AutoLock lock_scope(lock_);
   if (image_.IsEmpty()) {
     image_ = gfx::Image(gfx::ImageSkia(skia_rep));
   } else {
@@ -328,6 +318,7 @@ bool CefImageImpl::AddBitmap(float scale_factor, const SkBitmap& bitmap) {
 }
 
 const SkBitmap* CefImageImpl::GetBitmap(float scale_factor) const {
+  lock_.AssertAcquired();
   gfx::ImageSkia image_skia = image_.AsImageSkia();
   if (image_skia.isNull())
     return nullptr;

libcef/browser/image_impl.h

@@ -15,7 +15,7 @@
 class CefImageImpl : public CefImage {
  public:
   // Creates an empty image with no representations.
-  CefImageImpl();
+  CefImageImpl() = default;
 
   // Creates a new image by copying the ImageSkia for use as the default
   // representation.
@@ -23,7 +23,7 @@ class CefImageImpl : public CefImage {
 
   // Deletes the image and, if the only owner of the storage, all of its cached
   // representations.
-  ~CefImageImpl() override;
+  ~CefImageImpl() override = default;
 
   // CefImage methods:
   bool IsEmpty() override;
@@ -76,7 +76,8 @@ class CefImageImpl : public CefImage {
   // TODO(cef): Remove once https://crbug.com/597732 is resolved.
   gfx::ImageSkia GetForced1xScaleRepresentation(float scale_factor) const;
 
-  const gfx::Image& image() const { return image_; }
+  // Returns the skia representation of this Image.
+  gfx::ImageSkia AsImageSkia() const;
 
  private:
   // Add a bitmap.
@@ -114,6 +115,9 @@ class CefImageImpl : public CefImage {
                         std::vector<unsigned char>* compressed,
                         int quality);
 
+  mutable base::Lock lock_;
+
+  // Access to |image_| must be protected by |lock_|.
   gfx::Image image_;
 
   IMPLEMENT_REFCOUNTING_DELETE_ON_UIT(CefImageImpl);

libcef/browser/views/label_button_impl.h

@@ -79,7 +79,7 @@ CEF_LABEL_BUTTON_IMPL_T void CEF_LABEL_BUTTON_IMPL_D::SetImage(
   CEF_REQUIRE_VALID_RETURN_VOID();
   gfx::ImageSkia image_skia;
   if (image)
-    image_skia = static_cast<CefImageImpl*>(image.get())->image().AsImageSkia();
+    image_skia = static_cast<CefImageImpl*>(image.get())->AsImageSkia();
   ParentClass::root_view()->SetImage(
       static_cast<views::Button::ButtonState>(button_state), image_skia);
 }

tests/ceftests/image_unittest.cc

@@ -4,7 +4,6 @@
 
 #include "include/cef_image.h"
 #include "tests/ceftests/image_util.h"
-#include "tests/ceftests/thread_helper.h"
 #include "tests/gtest/include/gtest/gtest.h"
 
 namespace {
@@ -137,7 +136,9 @@ void VerifySaveAsJPEG(CefRefPtr<CefImage> image,
   VerifyScaleExists(image2, expected_scale_factor, expected_scale_factor);
 }
 
-void EmptyImpl() {
+}  // namespace
+
+TEST(ImageTest, Empty) {
   CefRefPtr<CefImage> image = CefImage::CreateImage();
   EXPECT_TRUE(image.get());
 
@@ -160,7 +161,7 @@ void EmptyImpl() {
   VerifyScaleEmpty(image, 2.0f);
 }
 
-void Scale1xImpl() {
+TEST(ImageTest, Scale1x) {
   CefRefPtr<CefImage> image = CefImage::CreateImage();
   EXPECT_TRUE(image.get());
 
@@ -173,7 +174,7 @@ void Scale1xImpl() {
   VerifyScaleEmpty(image, 2.0f);
 }
 
-void Scale2xImpl() {
+TEST(ImageTest, Scale2x) {
   CefRefPtr<CefImage> image = CefImage::CreateImage();
   EXPECT_TRUE(image.get());
 
@@ -186,7 +187,7 @@ void Scale2xImpl() {
   VerifyScaleExists(image, 2.0f, 2.0f);
 }
 
-void ScaleMultiImpl() {
+TEST(ImageTest, ScaleMulti) {
   CefRefPtr<CefImage> image = CefImage::CreateImage();
   EXPECT_TRUE(image.get());
 
@@ -200,7 +201,7 @@ void ScaleMultiImpl() {
   VerifyScaleExists(image, 2.0f, 2.0f);
 }
 
-void SaveBitmap1xImpl() {
+TEST(ImageTest, SaveBitmap1x) {
   CefRefPtr<CefImage> image = CefImage::CreateImage();
   EXPECT_TRUE(image.get());
 
@@ -209,7 +210,7 @@ void SaveBitmap1xImpl() {
   VerifySaveAsBitmap(image, 1.0f, 1.0f);
 }
 
-void SaveBitmap2xImpl() {
+TEST(ImageTest, SaveBitmap2x) {
   CefRefPtr<CefImage> image = CefImage::CreateImage();
   EXPECT_TRUE(image.get());
 
@@ -218,7 +219,7 @@ void SaveBitmap2xImpl() {
   VerifySaveAsBitmap(image, 2.0f, 2.0f);
 }
 
-void SaveBitmapMultiImpl() {
+TEST(ImageTest, SaveBitmapMulti) {
   CefRefPtr<CefImage> image = CefImage::CreateImage();
   EXPECT_TRUE(image.get());
 
@@ -227,7 +228,7 @@ void SaveBitmapMultiImpl() {
   VerifySaveAsBitmap(image, 1.0f, 2.0f);
 }
 
-void SavePNG1xImpl() {
+TEST(ImageTest, SavePNG1x) {
   CefRefPtr<CefImage> image = CefImage::CreateImage();
   EXPECT_TRUE(image.get());
 
@@ -236,7 +237,7 @@ void SavePNG1xImpl() {
   VerifySaveAsPNG(image, 1.0f, 1.0f);
 }
 
-void SavePNG2xImpl() {
+TEST(ImageTest, SavePNG2x) {
   CefRefPtr<CefImage> image = CefImage::CreateImage();
   EXPECT_TRUE(image.get());
 
@@ -245,7 +246,7 @@ void SavePNG2xImpl() {
   VerifySaveAsPNG(image, 2.0f, 2.0f);
 }
 
-void SavePNGMultiImpl() {
+TEST(ImageTest, SavePNGMulti) {
   CefRefPtr<CefImage> image = CefImage::CreateImage();
   EXPECT_TRUE(image.get());
 
@@ -254,7 +255,7 @@ void SavePNGMultiImpl() {
   VerifySaveAsPNG(image, 1.0f, 2.0f);
 }
 
-void SaveJPEG1xImpl() {
+TEST(ImageTest, SaveJPEG1x) {
   CefRefPtr<CefImage> image = CefImage::CreateImage();
   EXPECT_TRUE(image.get());
 
@@ -263,7 +264,7 @@ void SaveJPEG1xImpl() {
   VerifySaveAsJPEG(image, 1.0f, 1.0f);
 }
 
-void SaveJPEG2xImpl() {
+TEST(ImageTest, SaveJPEG2x) {
   CefRefPtr<CefImage> image = CefImage::CreateImage();
   EXPECT_TRUE(image.get());
 
@@ -272,7 +273,7 @@ void SaveJPEG2xImpl() {
   VerifySaveAsJPEG(image, 2.0f, 2.0f);
 }
 
-void SaveJPEGMultiImpl() {
+TEST(ImageTest, SaveJPEGMulti) {
   CefRefPtr<CefImage> image = CefImage::CreateImage();
   EXPECT_TRUE(image.get());
 
@@ -280,21 +281,3 @@ void SaveJPEGMultiImpl() {
 
   VerifySaveAsJPEG(image, 1.0f, 2.0f);
 }
-
-}  // namespace
-
-#define IMAGE_TEST(name) UI_THREAD_TEST(ImageTest, name)
-
-IMAGE_TEST(Empty)
-IMAGE_TEST(Scale1x)
-IMAGE_TEST(Scale2x)
-IMAGE_TEST(ScaleMulti)
-IMAGE_TEST(SaveBitmap1x)
-IMAGE_TEST(SaveBitmap2x)
-IMAGE_TEST(SaveBitmapMulti)
-IMAGE_TEST(SavePNG1x)
-IMAGE_TEST(SavePNG2x)
-IMAGE_TEST(SavePNGMulti)
-IMAGE_TEST(SaveJPEG1x)
-IMAGE_TEST(SaveJPEG2x)
-IMAGE_TEST(SaveJPEGMulti)