Revert "Return pdfium::span<wchar_t> from WideString::GetBuffer()."

This reverts commit 154e18f9a862975abecebe77b8f5fb418418d14c.

Reason for revert: Generate CL to merge to beta branch

Original change's description:
> Return pdfium::span<wchar_t> from WideString::GetBuffer().
> 
> Adds bounds checking "for free", but beware of span outliving
> a ReleaseBuffer() call.  Scoping as such avoids the possibility
> of using an invalid span (and it is flagged as a lifetime issue).
> 
> Change-Id: Ica63f4b1429823d0254ec6951aeaeb08160cb93c
> Reviewed-on: https://pdfium-review.googlesource.com/30310
> Reviewed-by: dsinclair <dsinclair@chromium.org>
> Commit-Queue: Tom Sepez <tsepez@chromium.org>

TBR=tsepez@chromium.org,dsinclair@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Change-Id: Ie1ec9434215584a024538ca8edeb59dea555af48
Reviewed-on: https://pdfium-review.googlesource.com/30830
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: dsinclair <dsinclair@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>

5 files changed, 72 insertions, 87 deletions

diff --git a/core/fpdfapi/parser/fpdf_parser_decode.cpp b/core/fpdfapi/parser/fpdf_parser_decode.cpp
index 90dca2edcb..d7114b66c5 100644
--- a/core/fpdfapi/parser/fpdf_parser_decode.cpp
+++ b/core/fpdfapi/parser/fpdf_parser_decode.cpp
@@ -425,7 +425,6 @@ bool PDF_DataDecode(const uint8_t* src_buf,
 }
 
 WideString PDF_DecodeText(const uint8_t* src_data, uint32_t src_len) {
-  int dest_pos = 0;
   WideString result;
   if (src_len >= 2 && ((src_data[0] == 0xfe && src_data[1] == 0xff) ||
                        (src_data[0] == 0xff && src_data[1] == 0xfe))) {
@@ -433,15 +432,17 @@ WideString PDF_DecodeText(const uint8_t* src_data, uint32_t src_len) {
     if (!max_chars)
       return result;
 
-    pdfium::span<wchar_t> dest_buf = result.GetBuffer(max_chars);
     bool bBE = src_data[0] == 0xfe || (src_data[0] == 0xff && !src_data[2]);
+    wchar_t* dest_buf = result.GetBuffer(max_chars);
     const uint8_t* uni_str = src_data + 2;
+    int dest_pos = 0;
     for (uint32_t i = 0; i < max_chars * 2; i += 2) {
       uint16_t unicode = GetUnicodeFromBytes(uni_str + i, bBE);
       if (unicode != 0x1b) {
         dest_buf[dest_pos++] = unicode;
         continue;
       }
+
       i += 2;
       while (i < max_chars * 2) {
         uint16_t unicode2 = GetUnicodeFromBytes(uni_str + i, bBE);
@@ -450,13 +451,13 @@ WideString PDF_DecodeText(const uint8_t* src_data, uint32_t src_len) {
           break;
       }
     }
+    result.ReleaseBuffer(dest_pos);
   } else {
-    pdfium::span<wchar_t> dest_buf = result.GetBuffer(src_len);
+    wchar_t* dest_buf = result.GetBuffer(src_len);
     for (uint32_t i = 0; i < src_len; ++i)
       dest_buf[i] = PDFDocEncoding[src_data[i]];
-    dest_pos = src_len;
+    result.ReleaseBuffer(src_len);
   }
-  result.ReleaseBuffer(dest_pos);
   return result;
 }
 
diff --git a/core/fxcrt/cfx_blockbuffer.cpp b/core/fxcrt/cfx_blockbuffer.cpp
index 6a7d98aa18..13134f0e7b 100644
--- a/core/fxcrt/cfx_blockbuffer.cpp
+++ b/core/fxcrt/cfx_blockbuffer.cpp
@@ -77,39 +77,36 @@ WideString CFX_BlockBuffer::GetTextData(size_t start, size_t length) const {
   size_t maybeDataLength = m_BufferSize - 1 - m_StartPosition;
   if (start > maybeDataLength)
     return WideString();
-
   length = std::min(length, maybeDataLength);
-  if (!length)
-    return WideString();
 
   WideString wsTextData;
-  {
-    // Span's lifetime must end before ReleaseBuffer() below.
-    pdfium::span<wchar_t> pBuf = wsTextData.GetBuffer(length);
-    size_t startBlock = 0;
-    size_t startInner = 0;
-    std::tie(startBlock, startInner) = TextDataIndex2BufIndex(start);
-
-    size_t endBlock = 0;
-    size_t endInner = 0;
-    std::tie(endBlock, endInner) = TextDataIndex2BufIndex(start + length);
-
-    size_t pointer = 0;
-    for (size_t i = startBlock; i <= endBlock; ++i) {
-      size_t bufferPointer = 0;
-      size_t copyLength = kAllocStep;
-      if (i == startBlock) {
-        copyLength -= startInner;
-        bufferPointer = startInner;
-      }
-      if (i == endBlock)
-        copyLength -= ((kAllocStep - 1) - endInner);
-
-      wchar_t* pBlockBuf = m_BlockArray[i].get();
-      memcpy(&pBuf[pointer], pBlockBuf + bufferPointer,
-             copyLength * sizeof(wchar_t));
-      pointer += copyLength;
+  wchar_t* pBuf = wsTextData.GetBuffer(length);
+  if (!pBuf)
+    return WideString();
+
+  size_t startBlock = 0;
+  size_t startInner = 0;
+  std::tie(startBlock, startInner) = TextDataIndex2BufIndex(start);
+
+  size_t endBlock = 0;
+  size_t endInner = 0;
+  std::tie(endBlock, endInner) = TextDataIndex2BufIndex(start + length);
+
+  size_t pointer = 0;
+  for (size_t i = startBlock; i <= endBlock; ++i) {
+    size_t bufferPointer = 0;
+    size_t copyLength = kAllocStep;
+    if (i == startBlock) {
+      copyLength -= startInner;
+      bufferPointer = startInner;
     }
+    if (i == endBlock)
+      copyLength -= ((kAllocStep - 1) - endInner);
+
+    wchar_t* pBlockBuf = m_BlockArray[i].get();
+    memcpy(pBuf + pointer, pBlockBuf + bufferPointer,
+           copyLength * sizeof(wchar_t));
+    pointer += copyLength;
   }
   wsTextData.ReleaseBuffer(length);
   return wsTextData;
diff --git a/core/fxcrt/widestring.cpp b/core/fxcrt/widestring.cpp
index a3525593ee..7b5bf66fd3 100644
--- a/core/fxcrt/widestring.cpp
+++ b/core/fxcrt/widestring.cpp
@@ -252,27 +252,22 @@ Optional<size_t> GuessSizeForVSWPrintf(const wchar_t* pFormat,
 Optional<WideString> TryVSWPrintf(size_t size,
                                   const wchar_t* pFormat,
                                   va_list argList) {
-  if (!size)
+  WideString str;
+  wchar_t* buffer = str.GetBuffer(size);
+
+  // In the following two calls, there's always space in the buffer for
+  // a terminating NUL that's not included in nMaxLen.
+  // For vswprintf(), MSAN won't untaint the buffer on a truncated write's
+  // -1 return code even though the buffer is written. Probably just as well
+  // not to trust the vendor's implementation to write anything anyways.
+  // See https://crbug.com/705912.
+  memset(buffer, 0, (size + 1) * sizeof(wchar_t));
+  int ret = vswprintf(buffer, size + 1, pFormat, argList);
+
+  bool bSufficientBuffer = ret >= 0 || buffer[size - 1] == 0;
+  if (!bSufficientBuffer)
     return {};
 
-  WideString str;
-  {
-    // Span's lifetime must end before ReleaseBuffer() below.
-    pdfium::span<wchar_t> buffer = str.GetBuffer(size);
-
-    // In the following two calls, there's always space in the WideString
-    // for a terminating NUL that's not included in the span.
-    // For vswprintf(), MSAN won't untaint the buffer on a truncated write's
-    // -1 return code even though the buffer is written. Probably just as well
-    // not to trust the vendor's implementation to write anything anyways.
-    // See https://crbug.com/705912.
-    memset(buffer.data(), 0, (size + 1) * sizeof(wchar_t));
-    int ret = vswprintf(buffer.data(), size + 1, pFormat, argList);
-
-    bool bSufficientBuffer = ret >= 0 || buffer[size - 1] == 0;
-    if (!bSufficientBuffer)
-      return {};
-  }
   str.ReleaseBuffer(str.GetStringLength());
   return {str};
 }
@@ -304,12 +299,9 @@ WideString GetWideString(uint16_t codepage, const ByteStringView& bstr) {
     return WideString();
 
   WideString wstr;
-  {
-    // Span's lifetime must end before ReleaseBuffer() below.
-    pdfium::span<wchar_t> dest_buf = wstr.GetBuffer(dest_len);
-    FXSYS_MultiByteToWideChar(codepage, 0, bstr.unterminated_c_str(), src_len,
-                              dest_buf.data(), dest_len);
-  }
+  wchar_t* dest_buf = wstr.GetBuffer(dest_len);
+  FXSYS_MultiByteToWideChar(codepage, 0, bstr.unterminated_c_str(), src_len,
+                            dest_buf, dest_len);
   wstr.ReleaseBuffer(dest_len);
   return wstr;
 }
@@ -594,29 +586,29 @@ void WideString::Reserve(size_t len) {
   GetBuffer(len);
 }
 
-pdfium::span<wchar_t> WideString::GetBuffer(size_t nMinBufLength) {
+wchar_t* WideString::GetBuffer(size_t nMinBufLength) {
   if (!m_pData) {
     if (nMinBufLength == 0)
-      return pdfium::span<wchar_t>();
+      return nullptr;
 
     m_pData.Reset(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);
+    return m_pData->m_String;
   }
 
   if (m_pData->CanOperateInPlace(nMinBufLength))
-    return pdfium::span<wchar_t>(m_pData->m_String, m_pData->m_nAllocLength);
+    return m_pData->m_String;
 
   nMinBufLength = std::max(nMinBufLength, m_pData->m_nDataLength);
   if (nMinBufLength == 0)
-    return pdfium::span<wchar_t>();
+    return nullptr;
 
   RetainPtr<StringData> pNewData(StringData::Create(nMinBufLength));
   pNewData->CopyContents(*m_pData);
   pNewData->m_nDataLength = m_pData->m_nDataLength;
   m_pData.Swap(pNewData);
-  return pdfium::span<wchar_t>(m_pData->m_String, m_pData->m_nAllocLength);
+  return m_pData->m_String;
 }
 
 size_t WideString::Delete(size_t index, size_t count) {
@@ -893,15 +885,14 @@ WideString WideString::FromUTF8(const ByteStringView& str) {
 
 // static
 WideString WideString::FromUTF16LE(const unsigned short* wstr, size_t wlen) {
-  if (!wstr || wlen == 0)
+  if (!wstr || wlen == 0) {
     return WideString();
+  }
 
   WideString result;
-  {
-    // Span's lifetime must end before ReleaseBuffer() below.
-    pdfium::span<wchar_t> buf = result.GetBuffer(wlen);
-    for (size_t i = 0; i < wlen; i++)
-      buf[i] = wstr[i];
+  wchar_t* buf = result.GetBuffer(wlen);
+  for (size_t i = 0; i < wlen; i++) {
+    buf[i] = wstr[i];
   }
   result.ReleaseBuffer(wlen);
   return result;
diff --git a/core/fxcrt/widestring.h b/core/fxcrt/widestring.h
index b668b0292c..cefdfabcd0 100644
--- a/core/fxcrt/widestring.h
+++ b/core/fxcrt/widestring.h
@@ -17,7 +17,7 @@
 #include "core/fxcrt/string_data_template.h"
 #include "core/fxcrt/string_view_template.h"
 #include "third_party/base/optional.h"
-#include "third_party/base/span.h"
+
 
 namespace fxcrt {
 
@@ -171,10 +171,7 @@ class WideString {
   void TrimRight(const WideStringView& targets);
 
   void Reserve(size_t len);
-
-  // Note: any modification of the string (including ReleaseBuffer()) may
-  // invalidate the span, which must not outlive its buffer.
-  pdfium::span<wchar_t> GetBuffer(size_t len);
+  wchar_t* GetBuffer(size_t len);
   void ReleaseBuffer(size_t len);
 
   int GetInteger() const;
diff --git a/core/fxcrt/widestring_unittest.cpp b/core/fxcrt/widestring_unittest.cpp
index ec0a55fc1e..48d5c7f8f5 100644
--- a/core/fxcrt/widestring_unittest.cpp
+++ b/core/fxcrt/widestring_unittest.cpp
@@ -817,21 +817,20 @@ TEST(WideString, Reserve) {
 }
 
 TEST(WideString, GetBuffer) {
-  WideString str1;
   {
-    pdfium::span<wchar_t> buffer = str1.GetBuffer(12);
-    wcscpy(buffer.data(), L"clams");
+    WideString str;
+    wchar_t* buffer = str.GetBuffer(12);
+    wcscpy(buffer, L"clams");
+    str.ReleaseBuffer(str.GetStringLength());
+    EXPECT_EQ(L"clams", str);
   }
-  str1.ReleaseBuffer(str1.GetStringLength());
-  EXPECT_EQ(L"clams", str1);
-
-  WideString str2(L"cl");
   {
-    pdfium::span<wchar_t> buffer = str2.GetBuffer(12);
-    wcscpy(buffer.data() + 2, L"ams");
+    WideString str(L"cl");
+    wchar_t* buffer = str.GetBuffer(12);
+    wcscpy(buffer + 2, L"ams");
+    str.ReleaseBuffer(str.GetStringLength());
+    EXPECT_EQ(L"clams", str);
   }
-  str2.ReleaseBuffer(str2.GetStringLength());
-  EXPECT_EQ(L"clams", str2);
 }
 
 TEST(WideString, ReleaseBuffer) {