Skip to content

Commit

Permalink
Reland "Adding network fetcher for fetching images."
Browse files Browse the repository at this point in the history
This is a reland of 7be16f9

Original change's description:
> 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}

Bug: 1115855
Change-Id: I6376cf4fb830147a093bf5a8ff301b2939c9367f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2354452
Commit-Queue: Jonathan Freed <freedjm@chromium.org>
Reviewed-by: Dan H <harringtond@chromium.org>
Reviewed-by: Ian Wells <iwells@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Auto-Submit: Jonathan Freed <freedjm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797899}
  • Loading branch information
Jonathan Freed authored and Commit Bot committed Aug 13, 2020
1 parent ca02dc4 commit 791b266
Show file tree
Hide file tree
Showing 12 changed files with 313 additions and 6 deletions.
3 changes: 3 additions & 0 deletions components/feed/core/v2/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
9 changes: 9 additions & 0 deletions components/feed/core/v2/feed_stream.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
Expand All @@ -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),
Expand Down Expand Up @@ -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,
Expand Down
6 changes: 6 additions & 0 deletions components/feed/core/v2/feed_stream.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ class PrefetchService;
namespace feed {
class FeedNetwork;
class FeedStore;
class ImageFetcher;
class MetricsReporter;
class OfflinePageSpy;
class RefreshTaskScheduler;
Expand Down Expand Up @@ -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,
Expand All @@ -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(
Expand Down Expand Up @@ -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_;
Expand Down
40 changes: 37 additions & 3 deletions components/feed/core/v2/feed_stream_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,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"
Expand All @@ -49,6 +50,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 {
Expand Down Expand Up @@ -253,6 +257,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.
Expand Down Expand Up @@ -522,6 +540,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();
}
Expand Down Expand Up @@ -557,9 +581,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_);
Expand Down Expand Up @@ -620,6 +644,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>(
Expand Down Expand Up @@ -846,6 +873,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");

Expand Down
62 changes: 62 additions & 0 deletions components/feed/core/v2/image_fetcher.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// 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 containing an ID for the image and "
"device specs (e.g. screen size) for resizing images."
destination: GOOGLE_OWNED_SERVICE
}
policy {
cookies_allowed: NO
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
44 changes: 44 additions & 0 deletions components/feed/core/v2/image_fetcher.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// 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_
Loading

0 comments on commit 791b266

Please sign in to comment.