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

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);

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; }
 

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;
 };

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;

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());
 }
 

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());
 }

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();
 }
 

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();
 }

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.

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();

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());
 }
 

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); }
+
+}  // namespace
+
+SK_DECLARE_STATIC_LAZY_PTR_ARRAY(SkTypeface, defaults, 4,
+                                 sk_create_default_typeface, sk_unref_typeface);
 
 SkTypeface* SkTypeface::GetDefaultTypeface(Style style) {
-    SK_DECLARE_STATIC_LAZY_PTR_ARRAY(SkTypeface, defaults, 4, CreateDefault, DeleteDefault);
-
     SkASSERT((int)style < 4);
     return defaults[style];
 }

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]);
 }
 

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());
 }

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();
 }