[PDF] Spanify PDFiumEngine::ReadLoadedBytes()

Modernize ReadLoadedBytes() to only take 1 parameter. Along the way:

- Document ReadLoadedBytes() behavior.
- Remove the DCHECK() in the production implementation, since
  DocumentLoader will handle the cases when the buffer is too big.
- Use base::span::copy_from() in the test implementation. Remove the
  DCHECK() here as well, since span APIs do even more rigorious checks.

Bug: 390223051
Change-Id: If958538615396ff9fd71e6122085a0aa559c1ccd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6350871
Reviewed-by: Alan Screen <awscreen@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1432238}

pdf/pdf_view_web_plugin.cc

@@ -2006,7 +2006,7 @@ void PdfViewWebPlugin::SaveToBuffer(SaveRequestType request_type,
     uint32_t length = engine_->GetLoadedByteSize();
     if (IsSaveDataSizeValid(length)) {
       base::Value::BlobStorage data(length);
-      if (engine_->ReadLoadedBytes(length, data.data())) {
+      if (engine_->ReadLoadedBytes(data)) {
         data_to_save = base::Value(std::move(data));
       }
     }

pdf/pdfium/pdfium_engine.cc

@@ -1258,9 +1258,8 @@ uint32_t PDFiumEngine::GetLoadedByteSize() {
   return doc_loader_->GetDocumentSize();
 }
 
-bool PDFiumEngine::ReadLoadedBytes(uint32_t length, void* buffer) {
+bool PDFiumEngine::ReadLoadedBytes(base::span<uint8_t> buffer) {
-  DCHECK_LE(length, GetLoadedByteSize());
+  return doc_loader_->GetBlock(0, buffer.size(), buffer.data());
-  return doc_loader_->GetBlock(0, length, buffer);
 }
 
 void PDFiumEngine::SetFormSelectedText(FPDF_FORMHANDLE form_handle,

pdf/pdfium/pdfium_engine.h

@@ -373,7 +373,12 @@ class PDFiumEngine : public DocumentLoader::Client, public IFSDK_PAUSE {
 
   virtual uint32_t GetLoadedByteSize();
 
-  virtual bool ReadLoadedBytes(uint32_t length, void* buffer);
+  // Copies data from `doc_loader_` into `buffer`.
+  // - `buffer` is completely filled, so its size should be less than or equal
+  //   to GetLoadedByteSize().
+  //
+  // Returns true on success and writes into `buffer. Returns false on failure.
+  virtual bool ReadLoadedBytes(base::span<uint8_t> buffer);
 
   // Requests rendering the page at `page_index` as a thumbnail at a given
   // `device_pixel_ratio`. Runs `send_callback` with the rendered thumbnail.

pdf/test/test_pdfium_engine.cc

@@ -2,20 +2,14 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifdef UNSAFE_BUFFERS_BUILD
-// TODO(crbug.com/390223051): Remove C-library calls to fix the errors.
-#pragma allow_unsafe_libc_calls
-#endif
-
 #include "pdf/test/test_pdfium_engine.h"
 
 #include <stdint.h>
-#include <string.h>
 
 #include <iterator>
 #include <vector>
 
-#include "base/check_op.h"
+#include "base/containers/span.h"
 #include "base/values.h"
 #include "pdf/document_attachment_info.h"
 #include "pdf/document_metadata.h"
@@ -59,9 +53,8 @@ uint32_t TestPDFiumEngine::GetLoadedByteSize() {
   return sizeof(kLoadedData);
 }
 
-bool TestPDFiumEngine::ReadLoadedBytes(uint32_t length, void* buffer) {
+bool TestPDFiumEngine::ReadLoadedBytes(base::span<uint8_t> buffer) {
-  DCHECK_LE(length, GetLoadedByteSize());
+  buffer.copy_from(base::span(kLoadedData).first(buffer.size()));
-  memcpy(buffer, kLoadedData, length);
   return true;
 }
 

pdf/test/test_pdfium_engine.h

@@ -100,7 +100,7 @@ class TestPDFiumEngine : public PDFiumEngine {
 
   uint32_t GetLoadedByteSize() override;
 
-  bool ReadLoadedBytes(uint32_t length, void* buffer) override;
+  bool ReadLoadedBytes(base::span<uint8_t> buffer) override;
 
   MOCK_METHOD(void,
               RequestThumbnail,