Modernize and trim down SkOnce.

The API and implementation are very much simplified.
You may not want to bother reading the diff.

As is our trend, SkOnce now uses <atomic> directly.

Member initialization means we don't need SK_DECLARE_STATIC_ONCE.
SkSpinlock already works this same way.

All uses of the old API taking an external bool* and Lock* were pessimal,
so I have not carried this sort of API forward.  It's simpler, faster,
and more space-efficient to always use this single SkOnce class interface.

SkOnce weighs 2 bytes: a done bool and an SkSpinlock, also a bool internally.
This API refactoring opens up the opportunity to fuse those into a single
three-state byte if we'd like.

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

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1894893002

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

15 files changed, 65 insertions, 167 deletions

diff --git a/include/effects/SkColorCubeFilter.h b/include/effects/SkColorCubeFilter.h
index 8b621292b0..10e370a3af 100644
--- a/include/effects/SkColorCubeFilter.h
+++ b/include/effects/SkColorCubeFilter.h
@@ -10,7 +10,7 @@
 
 #include "SkColorFilter.h"
 #include "SkData.h"
-#include "../private/SkMutex.h"
+#include "../private/SkOnce.h"
 #include "../private/SkTemplates.h"
 
 class SK_API SkColorCubeFilter : public SkColorFilter {
@@ -65,8 +65,7 @@ private:
         const int fCubeDimension;
 
         // Make sure we only initialize the caches once.
-        SkMutex fLutsMutex;
-        bool fLutsInited;
+        SkOnce fLutsInitOnce;
 
         static void initProcessingLuts(ColorCubeProcesingCache* cache);
     };
diff --git a/include/gpu/GrResourceKey.h b/include/gpu/GrResourceKey.h
index ff83a43104..9f8063d955 100644
--- a/include/gpu/GrResourceKey.h
+++ b/include/gpu/GrResourceKey.h
@@ -71,7 +71,7 @@ protected:
 
     /** size of the key data, excluding meta-data (hash, domain, etc).  */
     size_t dataSize() const { return this->size() - 4 * kMetaDataCnt; }
