From 33583b9c36caa8e4388a1ea73ea4f72ede988104 Mon Sep 17 00:00:00 2001
From: Herb Derby <herb@google.com>
Date: Wed, 4 May 2022 09:00:23 -0400
Subject: [PATCH] Add initializer class to improve safety of
 AllocateClassMemoryAndArena

Change-Id: If5f14828029473afef25c5ef69db3971796cdfc4
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/537076
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
---
 src/gpu/ganesh/GrSubRunAllocator.h | 17 +++++++++++++++-
 src/gpu/ganesh/text/GrTextBlob.cpp | 31 ++++++++++++------------------
 src/gpu/ganesh/text/GrTextBlob.h   | 10 +++++-----
 3 files changed, 33 insertions(+), 25 deletions(-)

diff --git a/src/gpu/ganesh/GrSubRunAllocator.h b/src/gpu/ganesh/GrSubRunAllocator.h
index df42545c02..83767fe327 100644
--- a/src/gpu/ganesh/GrSubRunAllocator.h
+++ b/src/gpu/ganesh/GrSubRunAllocator.h
@@ -165,6 +165,21 @@ private:
     SkFibBlockSizes<kMaxByteSize> fFibProgression;
 };
 
+template <typename T>
+class GrSubRunInitializer {
+public:
+    GrSubRunInitializer(void* memory) : fMemory{memory} { SkASSERT(memory != nullptr); }
+    template <typename... Args>
+    T* initialize(Args&&... args) {
+        // Warn on more than one initialization.
+        SkASSERT(fMemory != nullptr);
+        return new (std::exchange(fMemory, nullptr)) T(std::forward<Args>(args)...);
+    }
+
+private:
+    void* fMemory;
+};
+
 // GrSubRunAllocator provides fast allocation where the user takes care of calling the destructors
 // of the returned pointers, and GrSubRunAllocator takes care of deleting the storage. The
 // unique_ptrs returned, are to assist in assuring the object's destructor is called.
@@ -196,7 +211,7 @@ public:
     GrSubRunAllocator& operator=(GrSubRunAllocator&&) = default;
 
     template <typename T>
