Return RetainPtr<> from StringData::Create()

-- Pass spans to some internal StringData methods.
-- Move-assign rather than Swap()
-- Default move-constructors.

Change-Id: I0c5718d482ba66a9dc8f2028bd76feabcd840aa3
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/116350
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Thomas Sepez <tsepez@google.com>
diff --git a/core/fxcrt/bytestring.cpp b/core/fxcrt/bytestring.cpp
index 5155309..7d67743 100644
--- a/core/fxcrt/bytestring.cpp
+++ b/core/fxcrt/bytestring.cpp
@@ -108,26 +108,22 @@
 }
 
 ByteString::ByteString(const char* pStr, size_t nLen) {
-  if (nLen)
-    m_pData.Reset(StringData::Create(pStr, nLen));
+  if (nLen) {
+    m_pData = StringData::Create({pStr, nLen});
+  }
 }
 
-ByteString::ByteString(const uint8_t* pStr, size_t nLen) {
-  if (nLen)
-    m_pData.Reset(
-        StringData::Create(reinterpret_cast<const char*>(pStr), nLen));
-}
+ByteString::ByteString(const uint8_t* pStr, size_t nLen)
+    : ByteString(reinterpret_cast<const char*>(pStr), nLen) {}
 
 ByteString::ByteString() = default;
 
 ByteString::ByteString(const ByteString& other) = default;
 
-ByteString::ByteString(ByteString&& other) noexcept {
-  m_pData.Swap(other.m_pData);
-}
+ByteString::ByteString(ByteString&& other) noexcept = default;
 
 ByteString::ByteString(char ch) {
-  m_pData.Reset(StringData::Create(1));
+  m_pData = StringData::Create(1);
   m_pData->m_String[0] = ch;
 }
 
@@ -136,8 +132,7 @@
 
 ByteString::ByteString(ByteStringView bstrc) {
   if (!bstrc.IsEmpty()) {
-    m_pData.Reset(
-        StringData::Create(bstrc.unterminated_c_str(), bstrc.GetLength()));
+    m_pData = StringData::Create(bstrc.span());
   }
 }
 
@@ -149,10 +144,9 @@
   if (nNewLen == 0)
     return;
 
-  m_pData.Reset(StringData::Create(nNewLen));
-  m_pData->CopyContents(str1.unterminated_c_str(), str1.GetLength());
-  m_pData->CopyContentsAt(str1.GetLength(), str2.unterminated_c_str(),
-                          str2.GetLength());
+  m_pData = StringData::Create(nNewLen);
+  m_pData->CopyContents(str1.span());
+  m_pData->CopyContentsAt(str1.GetLength(), str2.span());
 }
 
 ByteString::ByteString(const std::initializer_list<ByteStringView>& list) {
@@ -164,20 +158,20 @@
   if (nNewLen == 0)
     return;
 
-  m_pData.Reset(StringData::Create(nNewLen));
+  m_pData = StringData::Create(nNewLen);
 
   size_t nOffset = 0;
   for (const auto& item : list) {
-    m_pData->CopyContentsAt(nOffset, item.unterminated_c_str(),
-                            item.GetLength());
+    m_pData->CopyContentsAt(nOffset, item.span());
     nOffset += item.GetLength();
   }
 }
 
 ByteString::ByteString(const fxcrt::ostringstream& outStream) {
   auto str = outStream.str();
-  if (!str.empty())
-    m_pData.Reset(StringData::Create(str.c_str(), str.size()));
+  if (!str.empty()) {
+    m_pData = StringData::Create({str.c_str(), str.size()});
+  }
 }
 
 ByteString::~ByteString() = default;
@@ -335,7 +329,7 @@
 
 void ByteString::AssignCopy(const char* pSrcData, size_t nSrcLen) {
   AllocBeforeWrite(nSrcLen);
-  m_pData->CopyContents(pSrcData, nSrcLen);
+  m_pData->CopyContents({pSrcData, nSrcLen});
   m_pData->m_nDataLength = nSrcLen;
 }
 
