Throttle loading in chrome_pdf::BlinkUrlLoader

Adds throttling that pauses loading when too much unread data
accumulates, then resumes loading when the backlog clears.

For consistency, switches all EXPECT_CALL() expectations without
argument matchers in url_loader_unittest.cc to the argument-less form.

Fixed: 1099022
Change-Id: Iac02f17bbcac166e92e19611e5ad35786a3c3a2f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2427844
Commit-Queue: K. Moon <kmoon@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#810369}

pdf/ppapi_migration/url_loader.cc

@@ -140,6 +140,11 @@ void BlinkUrlLoader::Open(const UrlRequest& request, ResultCallback callback) {
                                    GURL(request.custom_referrer_url));
   }
 
+  buffer_lower_threshold_ = request.buffer_lower_threshold;
+  buffer_upper_threshold_ = request.buffer_upper_threshold;
+  DCHECK_GT(buffer_lower_threshold_, 0u);
+  DCHECK_LE(buffer_lower_threshold_, buffer_upper_threshold_);
+
   blink_request.SetRequestContext(blink::mojom::RequestContextType::PLUGIN);
   blink_request.SetRequestDestination(
       network::mojom::RequestDestination::kEmbed);
@@ -235,6 +240,13 @@ void BlinkUrlLoader::DidReceiveData(const char* data, int data_length) {
     return;
 
   buffer_.insert(buffer_.end(), data, data + data_length);
+
+  // Defer loading if the buffer is too full.
+  if (!deferring_loading_ && buffer_.size() >= buffer_upper_threshold_) {
+    deferring_loading_ = true;
+    blink_loader_->SetDefersLoading(true);
+  }
+
   RunReadCallback();
 }
 
@@ -301,6 +313,12 @@ void BlinkUrlLoader::RunReadCallback() {
     auto read_end = read_begin + num_bytes;
     std::copy(read_begin, read_end, client_buffer_.data());
     buffer_.erase(read_begin, read_end);
+
+    // Resume loading if the buffer is too empty.
+    if (deferring_loading_ && buffer_.size() <= buffer_lower_threshold_) {
+      deferring_loading_ = false;
+      blink_loader_->SetDefersLoading(false);
+    }
   } else {
     DCHECK_EQ(state_, LoadingState::kLoadComplete);
     num_bytes = complete_result_;

pdf/ppapi_migration/url_loader.h

@@ -5,6 +5,7 @@
 #ifndef PDF_PPAPI_MIGRATION_URL_LOADER_H_
 #define PDF_PPAPI_MIGRATION_URL_LOADER_H_
 
+#include <stddef.h>
 #include <stdint.h>
 
 #include <memory>
@@ -60,6 +61,16 @@ struct UrlRequest final {
 
   // Request body.
   std::string body;
+
+  // Thresholds for throttling filling of the loader's internal buffer. Filling
+  // will stop after exceeding the upper threshold, and resume after dropping
+  // below the lower threshold.
+  //
+  // Default values taken from `ppapi/shared_impl/url_request_info_data.cc`. The
+  // PDF viewer never changes the defaults in production, so these fields mostly
+  // exist for testing purposes.
+  size_t buffer_lower_threshold = 50 * 1000 * 1000;
+  size_t buffer_upper_threshold = 100 * 1000 * 1000;
 };
 
 // Properties returned from a URL request. Does not include the response body.
@@ -211,7 +222,12 @@ class BlinkUrlLoader final : public UrlLoader,
   bool ignore_redirects_ = false;
   ResultCallback open_callback_;
 
+  // Thresholds control buffer throttling, as defined in `UrlRequest`.
+  size_t buffer_lower_threshold_ = 0;
+  size_t buffer_upper_threshold_ = 0;
+  bool deferring_loading_ = false;
   base::circular_deque<char> buffer_;
+
   ResultCallback read_callback_;
   base::span<char> client_buffer_;
 };

pdf/ppapi_migration/url_loader_unittest.cc

