From feb3c1a57faee39dc10ac904f6b215ba50e286b4 Mon Sep 17 00:00:00 2001 From: bungeman Date: Fri, 5 Aug 2016 06:51:50 -0700 Subject: [PATCH] Move to SkDataTable::MakeXXX and sk_sp. Change SkDataTable::NewXXX to SkDataTable::MakeXXX and return sk_sp. This updates users of SkDataTable to sk_sp as well. There do not appear to be any external users of these methods. Review-Url: https://codereview.chromium.org/2211143002 --- include/core/SkDataTable.h | 15 ++++--- include/ports/SkFontConfigInterface.h | 2 +- include/ports/SkFontMgr_indirect.h | 2 +- include/ports/SkRemotableFontMgr.h | 4 +- src/core/SkDataTable.cpp | 46 ++++++++++----------- src/fonts/SkFontMgr_indirect.cpp | 2 +- src/ports/SkFontConfigInterface_direct.cpp | 6 +-- src/ports/SkFontConfigInterface_direct.h | 2 +- src/ports/SkFontMgr_FontConfigInterface.cpp | 2 +- src/ports/SkFontMgr_fontconfig.cpp | 8 ++-- src/ports/SkRemotableFontMgr_win_dw.cpp | 2 +- tests/DataRefTest.cpp | 39 ++++++++--------- 12 files changed, 60 insertions(+), 70 deletions(-) diff --git a/include/core/SkDataTable.h b/include/core/SkDataTable.h index c9d915d255..2ec2d0f2e2 100644 --- a/include/core/SkDataTable.h +++ b/include/core/SkDataTable.h @@ -63,7 +63,7 @@ public: typedef void (*FreeProc)(void* context); - static SkDataTable* NewEmpty(); + static sk_sp MakeEmpty(); /** * Return a new DataTable that contains a copy of the data stored in each @@ -74,8 +74,8 @@ public: * ptrs[] array. * @param count the number of array elements in ptrs[] and sizes[] to copy. */ - static SkDataTable* NewCopyArrays(const void * const * ptrs, - const size_t sizes[], int count); + static sk_sp MakeCopyArrays(const void * const * ptrs, + const size_t sizes[], int count); /** * Return a new table that contains a copy of the data in array. @@ -85,11 +85,10 @@ public: * @param count the number of entries to be copied out of array. The number * of bytes that will be copied is count * elemSize. */ - static SkDataTable* NewCopyArray(const void* array, size_t elemSize, - int count); + static sk_sp MakeCopyArray(const void* array, size_t elemSize, int count); - static SkDataTable* NewArrayProc(const void* array, size_t elemSize, - int count, FreeProc proc, void* context); + static sk_sp MakeArrayProc(const void* array, size_t elemSize, int count, + FreeProc proc, void* context); private: struct Dir { @@ -164,7 +163,7 @@ public: * calls to append(). This call also clears any accumluated entries from * this builder, so its count() will be 0 after this call. */ - SkDataTable* detachDataTable(); + sk_sp detachDataTable(); private: SkTDArray fDir; diff --git a/include/ports/SkFontConfigInterface.h b/include/ports/SkFontConfigInterface.h index 536768a8bf..30fc8a35fb 100644 --- a/include/ports/SkFontConfigInterface.h +++ b/include/ports/SkFontConfigInterface.h @@ -111,7 +111,7 @@ public: // New APIS, which have default impls for now (which do nothing) - virtual SkDataTable* getFamilyNames() { return SkDataTable::NewEmpty(); } + virtual sk_sp getFamilyNames() { return SkDataTable::MakeEmpty(); } typedef SkRefCnt INHERITED; }; diff --git a/include/ports/SkFontMgr_indirect.h b/include/ports/SkFontMgr_indirect.h index d3f47cb82b..406a75a7ee 100644 --- a/include/ports/SkFontMgr_indirect.h +++ b/include/ports/SkFontMgr_indirect.h @@ -95,7 +95,7 @@ private: mutable SkTArray fDataCache; mutable SkMutex fDataCacheMutex; - mutable SkAutoTUnref fFamilyNames; + mutable sk_sp fFamilyNames; mutable SkOnce fFamilyNamesInitOnce; static void set_up_family_names(const SkFontMgr_Indirect* self); diff --git a/include/ports/SkRemotableFontMgr.h b/include/ports/SkRemotableFontMgr.h index ddc8e50a12..2e028cee26 100644 --- a/include/ports/SkRemotableFontMgr.h +++ b/include/ports/SkRemotableFontMgr.h @@ -61,10 +61,8 @@ public: * * The indexes may be used with getIndex(int) and * matchIndexStyle(int, SkFontStyle). - * - * The caller must unref() the returned object. */ - virtual SkDataTable* getFamilyNames() const = 0; + virtual sk_sp getFamilyNames() const = 0; /** * Returns all of the fonts with the given familyIndex. diff --git a/src/core/SkDataTable.cpp b/src/core/SkDataTable.cpp index 32e30af64d..ea2b91b903 100644 --- a/src/core/SkDataTable.cpp +++ b/src/core/SkDataTable.cpp @@ -7,6 +7,7 @@ #include "SkData.h" #include "SkDataTable.h" +#include "SkOnce.h" static void malloc_freeproc(void* context) { sk_free(context); @@ -76,19 +77,17 @@ const void* SkDataTable::at(int index, size_t* size) const { /////////////////////////////////////////////////////////////////////////////// -SkDataTable* SkDataTable::NewEmpty() { - static SkDataTable* gEmpty; - if (nullptr == gEmpty) { - gEmpty = new SkDataTable; - } - gEmpty->ref(); - return gEmpty; +sk_sp SkDataTable::MakeEmpty() { + static SkDataTable* singleton; + static SkOnce once; + once([]{ singleton = new SkDataTable(); }); + return sk_ref_sp(singleton); } -SkDataTable* SkDataTable::NewCopyArrays(const void * const * ptrs, - const size_t sizes[], int count) { +sk_sp SkDataTable::MakeCopyArrays(const void * const * ptrs, + const size_t sizes[], int count) { if (count <= 0) { - return SkDataTable::NewEmpty(); + return SkDataTable::MakeEmpty(); } size_t dataSize = 0; @@ -108,28 +107,27 @@ SkDataTable* SkDataTable::NewCopyArrays(const void * const * ptrs, elem += sizes[i]; } - return new SkDataTable(dir, count, malloc_freeproc, buffer); + return sk_sp(new SkDataTable(dir, count, malloc_freeproc, buffer)); } -SkDataTable* SkDataTable::NewCopyArray(const void* array, size_t elemSize, - int count) { +sk_sp SkDataTable::MakeCopyArray(const void* array, size_t elemSize, int count) { if (count <= 0) { - return SkDataTable::NewEmpty(); + return SkDataTable::MakeEmpty(); } size_t bufferSize = elemSize * count; void* buffer = sk_malloc_throw(bufferSize); memcpy(buffer, array, bufferSize); - return new SkDataTable(buffer, elemSize, count, malloc_freeproc, buffer); + return sk_sp(new SkDataTable(buffer, elemSize, count, malloc_freeproc, buffer)); } -SkDataTable* SkDataTable::NewArrayProc(const void* array, size_t elemSize, - int count, FreeProc proc, void* ctx) { +sk_sp SkDataTable::MakeArrayProc(const void* array, size_t elemSize, int count, + FreeProc proc, void* ctx) { if (count <= 0) { - return SkDataTable::NewEmpty(); + return SkDataTable::MakeEmpty(); } - return new SkDataTable(array, elemSize, count, proc, ctx); + return sk_sp(new SkDataTable(array, elemSize, count, proc, ctx)); } /////////////////////////////////////////////////////////////////////////////// @@ -164,18 +162,18 @@ void SkDataTableBuilder::append(const void* src, size_t size) { dir->fSize = size; } -SkDataTable* SkDataTableBuilder::detachDataTable() { +sk_sp SkDataTableBuilder::detachDataTable() { const int count = fDir.count(); if (0 == count) { - return SkDataTable::NewEmpty(); + return SkDataTable::MakeEmpty(); } // Copy the dir into the heap; - void* dir = fHeap->alloc(count * sizeof(SkDataTable::Dir), - SkChunkAlloc::kThrow_AllocFailType); + void* dir = fHeap->alloc(count * sizeof(SkDataTable::Dir), SkChunkAlloc::kThrow_AllocFailType); memcpy(dir, fDir.begin(), count * sizeof(SkDataTable::Dir)); - SkDataTable* table = new SkDataTable((SkDataTable::Dir*)dir, count, chunkalloc_freeproc, fHeap); + sk_sp table( + new SkDataTable((SkDataTable::Dir*)dir, count, chunkalloc_freeproc, fHeap)); // we have to detach our fHeap, since we are giving that to the table fHeap = nullptr; fDir.reset(); diff --git a/src/fonts/SkFontMgr_indirect.cpp b/src/fonts/SkFontMgr_indirect.cpp index 01a24c5174..070c0aa402 100644 --- a/src/fonts/SkFontMgr_indirect.cpp +++ b/src/fonts/SkFontMgr_indirect.cpp @@ -61,7 +61,7 @@ private: }; void SkFontMgr_Indirect::set_up_family_names(const SkFontMgr_Indirect* self) { - self->fFamilyNames.reset(self->fProxy->getFamilyNames()); + self->fFamilyNames = self->fProxy->getFamilyNames(); } int SkFontMgr_Indirect::onCountFamilies() const { diff --git a/src/ports/SkFontConfigInterface_direct.cpp b/src/ports/SkFontConfigInterface_direct.cpp index 3cde837b9e..3455447246 100644 --- a/src/ports/SkFontConfigInterface_direct.cpp +++ b/src/ports/SkFontConfigInterface_direct.cpp @@ -698,7 +698,7 @@ static bool find_name(const SkTDArray& list, const char* str) { return false; } -SkDataTable* SkFontConfigInterfaceDirect::getFamilyNames() { +sk_sp SkFontConfigInterfaceDirect::getFamilyNames() { FCLocker lock; FcPattern* pat = FcPatternCreate(); @@ -730,6 +730,6 @@ SkDataTable* SkFontConfigInterfaceDirect::getFamilyNames() { } } - return SkDataTable::NewCopyArrays((const void*const*)names.begin(), - sizes.begin(), names.count()); + return SkDataTable::MakeCopyArrays((const void*const*)names.begin(), + sizes.begin(), names.count()); } diff --git a/src/ports/SkFontConfigInterface_direct.h b/src/ports/SkFontConfigInterface_direct.h index 3ccb390a70..6cd0a8f9ba 100644 --- a/src/ports/SkFontConfigInterface_direct.h +++ b/src/ports/SkFontConfigInterface_direct.h @@ -25,7 +25,7 @@ public: SkStreamAsset* openStream(const FontIdentity&) override; // new APIs - SkDataTable* getFamilyNames() override; + sk_sp getFamilyNames() override; protected: virtual bool isAccessible(const char* filename); diff --git a/src/ports/SkFontMgr_FontConfigInterface.cpp b/src/ports/SkFontMgr_FontConfigInterface.cpp index 09b4d13dd4..133b503444 100644 --- a/src/ports/SkFontMgr_FontConfigInterface.cpp +++ b/src/ports/SkFontMgr_FontConfigInterface.cpp @@ -139,7 +139,7 @@ static bool find_by_FontIdentity(SkTypeface* cachedTypeface, void* ctx) { class SkFontMgr_FCI : public SkFontMgr { SkAutoTUnref fFCI; - SkAutoTUnref fFamilyNames; + sk_sp fFamilyNames; SkTypeface_FreeType::Scanner fScanner; mutable SkMutex fMutex; diff --git a/src/ports/SkFontMgr_fontconfig.cpp b/src/ports/SkFontMgr_fontconfig.cpp index 0336e5d921..fd5f1b0830 100644 --- a/src/ports/SkFontMgr_fontconfig.cpp +++ b/src/ports/SkFontMgr_fontconfig.cpp @@ -503,7 +503,7 @@ private: class SkFontMgr_fontconfig : public SkFontMgr { mutable SkAutoFcConfig fFC; - SkAutoTUnref fFamilyNames; + sk_sp fFamilyNames; SkTypeface_FreeType::Scanner fScanner; class StyleSet : public SkFontStyleSet { @@ -579,7 +579,7 @@ class SkFontMgr_fontconfig : public SkFontMgr { return false; } - static SkDataTable* GetFamilyNames(FcConfig* fcconfig) { + static sk_sp GetFamilyNames(FcConfig* fcconfig) { FCLocker lock; SkTDArray names; @@ -613,8 +613,8 @@ class SkFontMgr_fontconfig : public SkFontMgr { } } - return SkDataTable::NewCopyArrays((void const *const *)names.begin(), - sizes.begin(), names.count()); + return SkDataTable::MakeCopyArrays((void const *const *)names.begin(), + sizes.begin(), names.count()); } static bool FindByFcPattern(SkTypeface* cached, void* ctx) { diff --git a/src/ports/SkRemotableFontMgr_win_dw.cpp b/src/ports/SkRemotableFontMgr_win_dw.cpp index ea5562cfbc..a4c895ad0d 100644 --- a/src/ports/SkRemotableFontMgr_win_dw.cpp +++ b/src/ports/SkRemotableFontMgr_win_dw.cpp @@ -90,7 +90,7 @@ public: memcpy(fLocaleName.get(), localeName, localeNameLength * sizeof(WCHAR)); } - SkDataTable* getFamilyNames() const override { + sk_sp getFamilyNames() const override { int count = fFontCollection->GetFontFamilyCount(); SkDataTableBuilder names(1024); diff --git a/tests/DataRefTest.cpp b/tests/DataRefTest.cpp index 59b56fb362..391124e840 100644 --- a/tests/DataRefTest.cpp +++ b/tests/DataRefTest.cpp @@ -25,35 +25,31 @@ static void test_is_equal(skiatest::Reporter* reporter, } } -static void test_datatable_is_empty(skiatest::Reporter* reporter, - SkDataTable* table) { +static void test_datatable_is_empty(skiatest::Reporter* reporter, SkDataTable* table) { REPORTER_ASSERT(reporter, table->isEmpty()); REPORTER_ASSERT(reporter, 0 == table->count()); } static void test_emptytable(skiatest::Reporter* reporter) { - SkAutoTUnref table0(SkDataTable::NewEmpty()); - SkAutoTUnref table1(SkDataTable::NewCopyArrays(nullptr, nullptr, 0)); - SkAutoTUnref table2(SkDataTable::NewCopyArray(nullptr, 0, 0)); - SkAutoTUnref table3(SkDataTable::NewArrayProc(nullptr, 0, 0, - nullptr, nullptr)); + sk_sp table0(SkDataTable::MakeEmpty()); + sk_sp table1(SkDataTable::MakeCopyArrays(nullptr, nullptr, 0)); + sk_sp table2(SkDataTable::MakeCopyArray(nullptr, 0, 0)); + sk_sp table3(SkDataTable::MakeArrayProc(nullptr, 0, 0, nullptr, nullptr)); - test_datatable_is_empty(reporter, table0); - test_datatable_is_empty(reporter, table1); - test_datatable_is_empty(reporter, table2); - test_datatable_is_empty(reporter, table3); + test_datatable_is_empty(reporter, table0.get()); + test_datatable_is_empty(reporter, table1.get()); + test_datatable_is_empty(reporter, table2.get()); + test_datatable_is_empty(reporter, table3.get()); - test_is_equal(reporter, table0, table1); - test_is_equal(reporter, table0, table2); - test_is_equal(reporter, table0, table3); + test_is_equal(reporter, table0.get(), table1.get()); + test_is_equal(reporter, table0.get(), table2.get()); + test_is_equal(reporter, table0.get(), table3.get()); } static void test_simpletable(skiatest::Reporter* reporter) { const int idata[] = { 1, 4, 9, 16, 25, 63 }; int icount = SK_ARRAY_COUNT(idata); - SkAutoTUnref itable(SkDataTable::NewCopyArray(idata, - sizeof(idata[0]), - icount)); + sk_sp itable(SkDataTable::MakeCopyArray(idata, sizeof(idata[0]), icount)); REPORTER_ASSERT(reporter, itable->count() == icount); for (int i = 0; i < icount; ++i) { size_t size; @@ -73,8 +69,7 @@ static void test_vartable(skiatest::Reporter* reporter) { sizes[i] = strlen(str[i]) + 1; } - SkAutoTUnref table(SkDataTable::NewCopyArrays( - (const void*const*)str, sizes, count)); + sk_sp table(SkDataTable::MakeCopyArrays((const void*const*)str, sizes, count)); REPORTER_ASSERT(reporter, table->count() == count); for (int i = 0; i < count; ++i) { @@ -100,7 +95,7 @@ static void test_tablebuilder(skiatest::Reporter* reporter) { for (int i = 0; i < count; ++i) { builder.append(str[i], strlen(str[i]) + 1); } - SkAutoTUnref table(builder.detachDataTable()); + sk_sp table(builder.detachDataTable()); REPORTER_ASSERT(reporter, table->count() == count); for (int i = 0; i < count; ++i) { @@ -121,8 +116,8 @@ static void test_globaltable(skiatest::Reporter* reporter) { }; int count = SK_ARRAY_COUNT(gData); - SkAutoTUnref table(SkDataTable::NewArrayProc(gData, - sizeof(gData[0]), count, nullptr, nullptr)); + sk_sp table( + SkDataTable::MakeArrayProc(gData, sizeof(gData[0]), count, nullptr, nullptr)); REPORTER_ASSERT(reporter, table->count() == count); for (int i = 0; i < count; ++i) {