Receive data in chrome_pdf::BlinkUrlLoader

Implements DidReceiveData() to save data received from the
blink::WebAssociatedURLLoader, and ReadResponseBody() to return the data
to the caller.

Based largely on whatever
ppapi::proxy::URLLoaderResource::ReadResponseBody() does.

Bug: 1099022
Change-Id: Ibaa7db0a7af5996e153d8dc106066ca6133ba95c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2407175
Reviewed-by: K. Moon <kmoon@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Daniel Hosseinian <dhoss@chromium.org>
Commit-Queue: K. Moon <kmoon@chromium.org>
Cr-Commit-Position: refs/heads/master@{#807347}

pdf/ppapi_migration/url_loader.cc

@@ -4,8 +4,10 @@
 
 #include "pdf/ppapi_migration/url_loader.h"
 
+#include <stddef.h>
 #include <stdint.h>
 
+#include <algorithm>
 #include <utility>
 
 #include "base/bind.h"
@@ -109,11 +111,17 @@ void BlinkUrlLoader::ReadResponseBody(base::span<char> buffer,
          state_ == LoadingState::kLoadComplete)
       << static_cast<int>(state_);
 
+  if (buffer.empty()) {
+    std::move(callback).Run(PP_ERROR_BADARGUMENT);
+    return;
+  }
+
   DCHECK(!read_callback_);
   DCHECK(callback);
   read_callback_ = std::move(callback);
+  client_buffer_ = buffer;
 
-  if (state_ == LoadingState::kLoadComplete)
+  if (!buffer_.empty() || state_ == LoadingState::kLoadComplete)
     RunReadCallback();
 }
 
@@ -151,8 +159,16 @@ void BlinkUrlLoader::DidDownloadData(uint64_t data_length) {
   NOTREACHED();
 }
 