@@ -348,28 +342,27 @@
     return;
   }
 
-  RetainPtr<StringData> pNewData(StringData::Create(nNewLength));
+  RetainPtr<StringData> pNewData = StringData::Create(nNewLength);
   if (m_pData) {
     size_t nCopyLength = std::min(m_pData->m_nDataLength, nNewLength);
-    pNewData->CopyContents(m_pData->m_String, nCopyLength);
+    pNewData->CopyContents({m_pData->m_String, nCopyLength});
     pNewData->m_nDataLength = nCopyLength;
   } else {
     pNewData->m_nDataLength = 0;
   }
   pNewData->m_String[pNewData->m_nDataLength] = 0;
-  m_pData.Swap(pNewData);
+  m_pData = std::move(pNewData);
 }
 
 void ByteString::AllocBeforeWrite(size_t nNewLength) {
-  if (m_pData && m_pData->CanOperateInPlace(nNewLength))
+  if (m_pData && m_pData->CanOperateInPlace(nNewLength)) {
     return;
-
+  }
   if (nNewLength == 0) {
     clear();
     return;
   }
-
-  m_pData.Reset(StringData::Create(nNewLength));
+  m_pData = StringData::Create(nNewLength);
 }
 
 void ByteString::ReleaseBuffer(size_t nNewLength) {
@@ -399,10 +392,10 @@
 
 pdfium::span<char> ByteString::GetBuffer(size_t nMinBufLength) {
   if (!m_pData) {
-    if (nMinBufLength == 0)
+    if (nMinBufLength == 0) {
       return pdfium::span<char>();
-
-    m_pData.Reset(StringData::Create(nMinBufLength));
+    }
+    m_pData = StringData::Create(nMinBufLength);
     m_pData->m_nDataLength = 0;
     m_pData->m_String[0] = 0;
     return pdfium::span<char>(m_pData->m_String, m_pData->m_nAllocLength);
@@ -415,10 +408,10 @@
   if (nMinBufLength == 0)
     return pdfium::span<char>();
 
-  RetainPtr<StringData> pNewData(StringData::Create(nMinBufLength));
+  RetainPtr<StringData> pNewData = StringData::Create(nMinBufLength);
   pNewData->CopyContents(*m_pData);
   pNewData->m_nDataLength = m_pData->m_nDataLength;
-  m_pData.Swap(pNewData);
+  m_pData = std::move(pNewData);
   return pdfium::span<char>(m_pData->m_String, m_pData->m_nAllocLength);
 }
 
@@ -448,23 +441,23 @@
     return;
 
   if (!m_pData) {
-    m_pData.Reset(StringData::Create(pSrcData, nSrcLen));
+    m_pData = StringData::Create({pSrcData, nSrcLen});
     return;
   }
 
   if (m_pData->CanOperateInPlace(m_pData->m_nDataLength + nSrcLen)) {
-    m_pData->CopyContentsAt(m_pData->m_nDataLength, pSrcData, nSrcLen);
+    m_pData->CopyContentsAt(m_pData->m_nDataLength, {pSrcData, nSrcLen});
     m_pData->m_nDataLength += nSrcLen;
     return;
   }
 
   size_t nConcatLen = std::max(m_pData->m_nDataLength / 2, nSrcLen);
-  RetainPtr<StringData> pNewData(
-      StringData::Create(m_pData->m_nDataLength + nConcatLen));
+  RetainPtr<StringData> pNewData =
+      StringData::Create(m_pData->m_nDataLength + nConcatLen);
   pNewData->CopyContents(*m_pData);
-  pNewData->CopyContentsAt(m_pData->m_nDataLength, pSrcData, nSrcLen);
+  pNewData->CopyContentsAt(m_pData->m_nDataLength, {pSrcData, nSrcLen});
   pNewData->m_nDataLength = m_pData->m_nDataLength + nSrcLen;
-  m_pData.Swap(pNewData);
+  m_pData = std::move(pNewData);
 }
 
 intptr_t ByteString::ReferenceCountForTesting() const {
@@ -635,7 +628,7 @@
     return nCount;
   }
 
-  RetainPtr<StringData> pNewData(StringData::Create(nNewLength));
+  RetainPtr<StringData> pNewData = StringData::Create(nNewLength);
   {
     // Spans can't outlive the StringData buffers.
     pdfium::span<const char> search_span = m_pData->span();
diff --git a/core/fxcrt/string_data_template.cpp b/core/fxcrt/string_data_template.cpp
index a659cc2..fb5c3c4 100644
--- a/core/fxcrt/string_data_template.cpp
+++ b/core/fxcrt/string_data_template.cpp
@@ -20,7 +20,7 @@
 
 // static
 template <typename CharType>
-StringDataTemplate<CharType>* StringDataTemplate<CharType>::Create(
+RetainPtr<StringDataTemplate<CharType>> StringDataTemplate<CharType>::Create(
     size_t nLen) {
   DCHECK_GT(nLen, 0u);
 
@@ -42,16 +42,15 @@
   DCHECK(usableLen >= nLen);
 
   void* pData = FX_StringAlloc(char, totalSize);
-  return new (pData) StringDataTemplate(nLen, usableLen);
+  return pdfium::WrapRetain(new (pData) StringDataTemplate(nLen, usableLen));
 }
 
 // static
 template <typename CharType>
-StringDataTemplate<CharType>* StringDataTemplate<CharType>::Create(
-    const CharType* pStr,
-    size_t nLen) {
-  StringDataTemplate* result = Create(nLen);
-  result->CopyContents(pStr, nLen);
+RetainPtr<StringDataTemplate<CharType>> StringDataTemplate<CharType>::Create(
+    pdfium::span<const CharType> str) {
+  RetainPtr<StringDataTemplate> result = Create(str.size());
+  result->CopyContents(str);
   return result;
 }
 
@@ -70,30 +69,24 @@
 }
 
 template <typename CharType>
-void StringDataTemplate<CharType>::CopyContents(const CharType* pStr,
-                                                size_t nLen) {
-  DCHECK_GE(nLen, 0u);
-  DCHECK_LE(nLen, m_nAllocLength);
-  FXSYS_memcpy(m_String, pStr, nLen * sizeof(CharType));
-  m_String[nLen] = 0;
+void StringDataTemplate<CharType>::CopyContents(
+    pdfium::span<const CharType> str) {
+  FXSYS_memcpy(m_String, str.data(), str.size_bytes());
+  m_String[str.size()] = 0;
 }
 
 template <typename CharType>
-void StringDataTemplate<CharType>::CopyContentsAt(size_t offset,
-                                                  const CharType* pStr,
-                                                  size_t nLen) {
-  DCHECK_GE(offset, 0u);
-  DCHECK_GE(nLen, 0u);
-  DCHECK_LE(offset + nLen, m_nAllocLength);
-  FXSYS_memcpy(m_String + offset, pStr, nLen * sizeof(CharType));
-  m_String[offset + nLen] = 0;
+void StringDataTemplate<CharType>::CopyContentsAt(
+    size_t offset,
+    pdfium::span<const CharType> str) {
+  FXSYS_memcpy(m_String + offset, str.data(), str.size_bytes());
+  m_String[offset + str.size()] = 0;
 }
 
 template <typename CharType>
 StringDataTemplate<CharType>::StringDataTemplate(size_t dataLen,
                                                  size_t allocLen)
     : m_nDataLength(dataLen), m_nAllocLength(allocLen) {
-  DCHECK_GE(dataLen, 0u);
   DCHECK_LE(dataLen, allocLen);
   m_String[dataLen] = 0;
 }
diff --git a/core/fxcrt/string_data_template.h b/core/fxcrt/string_data_template.h
index da5617e..b827a81 100644
--- a/core/fxcrt/string_data_template.h
+++ b/core/fxcrt/string_data_template.h
@@ -10,6 +10,7 @@
 #include <stddef.h>
 #include <stdint.h>
 
+#include "core/fxcrt/retain_ptr.h"
 #include "third_party/base/containers/span.h"
 
 namespace fxcrt {
@@ -17,8 +18,8 @@
 template <typename CharType>
 class StringDataTemplate {
  public:
-  static StringDataTemplate* Create(size_t nLen);
-  static StringDataTemplate* Create(const CharType* pStr, size_t nLen);
+  static RetainPtr<StringDataTemplate> Create(size_t nLen);
+  static RetainPtr<StringDataTemplate> Create(pdfium::span<const CharType> str);
 
   void Retain() { ++m_nRefs; }
   void Release();
@@ -28,8 +29,8 @@
   }
 
   void CopyContents(const StringDataTemplate& other);
-  void CopyContents(const CharType* pStr, size_t nLen);
-  void CopyContentsAt(size_t offset, const CharType* pStr, size_t nLen);
+  void CopyContents(pdfium::span<const CharType> str);
+  void CopyContentsAt(size_t offset, pdfium::span<const CharType> str);
 
   pdfium::span<CharType> span() {
     return pdfium::make_span(m_String, m_nDataLength);
diff --git a/core/fxcrt/widestring.cpp b/core/fxcrt/widestring.cpp
index 2c83fad..179a03e 100644
--- a/core/fxcrt/widestring.cpp
+++ b/core/fxcrt/widestring.cpp
@@ -405,17 +405,16 @@
 
 WideString::WideString(const WideString& other) = default;
 
-WideString::WideString(WideString&& other) noexcept {
-  m_pData.Swap(other.m_pData);
-}
+WideString::WideString(WideString&& other) noexcept = default;
 
 WideString::WideString(const wchar_t* pStr, size_t nLen) {
-  if (nLen)
-    m_pData.Reset(StringData::Create(pStr, nLen));
+  if (nLen) {
+    m_pData = StringData::Create({pStr, nLen});
+  }
 }
 
 WideString::WideString(wchar_t ch) {
-  m_pData.Reset(StringData::Create(1));
+  m_pData = StringData::Create(1);
   m_pData->m_String[0] = ch;
 }
 
@@ -424,8 +423,7 @@
 
 WideString::WideString(WideStringView stringSrc) {
   if (!stringSrc.IsEmpty()) {
-    m_pData.Reset(StringData::Create(stringSrc.unterminated_c_str(),
-                                     stringSrc.GetLength()));
+    m_pData = StringData::Create(stringSrc.span());
   }
 }
 
@@ -437,10 +435,9 @@
   if (nNewLen == 0)
     return;
 
-  m_pData.Reset(StringData::Create(nNewLen));
-  m_pData->CopyContents(str1.unterminated_c_str(), str1.GetLength());
-  m_pData->CopyContentsAt(str1.GetLength(), str2.unterminated_c_str(),
-                          str2.GetLength());
+  m_pData = StringData::Create(nNewLen);
+  m_pData->CopyContents(str1.span());
+  m_pData->CopyContentsAt(str1.GetLength(), str2.span());
 }
 
 WideString::WideString(const std::initializer_list<WideStringView>& list) {
@@ -452,12 +449,11 @@
   if (nNewLen == 0)
     return;
 
-  m_pData.Reset(StringData::Create(nNewLen));
+  m_pData = StringData::Create(nNewLen);
 
   size_t nOffset = 0;
   for (const auto& item : list) {
-    m_pData->CopyContentsAt(nOffset, item.unterminated_c_str(),
-                            item.GetLength());
+    m_pData->CopyContentsAt(nOffset, item.span());
     nOffset += item.GetLength();
   }
 }
@@ -588,7 +584,7 @@
 
 void WideString::AssignCopy(const wchar_t* pSrcData, size_t nSrcLen) {
   AllocBeforeWrite(nSrcLen);
-  m_pData->CopyContents(pSrcData, nSrcLen);
+  m_pData->CopyContents({pSrcData, nSrcLen});
   m_pData->m_nDataLength = nSrcLen;
 }
 
@@ -601,16 +597,16 @@
     return;
   }
 
-  RetainPtr<StringData> pNewData(StringData::Create(nNewLength));
+  RetainPtr<StringData> pNewData = StringData::Create(nNewLength);
   if (m_pData) {
     size_t nCopyLength = std::min(m_pData->m_nDataLength, nNewLength);
-    pNewData->CopyContents(m_pData->m_String, nCopyLength);
+    pNewData->CopyContents({m_pData->m_String, nCopyLength});
     pNewData->m_nDataLength = nCopyLength;
   } else {
     pNewData->m_nDataLength = 0;
   }
   pNewData->m_String[pNewData->m_nDataLength] = 0;
-  m_pData.Swap(pNewData);
+  m_pData = std::move(pNewData);
 }
 
 void WideString::AllocBeforeWrite(size_t nNewLength) {
@@ -622,7 +618,7 @@
     return;
   }
 
-  m_pData.Reset(StringData::Create(nNewLength));
+  m_pData = StringData::Create(nNewLength);
 }
 
 void WideString::ReleaseBuffer(size_t nNewLength) {
@@ -655,7 +651,7 @@
     if (nMinBufLength == 0)
       return pdfium::span<wchar_t>();
 
-    m_pData.Reset(StringData::Create(nMinBufLength));
+    m_pData = StringData::Create(nMinBufLength);
     m_pData->m_nDataLength = 0;
     m_pData->m_String[0] = 0;
     return pdfium::span<wchar_t>(m_pData->m_String, m_pData->m_nAllocLength);
@@ -668,10 +664,10 @@
   if (nMinBufLength == 0)
     return pdfium::span<wchar_t>();
 
-  RetainPtr<StringData> pNewData(StringData::Create(nMinBufLength));
+  RetainPtr<StringData> pNewData = StringData::Create(nMinBufLength);
   pNewData->CopyContents(*m_pData);
   pNewData->m_nDataLength = m_pData->m_nDataLength;
-  m_pData.Swap(pNewData);
+  m_pData = std::move(pNewData);
   return pdfium::span<wchar_t>(m_pData->m_String, m_pData->m_nAllocLength);
 }
 
@@ -701,23 +697,23 @@
     return;
 
   if (!m_pData) {
-    m_pData.Reset(StringData::Create(pSrcData, nSrcLen));
+    m_pData = StringData::Create({pSrcData, nSrcLen});
     return;
   }
 
   if (m_pData->CanOperateInPlace(m_pData->m_nDataLength + nSrcLen)) {
-    m_pData->CopyContentsAt(m_pData->m_nDataLength, pSrcData, nSrcLen);
+    m_pData->CopyContentsAt(m_pData->m_nDataLength, {pSrcData, nSrcLen});
     m_pData->m_nDataLength += nSrcLen;
     return;
   }
 
   size_t nConcatLen = std::max(m_pData->m_nDataLength / 2, nSrcLen);
-  RetainPtr<StringData> pNewData(
-      StringData::Create(m_pData->m_nDataLength + nConcatLen));
+  RetainPtr<StringData> pNewData =
+      StringData::Create(m_pData->m_nDataLength + nConcatLen);
   pNewData->CopyContents(*m_pData);
-  pNewData->CopyContentsAt(m_pData->m_nDataLength, pSrcData, nSrcLen);
+  pNewData->CopyContentsAt(m_pData->m_nDataLength, {pSrcData, nSrcLen});
   pNewData->m_nDataLength = m_pData->m_nDataLength + nSrcLen;
-  m_pData.Swap(pNewData);
+  m_pData = std::move(pNewData);
 }
 
 intptr_t WideString::ReferenceCountForTesting() const {
@@ -948,7 +944,7 @@
     return count;
   }
 
-  RetainPtr<StringData> pNewData(StringData::Create(nNewLength));
+  RetainPtr<StringData> pNewData = StringData::Create(nNewLength);
   {
     // Spans can't outlive StrinData buffers.
     pdfium::span<const wchar_t> search_span = m_pData->span();