- 
+
     /** ptr to the key data, excluding meta-data (hash, domain, etc).  */
     const uint32_t* data() const {
         this->validate();
@@ -290,12 +290,12 @@ private:
  */
 
 /** Place outside of function/class definitions. */
-#define GR_DECLARE_STATIC_UNIQUE_KEY(name) SK_DECLARE_STATIC_ONCE(name##_once)
+#define GR_DECLARE_STATIC_UNIQUE_KEY(name) static SkOnce name##_once
 
 /** Place inside function where the key is used. */
 #define GR_DEFINE_STATIC_UNIQUE_KEY(name)                                                       \
     static SkAlignedSTStorage<1, GrUniqueKey> name##_storage;                                   \
-    SkOnce(&name##_once, gr_init_static_unique_key_once, &name##_storage);                      \
+    name##_once(gr_init_static_unique_key_once, &name##_storage);                               \
     static const GrUniqueKey& name = *reinterpret_cast<GrUniqueKey*>(name##_storage.get());
 
 static inline void gr_init_static_unique_key_once(SkAlignedSTStorage<1,GrUniqueKey>* keyStorage) {
diff --git a/include/ports/SkFontMgr_indirect.h b/include/ports/SkFontMgr_indirect.h
index 5e8f1ede70..d3f47cb82b 100644
--- a/include/ports/SkFontMgr_indirect.h
+++ b/include/ports/SkFontMgr_indirect.h
@@ -9,6 +9,7 @@
 #define SkFontMgr_indirect_DEFINED
 
 #include "../private/SkMutex.h"
+#include "../private/SkOnce.h"
 #include "../private/SkTArray.h"
 #include "SkDataTable.h"
 #include "SkFontMgr.h"
@@ -28,7 +29,7 @@ public:
     // In the future these calls should be broken out into their own interface
     // with a name like SkFontRenderer.
     SkFontMgr_Indirect(SkFontMgr* impl, SkRemotableFontMgr* proxy)
-        : fImpl(SkRef(impl)), fProxy(SkRef(proxy)), fFamilyNamesInited(false)
+        : fImpl(SkRef(impl)), fProxy(SkRef(proxy))
     { }
 
 protected:
@@ -95,8 +96,7 @@ private:
     mutable SkMutex fDataCacheMutex;
 
     mutable SkAutoTUnref<SkDataTable> fFamilyNames;
-    mutable bool fFamilyNamesInited;
-    mutable SkMutex fFamilyNamesMutex;
+    mutable SkOnce fFamilyNamesInitOnce;
     static void set_up_family_names(const SkFontMgr_Indirect* self);
 
     friend class SkStyleSet_Indirect;
diff --git a/include/private/SkOnce.h b/include/private/SkOnce.h
index 34eb79cd77..d83b63f283 100644
--- a/include/private/SkOnce.h
+++ b/include/private/SkOnce.h
@@ -8,132 +8,33 @@
 #ifndef SkOnce_DEFINED
 #define SkOnce_DEFINED
 
-// Before trying SkOnce, see if SkLazyPtr or SkLazyFnPtr will work for you.
-// They're smaller and faster, if slightly less versatile.
-
-
-// SkOnce.h defines SK_DECLARE_STATIC_ONCE and SkOnce(), which you can use
-// together to create a threadsafe way to call a function just once.  E.g.
-//
-// static void register_my_stuff(GlobalRegistry* registry) {
-//     registry->register(...);
-// }
-// ...
-// void EnsureRegistered() {
-//     SK_DECLARE_STATIC_ONCE(once);
-//     SkOnce(&once, register_my_stuff, GetGlobalRegistry());
-// }
-//
-// No matter how many times you call EnsureRegistered(), register_my_stuff will be called just once.
-// OnceTest.cpp also should serve as a few other simple examples.
-
-#include "../private/SkAtomics.h"
 #include "../private/SkSpinlock.h"
+#include <atomic>
+#include <utility>
 
-// This must be used in a global scope, not in function scope or as a class member.
-#define SK_DECLARE_STATIC_ONCE(name) namespace {} static SkOnceFlag name
-
-class SkOnceFlag;
-
-inline void SkOnce(SkOnceFlag* once, void (*f)());
-
-template <typename Arg>
-inline void SkOnce(SkOnceFlag* once, void (*f)(Arg), Arg arg);
-
-// If you've already got a lock and a flag to use, this variant lets you avoid an extra SkOnceFlag.
-template <typename Lock>
-inline void SkOnce(bool* done, Lock* lock, void (*f)());
-
-template <typename Lock, typename Arg>
-inline void SkOnce(bool* done, Lock* lock, void (*f)(Arg), Arg arg);
-
-//  ----------------------  Implementation details below here. -----------------------------
+// SkOnce provides call-once guarantees for Skia, much like std::once_flag/std::call_once().
+//
+// There should be no particularly error-prone gotcha use cases when using SkOnce.
+// It works correctly as a class member, a local, a global, a function-scoped static, whatever.
 
-// This class has no constructor and must be zero-initialized (the macro above does this).
-class SkOnceFlag {
+class SkOnce {
 public:
-    bool* mutableDone() { return &fDone; }
-
-    void acquire() { fSpinlock.acquire(); }
-    void release() { fSpinlock.release(); }
+    template <typename Fn, typename... Args>
+    void operator()(Fn&& fn, Args&&... args) {
+        // Vanilla double-checked locking.
+        if (!fDone.load(std::memory_order_acquire)) {
+            fLock.acquire();
+            if (!fDone.load(std::memory_order_relaxed)) {
+                fn(std::forward<Args>(args)...);
+                fDone.store(true, std::memory_order_release);
+            }
+            fLock.release();
+        }
+    }
 
 private:
-    bool fDone;
-    SkSpinlock fSpinlock;
+    std::atomic<bool> fDone{false};
+    SkSpinlock        fLock;
 };
 
-// We've pulled a pretty standard double-checked locking implementation apart
-// into its main fast path and a slow path that's called when we suspect the
-// one-time code hasn't run yet.
-
-// This is the guts of the code, called when we suspect the one-time code hasn't been run yet.
-// This should be rarely called, so we separate it from SkOnce and don't mark it as inline.
-// (We don't mind if this is an actual function call, but odds are it'll be inlined anyway.)
-template <typename Lock, typename Arg>
-static void sk_once_slow(bool* done, Lock* lock, void (*f)(Arg), Arg arg) {
-    lock->acquire();
-    if (!sk_atomic_load(done, sk_memory_order_relaxed)) {
-        f(arg);
-        // Also known as a store-store/load-store barrier, this makes sure that the writes
-        // done before here---in particular, those done by calling f(arg)---are observable
-        // before the writes after the line, *done = true.
-        //
-        // In version control terms this is like saying, "check in the work up
-        // to and including f(arg), then check in *done=true as a subsequent change".
-        //
-        // We'll use this in the fast path to make sure f(arg)'s effects are
-        // observable whenever we observe *done == true.
-        sk_atomic_store(done, true, sk_memory_order_release);
-    }
-    lock->release();
-}
-
-// This is our fast path, called all the time.  We do really want it to be inlined.
-template <typename Lock, typename Arg>
-inline void SkOnce(bool* done, Lock* lock, void (*f)(Arg), Arg arg) {
-    // When *done == true:
-    //   Also known as a load-load/load-store barrier, this acquire barrier makes
-    //   sure that anything we read from memory---in particular, memory written by
-    //   calling f(arg)---is at least as current as the value we read from done.
-    //
-    //   In version control terms, this is a lot like saying "sync up to the
-    //   commit where we wrote done = true".
-    //
-    //   The release barrier in sk_once_slow guaranteed that done = true
-    //   happens after f(arg), so by syncing to done = true here we're
-    //   forcing ourselves to also wait until the effects of f(arg) are readble.
-    //
-    // When *done == false:
-    //   We'll try to call f(arg) in sk_once_slow.
-    //   If we get the lock, great, we call f(arg), release true into done, and drop the lock.
-    //   If we race and don't get the lock first, we'll wait for the first guy to finish.
-    //   Then lock acquire() will give us at least an acquire memory barrier to get the same
-    //   effect as the acquire load in the *done == true fast case.  We'll see *done is true,
-    //   then just drop the lock and return.
-    if (!sk_atomic_load(done, sk_memory_order_acquire)) {
-        sk_once_slow(done, lock, f, arg);
-    }
-}
-
-template <typename Arg>
-inline void SkOnce(SkOnceFlag* once, void (*f)(Arg), Arg arg) {
-    return SkOnce(once->mutableDone(), once, f, arg);
-}
-
-// Calls its argument.
-// This lets us use functions that take no arguments with SkOnce methods above.
-// (We pass _this_ as the function and the no-arg function as its argument.  Cute eh?)
-static void sk_once_no_arg_adaptor(void (*f)()) {
-    f();
-}
-
-inline void SkOnce(SkOnceFlag* once, void (*func)()) {
-    return SkOnce(once, sk_once_no_arg_adaptor, func);
-}
-
-template <typename Lock>
-inline void SkOnce(bool* done, Lock* lock, void (*func)()) {
-    return SkOnce(done, lock, sk_once_no_arg_adaptor, func);
-}
-
 #endif  // SkOnce_DEFINED
diff --git a/src/core/SkGlobalInitialization_core.cpp b/src/core/SkGlobalInitialization_core.cpp
index ac37073eb3..d7b4a80003 100644
--- a/src/core/SkGlobalInitialization_core.cpp
+++ b/src/core/SkGlobalInitialization_core.cpp
@@ -53,7 +53,7 @@ void SkFlattenable::PrivateInitializer::InitCore() {
     InitEffects();
 };
 
-SK_DECLARE_STATIC_ONCE(once);
 void SkFlattenable::InitializeFlattenablesIfNeeded() {
-    SkOnce(&once, SkFlattenable::PrivateInitializer::InitCore);
+    static SkOnce once;
+    once(SkFlattenable::PrivateInitializer::InitCore);
 }
diff --git a/src/core/SkOpts.cpp b/src/core/SkOpts.cpp
index 570e329094..3c6b5c12c8 100644
--- a/src/core/SkOpts.cpp
+++ b/src/core/SkOpts.cpp
@@ -137,8 +137,10 @@ namespace SkOpts {
     #endif
     }
 
-    SK_DECLARE_STATIC_ONCE(gInitOnce);
-    void Init() { SkOnce(&gInitOnce, init); }
+    void Init() {
+        static SkOnce once;
+        once(init);
+    }
 
 #if SK_ALLOW_STATIC_GLOBAL_INITIALIZERS
     static struct AutoInit {
diff --git a/src/core/SkTaskGroup.cpp b/src/core/SkTaskGroup.cpp
index b696555d6f..2ae1b59971 100644
--- a/src/core/SkTaskGroup.cpp
+++ b/src/core/SkTaskGroup.cpp
@@ -26,11 +26,11 @@
     }
 #endif
 
-// We cache sk_num_cores() so we only query the OS once.
-SK_DECLARE_STATIC_ONCE(g_query_num_cores_once);
 int sk_num_cores() {
+    // We cache sk_num_cores() so we only query the OS once.
     static int num_cores = 0;
-    SkOnce(&g_query_num_cores_once, query_num_cores, &num_cores);
+    static SkOnce once;
+    once(query_num_cores, &num_cores);
     SkASSERT(num_cores > 0);
     return num_cores;
 }
diff --git a/src/effects/SkColorCubeFilter.cpp b/src/effects/SkColorCubeFilter.cpp
index d9e51127a4..fdf571cbf3 100644
--- a/src/effects/SkColorCubeFilter.cpp
+++ b/src/effects/SkColorCubeFilter.cpp
@@ -67,8 +67,7 @@ uint32_t SkColorCubeFilter::getFlags() const {
 }
 
 SkColorCubeFilter::ColorCubeProcesingCache::ColorCubeProcesingCache(int cubeDimension)
-  : fCubeDimension(cubeDimension)
-  , fLutsInited(false) {
+  : fCubeDimension(cubeDimension) {
     fColorToIndex[0] = fColorToIndex[1] = nullptr;
     fColorToFactors[0] = fColorToFactors[1] = nullptr;
     fColorToScalar = nullptr;
@@ -77,8 +76,8 @@ SkColorCubeFilter::ColorCubeProcesingCache::ColorCubeProcesingCache(int cubeDime
 void SkColorCubeFilter::ColorCubeProcesingCache::getProcessingLuts(
     const int* (*colorToIndex)[2], const SkScalar* (*colorToFactors)[2],
     const SkScalar** colorToScalar) {
-    SkOnce(&fLutsInited, &fLutsMutex,
-           SkColorCubeFilter::ColorCubeProcesingCache::initProcessingLuts, this);
+    fLutsInitOnce(SkColorCubeFilter::ColorCubeProcesingCache::initProcessingLuts, this);
+
     SkASSERT((fColorToIndex[0] != nullptr) &&
              (fColorToIndex[1] != nullptr) &&
              (fColorToFactors[0] != nullptr) &&
diff --git a/src/effects/gradients/SkGradientShader.cpp b/src/effects/gradients/SkGradientShader.cpp
index 6c141dcf80..779756f3ba 100644
--- a/src/effects/gradients/SkGradientShader.cpp
+++ b/src/effects/gradients/SkGradientShader.cpp
@@ -322,8 +322,6 @@ SkGradientShaderBase::GradientShaderCache::GradientShaderCache(
     : fCacheAlpha(alpha)
     , fCacheDither(dither)
     , fShader(shader)
-    , fCache16Inited(false)
-    , fCache32Inited(false)
 {
     // Only initialize the cache in getCache16/32.
     fCache16 = nullptr;
@@ -545,8 +543,7 @@ static inline int SkFixedToFFFF(SkFixed x) {
 }
 
 const uint16_t* SkGradientShaderBase::GradientShaderCache::getCache16() {
-    SkOnce(&fCache16Inited, &fCache16Mutex, SkGradientShaderBase::GradientShaderCache::initCache16,
-           this);
+    fCache16InitOnce(SkGradientShaderBase::GradientShaderCache::initCache16, this);
     SkASSERT(fCache16);
     return fCache16;
 }
@@ -579,8 +576,7 @@ void SkGradientShaderBase::GradientShaderCache::initCache16(GradientShaderCache*
 }
 
 const SkPMColor* SkGradientShaderBase::GradientShaderCache::getCache32() {
-    SkOnce(&fCache32Inited, &fCache32Mutex, SkGradientShaderBase::GradientShaderCache::initCache32,
-           this);
+    fCache32InitOnce(SkGradientShaderBase::GradientShaderCache::initCache32, this);
     SkASSERT(fCache32);
     return fCache32;
 }
diff --git a/src/effects/gradients/SkGradientShaderPriv.h b/src/effects/gradients/SkGradientShaderPriv.h
index 881f1eb836..0677cf0fd3 100644
--- a/src/effects/gradients/SkGradientShaderPriv.h
+++ b/src/effects/gradients/SkGradientShaderPriv.h
@@ -142,8 +142,8 @@ public:
         const SkGradientShaderBase& fShader;
 
         // Make sure we only initialize the caches once.
-        bool    fCache16Inited, fCache32Inited;
-        SkMutex fCache16Mutex, fCache32Mutex;
+        SkOnce fCache16InitOnce,
+               fCache32InitOnce;
 
         static void initCache16(GradientShaderCache* cache);
         static void initCache32(GradientShaderCache* cache);
diff --git a/src/fonts/SkFontMgr_indirect.cpp b/src/fonts/SkFontMgr_indirect.cpp
index d3fae91808..01a24c5174 100644
--- a/src/fonts/SkFontMgr_indirect.cpp
+++ b/src/fonts/SkFontMgr_indirect.cpp
@@ -65,12 +65,12 @@ void SkFontMgr_Indirect::set_up_family_names(const SkFontMgr_Indirect* self) {
 }
 
 int SkFontMgr_Indirect::onCountFamilies() const {
-    SkOnce(&fFamilyNamesInited, &fFamilyNamesMutex, SkFontMgr_Indirect::set_up_family_names, this);
+    fFamilyNamesInitOnce(SkFontMgr_Indirect::set_up_family_names, this);
     return fFamilyNames->count();
 }
 
 void SkFontMgr_Indirect::onGetFamilyName(int index, SkString* familyName) const {
-    SkOnce(&fFamilyNamesInited, &fFamilyNamesMutex, SkFontMgr_Indirect::set_up_family_names, this);
+    fFamilyNamesInitOnce(SkFontMgr_Indirect::set_up_family_names, this);
     if (index >= fFamilyNames->count()) {
         familyName->reset();
         return;
diff --git a/src/gpu/GrProgramElement.cpp b/src/gpu/GrProgramElement.cpp
index 6b56392839..f1f3f41343 100644
--- a/src/gpu/GrProgramElement.cpp
+++ b/src/gpu/GrProgramElement.cpp
@@ -7,6 +7,7 @@
 
 #include "GrProgramElement.h"
 #include "GrGpuResourceRef.h"
+#include "SkAtomics.h"
 
 uint32_t GrProgramElement::CreateUniqueID() {
     static int32_t gUniqueID = SK_InvalidUniqueID;
diff --git a/src/utils/win/SkDWrite.cpp b/src/utils/win/SkDWrite.cpp
index 2fdd6115b0..17613f6fad 100644
--- a/src/utils/win/SkDWrite.cpp
+++ b/src/utils/win/SkDWrite.cpp
@@ -43,9 +43,9 @@ static void create_dwrite_factory(IDWriteFactory** factory) {
 }
 
 
-SK_DECLARE_STATIC_ONCE(once);
 IDWriteFactory* sk_get_dwrite_factory() {
-    SkOnce(&once, create_dwrite_factory, &gDWriteFactory);
+    static SkOnce once;
+    once(create_dwrite_factory, &gDWriteFactory);
     return gDWriteFactory;
 }
 
diff --git a/tests/OnceTest.cpp b/tests/OnceTest.cpp
index 83ee66b6c8..ef8d3d9225 100644
--- a/tests/OnceTest.cpp
+++ b/tests/OnceTest.cpp
@@ -13,27 +13,27 @@ static void add_five(int* x) {
     *x += 5;
 }
 
-SK_DECLARE_STATIC_ONCE(st_once);
 DEF_TEST(SkOnce_Singlethreaded, r) {
     int x = 0;
 
     // No matter how many times we do this, x will be 5.
-    SkOnce(&st_once, add_five, &x);
-    SkOnce(&st_once, add_five, &x);
-    SkOnce(&st_once, add_five, &x);
-    SkOnce(&st_once, add_five, &x);
-    SkOnce(&st_once, add_five, &x);
+    SkOnce once;
+    once(add_five, &x);
+    once(add_five, &x);
+    once(add_five, &x);
+    once(add_five, &x);
+    once(add_five, &x);
 
     REPORTER_ASSERT(r, 5 == x);
 }
 
-SK_DECLARE_STATIC_ONCE(mt_once);
 DEF_TEST(SkOnce_Multithreaded, r) {
     int x = 0;
+
     // Run a bunch of tasks to be the first to add six to x.
+    SkOnce once;
     SkTaskGroup().batch(1021, [&](int) {
-        void(*add_six)(int*) = [](int* p) { *p += 6; };
-        SkOnce(&mt_once, add_six, &x);
+        once([&] { x += 6; });
     });
 
     // Only one should have done the +=.
@@ -43,10 +43,10 @@ DEF_TEST(SkOnce_Multithreaded, r) {
 static int gX = 0;
 static void inc_gX() { gX++; }
 
-SK_DECLARE_STATIC_ONCE(noarg_once);
 DEF_TEST(SkOnce_NoArg, r) {
-    SkOnce(&noarg_once, inc_gX);
-    SkOnce(&noarg_once, inc_gX);
-    SkOnce(&noarg_once, inc_gX);
+    SkOnce once;
+    once(inc_gX);
+    once(inc_gX);
+    once(inc_gX);
     REPORTER_ASSERT(r, 1 == gX);
 }
diff --git a/tools/gpu/gl/command_buffer/GLTestContext_command_buffer.cpp b/tools/gpu/gl/command_buffer/GLTestContext_command_buffer.cpp
index af0abc5853..850adaaf25 100644
--- a/tools/gpu/gl/command_buffer/GLTestContext_command_buffer.cpp
+++ b/tools/gpu/gl/command_buffer/GLTestContext_command_buffer.cpp
@@ -127,9 +127,9 @@ static GrGLFuncPtr command_buffer_get_gl_proc(void* ctx, const char name[]) {
     return gfGetProcAddress(name);
 }
 
-SK_DECLARE_STATIC_ONCE(loadCommandBufferOnce);
 static void load_command_buffer_once() {
-    SkOnce(&loadCommandBufferOnce, load_command_buffer_functions);
+    static SkOnce once;
+    once(load_command_buffer_functions);
 }
 
 static const GrGLInterface* create_command_buffer_interface() {