-    static std::tuple<void *, int, GrSubRunAllocator>
+    static std::tuple<GrSubRunInitializer<T>, int, GrSubRunAllocator>
     AllocateClassMemoryAndArena(int allocSizeHint) {
         SkASSERT_RELEASE(allocSizeHint >= 0);
         // Round the size after the object the optimal amount.
diff --git a/src/gpu/ganesh/text/GrTextBlob.cpp b/src/gpu/ganesh/text/GrTextBlob.cpp
index 6ab41f82bd..63ccb0b604 100644
--- a/src/gpu/ganesh/text/GrTextBlob.cpp
+++ b/src/gpu/ganesh/text/GrTextBlob.cpp
@@ -2178,13 +2178,11 @@ sk_sp<GrTextBlob> GrTextBlob::Make(const SkGlyphRunList& glyphRunList,
             + GrGlyphVector::GlyphVectorSize(totalGlyphCount)
             + glyphRunList.runCount() * (sizeof(DirectMaskSubRun) + vertexDataToSubRunPadding);
 
-    auto [memory, totalMemoryAllocated, alloc] =
+    auto [initializer, totalMemoryAllocated, alloc] =
             GrSubRunAllocator::AllocateClassMemoryAndArena<GrTextBlob>(subRunSizeHint);
     SkColor initialLuminance = SkPaintPriv::ComputeLuminanceColor(paint);
-    sk_sp<GrTextBlob> blob{new (memory) GrTextBlob(std::move(alloc),
-                                                   totalMemoryAllocated,
-                                                   positionMatrix,
-                                                   initialLuminance)};
+    sk_sp<GrTextBlob> blob = sk_sp<GrTextBlob>(initializer.initialize(
+            std::move(alloc), totalMemoryAllocated, positionMatrix, initialLuminance));
 
     const uint64_t uniqueID = glyphRunList.uniqueID();
     for (auto& glyphRun : glyphRunList) {
@@ -2451,13 +2449,11 @@ sk_sp<GrSlug> Slug::MakeFromBuffer(SkReadBuffer& buffer, const SkStrikeClient* c
         subRunsSizeHint = 128;
     }
 
-    auto [memory, _, alloc] = GrSubRunAllocator::AllocateClassMemoryAndArena<Slug>(subRunsSizeHint);
+    auto [initializer, _, alloc] =
+            GrSubRunAllocator::AllocateClassMemoryAndArena<Slug>(subRunsSizeHint);
 
-    sk_sp<Slug> slug{new (memory) Slug(std::move(alloc),
-                                       sourceBounds,
-                                       paint,
-                                       positionMatrix,
-                                       origin)};
+    sk_sp<Slug> slug = sk_sp<Slug>(
+            initializer.initialize(std::move(alloc), sourceBounds, paint, positionMatrix, origin));
 
     for (int i = 0; i < subRunCount; ++i) {
         auto subRun = GrSubRun::MakeFromBuffer(slug.get(), buffer, &slug->fAlloc, client);
@@ -2474,8 +2470,6 @@ sk_sp<GrSlug> Slug::MakeFromBuffer(SkReadBuffer& buffer, const SkStrikeClient* c
     return std::move(slug);
 }
 
-
-
 void Slug::processDeviceMasks(
         const SkZip<SkGlyphVariant, SkPoint>& accepted, sk_sp<SkStrike>&& strike) {
     auto addGlyphsWithSameFormat = [&] (const SkZip<SkGlyphVariant, SkPoint>& accepted,
@@ -2508,16 +2502,15 @@ sk_sp<Slug> Slug::Make(const SkMatrixProvider& viewMatrix,
             + GrGlyphVector::GlyphVectorSize(totalGlyphCount)
             + glyphRunList.runCount() * (sizeof(DirectMaskSubRun) + vertexDataToSubRunPadding);
 
-    auto [memory, _, alloc] = GrSubRunAllocator::AllocateClassMemoryAndArena<Slug>(subRunSizeHint);
+    auto [initializer, _, alloc] =
+            GrSubRunAllocator::AllocateClassMemoryAndArena<Slug>(subRunSizeHint);
 
     const SkMatrix positionMatrix =
             position_matrix(viewMatrix.localToDevice(), glyphRunList.origin());
 
-    sk_sp<Slug> slug{new (memory) Slug(std::move(alloc),
-                                       glyphRunList.sourceBounds(),
-                                       initialPaint,
-                                       positionMatrix,
-                                       glyphRunList.origin())};
+    sk_sp<Slug> slug = sk_sp<Slug>(initializer.initialize(
+            std::move(alloc), glyphRunList.sourceBounds(), initialPaint, positionMatrix,
+            glyphRunList.origin()));
 
     const uint64_t uniqueID = glyphRunList.uniqueID();
     for (auto& glyphRun : glyphRunList) {
diff --git a/src/gpu/ganesh/text/GrTextBlob.h b/src/gpu/ganesh/text/GrTextBlob.h
index 70f8e373fc..2510fc09e2 100644
--- a/src/gpu/ganesh/text/GrTextBlob.h
+++ b/src/gpu/ganesh/text/GrTextBlob.h
@@ -230,6 +230,11 @@ public:
                                   const GrSDFTControl& control,
                                   SkGlyphRunListPainter* painter);
 
+    GrTextBlob(GrSubRunAllocator&& alloc,
+               int totalMemorySize,
+               const SkMatrix& positionMatrix,
+               SkColor initialLuminance);
+
     ~GrTextBlob() override;
 
     // Change memory management to handle the data after GrTextBlob, but in the same allocation
@@ -258,11 +263,6 @@ public:
     const GrAtlasSubRun* testingOnlyFirstSubRun() const;
 
 private:
-    GrTextBlob(GrSubRunAllocator&& alloc,
-               int totalMemorySize,
-               const SkMatrix& positionMatrix,
-               SkColor initialLuminance);
-
     // Methods to satisfy SkGlyphRunPainterInterface
     void processDeviceMasks(const SkZip<SkGlyphVariant, SkPoint>& accepted,
                             sk_sp<SkStrike>&& strike) override;