Commit ccb95b1f authored by Antonio Gomes's avatar Antonio Gomes Committed by Commit Bot

Rewrite URLDownloaderTest::SingleDownloadPDF to exercise the new URLLoader path

In [1], iOS' URLDownloader switched from URLFetcher to SimpleURLLoader.
However, it was observed during the migration that its existing unit test
SingleDownloadPDF does not actually exercise the actual fetch/load code path.

This CL fixes SingleDownloadPDF so that it actually loads a testing PDF file
through SimpleURLLoader and URLLoaderFactory machinery.

[1] https://crrev.com/c/1234635

BUG=None

Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Change-Id: I6b3006f8e021bb97c8d839307c741f7071708683
Reviewed-on: https://chromium-review.googlesource.com/1236855Reviewed-by: default avatarOlivier Robin <olivierrobin@chromium.org>
Commit-Queue: Antonio Gomes <tonikitoo@igalia.com>
Cr-Commit-Position: refs/heads/master@{#593176}
parent eff49dd8
...@@ -98,6 +98,7 @@ source_set("unit_tests") { ...@@ -98,6 +98,7 @@ source_set("unit_tests") {
"//ios/web/public/test", "//ios/web/public/test",
"//ios/web/public/test/fakes", "//ios/web/public/test/fakes",
"//net", "//net",
"//services/network:test_support",
"//testing/gtest", "//testing/gtest",
"//url", "//url",
] ]
......
...@@ -16,7 +16,9 @@ ...@@ -16,7 +16,9 @@
#include "ios/chrome/browser/dom_distiller/distiller_viewer.h" #include "ios/chrome/browser/dom_distiller/distiller_viewer.h"
#include "ios/chrome/browser/reading_list/offline_url_utils.h" #include "ios/chrome/browser/reading_list/offline_url_utils.h"
#include "ios/chrome/browser/reading_list/reading_list_distiller_page.h" #include "ios/chrome/browser/reading_list/reading_list_distiller_page.h"
#include "services/network/public/cpp/shared_url_loader_factory.h" #include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
#include "services/network/test/test_url_loader_factory.h"
#include "services/network/test/test_utils.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "testing/platform_test.h" #include "testing/platform_test.h"
...@@ -65,12 +67,14 @@ void RemoveOfflineFilesDirectory(base::FilePath base_directory) { ...@@ -65,12 +67,14 @@ void RemoveOfflineFilesDirectory(base::FilePath base_directory) {
class MockURLDownloader : public URLDownloader { class MockURLDownloader : public URLDownloader {
public: public:
MockURLDownloader(base::FilePath path) MockURLDownloader(
base::FilePath path,
scoped_refptr<network::SharedURLLoaderFactory> url_loader_factory)
: URLDownloader(nullptr, : URLDownloader(nullptr,
nullptr, nullptr,
nullptr, nullptr,
path, path,
nullptr, std::move(url_loader_factory),
base::Bind(&MockURLDownloader::OnEndDownload, base::Bind(&MockURLDownloader::OnEndDownload,
base::Unretained(this)), base::Unretained(this)),
base::Bind(&MockURLDownloader::OnEndRemove, base::Bind(&MockURLDownloader::OnEndRemove,
...@@ -82,11 +86,13 @@ class MockURLDownloader : public URLDownloader { ...@@ -82,11 +86,13 @@ class MockURLDownloader : public URLDownloader {
removed_files_.clear(); removed_files_.clear();
} }
bool CheckExistenceOfOfflineURLPagePath(const GURL& url) { bool CheckExistenceOfOfflineURLPagePath(
const GURL& url,
reading_list::OfflineFileType file_type =
reading_list::OFFLINE_TYPE_HTML) {
return base::PathExists( return base::PathExists(
reading_list::OfflineURLAbsolutePathFromRelativePath( reading_list::OfflineURLAbsolutePathFromRelativePath(
base_directory_, reading_list::OfflinePagePath( base_directory_, reading_list::OfflinePagePath(url, file_type)));
url, reading_list::OFFLINE_TYPE_HTML)));
} }
void FakeWorking() { working_ = true; } void FakeWorking() { working_ = true; }
...@@ -101,7 +107,6 @@ class MockURLDownloader : public URLDownloader { ...@@ -101,7 +107,6 @@ class MockURLDownloader : public URLDownloader {
GURL redirect_url_; GURL redirect_url_;
std::string mime_type_; std::string mime_type_;
std::string html_; std::string html_;
bool fetching_pdf_;
private: private:
void DownloadURL(const GURL& url, bool offline_url_exists) override { void DownloadURL(const GURL& url, bool offline_url_exists) override {
...@@ -118,8 +123,6 @@ class MockURLDownloader : public URLDownloader { ...@@ -118,8 +123,6 @@ class MockURLDownloader : public URLDownloader {
this, html_, redirect_url_, mime_type_)); this, html_, redirect_url_, mime_type_));
} }
void FetchPDFFile() override { fetching_pdf_ = true; }
void OnEndDownload(const GURL& url, void OnEndDownload(const GURL& url,
const GURL& distilled_url, const GURL& distilled_url,
SuccessState success, SuccessState success,
...@@ -140,11 +143,15 @@ class MockURLDownloader : public URLDownloader { ...@@ -140,11 +143,15 @@ class MockURLDownloader : public URLDownloader {
namespace { namespace {
class URLDownloaderTest : public PlatformTest { class URLDownloaderTest : public PlatformTest {
public: public:
URLDownloaderTest() { URLDownloaderTest()
: test_shared_url_loader_factory_(
base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
&test_url_loader_factory_)) {
base::FilePath data_dir; base::FilePath data_dir;
base::PathService::Get(ios::DIR_USER_DATA, &data_dir); base::PathService::Get(ios::DIR_USER_DATA, &data_dir);
RemoveOfflineFilesDirectory(data_dir); RemoveOfflineFilesDirectory(data_dir);
downloader_.reset(new MockURLDownloader(data_dir)); downloader_.reset(
new MockURLDownloader(data_dir, test_shared_url_loader_factory_));
} }
~URLDownloaderTest() override {} ~URLDownloaderTest() override {}
...@@ -158,6 +165,11 @@ class URLDownloaderTest : public PlatformTest { ...@@ -158,6 +165,11 @@ class URLDownloaderTest : public PlatformTest {
protected: protected:
base::test::ScopedTaskEnvironment task_environment_; base::test::ScopedTaskEnvironment task_environment_;
network::TestURLLoaderFactory test_url_loader_factory_;
scoped_refptr<network::WeakWrapperSharedURLLoaderFactory>
test_shared_url_loader_factory_;
std::unique_ptr<MockURLDownloader> downloader_; std::unique_ptr<MockURLDownloader> downloader_;
private: private:
...@@ -198,7 +210,8 @@ TEST_F(URLDownloaderTest, SingleDownloadRedirect) { ...@@ -198,7 +210,8 @@ TEST_F(URLDownloaderTest, SingleDownloadRedirect) {
TEST_F(URLDownloaderTest, SingleDownloadPDF) { TEST_F(URLDownloaderTest, SingleDownloadPDF) {
GURL url = GURL("http://test.com"); GURL url = GURL("http://test.com");
ASSERT_FALSE(downloader_->CheckExistenceOfOfflineURLPagePath(url)); ASSERT_FALSE(downloader_->CheckExistenceOfOfflineURLPagePath(
url, reading_list::OFFLINE_TYPE_PDF));
ASSERT_EQ(0ul, downloader_->downloaded_files_.size()); ASSERT_EQ(0ul, downloader_->downloaded_files_.size());
ASSERT_EQ(0ul, downloader_->removed_files_.size()); ASSERT_EQ(0ul, downloader_->removed_files_.size());
...@@ -207,10 +220,20 @@ TEST_F(URLDownloaderTest, SingleDownloadPDF) { ...@@ -207,10 +220,20 @@ TEST_F(URLDownloaderTest, SingleDownloadPDF) {
downloader_->DownloadOfflineURL(url); downloader_->DownloadOfflineURL(url);
task_environment_.RunUntilIdle();
auto* pending_request = test_url_loader_factory_.GetPendingRequest(0);
auto response_info = network::CreateResourceResponseHead(net::HTTP_OK);
response_info.mime_type = "application/pdf";
test_url_loader_factory_.SimulateResponseForPendingRequest(
pending_request->request.url, network::URLLoaderCompletionStatus(net::OK),
response_info, std::string("123456789"));
// Wait for all asynchronous tasks to complete. // Wait for all asynchronous tasks to complete.
task_environment_.RunUntilIdle(); task_environment_.RunUntilIdle();
EXPECT_FALSE(downloader_->CheckExistenceOfOfflineURLPagePath(url)); EXPECT_TRUE(downloader_->CheckExistenceOfOfflineURLPagePath(
url, reading_list::OFFLINE_TYPE_PDF));
} }
TEST_F(URLDownloaderTest, DownloadAndRemove) { TEST_F(URLDownloaderTest, DownloadAndRemove) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment