diff options
23 files changed, 321 insertions, 94 deletions
diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi index ea34ae2..095e0ed 100644 --- a/chrome/chrome_tests_unit.gypi +++ b/chrome/chrome_tests_unit.gypi @@ -1607,6 +1607,7 @@ '../skia/ext/convolver_unittest.cc', '../skia/ext/image_operations_unittest.cc', '../skia/ext/platform_canvas_unittest.cc', + '../skia/ext/refptr_unittest.cc', '../skia/ext/skia_utils_ios_unittest.mm', '../skia/ext/skia_utils_mac_unittest.mm', '../skia/ext/vector_canvas_unittest.cc', diff --git a/chrome/renderer/automation/automation_renderer_helper.cc b/chrome/renderer/automation/automation_renderer_helper.cc index be20770..a8e3b17 100644 --- a/chrome/renderer/automation/automation_renderer_helper.cc +++ b/chrome/renderer/automation/automation_renderer_helper.cc @@ -69,7 +69,7 @@ bool AutomationRendererHelper::SnapshotEntirePage( skia::ScopedPlatformCanvas canvas(new_size.width, new_size.height, true); - view->paint(webkit_glue::ToWebCanvas(canvas), + view->paint(webkit_glue::ToWebCanvas(canvas.get()), gfx::Rect(0, 0, new_size.width, new_size.height)); frame->setCanHaveScrollbars(true); diff --git a/chrome/renderer/print_web_view_helper_linux.cc b/chrome/renderer/print_web_view_helper_linux.cc index f1ec865..ca1de1e 100644 --- a/chrome/renderer/print_web_view_helper_linux.cc +++ b/chrome/renderer/print_web_view_helper_linux.cc @@ -16,7 +16,6 @@ #include "printing/page_size_margins.h" #include "skia/ext/platform_device.h" #include "skia/ext/vector_canvas.h" -#include "third_party/skia/include/core/SkRefCnt.h" #include "third_party/WebKit/Source/WebKit/chromium/public/WebFrame.h" #if !defined(OS_CHROMEOS) @@ -194,8 +193,8 @@ void PrintWebViewHelper::PrintPageInternal( // The printPage method take a reference to the canvas we pass down, so it // can't be a stack object. - SkRefPtr<skia::VectorCanvas> canvas = new skia::VectorCanvas(device); - canvas->unref(); // SkRefPtr and new both took a reference. + skia::RefPtr<skia::VectorCanvas> canvas = + skia::AdoptRef(new skia::VectorCanvas(device)); printing::MetafileSkiaWrapper::SetMetafileOnCanvas(*canvas, metafile); skia::SetIsDraftMode(*canvas, is_print_ready_metafile_sent_); diff --git a/chrome/renderer/print_web_view_helper_mac.mm b/chrome/renderer/print_web_view_helper_mac.mm index e74739b..378e89c 100644 --- a/chrome/renderer/print_web_view_helper_mac.mm +++ b/chrome/renderer/print_web_view_helper_mac.mm @@ -122,8 +122,8 @@ void PrintWebViewHelper::RenderPage( if (!device) return; - SkRefPtr<skia::VectorCanvas> canvas = new skia::VectorCanvas(device); - canvas->unref(); // SkRefPtr and new both took a reference. + skia::RefPtr<skia::VectorCanvas> canvas = + skia::AdoptRef(new skia::VectorCanvas(device)); WebKit::WebCanvas* canvas_ptr = canvas.get(); printing::MetafileSkiaWrapper::SetMetafileOnCanvas(*canvas, metafile); skia::SetIsDraftMode(*canvas, is_print_ready_metafile_sent_); diff --git a/chrome/renderer/print_web_view_helper_win.cc b/chrome/renderer/print_web_view_helper_win.cc index d5868df..8968308 100644 --- a/chrome/renderer/print_web_view_helper_win.cc +++ b/chrome/renderer/print_web_view_helper_win.cc @@ -17,9 +17,9 @@ #include "printing/metafile_skia_wrapper.h" #include "printing/page_size_margins.h" #include "printing/units.h" -#include "skia/ext/vector_canvas.h" #include "skia/ext/platform_device.h" -#include "third_party/skia/include/core/SkRefCnt.h" +#include "skia/ext/refptr.h" +#include "skia/ext/vector_canvas.h" #include "third_party/WebKit/Source/WebKit/chromium/public/WebFrame.h" #include "ui/gfx/gdi_util.h" #include "ui/gfx/point.h" @@ -179,8 +179,8 @@ void PrintWebViewHelper::RenderPage( DCHECK(device); // The printPage method may take a reference to the canvas we pass down, so it // can't be a stack object. - SkRefPtr<skia::VectorCanvas> canvas = new skia::VectorCanvas(device); - canvas->unref(); // SkRefPtr and new both took a reference. + skia::RefPtr<skia::VectorCanvas> canvas = + skia::AdoptRef(new skia::VectorCanvas(device)); if (is_preview) { printing::MetafileSkiaWrapper::SetMetafileOnCanvas(*canvas, metafile); diff --git a/skia/ext/bitmap_platform_device_android.cc b/skia/ext/bitmap_platform_device_android.cc index c12abd6..3a043a8 100644 --- a/skia/ext/bitmap_platform_device_android.cc +++ b/skia/ext/bitmap_platform_device_android.cc @@ -76,7 +76,8 @@ void BitmapPlatformDevice::DrawToNativeContext( SkCanvas* CreatePlatformCanvas(int width, int height, bool is_opaque, uint8_t* data, OnFailureType failureType) { - SkDevice* dev = BitmapPlatformDevice::Create(width, height, is_opaque, data); + skia::RefPtr<SkDevice> dev = skia::AdoptRef( + BitmapPlatformDevice::Create(width, height, is_opaque, data)); return CreateCanvas(dev, failureType); } diff --git a/skia/ext/bitmap_platform_device_linux.cc b/skia/ext/bitmap_platform_device_linux.cc index 23a7683..3a049e5 100644 --- a/skia/ext/bitmap_platform_device_linux.cc +++ b/skia/ext/bitmap_platform_device_linux.cc @@ -180,7 +180,8 @@ void BitmapPlatformDevice::setMatrixClip(const SkMatrix& transform, SkCanvas* CreatePlatformCanvas(int width, int height, bool is_opaque, uint8_t* data, OnFailureType failureType) { - SkDevice* dev = BitmapPlatformDevice::Create(width, height, is_opaque, data); + skia::RefPtr<SkDevice> dev = skia::AdoptRef( + BitmapPlatformDevice::Create(width, height, is_opaque, data)); return CreateCanvas(dev, failureType); } diff --git a/skia/ext/bitmap_platform_device_mac.cc b/skia/ext/bitmap_platform_device_mac.cc index c97e6fb..b7b05e5 100644 --- a/skia/ext/bitmap_platform_device_mac.cc +++ b/skia/ext/bitmap_platform_device_mac.cc @@ -156,7 +156,7 @@ BitmapPlatformDevice* BitmapPlatformDevice::Create(CGContextRef context, CGContextRetain(context); BitmapPlatformDevice* rv = new BitmapPlatformDevice( - new BitmapPlatformDeviceData(context), bitmap); + skia::AdoptRef(new BitmapPlatformDeviceData(context)), bitmap); // The device object took ownership of the graphics context with its own // CGContextRetain call. @@ -195,14 +195,13 @@ BitmapPlatformDevice* BitmapPlatformDevice::CreateWithData(uint8_t* data, // The device will own the bitmap, which corresponds to also owning the pixel // data. Therefore, we do not transfer ownership to the SkDevice's bitmap. BitmapPlatformDevice::BitmapPlatformDevice( - BitmapPlatformDeviceData* data, const SkBitmap& bitmap) + const skia::RefPtr<BitmapPlatformDeviceData>& data, const SkBitmap& bitmap) : SkDevice(bitmap), data_(data) { SetPlatformDevice(this, this); } BitmapPlatformDevice::~BitmapPlatformDevice() { - data_->unref(); } CGContextRef BitmapPlatformDevice::GetBitmapContext() { @@ -265,14 +264,15 @@ SkDevice* BitmapPlatformDevice::onCreateCompatibleDevice( SkCanvas* CreatePlatformCanvas(CGContextRef ctx, int width, int height, bool is_opaque, OnFailureType failureType) { - SkDevice* dev = BitmapPlatformDevice::Create(ctx, width, height, is_opaque); + skia::RefPtr<SkDevice> dev = skia::AdoptRef( + BitmapPlatformDevice::Create(ctx, width, height, is_opaque)); return CreateCanvas(dev, failureType); } SkCanvas* CreatePlatformCanvas(int width, int height, bool is_opaque, uint8_t* data, OnFailureType failureType) { - SkDevice* dev = BitmapPlatformDevice::CreateWithData(data, width, height, - is_opaque); + skia::RefPtr<SkDevice> dev = skia::AdoptRef( + BitmapPlatformDevice::CreateWithData(data, width, height, is_opaque)); return CreateCanvas(dev, failureType); } diff --git a/skia/ext/bitmap_platform_device_mac.h b/skia/ext/bitmap_platform_device_mac.h index 0d00a0a..a1c5894 100644 --- a/skia/ext/bitmap_platform_device_mac.h +++ b/skia/ext/bitmap_platform_device_mac.h @@ -8,6 +8,7 @@ #include "base/basictypes.h" #include "base/compiler_specific.h" #include "skia/ext/platform_device.h" +#include "skia/ext/refptr.h" namespace skia { @@ -64,7 +65,7 @@ class SK_API BitmapPlatformDevice : public SkDevice, public PlatformDevice { // bitmaps used by the base device class are already refcounted and copyable. class BitmapPlatformDeviceData; - BitmapPlatformDevice(BitmapPlatformDeviceData* data, + BitmapPlatformDevice(const skia::RefPtr<BitmapPlatformDeviceData>& data, const SkBitmap& bitmap); // Flushes the CoreGraphics context so that the pixel data can be accessed @@ -76,9 +77,8 @@ class SK_API BitmapPlatformDevice : public SkDevice, public PlatformDevice { int height, bool isOpaque, Usage usage) OVERRIDE; - // Data associated with this device, guaranteed non-null. We hold a reference - // to this object. - BitmapPlatformDeviceData* data_; + // Data associated with this device, guaranteed non-null. + skia::RefPtr<BitmapPlatformDeviceData> data_; DISALLOW_COPY_AND_ASSIGN(BitmapPlatformDevice); }; diff --git a/skia/ext/bitmap_platform_device_win.cc b/skia/ext/bitmap_platform_device_win.cc index 4e5bbce..8c304e6 100644 --- a/skia/ext/bitmap_platform_device_win.cc +++ b/skia/ext/bitmap_platform_device_win.cc @@ -142,8 +142,8 @@ BitmapPlatformDevice* BitmapPlatformDevice::Create( // The device object will take ownership of the HBITMAP. The initial refcount // of the data object will be 1, which is what the constructor expects. - return new BitmapPlatformDevice(new BitmapPlatformDeviceData(hbitmap), - bitmap); + return new BitmapPlatformDevice( + skia::AdoptRef(new BitmapPlatformDeviceData(hbitmap)), bitmap); } // static @@ -166,7 +166,7 @@ BitmapPlatformDevice* BitmapPlatformDevice::CreateAndClear(int width, // The device will own the HBITMAP, which corresponds to also owning the pixel // data. Therefore, we do not transfer ownership to the SkDevice's bitmap. BitmapPlatformDevice::BitmapPlatformDevice( - BitmapPlatformDeviceData* data, + const skia::RefPtr<BitmapPlatformDeviceData>& data, const SkBitmap& bitmap) : SkDevice(bitmap), data_(data) { @@ -177,7 +177,6 @@ BitmapPlatformDevice::BitmapPlatformDevice( BitmapPlatformDevice::~BitmapPlatformDevice() { SkASSERT(begin_paint_count_ == 0); - data_->unref(); } HDC BitmapPlatformDevice::BeginPlatformPaint() { @@ -272,10 +271,8 @@ SkCanvas* CreatePlatformCanvas(int width, bool is_opaque, HANDLE shared_section, OnFailureType failureType) { - SkDevice* dev = BitmapPlatformDevice::Create(width, - height, - is_opaque, - shared_section); + skia::RefPtr<SkDevice> dev = skia::AdoptRef( + BitmapPlatformDevice::Create(width, height, is_opaque, shared_section)); return CreateCanvas(dev, failureType); } diff --git a/skia/ext/bitmap_platform_device_win.h b/skia/ext/bitmap_platform_device_win.h index 745ac10..c896c0a 100644 --- a/skia/ext/bitmap_platform_device_win.h +++ b/skia/ext/bitmap_platform_device_win.h @@ -8,6 +8,7 @@ #include "base/basictypes.h" #include "base/compiler_specific.h" #include "skia/ext/platform_device.h" +#include "skia/ext/refptr.h" namespace skia { @@ -79,13 +80,12 @@ class SK_API BitmapPlatformDevice : public SkDevice, public PlatformDevice { // bitmaps used by the base device class are already refcounted and copyable. class BitmapPlatformDeviceData; - // Private constructor. The data should already be ref'ed for us. - BitmapPlatformDevice(BitmapPlatformDeviceData* data, + // Private constructor. + BitmapPlatformDevice(const skia::RefPtr<BitmapPlatformDeviceData>& data, const SkBitmap& bitmap); - // Data associated with this device, guaranteed non-null. We hold a reference - // to this object. - BitmapPlatformDeviceData* data_; + // Data associated with this device, guaranteed non-null. + skia::RefPtr<BitmapPlatformDeviceData> data_; #ifdef SK_DEBUG int begin_paint_count_; diff --git a/skia/ext/platform_canvas.cc b/skia/ext/platform_canvas.cc index 461dba4..6a25481 100644 --- a/skia/ext/platform_canvas.cc +++ b/skia/ext/platform_canvas.cc @@ -54,7 +54,9 @@ void MakeOpaque(SkCanvas* canvas, int x, int y, int width, int height) { // so we don't draw anything on a device that ignores xfermodes paint.setColor(0); // install our custom mode - paint.setXfermode(new SkProcXfermode(MakeOpaqueXfermodeProc))->unref(); + skia::RefPtr<SkProcXfermode> xfermode = + skia::AdoptRef(new SkProcXfermode(MakeOpaqueXfermodeProc)); + paint.setXfermode(xfermode.get()); canvas->drawRect(rect, paint); } @@ -62,14 +64,13 @@ size_t PlatformCanvasStrideForWidth(unsigned width) { return 4 * width; } -SkCanvas* CreateCanvas(SkDevice* device, OnFailureType failureType) { +SkCanvas* CreateCanvas(const skia::RefPtr<SkDevice>& device, OnFailureType failureType) { if (!device) { if (CRASH_ON_FAILURE == failureType) SK_CRASH(); return NULL; } - SkAutoUnref aur(device); - return new SkCanvas(device); + return new SkCanvas(device.get()); } PlatformBitmap::PlatformBitmap() : surface_(0), platform_extra_(0) {} diff --git a/skia/ext/platform_canvas.h b/skia/ext/platform_canvas.h index c83e6bf..68e00072 100644 --- a/skia/ext/platform_canvas.h +++ b/skia/ext/platform_canvas.h @@ -9,6 +9,7 @@ // to get the surface type. #include "base/basictypes.h" #include "skia/ext/platform_device.h" +#include "skia/ext/refptr.h" #include "third_party/skia/include/core/SkCanvas.h" namespace skia { @@ -71,8 +72,8 @@ static inline SkCanvas* CreatePlatformCanvas(int width, return CreatePlatformCanvas(width, height, is_opaque, 0, CRASH_ON_FAILURE); } -// Takes ownership of the device, so the caller need not call unref(). -SK_API SkCanvas* CreateCanvas(SkDevice* device, OnFailureType failure_type); +SK_API SkCanvas* CreateCanvas(const skia::RefPtr<SkDevice>& device, + OnFailureType failure_type); static inline SkCanvas* CreateBitmapCanvas(int width, int height, @@ -87,10 +88,12 @@ static inline SkCanvas* TryCreateBitmapCanvas(int width, RETURN_NULL_ON_FAILURE); } -class SK_API ScopedPlatformCanvas : public SkAutoTUnref<SkCanvas> { +class SK_API ScopedPlatformCanvas : public RefPtr<SkCanvas> { public: ScopedPlatformCanvas(int width, int height, bool is_opaque) - : SkAutoTUnref<SkCanvas>(CreatePlatformCanvas(width, height, is_opaque)){} + : RefPtr<SkCanvas>(AdoptRef( + CreatePlatformCanvas(width, height, is_opaque))) + {} }; // Return the stride (length of a line in bytes) for the given width. Because diff --git a/skia/ext/platform_canvas_unittest.cc b/skia/ext/platform_canvas_unittest.cc index 5d71613..3c8b017 100644 --- a/skia/ext/platform_canvas_unittest.cc +++ b/skia/ext/platform_canvas_unittest.cc @@ -247,7 +247,7 @@ TEST(PlatformCanvas, FillLayer) { LayerSaver layer(*canvas, kLayerX, kLayerY, kLayerW, kLayerH); DrawNativeRect(*canvas, 0, 0, 100, 100); #if defined(OS_WIN) - MakeOpaque(canvas, 0, 0, 100, 100); + MakeOpaque(canvas.get(), 0, 0, 100, 100); #endif } EXPECT_TRUE(VerifyBlackRect(*canvas, kLayerX, kLayerY, kLayerW, kLayerH)); @@ -258,7 +258,7 @@ TEST(PlatformCanvas, FillLayer) { LayerSaver layer(*canvas, kLayerX, kLayerY, kLayerW, kLayerH); DrawNativeRect(*canvas, kInnerX, kInnerY, kInnerW, kInnerH); #if defined(OS_WIN) - MakeOpaque(canvas, kInnerX, kInnerY, kInnerW, kInnerH); + MakeOpaque(canvas.get(), kInnerX, kInnerY, kInnerW, kInnerH); #endif } EXPECT_TRUE(VerifyBlackRect(*canvas, kInnerX, kInnerY, kInnerW, kInnerH)); @@ -271,7 +271,7 @@ TEST(PlatformCanvas, FillLayer) { AddClip(*canvas, kInnerX, kInnerY, kInnerW, kInnerH); DrawNativeRect(*canvas, 0, 0, 100, 100); #if defined(OS_WIN) - MakeOpaque(canvas, kInnerX, kInnerY, kInnerW, kInnerH); + MakeOpaque(canvas.get(), kInnerX, kInnerY, kInnerW, kInnerH); #endif canvas->restore(); } @@ -285,7 +285,7 @@ TEST(PlatformCanvas, FillLayer) { LayerSaver layer(*canvas, kLayerX, kLayerY, kLayerW, kLayerH); DrawNativeRect(*canvas, 0, 0, 100, 100); #if defined(OS_WIN) - MakeOpaque(canvas, 0, 0, 100, 100); + MakeOpaque(canvas.get(), 0, 0, 100, 100); #endif } canvas->restore(); @@ -308,7 +308,7 @@ TEST(PlatformCanvas, TranslateLayer) { LayerSaver layer(*canvas, kLayerX, kLayerY, kLayerW, kLayerH); DrawNativeRect(*canvas, 0, 0, 100, 100); #if defined(OS_WIN) - MakeOpaque(canvas, 0, 0, 100, 100); + MakeOpaque(canvas.get(), 0, 0, 100, 100); #endif } canvas->restore(); @@ -323,7 +323,7 @@ TEST(PlatformCanvas, TranslateLayer) { LayerSaver layer(*canvas, kLayerX, kLayerY, kLayerW, kLayerH); DrawNativeRect(*canvas, kInnerX, kInnerY, kInnerW, kInnerH); #if defined(OS_WIN) - MakeOpaque(canvas, kInnerX, kInnerY, kInnerW, kInnerH); + MakeOpaque(canvas.get(), kInnerX, kInnerY, kInnerW, kInnerH); #endif } canvas->restore(); @@ -338,7 +338,7 @@ TEST(PlatformCanvas, TranslateLayer) { canvas->translate(1, 1); DrawNativeRect(*canvas, kInnerX, kInnerY, kInnerW, kInnerH); #if defined(OS_WIN) - MakeOpaque(canvas, kInnerX, kInnerY, kInnerW, kInnerH); + MakeOpaque(canvas.get(), kInnerX, kInnerY, kInnerW, kInnerH); #endif } canvas->restore(); @@ -356,7 +356,7 @@ TEST(PlatformCanvas, TranslateLayer) { AddClip(*canvas, kInnerX + 1, kInnerY + 1, kInnerW - 1, kInnerH - 1); DrawNativeRect(*canvas, 0, 0, 100, 100); #if defined(OS_WIN) - MakeOpaque(canvas, kLayerX, kLayerY, kLayerW, kLayerH); + MakeOpaque(canvas.get(), kLayerX, kLayerY, kLayerW, kLayerH); #endif } canvas->restore(); @@ -384,7 +384,7 @@ TEST(PlatformCanvas, TranslateLayer) { DrawNativeRect(*canvas, 0, 0, 100, 100); #if defined(OS_WIN) - MakeOpaque(canvas, kLayerX, kLayerY, kLayerW, kLayerH); + MakeOpaque(canvas.get(), kLayerX, kLayerY, kLayerW, kLayerH); #endif } canvas->restore(); diff --git a/skia/ext/refptr.h b/skia/ext/refptr.h new file mode 100644 index 0000000..514d453 --- /dev/null +++ b/skia/ext/refptr.h @@ -0,0 +1,98 @@ +// 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. + +#ifndef SKIA_EXT_REFPTR_H_ +#define SKIA_EXT_REFPTR_H_ + +#include "third_party/skia/include/core/SkRefCnt.h" + +namespace skia { + +// When creating/receiving a ref-counted pointer from Skia, wrap that pointer in +// this class to avoid dealing with the ref-counting and prevent leaks/crashes +// due to ref-counting bugs. +// +// Example of Creating an SkShader* and setting it on a SkPaint: +// skia::RefPtr<SkShader> shader = skia::AdoptRef(SkGradientShader::Create()); +// paint.setShader(shader.get()); +// +// When passing around a ref-counted pointer to methods outside of Skia, always +// pass around the skia::RefPtr instead of the raw pointer. An example method +// that takes a SkShader* parameter and saves the SkShader* in the class. +// void AMethodThatSavesAShader(const skia::RefPtr<SkShader>& shader) { +// member_refptr_ = shader; +// } +// skia::RefPtr<SkShader> member_refptr_; +// +// When returning a ref-counted ponter, also return the skia::RefPtr instead. An +// example method that creates an SkShader* and returns it: +// skia::RefPtr<SkShader> MakeAShader() { +// return skia::AdoptRef(SkGradientShader::Create()); +// } +// +// Never call ref() or unref() on the underlying ref-counted pointer. If you +// AdoptRef() the raw pointer immediately into a skia::RefPtr and always work +// with skia::RefPtr instances instead, the ref-counting will be taken care of +// for you. +template<typename T> +class RefPtr { + public: + RefPtr() : ptr_(NULL) {} + + RefPtr(const RefPtr& other) + : ptr_(other.get()) { + SkSafeRef(ptr_); + } + + template<typename U> + RefPtr(const RefPtr<U>& other) + : ptr_(other.get()) { + SkSafeRef(ptr_); + } + + ~RefPtr() { + clear(); + } + + RefPtr& operator=(const RefPtr& other) { + SkRefCnt_SafeAssign(ptr_, other.get()); + return *this; + } + + template<typename U> + RefPtr& operator=(const RefPtr<U>& other) { + SkRefCnt_SafeAssign(ptr_, other.get()); + return *this; + } + + void clear() { + T* to_unref = ptr_; + ptr_ = NULL; + SkSafeUnref(to_unref); + } + + T* get() const { return ptr_; } + T& operator*() const { return *ptr_; } + T* operator->() const { return ptr_; } + + typedef T* RefPtr::*unspecified_bool_type; + operator unspecified_bool_type() const { + return ptr_ ? &RefPtr::ptr_ : NULL; + } + + private: + T* ptr_; + + explicit RefPtr(T* ptr) : ptr_(ptr) {} + + template<typename U> + friend RefPtr<U> AdoptRef(U* ptr); +}; + +template<typename T> +RefPtr<T> AdoptRef(T* ptr) { return RefPtr<T>(ptr); } + +} // namespace skia + +#endif // SKIA_EXT_REFPTR_H_ diff --git a/skia/ext/refptr_unittest.cc b/skia/ext/refptr_unittest.cc new file mode 100644 index 0000000..1d63ed1 --- /dev/null +++ b/skia/ext/refptr_unittest.cc @@ -0,0 +1,125 @@ +// 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 "skia/ext/refptr.h" + +#include "testing/gtest/include/gtest/gtest.h" + +namespace skia { +namespace { + +TEST(RefPtrTest, ReferenceCounting) { + SkRefCnt* ref = new SkRefCnt(); + EXPECT_EQ(1, ref->getRefCnt()); + + { + // Adopt the reference from the caller on creation. + RefPtr<SkRefCnt> refptr1 = AdoptRef(ref); + EXPECT_EQ(1, ref->getRefCnt()); + EXPECT_EQ(1, refptr1->getRefCnt()); + + EXPECT_EQ(ref, &*refptr1); + EXPECT_EQ(ref, refptr1.get()); + + { + // Take a second reference for the second instance. + RefPtr<SkRefCnt> refptr2(refptr1); + EXPECT_EQ(2, ref->getRefCnt()); + + RefPtr<SkRefCnt> refptr3; + EXPECT_FALSE(refptr3); + + // Take a third reference for the third instance. + refptr3 = refptr1; + EXPECT_EQ(3, ref->getRefCnt()); + + // Same object, so should have the same refcount. + refptr2 = refptr3; + EXPECT_EQ(3, ref->getRefCnt()); + + // Drop the object from refptr2, so it should lose its reference. + EXPECT_TRUE(refptr2); + refptr2.clear(); + EXPECT_EQ(2, ref->getRefCnt()); + + EXPECT_FALSE(refptr2); + EXPECT_EQ(NULL, refptr2.get()); + + EXPECT_TRUE(refptr3); + EXPECT_EQ(2, refptr3->getRefCnt()); + EXPECT_EQ(ref, &*refptr3); + EXPECT_EQ(ref, refptr3.get()); + } + + // Drop a reference when the third object is destroyed. + EXPECT_EQ(1, ref->getRefCnt()); + } +} + +TEST(RefPtrTest, Construct) { + SkRefCnt* ref = new SkRefCnt(); + EXPECT_EQ(1, ref->getRefCnt()); + + // Adopt the reference from the caller on creation. + RefPtr<SkRefCnt> refptr1(AdoptRef(ref)); + EXPECT_EQ(1, ref->getRefCnt()); + EXPECT_EQ(1, refptr1->getRefCnt()); + + EXPECT_EQ(ref, &*refptr1); + EXPECT_EQ(ref, refptr1.get()); + + RefPtr<SkRefCnt> refptr2(refptr1); + EXPECT_EQ(2, ref->getRefCnt()); +} + +TEST(RefPtrTest, DeclareAndAssign) { + SkRefCnt* ref = new SkRefCnt(); + EXPECT_EQ(1, ref->getRefCnt()); + + // Adopt the reference from the caller on creation. + RefPtr<SkRefCnt> refptr1 = AdoptRef(ref); + EXPECT_EQ(1, ref->getRefCnt()); + EXPECT_EQ(1, refptr1->getRefCnt()); + + EXPECT_EQ(ref, &*refptr1); + EXPECT_EQ(ref, refptr1.get()); + + RefPtr<SkRefCnt> refptr2 = refptr1; + EXPECT_EQ(2, ref->getRefCnt()); +} + +TEST(RefPtrTest, Assign) { + SkRefCnt* ref = new SkRefCnt(); + EXPECT_EQ(1, ref->getRefCnt()); + + // Adopt the reference from the caller on creation. + RefPtr<SkRefCnt> refptr1; + refptr1 = AdoptRef(ref); + EXPECT_EQ(1, ref->getRefCnt()); + EXPECT_EQ(1, refptr1->getRefCnt()); + + EXPECT_EQ(ref, &*refptr1); + EXPECT_EQ(ref, refptr1.get()); + + RefPtr<SkRefCnt> refptr2; + refptr2 = refptr1; + EXPECT_EQ(2, ref->getRefCnt()); +} + +class Subclass : public SkRefCnt {}; + +TEST(RefPtrTest, Upcast) { + RefPtr<Subclass> child = AdoptRef(new Subclass()); + EXPECT_EQ(1, child->getRefCnt()); + + RefPtr<SkRefCnt> parent = child; + EXPECT_TRUE(child); + EXPECT_TRUE(parent); + + EXPECT_EQ(2, child->getRefCnt()); + EXPECT_EQ(2, parent->getRefCnt()); +} + +} // namespace +} // namespace skia diff --git a/skia/ext/vector_canvas.cc b/skia/ext/vector_canvas.cc index 96c23e9..9de6b3d 100644 --- a/skia/ext/vector_canvas.cc +++ b/skia/ext/vector_canvas.cc @@ -7,8 +7,8 @@ namespace skia { -VectorCanvas::VectorCanvas(SkDevice* device) { - setDevice(device)->unref(); // Created with refcount 1, and setDevice refs. +VectorCanvas::VectorCanvas(SkDevice* device) + : PlatformCanvas(device) { } VectorCanvas::~VectorCanvas() { diff --git a/skia/ext/vector_canvas_unittest.cc b/skia/ext/vector_canvas_unittest.cc index 8efbcdb..9f28645 100644 --- a/skia/ext/vector_canvas_unittest.cc +++ b/skia/ext/vector_canvas_unittest.cc @@ -390,8 +390,9 @@ class VectorCanvasTest : public ImageTest { size_ = size; context_ = new Context(); bitmap_ = new Bitmap(*context_, size_, size_); - vcanvas_ = new VectorCanvas(VectorPlatformDeviceEmf::CreateDevice( - size_, size_, true, context_->context())); + vcanvas_ = new VectorCanvas( + VectorPlatformDeviceEmf::CreateDevice( + size_, size_, true, context_->context())); pcanvas_ = CreatePlatformCanvas(size_, size_, false); // Clear white. @@ -729,9 +730,9 @@ TEST_F(VectorCanvasTest, MAYBE_PathEffects) { { SkPaint paint; SkScalar intervals[] = { 1, 1 }; - SkPathEffect* effect = new SkDashPathEffect(intervals, arraysize(intervals), - 0); - paint.setPathEffect(effect)->unref(); + skia::RefPtr<SkPathEffect> effect = skia::AdoptRef( + new SkDashPathEffect(intervals, arraysize(intervals), 0)); + paint.setPathEffect(effect.get()); paint.setColor(SK_ColorMAGENTA); paint.setStyle(SkPaint::kStroke_Style); @@ -749,9 +750,9 @@ TEST_F(VectorCanvasTest, MAYBE_PathEffects) { { SkPaint paint; SkScalar intervals[] = { 3, 5 }; - SkPathEffect* effect = new SkDashPathEffect(intervals, arraysize(intervals), - 0); - paint.setPathEffect(effect)->unref(); + skia::RefPtr<SkPathEffect> effect = skia::AdoptRef( + new SkDashPathEffect(intervals, arraysize(intervals), 0)); + paint.setPathEffect(effect.get()); paint.setColor(SK_ColorMAGENTA); paint.setStyle(SkPaint::kStroke_Style); @@ -767,9 +768,9 @@ TEST_F(VectorCanvasTest, MAYBE_PathEffects) { { SkPaint paint; SkScalar intervals[] = { 2, 1 }; - SkPathEffect* effect = new SkDashPathEffect(intervals, arraysize(intervals), - 0); - paint.setPathEffect(effect)->unref(); + skia::RefPtr<SkPathEffect> effect = skia::AdoptRef( + new SkDashPathEffect(intervals, arraysize(intervals), 0)); + paint.setPathEffect(effect.get()); paint.setColor(SK_ColorMAGENTA); paint.setStyle(SkPaint::kStroke_Style); @@ -783,9 +784,9 @@ TEST_F(VectorCanvasTest, MAYBE_PathEffects) { { SkPaint paint; SkScalar intervals[] = { 1, 1 }; - SkPathEffect* effect = new SkDashPathEffect(intervals, arraysize(intervals), - 0); - paint.setPathEffect(effect)->unref(); + skia::RefPtr<SkPathEffect> effect = skia::AdoptRef( + new SkDashPathEffect(intervals, arraysize(intervals), 0)); + paint.setPathEffect(effect.get()); paint.setColor(SK_ColorMAGENTA); paint.setStyle(SkPaint::kStroke_Style); diff --git a/skia/ext/vector_platform_device_emf_win.cc b/skia/ext/vector_platform_device_emf_win.cc index 243e808..6d8fbee 100644 --- a/skia/ext/vector_platform_device_emf_win.cc +++ b/skia/ext/vector_platform_device_emf_win.cc @@ -190,7 +190,7 @@ void VectorPlatformDeviceEmf::drawRect(const SkDraw& draw, // Removes the path effect from the temporary SkPaint object. SkPaint paint_no_effet(paint); - SkSafeUnref(paint_no_effet.setPathEffect(NULL)); + paint_no_effet.setPathEffect(NULL); // Draw the calculated path. drawPath(draw, path_modified, paint_no_effet); @@ -224,7 +224,7 @@ void VectorPlatformDeviceEmf::drawPath(const SkDraw& draw, // Removes the path effect from the temporary SkPaint object. SkPaint paint_no_effet(paint); - SkSafeUnref(paint_no_effet.setPathEffect(NULL)); + paint_no_effet.setPathEffect(NULL); // Draw the calculated path. drawPath(draw, path_modified, paint_no_effet); diff --git a/skia/ext/vector_platform_device_skia.cc b/skia/ext/vector_platform_device_skia.cc index 14376222..d8d3084 100644 --- a/skia/ext/vector_platform_device_skia.cc +++ b/skia/ext/vector_platform_device_skia.cc @@ -41,9 +41,8 @@ PlatformSurface VectorPlatformDeviceSkia::BeginPlatformPaint() { // and return the context from it, then layer on the raster data as an // image in EndPlatformPaint. DCHECK(raster_surface_ == NULL); - raster_surface_ = BitmapPlatformDevice::CreateAndClear(width(), height(), - false); - raster_surface_->unref(); // SkRefPtr and create both took a reference. + raster_surface_ = skia::AdoptRef( + BitmapPlatformDevice::CreateAndClear(width(), height(), false)); return raster_surface_->BeginPlatformPaint(); } @@ -58,7 +57,7 @@ void VectorPlatformDeviceSkia::EndPlatformPaint() { drawSprite(draw, raster_surface_->accessBitmap(false), 0, 0, paint); // BitmapPlatformDevice matches begin and end calls. raster_surface_->EndPlatformPaint(); - raster_surface_ = NULL; + raster_surface_.clear(); } #if defined(OS_WIN) diff --git a/skia/ext/vector_platform_device_skia.h b/skia/ext/vector_platform_device_skia.h index 0575563..4a6d2d9 100644 --- a/skia/ext/vector_platform_device_skia.h +++ b/skia/ext/vector_platform_device_skia.h @@ -9,7 +9,7 @@ #include "base/compiler_specific.h" #include "base/logging.h" #include "skia/ext/platform_device.h" -#include "third_party/skia/include/core/SkRefCnt.h" +#include "skia/ext/refptr.h" #include "third_party/skia/include/core/SkTScopedPtr.h" #include "third_party/skia/include/pdf/SkPDFDevice.h" @@ -50,7 +50,7 @@ class VectorPlatformDeviceSkia : public SkPDFDevice, public PlatformDevice { #endif private: - SkRefPtr<BitmapPlatformDevice> raster_surface_; + skia::RefPtr<BitmapPlatformDevice> raster_surface_; DISALLOW_COPY_AND_ASSIGN(VectorPlatformDeviceSkia); }; diff --git a/skia/skia.gyp b/skia/skia.gyp index 0decae1..7a3e20c 100644 --- a/skia/skia.gyp +++ b/skia/skia.gyp @@ -176,6 +176,7 @@ 'ext/platform_device_linux.cc', 'ext/platform_device_mac.cc', 'ext/platform_device_win.cc', + 'ext/refptr.h', 'ext/SkMemory_new_handler.cpp', 'ext/skia_sandbox_support_win.h', 'ext/skia_sandbox_support_win.cc', diff --git a/ui/gfx/blit_unittest.cc b/ui/gfx/blit_unittest.cc index 1d59d9b..f71be18 100644 --- a/ui/gfx/blit_unittest.cc +++ b/ui/gfx/blit_unittest.cc @@ -68,71 +68,71 @@ TEST(Blit, ScrollCanvas) { { 0x20, 0x21, 0x22, 0x23, 0x24 }, { 0x30, 0x31, 0x32, 0x33, 0x34 }, { 0x40, 0x41, 0x42, 0x43, 0x44 }}; - SetToCanvas<5, 5>(canvas, initial_values); + SetToCanvas<5, 5>(canvas.get(), initial_values); // Sanity check on input. - VerifyCanvasValues<5, 5>(canvas, initial_values); + VerifyCanvasValues<5, 5>(canvas.get(), initial_values); // Scroll none and make sure it's a NOP. - gfx::ScrollCanvas(canvas, + gfx::ScrollCanvas(canvas.get(), gfx::Rect(0, 0, kCanvasWidth, kCanvasHeight), gfx::Vector2d(0, 0)); - VerifyCanvasValues<5, 5>(canvas, initial_values); + VerifyCanvasValues<5, 5>(canvas.get(), initial_values); // Scroll the center 3 pixels up one. gfx::Rect center_three(1, 1, 3, 3); - gfx::ScrollCanvas(canvas, center_three, gfx::Vector2d(0, -1)); + gfx::ScrollCanvas(canvas.get(), center_three, gfx::Vector2d(0, -1)); uint8 scroll_up_expected[kCanvasHeight][kCanvasWidth] = { { 0x00, 0x01, 0x02, 0x03, 0x04 }, { 0x10, 0x21, 0x22, 0x23, 0x14 }, { 0x20, 0x31, 0x32, 0x33, 0x24 }, { 0x30, 0x31, 0x32, 0x33, 0x34 }, { 0x40, 0x41, 0x42, 0x43, 0x44 }}; - VerifyCanvasValues<5, 5>(canvas, scroll_up_expected); + VerifyCanvasValues<5, 5>(canvas.get(), scroll_up_expected); // Reset and scroll the center 3 pixels down one. - SetToCanvas<5, 5>(canvas, initial_values); - gfx::ScrollCanvas(canvas, center_three, gfx::Vector2d(0, 1)); + SetToCanvas<5, 5>(canvas.get(), initial_values); + gfx::ScrollCanvas(canvas.get(), center_three, gfx::Vector2d(0, 1)); uint8 scroll_down_expected[kCanvasHeight][kCanvasWidth] = { { 0x00, 0x01, 0x02, 0x03, 0x04 }, { 0x10, 0x11, 0x12, 0x13, 0x14 }, { 0x20, 0x11, 0x12, 0x13, 0x24 }, { 0x30, 0x21, 0x22, 0x23, 0x34 }, { 0x40, 0x41, 0x42, 0x43, 0x44 }}; - VerifyCanvasValues<5, 5>(canvas, scroll_down_expected); + VerifyCanvasValues<5, 5>(canvas.get(), scroll_down_expected); // Reset and scroll the center 3 pixels right one. - SetToCanvas<5, 5>(canvas, initial_values); - gfx::ScrollCanvas(canvas, center_three, gfx::Vector2d(1, 0)); + SetToCanvas<5, 5>(canvas.get(), initial_values); + gfx::ScrollCanvas(canvas.get(), center_three, gfx::Vector2d(1, 0)); uint8 scroll_right_expected[kCanvasHeight][kCanvasWidth] = { { 0x00, 0x01, 0x02, 0x03, 0x04 }, { 0x10, 0x11, 0x11, 0x12, 0x14 }, { 0x20, 0x21, 0x21, 0x22, 0x24 }, { 0x30, 0x31, 0x31, 0x32, 0x34 }, { 0x40, 0x41, 0x42, 0x43, 0x44 }}; - VerifyCanvasValues<5, 5>(canvas, scroll_right_expected); + VerifyCanvasValues<5, 5>(canvas.get(), scroll_right_expected); // Reset and scroll the center 3 pixels left one. - SetToCanvas<5, 5>(canvas, initial_values); - gfx::ScrollCanvas(canvas, center_three, gfx::Vector2d(-1, 0)); + SetToCanvas<5, 5>(canvas.get(), initial_values); + gfx::ScrollCanvas(canvas.get(), center_three, gfx::Vector2d(-1, 0)); uint8 scroll_left_expected[kCanvasHeight][kCanvasWidth] = { { 0x00, 0x01, 0x02, 0x03, 0x04 }, { 0x10, 0x12, 0x13, 0x13, 0x14 }, { 0x20, 0x22, 0x23, 0x23, 0x24 }, { 0x30, 0x32, 0x33, 0x33, 0x34 }, { 0x40, 0x41, 0x42, 0x43, 0x44 }}; - VerifyCanvasValues<5, 5>(canvas, scroll_left_expected); + VerifyCanvasValues<5, 5>(canvas.get(), scroll_left_expected); // Diagonal scroll. - SetToCanvas<5, 5>(canvas, initial_values); - gfx::ScrollCanvas(canvas, center_three, gfx::Vector2d(2, 2)); + SetToCanvas<5, 5>(canvas.get(), initial_values); + gfx::ScrollCanvas(canvas.get(), center_three, gfx::Vector2d(2, 2)); uint8 scroll_diagonal_expected[kCanvasHeight][kCanvasWidth] = { { 0x00, 0x01, 0x02, 0x03, 0x04 }, { 0x10, 0x11, 0x12, 0x13, 0x14 }, { 0x20, 0x21, 0x22, 0x23, 0x24 }, { 0x30, 0x31, 0x32, 0x11, 0x34 }, { 0x40, 0x41, 0x42, 0x43, 0x44 }}; - VerifyCanvasValues<5, 5>(canvas, scroll_diagonal_expected); + VerifyCanvasValues<5, 5>(canvas.get(), scroll_diagonal_expected); } #if defined(OS_WIN) |