Bug 1660177 - Clarify accesses to SpliceableChunkedJSONWriter's WriteFunc - r=canaltinova

`SpliceableChunkedJSONWriter::ChunkedWriteFunc` returns a `ChunkedJSONWriteFunc*`, which is never null and is either used to:
1. Copy data.
2. Or take ownership of the chunks.

In the first case, `ChunkedWriteFunc()` now returns a `const ChunkedJSONWriteFunc&` (notice "const &"), so only const members may be used to copy the data.

In the second case, a new function `TakeChunkedWriteFunc()` returns `ChunkedJSONWriteFunc&&` (notice "&&"), so it's clear that its chunks can be taken away. Some `DEBUG` assertions help ensure that it's not used anymore after that.
`TakeAndSplice()` now takes a `ChunkedJSONWriteFunc&&`.

All callers have been updated to the more appropriate functions.

Differential Revision: https://phabricator.services.mozilla.com/D87702

mozglue/baseprofiler/core/ProfileBufferEntry.cpp

@@ -206,7 +206,7 @@ UniqueJSONStrings::UniqueJSONStrings(const UniqueJSONStrings& aOther) {
                                              iter.get().value());
     }
     UniquePtr<char[]> stringTableJSON =
-        aOther.mStringTableWriter.ChunkedWriteFunc()->CopyData();
+        aOther.mStringTableWriter.ChunkedWriteFunc().CopyData();
     mStringTableWriter.Splice(stringTableJSON.get());
   }
 }
