Require SK_DECLARE_STATIC_LAZY_PTR is used in global scope.

Function- or method- local scope isn't threadsafe; the pointer is generally
zero-initialized on first use in function scope (i.e. lazily... we have to go
deeper), but for globals we can be pretty sure the linker will do that for us.

BUG=skia:

No public API changes.
TBR=reed@google.com

Review URL: https://codereview.chromium.org/651723003

diff --git a/include/core/SkData.h b/include/core/SkData.h
index e25ef50d711fcbfd0a262c122d1f8b53d1ca11b8..4f0c213da9bc6f07c9e89f87f77831fb66e763f4 100644 (file)
--- a/include/core/SkData.h
+++ b/include/core/SkData.h
@@ -171,8 +171,7 @@ private:
     virtual void internal_dispose() const SK_OVERRIDE;
 
     // Called the first time someone calls NewEmpty to initialize the singleton.
-    static SkData* NewEmptyImpl();
-    static void DeleteEmpty(SkData*);
+    friend SkData* sk_new_empty_data();
 
     // shared internal factory
     static SkData* PrivateNewWithCopy(const void* srcOrNull, size_t length);

diff --git a/include/core/SkPathRef.h b/include/core/SkPathRef.h
index 0b661b44cb2541bd1302d320fa90bdc2adae2460..4b57fc80e4c39fe595e9a7275ce282584818b02e 100644 (file)
--- a/include/core/SkPathRef.h
+++ b/include/core/SkPathRef.h
@@ -421,7 +421,7 @@ private:
     /**
      * Called the first time someone calls CreateEmpty to actually create the singleton.
      */
-    static SkPathRef* CreateEmptyImpl();
+    friend SkPathRef* sk_create_empty_pathref();
 
     void setIsOval(bool isOval) { fIsOval = isOval; }
 

diff --git a/include/ports/SkFontMgr.h b/include/ports/SkFontMgr.h
index bb8c7b7d988b669e2c6f43e9956029116bba30ad..181fe9f6aa93dce9ef833ec38e7ded346ec25cd3 100644 (file)
--- a/include/ports/SkFontMgr.h
+++ b/include/ports/SkFontMgr.h
@@ -131,7 +131,7 @@ protected:
                                                unsigned styleBits) const = 0;
 private:
     static SkFontMgr* Factory();    // implemented by porting layer
-    static SkFontMgr* CreateDefault();
+    friend SkFontMgr* sk_fontmgr_create_default();
 
     typedef SkRefCnt INHERITED;
 };

diff --git a/include/ports/SkRemotableFontMgr.h b/include/ports/SkRemotableFontMgr.h
index bd99497cda276d6440f2467bb8f628795953b230..bf2459955f5ce27d8bdaf25a2e2eb3cbde2baef1 100644 (file)
--- a/include/ports/SkRemotableFontMgr.h
+++ b/include/ports/SkRemotableFontMgr.h
@@ -46,7 +46,8 @@ public:
 
 private:
     SkRemotableFontIdentitySet() : fCount(0), fData() { }
-    static SkRemotableFontIdentitySet* NewEmptyImpl();
+
+    friend SkRemotableFontIdentitySet* sk_remotable_font_identity_set_new();
 
     int fCount;
     SkAutoTMalloc<SkFontIdentity> fData;

