Add |SetImageDownloadLimit| to ImageFetcher to limit downloaded bytes

components/image_fetcher/core/image_data_fetcher_unittest.cc

Index: components/image_fetcher/core/image_data_fetcher_unittest.cc
diff --git a/components/image_fetcher/core/image_data_fetcher_unittest.cc b/components/image_fetcher/core/image_data_fetcher_unittest.cc
index fad79ee59a59772fb42602748c3958f996c38b26..7fb828786f0ffda2ae72e24e1d4816b92f97f002 100644
--- a/components/image_fetcher/core/image_data_fetcher_unittest.cc
+++ b/components/image_fetcher/core/image_data_fetcher_unittest.cc
@@ -201,4 +201,52 @@ TEST_F(ImageDataFetcherTest, FetchImageData_MultipleRequests) {
   test_url_fetcher->delegate()->OnURLFetchComplete(test_url_fetcher);
 }
 
+TEST_F(ImageDataFetcherTest, FetchImageData_CancelFetchIfImageExceedsMaxSize) {
+  // In order to know whether the fetcher was canceled, it must notify about its
+  // deletion.
+  fetcher_factory_.set_remove_fetcher_on_delete(true);
+
+  int64_t max_download_bytes = 1024 * 1024;
+  image_data_fetcher_.SetImageDownloadLimit(max_download_bytes);
+  image_data_fetcher_.FetchImageData(
+      GURL(kImageURL), base::Bind(&ImageDataFetcherTest::OnImageDataFetched,
+                                  base::Unretained(this)));
+
+  // Fetching an oversized image will behave like any other failed request.
+  // There will be exactly one call to OnImageDataFetched containing a response
+  // code that would be possible for a completed fetch.

[Marc Treib, 2017/03/27 13:16:43]

s/possible/impossible/ ?

[fhorschig, 2017/03/27 14:33:43]

Absolutely.

+  RequestMetadata expected_metadata;
+  expected_metadata.http_response_code = net::URLFetcher::RESPONSE_CODE_INVALID;
+  EXPECT_CALL(*this, OnImageDataFetched(std::string(), expected_metadata))
+      .Times(1);

[Marc Treib, 2017/03/27 13:16:42]

nit: ".Times(1)" is a noop

[fhorschig, 2017/03/27 14:33:43]

Done.

+
+  // Get and configure the TestURLFetcher.
+  net::TestURLFetcher* test_url_fetcher = fetcher_factory_.GetFetcherByID(0);
+  ASSERT_NE(nullptr, test_url_fetcher);
+
+  // Create a completely valid response to make sure that the answer isn't
+  // accidentally invalid but intentionally.
+  test_url_fetcher->set_status(
+      net::URLRequestStatus(net::URLRequestStatus::SUCCESS, net::OK));
+  test_url_fetcher->SetResponseString(kURLResponseData);

[Marc Treib, 2017/03/27 13:16:42]

nit: add a comment saying that this isn't actually used here?

[fhorschig, 2017/03/27 14:33:43]

Do you mean this particular line? Because nothing I set here is
used. (Adjusted the comment above a little)

+  test_url_fetcher->set_response_code(net::HTTP_OK);
+
+  std::string raw_header =
+      "HTTP/1.1 200 OK\n"
+      "Content-type: image/png\n\n";
+  std::replace(raw_header.begin(), raw_header.end(), '\n', '\0');
+  scoped_refptr<net::HttpResponseHeaders> headers(
+      new net::HttpResponseHeaders(raw_header));
+  test_url_fetcher->set_response_headers(headers);
+

[Marc Treib, 2017/03/27 13:16:42]

Maybe first call OnURLFetchDownloadProgress with a value below the limit, and
make sure the fetcher is still there?

[fhorschig, 2017/03/27 14:33:43]

Good idea. I introduced some edge cases.

+  // Call the URLFetcher delegate to continue the test.

[Marc Treib, 2017/03/27 13:16:43]

nit: IMO this comment isn't really helpful.

[fhorschig, 2017/03/27 14:33:43]

Gone.

+  test_url_fetcher->delegate()->OnURLFetchDownloadProgress(
+      test_url_fetcher,
+      /*current=*/2 * max_download_bytes,    // Bytes received up to the call.
+      /*total=*/-1,                          // not determined
+      /*current_network_bytes=*/24 * 1024);  // not relevant
+  // The fetcher has to be deleted for the request to be canceled.

[Marc Treib, 2017/03/27 13:16:42]

"The URL fetch should have been canceled" ?

[fhorschig, 2017/03/27 14:33:43]

Done.

+  EXPECT_EQ(nullptr, fetcher_factory_.GetFetcherByID(0));
+}
+
 }  // namespace image_fetcher