@@ -276,12 +276,12 @@ uint32_t UniqueStacks::GetOrAddFrameIndex(const FrameKey& aFrame) {
 
 void UniqueStacks::SpliceFrameTableElements(SpliceableJSONWriter& aWriter) {
   mFrameTableWriter.EndBareList();
-  aWriter.TakeAndSplice(mFrameTableWriter.ChunkedWriteFunc());
+  aWriter.TakeAndSplice(mFrameTableWriter.TakeChunkedWriteFunc());
 }
 
 void UniqueStacks::SpliceStackTableElements(SpliceableJSONWriter& aWriter) {
   mStackTableWriter.EndBareList();
-  aWriter.TakeAndSplice(mStackTableWriter.ChunkedWriteFunc());
+  aWriter.TakeAndSplice(mStackTableWriter.TakeChunkedWriteFunc());
 }
 
 void UniqueStacks::StreamStack(const StackKey& aStack) {

mozglue/baseprofiler/core/ProfileBufferEntry.h

@@ -86,7 +86,7 @@ class UniqueJSONStrings {
   explicit UniqueJSONStrings(const UniqueJSONStrings& aOther);
 
   void SpliceStringTableElements(SpliceableJSONWriter& aWriter) {
-    aWriter.TakeAndSplice(mStringTableWriter.ChunkedWriteFunc());
+    aWriter.TakeAndSplice(mStringTableWriter.TakeChunkedWriteFunc());
   }
 
   void WriteProperty(JSONWriter& aWriter, const char* aName, const char* aStr) {

mozglue/baseprofiler/core/platform.cpp

@@ -2803,7 +2803,7 @@ UniquePtr<char[]> profiler_get_profile(double aSinceTime, bool aIsShuttingDown,
   if (!WriteProfileToJSONWriter(b, aSinceTime, aIsShuttingDown, aOnlyThreads)) {
     return nullptr;
   }
-  return b.ChunkedWriteFunc()->CopyData();
+  return b.ChunkedWriteFunc().CopyData();
 }
 
 void profiler_get_profile_json_into_lazily_allocated_buffer(
@@ -2816,7 +2816,7 @@ void profiler_get_profile_json_into_lazily_allocated_buffer(
     return;
   }
 
-  b.ChunkedWriteFunc()->CopyDataIntoLazilyAllocatedBuffer(aAllocator);
+  b.ChunkedWriteFunc().CopyDataIntoLazilyAllocatedBuffer(aAllocator);
 }
 
 void profiler_get_start_params(int* aCapacity, Maybe<double>* aDuration,

mozglue/baseprofiler/public/BaseProfileJSONWriter.h

@@ -195,15 +195,15 @@ class SpliceableJSONWriter : public JSONWriter {
   // Takes the chunks from aFunc and write them. If move is not possible
   // (e.g., using OStreamJSONWriteFunc), aFunc's chunks are copied and its
   // storage cleared.
-  virtual void TakeAndSplice(ChunkedJSONWriteFunc* aFunc) {
+  virtual void TakeAndSplice(ChunkedJSONWriteFunc&& aFunc) {
     Separator();
-    for (size_t i = 0; i < aFunc->mChunkList.length(); i++) {
-      WriteFunc()->Write(aFunc->mChunkList[i].get());
+    for (size_t i = 0; i < aFunc.mChunkList.length(); i++) {
+      WriteFunc()->Write(aFunc.mChunkList[i].get());
     }
-    aFunc->mChunkPtr = nullptr;
-    aFunc->mChunkEnd = nullptr;
-    aFunc->mChunkList.clear();
-    aFunc->mChunkLengths.clear();
+    aFunc.mChunkPtr = nullptr;
+    aFunc.mChunkEnd = nullptr;
+    aFunc.mChunkList.clear();
+    aFunc.mChunkLengths.clear();
     mNeedComma[mDepth] = true;
   }
 };
@@ -213,18 +213,41 @@ class SpliceableChunkedJSONWriter final : public SpliceableJSONWriter {
   explicit SpliceableChunkedJSONWriter()
       : SpliceableJSONWriter(MakeUnique<ChunkedJSONWriteFunc>()) {}
 
-  ChunkedJSONWriteFunc* ChunkedWriteFunc() const {
-    // The WriteFunc was initialized with a ChunkedJSONWriteFunc in the only
-    // constructor above, so it's safe to cast to ChunkedJSONWriteFunc*.
-    return static_cast<ChunkedJSONWriteFunc*>(WriteFunc());
+  // Access the ChunkedJSONWriteFunc as reference-to-const, usually to copy data
+  // out.
+  const ChunkedJSONWriteFunc& ChunkedWriteFunc() const {
+    MOZ_ASSERT(!mTaken);
+    // The WriteFunc was non-fallibly allocated as a ChunkedJSONWriteFunc in the
+    // only constructor above, so it's safe to cast to ChunkedJSONWriteFunc*.
+    return *static_cast<const ChunkedJSONWriteFunc*>(WriteFunc());
+  }
+
+  // Access the ChunkedJSONWriteFunc as rvalue-reference, usually to take its
+  // data out. This writer shouldn't be used anymore after this.
+  ChunkedJSONWriteFunc&& TakeChunkedWriteFunc() {
+#ifdef DEBUG
+    MOZ_ASSERT(!mTaken);
+    mTaken = true;
+#endif  //
+    // The WriteFunc was non-fallibly allocated as a ChunkedJSONWriteFunc in the
+    // only constructor above, so it's safe to cast to ChunkedJSONWriteFunc*.
+    return std::move(*static_cast<ChunkedJSONWriteFunc*>(WriteFunc()));
   }
 
   // Adopts the chunks from aFunc without copying.
-  void TakeAndSplice(ChunkedJSONWriteFunc* aFunc) override {
+  void TakeAndSplice(ChunkedJSONWriteFunc&& aFunc) override {
+    MOZ_ASSERT(!mTaken);
     Separator();
-    ChunkedWriteFunc()->Take(std::move(*aFunc));
+    // The WriteFunc was non-fallibly allocated as a ChunkedJSONWriteFunc in the
+    // only constructor above, so it's safe to cast to ChunkedJSONWriteFunc*.
+    static_cast<ChunkedJSONWriteFunc*>(WriteFunc())->Take(std::move(aFunc));
     mNeedComma[mDepth] = true;
   }
+
+#ifdef DEBUG
+ private:
+  bool mTaken = false;
+#endif  //
 };
 
 class JSONSchemaWriter {

tools/profiler/core/ProfileBufferEntry.cpp

@@ -218,7 +218,7 @@ UniqueJSONStrings::UniqueJSONStrings(const UniqueJSONStrings& aOther) {
                                              iter.get().value());
     }
     UniquePtr<char[]> stringTableJSON =
-        aOther.mStringTableWriter.ChunkedWriteFunc()->CopyData();
+        aOther.mStringTableWriter.ChunkedWriteFunc().CopyData();
     mStringTableWriter.Splice(stringTableJSON.get());
   }
 }
@@ -380,12 +380,12 @@ uint32_t UniqueStacks::GetOrAddFrameIndex(const FrameKey& aFrame) {
 
 void UniqueStacks::SpliceFrameTableElements(SpliceableJSONWriter& aWriter) {
   mFrameTableWriter.EndBareList();
-  aWriter.TakeAndSplice(mFrameTableWriter.ChunkedWriteFunc());
+  aWriter.TakeAndSplice(mFrameTableWriter.TakeChunkedWriteFunc());
 }
 
 void UniqueStacks::SpliceStackTableElements(SpliceableJSONWriter& aWriter) {
   mStackTableWriter.EndBareList();
-  aWriter.TakeAndSplice(mStackTableWriter.ChunkedWriteFunc());
+  aWriter.TakeAndSplice(mStackTableWriter.TakeChunkedWriteFunc());
 }
 
 void UniqueStacks::StreamStack(const StackKey& aStack) {

tools/profiler/core/ProfileBufferEntry.h

@@ -85,7 +85,7 @@ class UniqueJSONStrings {
   explicit UniqueJSONStrings(const UniqueJSONStrings& aOther);
 
   void SpliceStringTableElements(SpliceableJSONWriter& aWriter) {
-    aWriter.TakeAndSplice(mStringTableWriter.ChunkedWriteFunc());
+    aWriter.TakeAndSplice(mStringTableWriter.TakeChunkedWriteFunc());
   }
 
   void WriteProperty(mozilla::JSONWriter& aWriter, const char* aName,

tools/profiler/core/platform.cpp

@@ -4261,7 +4261,7 @@ UniquePtr<char[]> profiler_get_profile(double aSinceTime,
                                 service.get())) {
     return nullptr;
   }
-  return b.ChunkedWriteFunc()->CopyData();
+  return b.ChunkedWriteFunc().CopyData();
 }
 
 void profiler_get_profile_json_into_lazily_allocated_buffer(
@@ -4278,7 +4278,7 @@ void profiler_get_profile_json_into_lazily_allocated_buffer(
     return;
   }
 
-  b.ChunkedWriteFunc()->CopyDataIntoLazilyAllocatedBuffer(aAllocator);
+  b.ChunkedWriteFunc().CopyDataIntoLazilyAllocatedBuffer(aAllocator);
 }
 
 void profiler_get_start_params(int* aCapacity, Maybe<double>* aDuration,

tools/profiler/gecko/nsProfiler.cpp

@@ -991,7 +991,7 @@ void nsProfiler::FinishGathering() {
   // Close the root object of the generated JSON.
   mWriter->End();
 
-  UniquePtr<char[]> buf = mWriter->ChunkedWriteFunc()->CopyData();
+  UniquePtr<char[]> buf = mWriter->ChunkedWriteFunc().CopyData();
   size_t len = strlen(buf.get());
   nsCString result;
   result.Adopt(buf.release(), len);

tools/profiler/tests/gtest/GeckoProfiler.cpp

@@ -842,7 +842,7 @@ TEST(GeckoProfiler, Markers)
   EXPECT_EQ(GTestMarkerPayload::sNumStreamed, 0 + 10);
   EXPECT_EQ(GTestMarkerPayload::sNumDestroyed, 10 + 10);
 
-  UniquePtr<char[]> profile = w.ChunkedWriteFunc()->CopyData();
+  UniquePtr<char[]> profile = w.ChunkedWriteFunc().CopyData();
   ASSERT_TRUE(!!profile.get());
 
   // Expected markers, in order.
@@ -1548,7 +1548,7 @@ TEST(GeckoProfiler, Counters)
   SpliceableChunkedJSONWriter w;
   ASSERT_TRUE(::profiler_stream_json_for_this_process(w));
 
-  UniquePtr<char[]> profile = w.ChunkedWriteFunc()->CopyData();
+  UniquePtr<char[]> profile = w.ChunkedWriteFunc().CopyData();
 
   // counter name and description should appear as is.
   ASSERT_TRUE(strstr(profile.get(), COUNTER_NAME));
@@ -1561,7 +1561,7 @@ TEST(GeckoProfiler, Counters)
 
   ASSERT_TRUE(::profiler_stream_json_for_this_process(w));
 
-  profile = w.ChunkedWriteFunc()->CopyData();
+  profile = w.ChunkedWriteFunc().CopyData();
   ASSERT_TRUE(strstr(profile.get(), COUNTER_NAME));
   ASSERT_TRUE(strstr(profile.get(), COUNTER_DESCRIPTION));
   ASSERT_TRUE(strstr(profile.get(), COUNTER_NAME2));
@@ -1648,7 +1648,7 @@ TEST(GeckoProfiler, StreamJSONForThisProcess)
   ASSERT_TRUE(::profiler_stream_json_for_this_process(w));
   w.End();
 
-  UniquePtr<char[]> profile = w.ChunkedWriteFunc()->CopyData();
+  UniquePtr<char[]> profile = w.ChunkedWriteFunc().CopyData();
 
   JSONOutputCheck(profile.get());
 
@@ -1685,7 +1685,7 @@ TEST(GeckoProfiler, StreamJSONForThisProcessThreaded)
           }),
       NS_DISPATCH_SYNC);
 
-  UniquePtr<char[]> profile = w.ChunkedWriteFunc()->CopyData();
+  UniquePtr<char[]> profile = w.ChunkedWriteFunc().CopyData();
 
   JSONOutputCheck(profile.get());