+// Modeled on `content::PepperURLLoaderHost::DidReceiveData()`.
 void BlinkUrlLoader::DidReceiveData(const char* data, int data_length) {
-  NOTIMPLEMENTED();
+  DCHECK_EQ(state_, LoadingState::kStreamingData);
+
+  // It's surprisingly difficult to guarantee that this is always >0.
+  if (data_length < 1)
+    return;
+
+  buffer_.insert(buffer_.end(), data, data + data_length);
+  RunReadCallback();
 }
 
 void BlinkUrlLoader::DidReceiveCachedMetadata(const char* data,
@@ -177,19 +193,38 @@ void BlinkUrlLoader::AbortLoad(int32_t result) {
   DCHECK_LT(result, 0);
 
   SetLoadComplete(result);
+  buffer_.clear();
 
   if (open_callback_) {
     DCHECK(!read_callback_);
     std::move(open_callback_).Run(complete_result_);
   } else if (read_callback_) {
-    std::move(read_callback_).Run(complete_result_);
+    RunReadCallback();
   }
 }
 
-// TODO(crbug.com/1099022): Need to handle buffered data.
+// Modeled on `ppapi::proxy::URLLoaderResource::FillUserBuffer()`.
 void BlinkUrlLoader::RunReadCallback() {
-  if (read_callback_)
-    std::move(read_callback_).Run(complete_result_);
+  if (!read_callback_)
+    return;
+
+  DCHECK(!client_buffer_.empty());
+  int32_t num_bytes = std::min(
+      {buffer_.size(), client_buffer_.size(), static_cast<size_t>(INT32_MAX)});
+  if (num_bytes > 0) {
+    auto read_begin = buffer_.begin();
+    auto read_end = read_begin + num_bytes;
+    std::copy(read_begin, read_end, client_buffer_.data());
+    buffer_.erase(read_begin, read_end);
+  } else {
+    DCHECK_EQ(state_, LoadingState::kLoadComplete);
+    num_bytes = complete_result_;
+    DCHECK_LE(num_bytes, 0);
+    static_assert(PP_OK == 0, "PP_OK should be equivalent to 0 bytes");
+  }
+
+  client_buffer_ = {};
+  std::move(read_callback_).Run(num_bytes);
 }
 
 void BlinkUrlLoader::SetLoadComplete(int32_t result) {

pdf/ppapi_migration/url_loader.h

@@ -11,6 +11,7 @@
 #include <string>
 
 #include "base/callback.h"
+#include "base/containers/circular_deque.h"
 #include "base/containers/span.h"
 #include "base/memory/weak_ptr.h"
 #include "base/optional.h"
@@ -186,7 +187,10 @@ class BlinkUrlLoader final : public UrlLoader,
   std::unique_ptr<blink::WebAssociatedURLLoader> blink_loader_;
 
   ResultCallback open_callback_;
+
+  base::circular_deque<char> buffer_;
   ResultCallback read_callback_;
+  base::span<char> client_buffer_;
 };
 
 // A Pepper URL loader.

pdf/ppapi_migration/url_loader_unittest.cc

@@ -31,10 +31,14 @@ namespace chrome_pdf {
 namespace {
 
 using ::testing::_;
+using ::testing::Each;
+using ::testing::ElementsAreArray;
 using ::testing::Invoke;
 using ::testing::NiceMock;
 using ::testing::ReturnNull;
 
+constexpr base::span<const char> kFakeData = "fake data";
+
 class MockWebAssociatedURLLoader : public blink::WebAssociatedURLLoader {
  public:
   // blink::WebAssociatedURLLoader:
@@ -158,31 +162,146 @@ TEST_F(BlinkUrlLoaderTest, DidReceiveResponse) {
   EXPECT_EQ(204, loader_->response().status_code);
 }
 
+TEST_F(BlinkUrlLoaderTest, DidReceiveData) {
+  char buffer[kFakeData.size()] = {};
+  loader_->Open(UrlRequest(), mock_callback_.Get());
+  loader_->DidReceiveResponse(blink::WebURLResponse());
+  loader_->ReadResponseBody(buffer, mock_callback_.Get());
+  EXPECT_CALL(mock_callback_, Run(kFakeData.size()));
+
+  loader_->DidReceiveData(kFakeData.data(), kFakeData.size());
+
+  EXPECT_THAT(buffer, ElementsAreArray(kFakeData));
+}
+
+TEST_F(BlinkUrlLoaderTest, DidReceiveDataWithZeroLength) {
+  char buffer[kFakeData.size()] = {};
+  loader_->Open(UrlRequest(), mock_callback_.Get());
+  loader_->DidReceiveResponse(blink::WebURLResponse());
+  loader_->ReadResponseBody(buffer, mock_callback_.Get());
+  EXPECT_CALL(mock_callback_, Run(_)).Times(0);
+
+  loader_->DidReceiveData(kFakeData.data(), 0);
+
+  EXPECT_THAT(buffer, Each(0));
+}
+
 TEST_F(BlinkUrlLoaderTest, ReadResponseBody) {
+  loader_->Open(UrlRequest(), mock_callback_.Get());
+  loader_->DidReceiveResponse(blink::WebURLResponse());
+  loader_->DidReceiveData(kFakeData.data(), kFakeData.size());
+  EXPECT_CALL(mock_callback_, Run(kFakeData.size()));
+
+  char buffer[kFakeData.size()] = {};
+  loader_->ReadResponseBody(buffer, mock_callback_.Get());
+
+  EXPECT_THAT(buffer, ElementsAreArray(kFakeData));
+
+  // Verify no more data returned on next call.
+  EXPECT_CALL(mock_callback_, Run(_)).Times(0);
+  loader_->ReadResponseBody(buffer, mock_callback_.Get());
+}
+
+TEST_F(BlinkUrlLoaderTest, ReadResponseBodyWithoutData) {
   loader_->Open(UrlRequest(), mock_callback_.Get());
   loader_->DidReceiveResponse(blink::WebURLResponse());
   EXPECT_CALL(mock_callback_, Run(_)).Times(0);
 
-  char buffer[1];
+  char buffer[kFakeData.size()] = {};
   loader_->ReadResponseBody(buffer, mock_callback_.Get());
+
+  EXPECT_THAT(buffer, Each(0));
 }
 
 TEST_F(BlinkUrlLoaderTest, ReadResponseBodyWithEmptyBuffer) {
   loader_->Open(UrlRequest(), mock_callback_.Get());
   loader_->DidReceiveResponse(blink::WebURLResponse());
-  EXPECT_CALL(mock_callback_, Run(_)).Times(0);
+  EXPECT_CALL(mock_callback_, Run(PP_ERROR_BADARGUMENT));
 
   loader_->ReadResponseBody(base::span<char>(), mock_callback_.Get());
 }
 
+TEST_F(BlinkUrlLoaderTest, ReadResponseBodyWithSmallerBuffer) {
+  static constexpr size_t kTailSize = 1;
+  static constexpr size_t kBufferSize = kFakeData.size() - kTailSize;
+
+  loader_->Open(UrlRequest(), mock_callback_.Get());
+  loader_->DidReceiveResponse(blink::WebURLResponse());
+  loader_->DidReceiveData(kFakeData.data(), kFakeData.size());
+  EXPECT_CALL(mock_callback_, Run(kBufferSize));
+
+  char buffer[kBufferSize] = {};
+  loader_->ReadResponseBody(buffer, mock_callback_.Get());
+
+  EXPECT_THAT(buffer, ElementsAreArray(kFakeData.first(kBufferSize)));
+
+  // Verify remaining data returned on next call.
+  char tail_buffer[kTailSize];
+  EXPECT_CALL(mock_callback_, Run(kTailSize));
+  loader_->ReadResponseBody(tail_buffer, mock_callback_.Get());
+  EXPECT_THAT(tail_buffer, ElementsAreArray(kFakeData.subspan(kBufferSize)));
+}
+
+TEST_F(BlinkUrlLoaderTest, ReadResponseBodyWithBiggerBuffer) {
+  loader_->Open(UrlRequest(), mock_callback_.Get());
+  loader_->DidReceiveResponse(blink::WebURLResponse());
+  loader_->DidReceiveData(kFakeData.data(), kFakeData.size());
+  EXPECT_CALL(mock_callback_, Run(kFakeData.size()));
+
+  char buffer[kFakeData.size() + 1] = {};
+  loader_->ReadResponseBody(buffer, mock_callback_.Get());
+
+  base::span<char> buffer_span = buffer;
+  EXPECT_THAT(buffer_span.first(kFakeData.size()), ElementsAreArray(kFakeData));
+  EXPECT_THAT(buffer_span.subspan(kFakeData.size()), Each(0));
+
+  // Verify no more data returned on next call.
+  EXPECT_CALL(mock_callback_, Run(_)).Times(0);
+  loader_->ReadResponseBody(buffer, mock_callback_.Get());
+}
+
 TEST_F(BlinkUrlLoaderTest, ReadResponseBodyWhileLoadComplete) {
   loader_->Open(UrlRequest(), mock_callback_.Get());
   loader_->DidReceiveResponse(blink::WebURLResponse());
+  loader_->DidReceiveData(kFakeData.data(), kFakeData.size());
   loader_->DidFinishLoading();
-  EXPECT_CALL(mock_callback_, Run(0));  // Result represents read bytes.
+  EXPECT_CALL(mock_callback_, Run(kFakeData.size()));
 
-  char buffer[1];
+  char buffer[kFakeData.size()] = {};
   loader_->ReadResponseBody(buffer, mock_callback_.Get());
+
+  EXPECT_THAT(buffer, ElementsAreArray(kFakeData));
+
+  // Verify no more data returned on next call.
+  char tail_buffer[kFakeData.size()] = {};
+  EXPECT_CALL(mock_callback_, Run(0));
+  loader_->ReadResponseBody(tail_buffer, mock_callback_.Get());
+  EXPECT_THAT(tail_buffer, Each(0));
+}
+
+TEST_F(BlinkUrlLoaderTest, ReadResponseBodyWhileLoadCompleteWithoutData) {
+  loader_->Open(UrlRequest(), mock_callback_.Get());
+  loader_->DidReceiveResponse(blink::WebURLResponse());
+  loader_->DidFinishLoading();
+  EXPECT_CALL(mock_callback_, Run(0));
+
+  char buffer[kFakeData.size()] = {};
+  loader_->ReadResponseBody(buffer, mock_callback_.Get());
+
+  EXPECT_THAT(buffer, Each(0));
+}
+
+TEST_F(BlinkUrlLoaderTest, ReadResponseBodyWhileLoadCompleteWithError) {
+  loader_->Open(UrlRequest(), mock_callback_.Get());
+  loader_->DidReceiveResponse(blink::WebURLResponse());
+  loader_->DidReceiveData(kFakeData.data(), kFakeData.size());
+  loader_->Close();
+  EXPECT_CALL(mock_callback_, Run(PP_ERROR_ABORTED));
+
+  char buffer[kFakeData.size()] = {};
+  loader_->ReadResponseBody(buffer, mock_callback_.Get());
+
+  EXPECT_THAT(buffer, Each(0));
 }
 
 TEST_F(BlinkUrlLoaderTest, DidFinishLoading) {