Adding network fetcher for fetching images.

Change-Id: If4d19d6ba2367f7770c4e77c8f885767171bebe3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2350148
Commit-Queue: Jonathan Freed <freedjm@chromium.org>
Reviewed-by: Dan H <harringtond@chromium.org>
Reviewed-by: Ian Wells <iwells@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797494}

components/feed/core/v2/BUILD.gn

@@ -23,6 +23,8 @@ source_set("feed_core_v2") {
     "feed_store.h",
     "feed_stream.cc",
     "feed_stream.h",
+    "image_fetcher.cc",
+    "image_fetcher.h",
     "metrics_reporter.cc",
     "metrics_reporter.h",
     "offline_page_spy.cc",
@@ -102,6 +104,7 @@ source_set("core_unit_tests") {
     "feed_network_impl_unittest.cc",
     "feed_store_unittest.cc",
     "feed_stream_unittest.cc",
+    "image_fetcher_unittest.cc",
     "metrics_reporter_unittest.cc",
     "proto_util_unittest.cc",
     "protocol_translator_unittest.cc",

components/feed/core/v2/feed_stream.cc

@@ -22,6 +22,7 @@
 #include "components/feed/core/v2/enums.h"
 #include "components/feed/core/v2/feed_network.h"
 #include "components/feed/core/v2/feed_store.h"
+#include "components/feed/core/v2/image_fetcher.h"
 #include "components/feed/core/v2/metrics_reporter.h"
 #include "components/feed/core/v2/offline_page_spy.h"
 #include "components/feed/core/v2/prefs.h"
@@ -131,6 +132,7 @@ FeedStream::FeedStream(RefreshTaskScheduler* refresh_task_scheduler,
                        Delegate* delegate,
                        PrefService* profile_prefs,
                        FeedNetwork* feed_network,
+                       ImageFetcher* image_fetcher,
                        FeedStore* feed_store,
                        offline_pages::PrefetchService* prefetch_service,
                        offline_pages::OfflinePageModel* offline_page_model,
@@ -143,6 +145,7 @@ FeedStream::FeedStream(RefreshTaskScheduler* refresh_task_scheduler,
       delegate_(delegate),
       profile_prefs_(profile_prefs),
       feed_network_(feed_network),
+      image_fetcher_(image_fetcher),
       store_(feed_store),
       clock_(clock),
       tick_clock_(tick_clock),
@@ -596,6 +599,12 @@ void FeedStream::FinishClearAll() {
   delegate_->ClearAll();
 }
 
+void FeedStream::FetchImage(
+    const GURL& url,
+    base::OnceCallback<void(std::unique_ptr<std::string>)> callback) {
+  image_fetcher_->Fetch(url, std::move(callback));
+}
+
 void FeedStream::UploadAction(
     feedwire::FeedAction action,
     bool upload_now,

components/feed/core/v2/feed_stream.h

@@ -43,6 +43,7 @@ class PrefetchService;
 namespace feed {
 class FeedNetwork;
 class FeedStore;
+class ImageFetcher;
 class MetricsReporter;
 class OfflinePageSpy;
 class RefreshTaskScheduler;
@@ -102,6 +103,7 @@ class FeedStream : public FeedStreamApi,
              Delegate* delegate,
              PrefService* profile_prefs,
              FeedNetwork* feed_network,
+             ImageFetcher* image_fetcher,
              FeedStore* feed_store,
              offline_pages::PrefetchService* prefetch_service,
              offline_pages::OfflinePageModel* offline_page_model,
@@ -124,6 +126,9 @@ class FeedStream : public FeedStreamApi,
   bool IsArticlesListVisible() override;
   std::string GetClientInstanceId() override;
   void ExecuteRefreshTask() override;
+  void FetchImage(
+      const GURL& url,
+      base::OnceCallback<void(std::unique_ptr<std::string>)> callback) override;
   void LoadMore(SurfaceId surface_id,
                 base::OnceCallback<void(bool)> callback) override;
   void ExecuteOperations(
@@ -291,6 +296,7 @@ class FeedStream : public FeedStreamApi,
   Delegate* delegate_;
   PrefService* profile_prefs_;  // May be null.
   FeedNetwork* feed_network_;
+  ImageFetcher* image_fetcher_;
   FeedStore* store_;
   const base::Clock* clock_;
   const base::TickClock* tick_clock_;

components/feed/core/v2/feed_stream_unittest.cc

@@ -34,6 +34,7 @@
 #include "components/feed/core/shared_prefs/pref_names.h"
 #include "components/feed/core/v2/config.h"
 #include "components/feed/core/v2/feed_network.h"
+#include "components/feed/core/v2/image_fetcher.h"
 #include "components/feed/core/v2/metrics_reporter.h"
 #include "components/feed/core/v2/protocol_translator.h"
 #include "components/feed/core/v2/refresh_task_scheduler.h"
@@ -50,6 +51,9 @@
 #include "components/offline_pages/core/stub_offline_page_model.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/testing_pref_service.h"
+#include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
+#include "services/network/test/test_url_loader_factory.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace feed {
@@ -254,6 +258,20 @@ class TestSurface : public FeedStream::SurfaceInterface {
   std::map<std::string, std::string> data_store_entries_;
 };
 
+class TestImageFetcher : public ImageFetcher {
+ public:
+  explicit TestImageFetcher(
+      scoped_refptr<::network::SharedURLLoaderFactory> url_loader_factory)
+      : ImageFetcher(url_loader_factory) {}
+  void Fetch(const GURL& url,
+             base::OnceCallback<void(std::unique_ptr<std::string>)> callback)
+      override {
+    // Emulate a response.
+    auto response = std::make_unique<std::string>("dummyresponse");
+    std::move(callback).Run(std::move(response));
+  }
+};
+
 class TestFeedNetwork : public FeedNetwork {
  public:
   // FeedNetwork implementation.
@@ -526,6 +544,12 @@ class FeedStreamTest : public testing::Test, public FeedStream::Delegate {
     metrics_reporter_ = std::make_unique<TestMetricsReporter>(
         task_environment_.GetMockTickClock(), &profile_prefs_);
 
+    shared_url_loader_factory_ =
+        base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
+            &test_factory_);
+    image_fetcher_ =
+        std::make_unique<TestImageFetcher>(shared_url_loader_factory_);
+
     CHECK_EQ(kTestTimeEpoch, task_environment_.GetMockClock()->Now());
     CreateStream();
   }
@@ -561,9 +585,9 @@ class FeedStreamTest : public testing::Test, public FeedStream::Delegate {
     chrome_info.version = base::Version({99, 1, 9911, 2});
     stream_ = std::make_unique<FeedStream>(
         &refresh_scheduler_, metrics_reporter_.get(), this, &profile_prefs_,
-        &network_, store_.get(), &prefetch_service_, &offline_page_model_,
-        task_environment_.GetMockClock(), task_environment_.GetMockTickClock(),
-        chrome_info);
+        &network_, image_fetcher_.get(), store_.get(), &prefetch_service_,
+        &offline_page_model_, task_environment_.GetMockClock(),
+        task_environment_.GetMockTickClock(), chrome_info);
 
     WaitForIdleTaskQueue();  // Wait for any initialization.
     stream_->SetWireResponseTranslatorForTesting(&response_translator_);
@@ -624,6 +648,9 @@ class FeedStreamTest : public testing::Test, public FeedStream::Delegate {
   std::unique_ptr<TestMetricsReporter> metrics_reporter_;
   TestFeedNetwork network_;
   TestWireResponseTranslator response_translator_;
+  std::unique_ptr<TestImageFetcher> image_fetcher_;
+  network::TestURLLoaderFactory test_factory_;
+  scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory_;
 
   std::unique_ptr<FeedStore> store_ = std::make_unique<FeedStore>(
       leveldb_proto::ProtoDatabaseProvider::GetUniqueDB<feedstore::Record>(
@@ -850,6 +877,13 @@ TEST_F(FeedStreamTest, DetachSurface) {
   EXPECT_FALSE(surface.update);
 }
 
+TEST_F(FeedStreamTest, FetchImage) {
+  CallbackReceiver<std::unique_ptr<std::string>> receiver;
+  stream_->FetchImage(GURL("https://example.com"), receiver.Bind());
+
+  EXPECT_EQ("dummyresponse", **receiver.GetResult());
+}
+
 TEST_F(FeedStreamTest, LoadFromNetwork) {
   stream_->GetMetadata()->SetConsistencyToken("token");
 

components/feed/core/v2/image_fetcher.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/feed/core/v2/image_fetcher.h"
+
+#include "net/http/http_request_headers.h"
+#include "services/network/public/cpp/shared_url_loader_factory.h"
+#include "services/network/public/cpp/simple_url_loader.h"
+
+namespace feed {
+
+ImageFetcher::ImageFetcher(
+    scoped_refptr<::network::SharedURLLoaderFactory> url_loader_factory)
+    : url_loader_factory_(url_loader_factory) {}
+
+ImageFetcher::~ImageFetcher() = default;
+
+void ImageFetcher::Fetch(const GURL& url, ImageCallback callback) {
+  net::NetworkTrafficAnnotationTag traffic_annotation =
+      net::DefineNetworkTrafficAnnotation("interest_feedv2_image_send", R"(
+        semantics {
+          sender: "Feed Library"
+          description: "Images for articles in the feed."
+          trigger: "Triggered when viewing the feed on the NTP."
+          data: "Request for an image associated with a feed article."
+          destination: GOOGLE_OWNED_SERVICE
+        }
+        policy {
+          setting: "This can be disabled from the New Tab Page by collapsing "
+          "the articles section."
+          chrome_policy {
+            NTPContentSuggestionsEnabled {
+              policy_options {mode: MANDATORY}
+              NTPContentSuggestionsEnabled: false
+            }
+          }
+        })");
+  auto resource_request = std::make_unique<::network::ResourceRequest>();
+  resource_request->url = url;
+  resource_request->method = net::HttpRequestHeaders::kGetMethod;
+  resource_request->credentials_mode = network::mojom::CredentialsMode::kOmit;
+
+  auto simple_loader = network::SimpleURLLoader::Create(
+      std::move(resource_request), traffic_annotation);
+  simple_loader->DownloadToString(
+      url_loader_factory_.get(),
+      base::BindOnce(&ImageFetcher::OnFetchComplete, weak_factory_.GetWeakPtr(),
+                     std::move(simple_loader), std::move(callback)),
+      network::SimpleURLLoader::kMaxBoundedStringDownloadSize);
+}
+
+void ImageFetcher::OnFetchComplete(
+    std::unique_ptr<network::SimpleURLLoader> simple_loader,
+    ImageCallback callback,
+    std::unique_ptr<std::string> response_data) {
+  std::move(callback).Run(std::move(response_data));
+}
+
+}  // namespace feed
\ No newline at end of file

components/feed/core/v2/image_fetcher.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef COMPONENTS_FEED_CORE_V2_IMAGE_FETCHER_H_
+#define COMPONENTS_FEED_CORE_V2_IMAGE_FETCHER_H_
+
+#include "base/callback.h"
+#include "base/memory/scoped_refptr.h"
+#include "base/memory/weak_ptr.h"
+#include "url/gurl.h"
+
+namespace network {
+class SharedURLLoaderFactory;
+class SimpleURLLoader;
+}  // namespace network
+
+namespace feed {
+
+// Fetcher object to retrieve an image resource from a URL.
+class ImageFetcher {
+ public:
+  using ImageCallback = base::OnceCallback<void(std::unique_ptr<std::string>)>;
+  explicit ImageFetcher(
+      scoped_refptr<::network::SharedURLLoaderFactory> url_loader_factory);
+  virtual ~ImageFetcher();
+  ImageFetcher(const ImageFetcher&) = delete;
+  ImageFetcher& operator=(const ImageFetcher&) = delete;
+
+  virtual void Fetch(const GURL& url, ImageCallback callback);
+
+ private:
+  // Called when fetch request completes.
+  void OnFetchComplete(std::unique_ptr<network::SimpleURLLoader> simple_loader,
+                       ImageCallback callback,
+                       std::unique_ptr<std::string> response_data);
+
+  const scoped_refptr<::network::SharedURLLoaderFactory> url_loader_factory_;
+  base::WeakPtrFactory<ImageFetcher> weak_factory_{this};
+};
+
+}  // namespace feed
+
+#endif  // COMPONENTS_FEED_CORE_V2_IMAGE_FETCHER_H_
\ No newline at end of file

components/feed/core/v2/image_fetcher_unittest.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/feed/core/v2/image_fetcher.h"
+
+#include <memory>
+#include <utility>
+
+#include "base/bind.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/strings/string_split.h"
+#include "base/test/bind_test_util.h"
+#include "base/test/task_environment.h"
+#include "components/feed/core/v2/test/callback_receiver.h"
+#include "net/http/http_response_headers.h"
+#include "net/http/http_status_code.h"
+#include "net/http/http_util.h"
+#include "net/traffic_annotation/network_traffic_annotation_test_helper.h"
+#include "services/network/public/cpp/url_loader_completion_status.h"
+#include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
+#include "services/network/test/test_url_loader_factory.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/protobuf/src/google/protobuf/io/coded_stream.h"
+#include "third_party/protobuf/src/google/protobuf/io/zero_copy_stream_impl_lite.h"
+#include "url/gurl.h"
+
+using ::testing::HasSubstr;
+
+namespace feed {
+namespace {
+
+class ImageFetcherTest : public testing::Test {
+ public:
+  ImageFetcherTest() = default;
+  ImageFetcherTest(ImageFetcherTest&) = delete;
+  ImageFetcherTest& operator=(const ImageFetcherTest&) = delete;
+  ~ImageFetcherTest() override = default;
+
+  void SetUp() override {
+    shared_url_loader_factory_ =
+        base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
+            &test_factory_);
+    image_fetcher_ = std::make_unique<ImageFetcher>(shared_url_loader_factory_);
+  }
+
+  ImageFetcher* image_fetcher() { return image_fetcher_.get(); }
+
+  void Respond(const GURL& url,
+               const std::string& response_string,
+               net::HttpStatusCode code = net::HTTP_OK,
+               network::URLLoaderCompletionStatus status =
+                   network::URLLoaderCompletionStatus()) {
+    auto head = network::mojom::URLResponseHead::New();
+    if (code >= 0) {
+      head->headers = base::MakeRefCounted<net::HttpResponseHeaders>(
+          "HTTP/1.1 " + base::NumberToString(code));
+      status.decoded_body_length = response_string.length();
+    }
+
+    test_factory_.AddResponse(url, std::move(head), response_string, status);
+  }
+
+  network::ResourceRequest RespondToRequest(const std::string& response_string,
+                                            net::HttpStatusCode code) {
+    task_environment_.RunUntilIdle();
+    network::TestURLLoaderFactory::PendingRequest* pending_request =
+        test_factory_.GetPendingRequest(0);
+    CHECK(pending_request);
+    network::ResourceRequest resource_request = pending_request->request;
+    Respond(pending_request->request.url, response_string, code);
+    task_environment_.FastForwardUntilNoTasksRemain();
+    return resource_request;
+  }
+
+ private:
+  std::unique_ptr<ImageFetcher> image_fetcher_;
+  network::TestURLLoaderFactory test_factory_;
+  scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory_;
+  base::test::TaskEnvironment task_environment_{
+      base::test::TaskEnvironment::TimeSource::MOCK_TIME};
+};
+
+TEST_F(ImageFetcherTest, SendRequestSendsValidRequest) {
+  CallbackReceiver<std::unique_ptr<std::string>> receiver;
+  image_fetcher()->Fetch(GURL("https://example.com"), receiver.Bind());
+  network::ResourceRequest resource_request =
+      RespondToRequest("", net::HTTP_OK);
+
+  EXPECT_EQ(GURL("https://example.com"), resource_request.url);
+  EXPECT_EQ("GET", resource_request.method);
+}
+
+TEST_F(ImageFetcherTest, SendRequestValidResponse) {
+  CallbackReceiver<std::unique_ptr<std::string>> receiver;
+  image_fetcher()->Fetch(GURL("https://example.com"), receiver.Bind());
+  RespondToRequest("example_response", net::HTTP_OK);
+
+  ASSERT_TRUE(receiver.GetResult());
+  EXPECT_THAT(**receiver.GetResult(), HasSubstr("example_response"));
+}
+
+TEST_F(ImageFetcherTest, SendSequentialRequestsValidResponses) {
+  CallbackReceiver<std::unique_ptr<std::string>> receiver1;
+  image_fetcher()->Fetch(GURL("https://example1.com"), receiver1.Bind());
+  RespondToRequest("example1_response", net::HTTP_OK);
+
+  CallbackReceiver<std::unique_ptr<std::string>> receiver2;
+  image_fetcher()->Fetch(GURL("https://example2.com"), receiver2.Bind());
+  RespondToRequest("example2_response", net::HTTP_OK);
+
+  ASSERT_TRUE(receiver1.GetResult());
+  EXPECT_THAT(**receiver1.GetResult(), HasSubstr("example1_response"));
+  ASSERT_TRUE(receiver2.GetResult());
+  EXPECT_THAT(**receiver2.GetResult(), HasSubstr("example2_response"));
+}
+
+TEST_F(ImageFetcherTest, SendParallelRequestsValidResponses) {
+  CallbackReceiver<std::unique_ptr<std::string>> receiver1;
+  image_fetcher()->Fetch(GURL("https://example1.com"), receiver1.Bind());
+  CallbackReceiver<std::unique_ptr<std::string>> receiver2;
+  image_fetcher()->Fetch(GURL("https://example2.com"), receiver2.Bind());
+
+  RespondToRequest("example1_response", net::HTTP_OK);
+  RespondToRequest("example2_response", net::HTTP_OK);
+
+  ASSERT_TRUE(receiver1.GetResult());
+  EXPECT_THAT(**receiver1.GetResult(), HasSubstr("example1_response"));
+  ASSERT_TRUE(receiver2.GetResult());
+  EXPECT_THAT(**receiver2.GetResult(), HasSubstr("example2_response"));
+}
+
+}  // namespace
+}  // namespace feed

components/feed/core/v2/public/feed_service.cc

@@ -13,6 +13,7 @@
 #include "components/feed/core/v2/feed_network_impl.h"
 #include "components/feed/core/v2/feed_store.h"
 #include "components/feed/core/v2/feed_stream.h"
+#include "components/feed/core/v2/image_fetcher.h"
 #include "components/feed/core/v2/metrics_reporter.h"
 #include "components/feed/core/v2/refresh_task_scheduler.h"
 #include "components/feed/feed_feature_list.h"
@@ -172,13 +173,15 @@ FeedService::FeedService(
   feed_network_ = std::make_unique<FeedNetworkImpl>(
       network_delegate_.get(), identity_manager, api_key, url_loader_factory,
       base::DefaultTickClock::GetInstance(), profile_prefs);
+  image_fetcher_ = std::make_unique<ImageFetcher>(url_loader_factory);
   store_ = std::make_unique<FeedStore>(std::move(database));
 
   stream_ = std::make_unique<FeedStream>(
       refresh_task_scheduler_.get(), metrics_reporter_.get(),
-      stream_delegate_.get(), profile_prefs, feed_network_.get(), store_.get(),
-      prefetch_service, offline_page_model, base::DefaultClock::GetInstance(),
-      base::DefaultTickClock::GetInstance(), chrome_info);
+      stream_delegate_.get(), profile_prefs, feed_network_.get(),
+      image_fetcher_.get(), store_.get(), prefetch_service, offline_page_model,
+      base::DefaultClock::GetInstance(), base::DefaultTickClock::GetInstance(),
+      chrome_info);
 
   history_observer_ = std::make_unique<HistoryObserverImpl>(
       history_service, static_cast<FeedStream*>(stream_.get()));

components/feed/core/v2/public/feed_service.h

@@ -48,6 +48,7 @@ class MetricsReporter;
 class FeedNetwork;
 class FeedStore;
 class FeedStream;
+class ImageFetcher;
 
 namespace internal {
 bool ShouldClearFeed(const history::DeletionInfo& deletion_info);
@@ -117,6 +118,7 @@ class FeedService : public KeyedService {
   std::unique_ptr<MetricsReporter> metrics_reporter_;
   std::unique_ptr<NetworkDelegateImpl> network_delegate_;
   std::unique_ptr<FeedNetwork> feed_network_;
+  std::unique_ptr<ImageFetcher> image_fetcher_;
   std::unique_ptr<FeedStore> store_;
   std::unique_ptr<RefreshTaskScheduler> refresh_task_scheduler_;
   std::unique_ptr<HistoryObserverImpl> history_observer_;

components/feed/core/v2/public/feed_stream_api.h

@@ -13,6 +13,7 @@
 #include "base/strings/string_piece_forward.h"
 #include "base/time/time.h"
 #include "components/feed/core/v2/public/types.h"
+#include "url/gurl.h"
 
 namespace feedui {
 class StreamUpdate;
@@ -71,6 +72,12 @@ class FeedStreamApi {
   virtual void LoadMore(SurfaceId surface,
                         base::OnceCallback<void(bool)> callback) = 0;
 
+  // Request to fetch and image for use in the feed. Calls |callback|
+  // with the network response when complete.
+  virtual void FetchImage(
+      const GURL& url,
+      base::OnceCallback<void(std::unique_ptr<std::string>)> callback) = 0;
+
   // Apply |operations| to the stream model. Does nothing if the model is not
   // yet loaded.
   virtual void ExecuteOperations(