diff --git a/src/core/SkData.cpp b/src/core/SkData.cpp
index 11cab7e98be73388c67bfecc2845dd8c1278ea92..c5d10775f8024cad028c69dbb7e107325c157586 100644 (file)
--- a/src/core/SkData.cpp
+++ b/src/core/SkData.cpp
@@ -92,14 +92,13 @@ SkData* SkData::PrivateNewWithCopy(const void* srcOrNull, size_t length) {
 
 ///////////////////////////////////////////////////////////////////////////////
 
-SkData* SkData::NewEmptyImpl() {
-    return new SkData(NULL, 0, NULL, NULL);
-}
+// As a template argument these must have external linkage.
+SkData* sk_new_empty_data() { return new SkData(NULL, 0, NULL, NULL); }
+namespace { void sk_unref_data(SkData* ptr) { return SkSafeUnref(ptr); } }
 
-void SkData::DeleteEmpty(SkData* ptr) { SkDELETE(ptr); }
+SK_DECLARE_STATIC_LAZY_PTR(SkData, empty, sk_new_empty_data, sk_unref_data);
 
 SkData* SkData::NewEmpty() {
-    SK_DECLARE_STATIC_LAZY_PTR(SkData, empty, NewEmptyImpl, DeleteEmpty);
     return SkRef(empty.get());
 }
 

diff --git a/src/core/SkFontHost.cpp b/src/core/SkFontHost.cpp
index c582ba5bfd4ca1ba11619c766f8ee53cc710a4e3..ce73491c7271e4fdc40a510ecf1f0671a7156eee 100644 (file)
--- a/src/core/SkFontHost.cpp
+++ b/src/core/SkFontHost.cpp
@@ -198,12 +198,14 @@ SkTypeface* SkFontMgr::legacyCreateTypeface(const char familyName[],
     return this->onLegacyCreateTypeface(familyName, styleBits);
 }
 
-SkFontMgr* SkFontMgr::CreateDefault() {
+// As a template argument this must have external linkage.
+SkFontMgr* sk_fontmgr_create_default() {
     SkFontMgr* fm = SkFontMgr::Factory();
     return fm ? fm : SkNEW(SkEmptyFontMgr);
 }
 
+SK_DECLARE_STATIC_LAZY_PTR(SkFontMgr, singleton, sk_fontmgr_create_default);
+
 SkFontMgr* SkFontMgr::RefDefault() {
-    SK_DECLARE_STATIC_LAZY_PTR(SkFontMgr, singleton, CreateDefault);
     return SkRef(singleton.get());
 }

diff --git a/src/core/SkGlyphCache.cpp b/src/core/SkGlyphCache.cpp
index ab816f940707984b8c309be6e21a10bb30b76c82..17f42365fb69c35b4a524f8a43be94a51dd6ebd2 100755 (executable)
--- a/src/core/SkGlyphCache.cpp
+++ b/src/core/SkGlyphCache.cpp
@@ -29,9 +29,10 @@ SkGlyphCache_Globals* create_globals() {
 
 }  // namespace
 
+SK_DECLARE_STATIC_LAZY_PTR(SkGlyphCache_Globals, globals, create_globals);
+
 // Returns the shared globals
 static SkGlyphCache_Globals& getSharedGlobals() {
-    SK_DECLARE_STATIC_LAZY_PTR(SkGlyphCache_Globals, globals, create_globals);
     return *globals.get();
 }
 

diff --git a/src/core/SkImageFilter.cpp b/src/core/SkImageFilter.cpp
index 6f7762ac466d68e010caedd3c7b97a350e3b71eb..c431ece9477b88a6a90e03c55002a0a933804319 100644 (file)
--- a/src/core/SkImageFilter.cpp
+++ b/src/core/SkImageFilter.cpp
@@ -97,7 +97,7 @@ bool SkImageFilter::Common::unflatten(SkReadBuffer& buffer, int expectedCount) {
     if (!buffer.isValid() || !buffer.validate(SkIsValidRect(rect))) {
         return false;
     }
-    
+
     uint32_t flags = buffer.readUInt();
     fCropRect = CropRect(rect, flags);
     if (buffer.isVersionLT(SkReadBuffer::kImageFilterUniqueID_Version)) {
@@ -495,7 +495,8 @@ SkImageFilter::Cache* SkImageFilter::Cache::Create(size_t maxBytes) {
     return SkNEW_ARGS(CacheImpl, (maxBytes));
 }
 
+SK_DECLARE_STATIC_LAZY_PTR(SkImageFilter::Cache, cache, CreateCache);
+
 SkImageFilter::Cache* SkImageFilter::Cache::Get() {
-    SK_DECLARE_STATIC_LAZY_PTR(SkImageFilter::Cache, cache, CreateCache);
     return cache.get();
 }

diff --git a/src/core/SkLazyPtr.h b/src/core/SkLazyPtr.h
index 13218a749ef138e0751ee29b990417a3b8a51bea..0612327dab4073226b2725fc137b307dceef067f 100644 (file)
--- a/src/core/SkLazyPtr.h
+++ b/src/core/SkLazyPtr.h
@@ -47,15 +47,18 @@
  *  We may call Create more than once, but all threads will see the same pointer
  *  returned from get().  Any extra calls to Create will be cleaned up.
  *
- *  These macros must be used in a global or function scope, not as a class member.
+ *  These macros must be used in a global scope, not in function scope or as a class member.
  */
 
 #define SK_DECLARE_STATIC_LAZY_PTR(T, name, ...) \
-    static Private::SkLazyPtr<T, ##__VA_ARGS__> name
+    namespace {} static Private::SkLazyPtr<T, ##__VA_ARGS__> name
 
 #define SK_DECLARE_STATIC_LAZY_PTR_ARRAY(T, name, N, ...) \
-    static Private::SkLazyPtrArray<T, N, ##__VA_ARGS__> name
+    namespace {} static Private::SkLazyPtrArray<T, N, ##__VA_ARGS__> name
 
+// namespace {} forces these macros to only be legal in global scopes.  Chrome has thread-safety
+// problems with them in function-local statics because it uses -fno-threadsafe-statics, and even
+// in builds with threadsafe statics, those threadsafe statics are just unnecessary overhead.
 
 
 // Everything below here is private implementation details.  Don't touch, don't even look.

diff --git a/src/core/SkMessageBus.h b/src/core/SkMessageBus.h
index 1290ea965f9c133faaf25037a85cf2061742a293..a005c3b9dfadd1c121397f979c9eec9aacdfb6e1 100644 (file)
--- a/src/core/SkMessageBus.h
+++ b/src/core/SkMessageBus.h
@@ -38,19 +38,21 @@ public:
 private:
     SkMessageBus();
     static SkMessageBus* Get();
-    static SkMessageBus* New();
+
+    // Allow SkLazyPtr to call SkMessageBus::SkMessageBus().
+    template <typename T> friend T* Private::sk_new();
 
     SkTDArray<Inbox*> fInboxes;
     SkMutex           fInboxesMutex;
 };
 
 // This must go in a single .cpp file, not some .h, or we risk creating more than one global
-// SkMessageBus per type when using shared libraries.
-#define DECLARE_SKMESSAGEBUS_MESSAGE(Message)                        \
-    template <>                                                      \
-    SkMessageBus<Message>* SkMessageBus<Message>::Get() {            \
-        SK_DECLARE_STATIC_LAZY_PTR(SkMessageBus<Message>, bus, New); \
-        return bus.get();                                            \
+// SkMessageBus per type when using shared libraries.  NOTE: at most one per file will compile.
+#define DECLARE_SKMESSAGEBUS_MESSAGE(Message)               \
+    SK_DECLARE_STATIC_LAZY_PTR(SkMessageBus<Message>, bus); \
+    template <>                                             \
+    SkMessageBus<Message>* SkMessageBus<Message>::Get() {   \
+        return bus.get();                                   \
     }
 
 //   ----------------------- Implementation of SkMessageBus::Inbox -----------------------
@@ -96,11 +98,6 @@ void SkMessageBus<Message>::Inbox::poll(SkTDArray<Message>* messages) {
 template <typename Message>
 SkMessageBus<Message>::SkMessageBus() {}
 
-template <typename Message>
-/*static*/ SkMessageBus<Message>* SkMessageBus<Message>::New() {
-    return SkNEW(SkMessageBus<Message>);
-}
-
 template <typename Message>
 /*static*/ void SkMessageBus<Message>::Post(const Message& m) {
     SkMessageBus<Message>* bus = SkMessageBus<Message>::Get();

diff --git a/src/core/SkPathRef.cpp b/src/core/SkPathRef.cpp
index 57076939b67927c431faa02de36d1a5a3255f8cd..dffb6a3646aefd27727ff357d63f855e4e4247b0 100644 (file)
--- a/src/core/SkPathRef.cpp
+++ b/src/core/SkPathRef.cpp
@@ -29,14 +29,16 @@ SkPathRef::Editor::Editor(SkAutoTUnref<SkPathRef>* pathRef,
 
 //////////////////////////////////////////////////////////////////////////////
 
-SkPathRef* SkPathRef::CreateEmptyImpl() {
+// As a template argument, this must have external linkage.
+SkPathRef* sk_create_empty_pathref() {
     SkPathRef* empty = SkNEW(SkPathRef);
     empty->computeBounds();   // Avoids races later to be the first to do this.
     return empty;
 }
 
+SK_DECLARE_STATIC_LAZY_PTR(SkPathRef, empty, sk_create_empty_pathref);
+
 SkPathRef* SkPathRef::CreateEmpty() {
-    SK_DECLARE_STATIC_LAZY_PTR(SkPathRef, empty, CreateEmptyImpl);
     return SkRef(empty.get());
 }
 

diff --git a/src/core/SkTypeface.cpp b/src/core/SkTypeface.cpp
index f9487870ccc537ad13e669083ab9022f554b6151..81038bc986e510e35922c21625b519839ae84dc0 100644 (file)
--- a/src/core/SkTypeface.cpp
+++ b/src/core/SkTypeface.cpp
@@ -79,8 +79,12 @@ protected:
     }
 };
 
+namespace {
+
 SK_DECLARE_STATIC_MUTEX(gCreateDefaultMutex);
-SkTypeface* SkTypeface::CreateDefault(int style) {
+
+// As a template arguments, these must have external linkage.
+SkTypeface* sk_create_default_typeface(int style) {
     // If backed by fontconfig, it's not safe to call SkFontHost::CreateTypeface concurrently.
     // To be safe, we serialize here with a mutex so only one call to
     // CreateTypeface is happening at any given time.
@@ -92,16 +96,14 @@ SkTypeface* SkTypeface::CreateDefault(int style) {
     return t ? t : SkEmptyTypeface::Create();
 }
 
-void SkTypeface::DeleteDefault(SkTypeface* t) {
-    // The SkTypeface returned by SkFontHost::CreateTypeface may _itself_ be a
-    // cleverly-shared singleton.  This is less than ideal.  This means we
-    // cannot just assert our ownership and SkDELETE(t) like we'd want to.
-    SkSafeUnref(t);
-}
+void sk_unref_typeface(SkTypeface* ptr) { SkSafeUnref(ptr); }
 
-SkTypeface* SkTypeface::GetDefaultTypeface(Style style) {
-    SK_DECLARE_STATIC_LAZY_PTR_ARRAY(SkTypeface, defaults, 4, CreateDefault, DeleteDefault);
+}  // namespace
+
+SK_DECLARE_STATIC_LAZY_PTR_ARRAY(SkTypeface, defaults, 4,
+                                 sk_create_default_typeface, sk_unref_typeface);
 
+SkTypeface* SkTypeface::GetDefaultTypeface(Style style) {
     SkASSERT((int)style < 4);
     return defaults[style];
 }

diff --git a/src/core/SkXfermode.cpp b/src/core/SkXfermode.cpp
index 7fcdeb8f3414c6b962791d1de8caeadffa9bc8aa..e115f6fd98ba5ea5cd83d5400683c0f93f190421 100644 (file)
--- a/src/core/SkXfermode.cpp
+++ b/src/core/SkXfermode.cpp
@@ -1211,7 +1211,7 @@ private:
         return fMode == s.fMode &&
                fBackgroundAccess.getTexture() == s.fBackgroundAccess.getTexture();
     }
-    
+
     virtual void onComputeInvariantOutput(InvariantOutput* inout) const SK_OVERRIDE {
         inout->setToUnknown();
     }
@@ -1708,6 +1708,7 @@ SkXfermode* create_mode(int iMode) {
 }
 }  // namespace
 
+SK_DECLARE_STATIC_LAZY_PTR_ARRAY(SkXfermode, cached, SkXfermode::kLastMode + 1, create_mode);
 
 SkXfermode* SkXfermode::Create(Mode mode) {
     SkASSERT(SK_ARRAY_COUNT(gProcCoeffs) == kModeCount);
@@ -1723,7 +1724,6 @@ SkXfermode* SkXfermode::Create(Mode mode) {
         return NULL;
     }
 
-    SK_DECLARE_STATIC_LAZY_PTR_ARRAY(SkXfermode, cached, kModeCount, create_mode);
     return SkSafeRef(cached[mode]);
 }
 

diff --git a/src/fonts/SkRemotableFontMgr.cpp b/src/fonts/SkRemotableFontMgr.cpp
index 633e91458bbb34d5c65fb3d64dc3ac5f877ebd5d..3681f2c8c77f9d9db22c4dfb18025f4f50266352 100644 (file)
--- a/src/fonts/SkRemotableFontMgr.cpp
+++ b/src/fonts/SkRemotableFontMgr.cpp
@@ -16,11 +16,12 @@ SkRemotableFontIdentitySet::SkRemotableFontIdentitySet(int count, SkFontIdentity
     *data = fData;
 }
 
-SkRemotableFontIdentitySet* SkRemotableFontIdentitySet::NewEmptyImpl() {
+// As a template argument, this must have external linkage.
+SkRemotableFontIdentitySet* sk_remotable_font_identity_set_new() {
     return SkNEW(SkRemotableFontIdentitySet);
 }
 
+SK_DECLARE_STATIC_LAZY_PTR(SkRemotableFontIdentitySet, empty, sk_remotable_font_identity_set_new);
 SkRemotableFontIdentitySet* SkRemotableFontIdentitySet::NewEmpty() {
-    SK_DECLARE_STATIC_LAZY_PTR(SkRemotableFontIdentitySet, empty, NewEmptyImpl);
     return SkRef(empty.get());
 }

diff --git a/src/lazy/SkDiscardableMemoryPool.cpp b/src/lazy/SkDiscardableMemoryPool.cpp
index f8f44d0c07cb2869b92a55ee2e0f047e9808a6fc..96d6046f3fb6c4f6e80eff1d8cf91a22b06dd946 100644 (file)
--- a/src/lazy/SkDiscardableMemoryPool.cpp
+++ b/src/lazy/SkDiscardableMemoryPool.cpp
@@ -260,8 +260,9 @@ SkDiscardableMemoryPool* SkDiscardableMemoryPool::Create(size_t size, SkBaseMute
     return SkNEW_ARGS(DiscardableMemoryPool, (size, mutex));
 }
 
+SK_DECLARE_STATIC_LAZY_PTR(SkDiscardableMemoryPool, global, create_global_pool);
+
 SkDiscardableMemoryPool* SkGetGlobalDiscardableMemoryPool() {
-    SK_DECLARE_STATIC_LAZY_PTR(SkDiscardableMemoryPool, global, create_global_pool);
     return global.get();
 }