Commit 06b5e1b6 authored by Steven Bennetts's avatar Steven Bennetts Committed by Commit Bot

Elim GetBlockingPool from ios_chrome_large_icon_service_factory

Bug: 750741
Change-Id: I2e9d55b9e08901d9ac80bd91634936c99831b5b7
Reviewed-on: https://chromium-review.googlesource.com/594640
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: default avatarJan Krcal <jkrcal@chromium.org>
Reviewed-by: default avatarRohit Rao (ping after 24h) <rohitrao@chromium.org>
Reviewed-by: default avatarGabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491807}
parent 850e8aa5
......@@ -51,9 +51,6 @@ KeyedService* LargeIconServiceFactory::BuildServiceInstanceFor(
ServiceAccessType::EXPLICIT_ACCESS);
return new favicon::LargeIconService(
favicon_service,
base::CreateTaskRunnerWithTraits(
{base::MayBlock(), base::TaskPriority::BACKGROUND,
base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN}),
base::MakeUnique<image_fetcher::ImageFetcherImpl>(
base::MakeUnique<suggestions::ImageDecoderImpl>(),
profile->GetRequestContext()));
......
......@@ -17,6 +17,7 @@
#include "base/metrics/histogram_macros.h"
#include "base/strings/stringprintf.h"
#include "base/task_runner.h"
#include "base/task_scheduler/post_task.h"
#include "base/threading/sequenced_worker_pool.h"
#include "base/threading/thread_task_runner_handle.h"
#include "components/data_use_measurement/core/data_use_user_data.h"
......@@ -182,7 +183,6 @@ class LargeIconWorker : public base::RefCountedThreadSafe<LargeIconWorker> {
int desired_size_in_pixel,
favicon_base::LargeIconCallback raw_bitmap_callback,
favicon_base::LargeIconImageCallback image_callback,
scoped_refptr<base::TaskRunner> background_task_runner,
base::CancelableTaskTracker* tracker);
// Must run on the owner (UI) thread in production.
......@@ -221,13 +221,14 @@ LargeIconWorker::LargeIconWorker(
int desired_size_in_pixel,
favicon_base::LargeIconCallback raw_bitmap_callback,
favicon_base::LargeIconImageCallback image_callback,
scoped_refptr<base::TaskRunner> background_task_runner,
base::CancelableTaskTracker* tracker)
: min_source_size_in_pixel_(min_source_size_in_pixel),
desired_size_in_pixel_(desired_size_in_pixel),
raw_bitmap_callback_(raw_bitmap_callback),
image_callback_(image_callback),
background_task_runner_(background_task_runner),
background_task_runner_(base::CreateTaskRunnerWithTraits(
{base::MayBlock(), base::TaskPriority::BACKGROUND,
base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN})),
tracker_(tracker),
fallback_icon_style_(
base::MakeUnique<favicon_base::FallbackIconStyle>()) {}
......@@ -326,10 +327,8 @@ void OnFetchIconFromGoogleServerComplete(
LargeIconService::LargeIconService(
FaviconService* favicon_service,
const scoped_refptr<base::TaskRunner>& background_task_runner,
std::unique_ptr<image_fetcher::ImageFetcher> image_fetcher)
: favicon_service_(favicon_service),
background_task_runner_(background_task_runner),
image_fetcher_(std::move(image_fetcher)) {
large_icon_types_.push_back(favicon_base::IconType::WEB_MANIFEST_ICON);
large_icon_types_.push_back(favicon_base::IconType::FAVICON);
......@@ -433,9 +432,9 @@ LargeIconService::GetLargeIconOrFallbackStyleImpl(
DCHECK_LE(1, min_source_size_in_pixel);
DCHECK_LE(0, desired_size_in_pixel);
scoped_refptr<LargeIconWorker> worker = new LargeIconWorker(
min_source_size_in_pixel, desired_size_in_pixel, raw_bitmap_callback,
image_callback, background_task_runner_, tracker);
scoped_refptr<LargeIconWorker> worker =
new LargeIconWorker(min_source_size_in_pixel, desired_size_in_pixel,
raw_bitmap_callback, image_callback, tracker);
int max_size_in_pixel =
std::max(desired_size_in_pixel, min_source_size_in_pixel);
......
......@@ -16,10 +16,6 @@
class GURL;
namespace base {
class TaskRunner;
}
namespace image_fetcher {
class ImageFetcher;
}
......@@ -34,7 +30,6 @@ class LargeIconService : public KeyedService {
public:
LargeIconService(
FaviconService* favicon_service,
const scoped_refptr<base::TaskRunner>& background_task_runner,
std::unique_ptr<image_fetcher::ImageFetcher> image_fetcher);
~LargeIconService() override;
......@@ -120,7 +115,6 @@ class LargeIconService : public KeyedService {
base::CancelableTaskTracker* tracker);
FaviconService* favicon_service_;
scoped_refptr<base::TaskRunner> background_task_runner_;
// A pre-populated list of icon types to consider when looking for large
// icons. This is an optimization over populating an icon type vector on each
......
......@@ -11,11 +11,10 @@
#include "base/macros.h"
#include "base/memory/ptr_util.h"
#include "base/memory/ref_counted_memory.h"
#include "base/message_loop/message_loop.h"
#include "base/run_loop.h"
#include "base/task/cancelable_task_tracker.h"
#include "base/test/histogram_tester.h"
#include "base/test/mock_callback.h"
#include "base/test/scoped_task_environment.h"
#include "base/threading/thread_task_runner_handle.h"
#include "components/favicon/core/favicon_client.h"
#include "components/favicon/core/test/mock_favicon_service.h"
......@@ -126,14 +125,12 @@ class LargeIconServiceTest : public testing::Test {
LargeIconServiceTest()
: mock_image_fetcher_(new NiceMock<MockImageFetcher>()),
large_icon_service_(&mock_favicon_service_,
base::ThreadTaskRunnerHandle::Get(),
base::WrapUnique(mock_image_fetcher_)) {}
~LargeIconServiceTest() override {}
protected:
base::MessageLoopForIO loop_;
base::test::ScopedTaskEnvironment scoped_task_environment_;
NiceMock<MockImageFetcher>* mock_image_fetcher_;
testing::NiceMock<MockFaviconService> mock_favicon_service_;
LargeIconService large_icon_service_;
......@@ -169,7 +166,7 @@ TEST_F(LargeIconServiceTest, ShouldGetFromGoogleServer) {
EXPECT_CALL(callback,
Run(favicon_base::GoogleFaviconServerRequestStatus::SUCCESS));
base::RunLoop().RunUntilIdle();
scoped_task_environment_.RunUntilIdle();
histogram_tester_.ExpectUniqueSample(
"Favicons.LargeIconService.DownloadedSize", 64, /*expected_count=*/1);
}
......@@ -207,7 +204,7 @@ TEST_F(LargeIconServiceTest, ShouldGetFromGoogleServerWithCustomUrl) {
EXPECT_CALL(callback,
Run(favicon_base::GoogleFaviconServerRequestStatus::SUCCESS));
base::RunLoop().RunUntilIdle();
scoped_task_environment_.RunUntilIdle();
}
TEST_F(LargeIconServiceTest, ShouldGetFromGoogleServerWithOriginalUrl) {
......@@ -239,7 +236,7 @@ TEST_F(LargeIconServiceTest, ShouldGetFromGoogleServerWithOriginalUrl) {
EXPECT_CALL(callback,
Run(favicon_base::GoogleFaviconServerRequestStatus::SUCCESS));
base::RunLoop().RunUntilIdle();
scoped_task_environment_.RunUntilIdle();
}
TEST_F(LargeIconServiceTest, ShouldTrimQueryParametersForGoogleServer) {
......@@ -264,7 +261,7 @@ TEST_F(LargeIconServiceTest, ShouldTrimQueryParametersForGoogleServer) {
TRAFFIC_ANNOTATION_FOR_TESTS,
favicon_base::GoogleFaviconServerCallback());
base::RunLoop().RunUntilIdle();
scoped_task_environment_.RunUntilIdle();
}
TEST_F(LargeIconServiceTest, ShouldNotCheckOnPublicUrls) {
......@@ -285,7 +282,7 @@ TEST_F(LargeIconServiceTest, ShouldNotCheckOnPublicUrls) {
EXPECT_CALL(callback, Run(favicon_base::GoogleFaviconServerRequestStatus::
FAILURE_CONNECTION_ERROR));
base::RunLoop().RunUntilIdle();
scoped_task_environment_.RunUntilIdle();
}
TEST_F(LargeIconServiceTest, ShouldNotQueryGoogleServerIfInvalidScheme) {
......@@ -305,7 +302,7 @@ TEST_F(LargeIconServiceTest, ShouldNotQueryGoogleServerIfInvalidScheme) {
EXPECT_CALL(
callback,
Run(favicon_base::GoogleFaviconServerRequestStatus::FAILURE_INVALID));
base::RunLoop().RunUntilIdle();
scoped_task_environment_.RunUntilIdle();
EXPECT_THAT(histogram_tester_.GetAllSamples(
"Favicons.LargeIconService.DownloadedSize"),
IsEmpty());
......@@ -336,7 +333,7 @@ TEST_F(LargeIconServiceTest, ShouldReportUnavailableIfFetchFromServerFails) {
EXPECT_CALL(callback, Run(favicon_base::GoogleFaviconServerRequestStatus::
FAILURE_CONNECTION_ERROR));
base::RunLoop().RunUntilIdle();
scoped_task_environment_.RunUntilIdle();
// Verify that download failure gets recorded.
histogram_tester_.ExpectUniqueSample(
"Favicons.LargeIconService.DownloadedSize", 0, /*expected_count=*/1);
......@@ -366,7 +363,7 @@ TEST_F(LargeIconServiceTest, ShouldNotGetFromGoogleServerIfUnavailable) {
EXPECT_CALL(callback, Run(favicon_base::GoogleFaviconServerRequestStatus::
FAILURE_HTTP_ERROR_CACHED));
base::RunLoop().RunUntilIdle();
scoped_task_environment_.RunUntilIdle();
EXPECT_THAT(histogram_tester_.GetAllSamples(
"Favicons.LargeIconService.DownloadedSize"),
IsEmpty());
......@@ -396,7 +393,7 @@ class LargeIconServiceGetterTest : public LargeIconServiceTest,
base::Unretained(this)),
&cancelable_task_tracker_);
}
base::RunLoop().RunUntilIdle();
scoped_task_environment_.RunUntilIdle();
}
void RawBitmapResultCallback(const favicon_base::LargeIconResult& result) {
......
......@@ -437,9 +437,7 @@ TEST_F(IconCacherTestPopularSites, LargeNotCachedAndFetchPerformedOnlyOnce) {
class IconCacherTestMostLikely : public IconCacherTestBase {
protected:
IconCacherTestMostLikely()
: large_icon_service_background_task_runner_(
new base::TestSimpleTaskRunner()),
fetcher_for_large_icon_service_(
: fetcher_for_large_icon_service_(
base::MakeUnique<::testing::StrictMock<MockImageFetcher>>()),
fetcher_for_icon_cacher_(
base::MakeUnique<::testing::StrictMock<MockImageFetcher>>()) {
......@@ -453,8 +451,6 @@ class IconCacherTestMostLikely : public IconCacherTestBase {
SetDesiredImageFrameSize(gfx::Size(128, 128)));
}
scoped_refptr<base::TestSimpleTaskRunner>
large_icon_service_background_task_runner_;
std::unique_ptr<MockImageFetcher> fetcher_for_large_icon_service_;
std::unique_ptr<MockImageFetcher> fetcher_for_icon_cacher_;
};
......@@ -467,8 +463,7 @@ TEST_F(IconCacherTestMostLikely, Cached) {
PreloadIcon(page_url, icon_url, favicon_base::TOUCH_ICON, 128, 128);
favicon::LargeIconService large_icon_service(
&favicon_service_, large_icon_service_background_task_runner_,
std::move(fetcher_for_large_icon_service_));
&favicon_service_, std::move(fetcher_for_large_icon_service_));
IconCacherImpl cacher(&favicon_service_, &large_icon_service,
std::move(fetcher_for_icon_cacher_));
......@@ -502,8 +497,7 @@ TEST_F(IconCacherTestMostLikely, NotCachedAndFetchSucceeded) {
}
favicon::LargeIconService large_icon_service(
&favicon_service_, large_icon_service_background_task_runner_,
std::move(fetcher_for_large_icon_service_));
&favicon_service_, std::move(fetcher_for_large_icon_service_));
IconCacherImpl cacher(&favicon_service_, &large_icon_service,
std::move(fetcher_for_icon_cacher_));
......@@ -511,7 +505,7 @@ TEST_F(IconCacherTestMostLikely, NotCachedAndFetchSucceeded) {
// Both these task runners need to be flushed in order to get |done| called by
// running the main loop.
WaitForHistoryThreadTasksToFinish();
large_icon_service_background_task_runner_->RunUntilIdle();
scoped_task_environment_.RunUntilIdle();
loop.Run();
EXPECT_FALSE(IconIsCachedFor(page_url, favicon_base::FAVICON));
......@@ -541,8 +535,7 @@ TEST_F(IconCacherTestMostLikely, NotCachedAndFetchFailed) {
}
favicon::LargeIconService large_icon_service(
&favicon_service_, large_icon_service_background_task_runner_,
std::move(fetcher_for_large_icon_service_));
&favicon_service_, std::move(fetcher_for_large_icon_service_));
IconCacherImpl cacher(&favicon_service_, &large_icon_service,
std::move(fetcher_for_icon_cacher_));
......@@ -550,7 +543,7 @@ TEST_F(IconCacherTestMostLikely, NotCachedAndFetchFailed) {
// Both these task runners need to be flushed before flushing the main thread
// queue in order to finish the work.
WaitForHistoryThreadTasksToFinish();
large_icon_service_background_task_runner_->RunUntilIdle();
scoped_task_environment_.RunUntilIdle();
WaitForMainThreadTasksToFinish();
EXPECT_FALSE(IconIsCachedFor(page_url, favicon_base::FAVICON));
......@@ -573,8 +566,7 @@ TEST_F(IconCacherTestMostLikely, HandlesEmptyCallbacksNicely) {
.WillOnce(PassFetch(128, 128));
favicon::LargeIconService large_icon_service(
&favicon_service_, large_icon_service_background_task_runner_,
std::move(fetcher_for_large_icon_service_));
&favicon_service_, std::move(fetcher_for_large_icon_service_));
IconCacherImpl cacher(&favicon_service_, &large_icon_service,
std::move(fetcher_for_icon_cacher_));
......@@ -582,7 +574,7 @@ TEST_F(IconCacherTestMostLikely, HandlesEmptyCallbacksNicely) {
// Both these task runners need to be flushed before flushing the main thread
// queue in order to finish the work.
WaitForHistoryThreadTasksToFinish();
large_icon_service_background_task_runner_->RunUntilIdle();
scoped_task_environment_.RunUntilIdle();
WaitForMainThreadTasksToFinish();
// Even though the callbacks are not called, the icon gets written out.
......@@ -609,8 +601,7 @@ TEST_F(IconCacherTestMostLikely, NotCachedAndFetchPerformedOnlyOnce) {
}
favicon::LargeIconService large_icon_service(
&favicon_service_, large_icon_service_background_task_runner_,
std::move(fetcher_for_large_icon_service_));
&favicon_service_, std::move(fetcher_for_large_icon_service_));
IconCacherImpl cacher(&favicon_service_, &large_icon_service,
std::move(fetcher_for_icon_cacher_));
......@@ -619,7 +610,7 @@ TEST_F(IconCacherTestMostLikely, NotCachedAndFetchPerformedOnlyOnce) {
// Both these task runners need to be flushed in order to get |done| called by
// running the main loop.
WaitForHistoryThreadTasksToFinish();
large_icon_service_background_task_runner_->RunUntilIdle();
scoped_task_environment_.RunUntilIdle();
loop.Run();
EXPECT_FALSE(IconIsCachedFor(page_url, favicon_base::FAVICON));
......
......@@ -58,6 +58,7 @@ source_set("unit_tests") {
deps = [
":spotlight",
"//base",
"//base/test:test_support",
"//components/bookmarks/browser",
"//components/bookmarks/test",
"//components/favicon/core",
......
......@@ -8,11 +8,9 @@
#import <Foundation/Foundation.h>
#include "base/location.h"
#include "base/message_loop/message_loop.h"
#include "base/single_thread_task_runner.h"
#include "base/strings/sys_string_conversions.h"
#include "base/strings/utf_string_conversions.h"
#include "base/threading/thread_task_runner_handle.h"
#include "base/test/scoped_task_environment.h"
#include "components/bookmarks/browser/bookmark_model.h"
#include "components/bookmarks/test/bookmark_test_helpers.h"
#include "components/bookmarks/test/test_bookmark_client.h"
......@@ -67,8 +65,7 @@ class SpotlightManagerTest : public testing::Test {
SpotlightManagerTest() {
model_ = bookmarks::TestBookmarkClient::CreateModel();
large_icon_service_.reset(new favicon::LargeIconService(
&mock_favicon_service_, base::ThreadTaskRunnerHandle::Get(),
/*image_fetcher=*/nullptr));
&mock_favicon_service_, /*image_fetcher=*/nullptr));
bookmarksSpotlightManager_ = [[BookmarksSpotlightManager alloc]
initWithLargeIconService:large_icon_service_.get()
bookmarkModel:model_.get()];
......@@ -80,7 +77,7 @@ class SpotlightManagerTest : public testing::Test {
~SpotlightManagerTest() override { [bookmarksSpotlightManager_ shutdown]; }
base::MessageLoop loop_;
base::test::ScopedTaskEnvironment scoped_task_environment_;
testing::StrictMock<favicon::MockFaviconService> mock_favicon_service_;
std::unique_ptr<favicon::LargeIconService> large_icon_service_;
base::CancelableTaskTracker cancelable_task_tracker_;
......
......@@ -6,7 +6,6 @@
#include "base/memory/ptr_util.h"
#include "base/memory/singleton.h"
#include "base/task_scheduler/post_task.h"
#include "components/favicon/core/large_icon_service.h"
#include "components/image_fetcher/core/image_fetcher_impl.h"
#include "components/image_fetcher/ios/ios_image_decoder_impl.h"
......@@ -43,15 +42,9 @@ IOSChromeLargeIconServiceFactory::BuildServiceInstanceFor(
web::BrowserState* context) const {
ios::ChromeBrowserState* browser_state =
ios::ChromeBrowserState::FromBrowserState(context);
scoped_refptr<base::SequencedTaskRunner> task_runner =
base::CreateSequencedTaskRunnerWithTraits(
{base::MayBlock(), base::TaskPriority::BACKGROUND,
base::TaskShutdownBehavior::CONTINUE_ON_SHUTDOWN});
return base::MakeUnique<favicon::LargeIconService>(
ios::FaviconServiceFactory::GetForBrowserState(
browser_state, ServiceAccessType::EXPLICIT_ACCESS),
task_runner,
base::MakeUnique<image_fetcher::ImageFetcherImpl>(
image_fetcher::CreateIOSImageDecoder(),
browser_state->GetRequestContext()));
......
......@@ -10,9 +10,8 @@
#include "base/files/file_util.h"
#include "base/location.h"
#include "base/path_service.h"
#include "base/single_thread_task_runner.h"
#include "base/strings/sys_string_conversions.h"
#include "base/threading/thread_task_runner_handle.h"
#include "base/task/cancelable_task_tracker.h"
#include "components/favicon/core/large_icon_service.h"
#include "components/favicon/core/test/mock_favicon_service.h"
#include "components/favicon_base/fallback_icon_style.h"
......@@ -83,8 +82,7 @@ class FaviconViewProviderTest : public PlatformTest {
DCHECK_CURRENTLY_ON(web::WebThread::UI);
PlatformTest::SetUp();
large_icon_service_.reset(new favicon::LargeIconService(
&mock_favicon_service_, base::ThreadTaskRunnerHandle::Get(),
/*image_fetcher=*/nullptr));
&mock_favicon_service_, /*image_fetcher=*/nullptr));
EXPECT_CALL(mock_favicon_service_, GetLargestRawFaviconForPageURL(
GURL(kTestFaviconURL), _, _, _, _))
......
......@@ -8,9 +8,7 @@
#import "base/mac/foundation_util.h"
#include "base/memory/ptr_util.h"
#include "base/single_thread_task_runner.h"
#include "base/strings/sys_string_conversions.h"
#include "base/threading/thread_task_runner_handle.h"
#include "base/time/default_clock.h"
#include "components/favicon/core/large_icon_service.h"
#include "components/favicon/core/test/mock_favicon_service.h"
......@@ -62,8 +60,7 @@ class ReadingListCollectionViewControllerTest : public testing::Test {
reading_list_model_.reset(new ReadingListModelImpl(
nullptr, nullptr, base::MakeUnique<base::DefaultClock>()));
large_icon_service_.reset(new favicon::LargeIconService(
&mock_favicon_service_, base::ThreadTaskRunnerHandle::Get(),
/*image_fetcher=*/nullptr));
&mock_favicon_service_, /*image_fetcher=*/nullptr));
mediator_ =
[[ReadingListMediator alloc] initWithModel:reading_list_model_.get()
largeIconService:large_icon_service_.get()];
......
......@@ -5,7 +5,6 @@
#import "ios/chrome/browser/ui/reading_list/reading_list_coordinator.h"
#include "base/memory/ptr_util.h"
#include "base/threading/thread_task_runner_handle.h"
#include "base/time/default_clock.h"
#include "components/favicon/core/large_icon_service.h"
#include "components/favicon/core/test/mock_favicon_service.h"
......@@ -134,8 +133,7 @@ class ReadingListCoordinatorTest : public web::WebTestWithWebState {
reading_list_model_.reset(new ReadingListModelImpl(
nullptr, nullptr, base::MakeUnique<base::DefaultClock>()));
large_icon_service_.reset(new favicon::LargeIconService(
&mock_favicon_service_, base::ThreadTaskRunnerHandle::Get(),
/*image_fetcher=*/nullptr));
&mock_favicon_service_, /*image_fetcher=*/nullptr));
mediator_ =
[[ReadingListMediator alloc] initWithModel:reading_list_model_.get()
largeIconService:large_icon_service_.get()];
......
......@@ -7,7 +7,6 @@
#include "base/memory/ptr_util.h"
#include "base/strings/sys_string_conversions.h"
#include "base/test/simple_test_clock.h"
#include "base/threading/thread_task_runner_handle.h"
#include "components/favicon/core/large_icon_service.h"
#include "components/favicon/core/test/mock_favicon_service.h"
#include "components/reading_list/core/reading_list_model_impl.h"
......@@ -58,8 +57,7 @@ class ReadingListMediatorTest : public PlatformTest {
model_->SetReadStatus(GURL("http://chromium.org/read2"), true);
large_icon_service_.reset(new favicon::LargeIconService(
&mock_favicon_service_, base::ThreadTaskRunnerHandle::Get(),
/*image_fetcher=*/nullptr));
&mock_favicon_service_, /*image_fetcher=*/nullptr));
mediator_ =
[[ReadingListMediator alloc] initWithModel:model_.get()
......
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