@@ -42,9 +42,9 @@
 namespace chrome_pdf {
 namespace {
 
-using ::testing::_;
 using ::testing::Each;
 using ::testing::ElementsAreArray;
+using ::testing::InSequence;
 using ::testing::Invoke;
 using ::testing::NiceMock;
 using ::testing::Return;
@@ -100,7 +100,9 @@ class FakeBlinkUrlLoaderClient : public BlinkUrlLoader::Client {
 
   void Invalidate() { valid_ = false; }
 
-  MockWebAssociatedURLLoader& mock_url_loader() { return *mock_url_loader_; }
+  MockWebAssociatedURLLoader* mock_url_loader() {
+    return mock_url_loader_.get();
+  }
 
   const blink::WebAssociatedURLLoaderOptions& saved_options() const {
     return saved_options_;
@@ -148,7 +150,7 @@ class FakeBlinkUrlLoaderClient : public BlinkUrlLoader::Client {
 class BlinkUrlLoaderTest : public testing::Test {
  protected:
   BlinkUrlLoaderTest() {
-    ON_CALL(fake_client_.mock_url_loader(), LoadAsynchronously(_, _))
+    ON_CALL(*mock_url_loader_, LoadAsynchronously)
         .WillByDefault(
             Invoke(this, &BlinkUrlLoaderTest::FakeLoadAsynchronously));
     loader_ = std::make_unique<BlinkUrlLoader>(fake_client_.GetWeakPtr());
@@ -160,10 +162,18 @@ class BlinkUrlLoaderTest : public testing::Test {
     EXPECT_EQ(loader_.get(), client);
   }
 
+  void StartLoadWithThresholds(size_t lower, size_t upper) {
+    UrlRequest request;
+    request.buffer_lower_threshold = lower;
+    request.buffer_upper_threshold = upper;
+    loader_->Open(request, mock_callback_.Get());
+    loader_->DidReceiveResponse(blink::WebURLResponse());
+  }
+
   int32_t DidFailWithError(const blink::WebURLError& error) {
     int32_t result = 0;
     loader_->Open(UrlRequest(), mock_callback_.Get());
-    EXPECT_CALL(mock_callback_, Run(_)).WillOnce(SaveArg<0>(&result));
+    EXPECT_CALL(mock_callback_, Run).WillOnce(SaveArg<0>(&result));
 
     loader_->DidFail(error);
     return result;
@@ -173,6 +183,9 @@ class BlinkUrlLoaderTest : public testing::Test {
   NiceMock<base::MockCallback<ResultCallback>> mock_callback_;
   std::unique_ptr<BlinkUrlLoader> loader_;
 
+  // Becomes invalid if `loader_` is closed or destructed.
+  MockWebAssociatedURLLoader* mock_url_loader_ = fake_client_.mock_url_loader();
+
   blink::WebURLRequest saved_request_;
 };
 
@@ -183,8 +196,8 @@ TEST_F(BlinkUrlLoaderTest, GrantUniversalAccess) {
 }
 
 TEST_F(BlinkUrlLoaderTest, Open) {
-  EXPECT_CALL(fake_client_.mock_url_loader(), LoadAsynchronously(_, _));
-  EXPECT_CALL(mock_callback_, Run(_)).Times(0);
+  EXPECT_CALL(*mock_url_loader_, LoadAsynchronously);
+  EXPECT_CALL(mock_callback_, Run).Times(0);
 
   UrlRequest request;
   request.url = "http://example.com/fake.pdf";
@@ -205,8 +218,7 @@ TEST_F(BlinkUrlLoaderTest, Open) {
 }
 
 TEST_F(BlinkUrlLoaderTest, OpenWithInvalidatedClientWeakPtr) {
-  EXPECT_CALL(fake_client_.mock_url_loader(), LoadAsynchronously(_, _))
-      .Times(0);
+  EXPECT_CALL(*mock_url_loader_, LoadAsynchronously).Times(0);
   EXPECT_CALL(mock_callback_, Run(PP_ERROR_FAILED));
 
   fake_client_.InvalidateWeakPtrs();
@@ -214,8 +226,7 @@ TEST_F(BlinkUrlLoaderTest, OpenWithInvalidatedClientWeakPtr) {
 }
 
 TEST_F(BlinkUrlLoaderTest, OpenWithInvalidatedClient) {
-  EXPECT_CALL(fake_client_.mock_url_loader(), LoadAsynchronously(_, _))
-      .Times(0);
+  EXPECT_CALL(*mock_url_loader_, LoadAsynchronously).Times(0);
   EXPECT_CALL(mock_callback_, Run(PP_ERROR_FAILED));
 
   fake_client_.Invalidate();
@@ -343,13 +354,46 @@ TEST_F(BlinkUrlLoaderTest, DidReceiveDataWithZeroLength) {
   loader_->Open(UrlRequest(), mock_callback_.Get());
   loader_->DidReceiveResponse(blink::WebURLResponse());
   loader_->ReadResponseBody(buffer, mock_callback_.Get());
-  EXPECT_CALL(mock_callback_, Run(_)).Times(0);
+  EXPECT_CALL(mock_callback_, Run).Times(0);
 
   loader_->DidReceiveData(kFakeData.data(), 0);
 
   EXPECT_THAT(buffer, Each(0));
 }
 
+TEST_F(BlinkUrlLoaderTest, DidReceiveDataBelowUpperThreshold) {
+  StartLoadWithThresholds(/*lower=*/2, /*upper=*/4);
+  EXPECT_CALL(*mock_url_loader_, SetDefersLoading).Times(0);
+
+  char buffer[3] = {};
+  loader_->DidReceiveData(buffer, sizeof(buffer));
+}
+
+TEST_F(BlinkUrlLoaderTest, DidReceiveDataCrossUpperThreshold) {
+  StartLoadWithThresholds(/*lower=*/2, /*upper=*/4);
+
+  char read_buffer[1];
+  loader_->ReadResponseBody(read_buffer, mock_callback_.Get());
+  {
+    InSequence defer_before_read_callback;
+    EXPECT_CALL(*mock_url_loader_, SetDefersLoading(true));
+    EXPECT_CALL(mock_callback_, Run);
+  }
+
+  char buffer[4] = {};
+  loader_->DidReceiveData(buffer, sizeof(buffer));
+}
+
+TEST_F(BlinkUrlLoaderTest, DidReceiveDataAboveUpperThreshold) {
+  StartLoadWithThresholds(/*lower=*/2, /*upper=*/4);
+
+  char buffer[4] = {};
+  loader_->DidReceiveData(buffer, sizeof(buffer));
+  EXPECT_CALL(*mock_url_loader_, SetDefersLoading).Times(0);
+
+  loader_->DidReceiveData(buffer, sizeof(buffer));
+}
+
 TEST_F(BlinkUrlLoaderTest, ReadResponseBody) {
   loader_->Open(UrlRequest(), mock_callback_.Get());
   loader_->DidReceiveResponse(blink::WebURLResponse());
@@ -362,14 +406,14 @@ TEST_F(BlinkUrlLoaderTest, ReadResponseBody) {
   EXPECT_THAT(buffer, ElementsAreArray(kFakeData));
 
   // Verify no more data returned on next call.
-  EXPECT_CALL(mock_callback_, Run(_)).Times(0);
+  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);
+  EXPECT_CALL(mock_callback_, Run).Times(0);
 
   char buffer[kFakeData.size()] = {};
   loader_->ReadResponseBody(buffer, mock_callback_.Get());
@@ -420,7 +464,7 @@ TEST_F(BlinkUrlLoaderTest, ReadResponseBodyWithBiggerBuffer) {
   EXPECT_THAT(buffer_span.subspan(kFakeData.size()), Each(0));
 
   // Verify no more data returned on next call.
-  EXPECT_CALL(mock_callback_, Run(_)).Times(0);
+  EXPECT_CALL(mock_callback_, Run).Times(0);
   loader_->ReadResponseBody(buffer, mock_callback_.Get());
 }
 
@@ -468,10 +512,49 @@ TEST_F(BlinkUrlLoaderTest, ReadResponseBodyWhileLoadCompleteWithError) {
   EXPECT_THAT(buffer, Each(0));
 }
 
+TEST_F(BlinkUrlLoaderTest, ReadResponseBodyAboveLowerThreshold) {
+  StartLoadWithThresholds(/*lower=*/2, /*upper=*/4);
+
+  char write_buffer[5] = {};
+  loader_->DidReceiveData(write_buffer, sizeof(write_buffer));
+  EXPECT_CALL(*mock_url_loader_, SetDefersLoading).Times(0);
+
+  char buffer[2] = {};
+  loader_->ReadResponseBody(buffer, mock_callback_.Get());
+}
+
+TEST_F(BlinkUrlLoaderTest, ReadResponseBodyCrossLowerThreshold) {
+  StartLoadWithThresholds(/*lower=*/2, /*upper=*/4);
+
+  char write_buffer[5] = {};
+  loader_->DidReceiveData(write_buffer, sizeof(write_buffer));
+  {
+    InSequence resume_before_read_callback;
+    EXPECT_CALL(*mock_url_loader_, SetDefersLoading(false));
+    EXPECT_CALL(mock_callback_, Run);
+  }
+
+  char buffer[3] = {};
+  loader_->ReadResponseBody(buffer, mock_callback_.Get());
+}
+
+TEST_F(BlinkUrlLoaderTest, ReadResponseBodyBelowLowerThreshold) {
+  StartLoadWithThresholds(/*lower=*/2, /*upper=*/4);
+
+  char write_buffer[5] = {};
+  loader_->DidReceiveData(write_buffer, sizeof(write_buffer));
+
+  char buffer[3] = {};
+  loader_->ReadResponseBody(buffer, mock_callback_.Get());
+  EXPECT_CALL(*mock_url_loader_, SetDefersLoading).Times(0);
+
+  loader_->ReadResponseBody(buffer, mock_callback_.Get());
+}
+
 TEST_F(BlinkUrlLoaderTest, DidFinishLoading) {
   loader_->Open(UrlRequest(), mock_callback_.Get());
   loader_->DidReceiveResponse(blink::WebURLResponse());
-  EXPECT_CALL(mock_callback_, Run(_)).Times(0);
+  EXPECT_CALL(mock_callback_, Run).Times(0);
 
   loader_->DidFinishLoading();
 }
@@ -527,7 +610,7 @@ TEST_F(BlinkUrlLoaderTest, DidFailWithWebSecurityViolationError) {
 }
 
 TEST_F(BlinkUrlLoaderTest, CloseWhileWaitingToOpen) {
-  EXPECT_CALL(mock_callback_, Run(_)).Times(0);
+  EXPECT_CALL(mock_callback_, Run).Times(0);
 
   loader_->Close();
 }
@@ -542,7 +625,7 @@ TEST_F(BlinkUrlLoaderTest, CloseWhileOpening) {
 TEST_F(BlinkUrlLoaderTest, CloseWhileStreamingData) {
   loader_->Open(UrlRequest(), mock_callback_.Get());
   loader_->DidReceiveResponse(blink::WebURLResponse());
-  EXPECT_CALL(mock_callback_, Run(_)).Times(0);
+  EXPECT_CALL(mock_callback_, Run).Times(0);
 
   loader_->Close();
 }
@@ -561,7 +644,7 @@ TEST_F(BlinkUrlLoaderTest, CloseWhileLoadComplete) {
   loader_->Open(UrlRequest(), mock_callback_.Get());
   loader_->DidReceiveResponse(blink::WebURLResponse());
   loader_->DidFinishLoading();
-  EXPECT_CALL(mock_callback_, Run(_)).Times(0);
+  EXPECT_CALL(mock_callback_, Run).Times(0);
 
   loader_->Close();
 }
@@ -569,7 +652,7 @@ TEST_F(BlinkUrlLoaderTest, CloseWhileLoadComplete) {
 TEST_F(BlinkUrlLoaderTest, CloseAgain) {
   loader_->Open(UrlRequest(), mock_callback_.Get());
   loader_->Close();
-  EXPECT_CALL(mock_callback_, Run(_)).Times(0);
+  EXPECT_CALL(mock_callback_, Run).Times(0);
 
   loader_->Close();
 }