Skip to content

Commit

Permalink
[Large Icon Service] Move icon resizing into worker thread.
Browse files Browse the repository at this point in the history
Using https://crrev.com/15679025 as template, but with some complication
involving base::CancelableTaskTracker.

BUG=467712

Review URL: https://codereview.chromium.org/1122103003

Cr-Commit-Position: refs/heads/master@{#329248}
  • Loading branch information
samuelhuang authored and Commit bot committed May 11, 2015
1 parent 9ab9648 commit 7f7237f
Show file tree
Hide file tree
Showing 6 changed files with 174 additions and 89 deletions.
1 change: 1 addition & 0 deletions components/favicon.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
'type': 'static_library',
'dependencies': [
'../base/base.gyp:base',
'../content/content.gyp:content_browser',
'../skia/skia.gyp:skia',
'../ui/base/ui_base.gyp:ui_base',
'../ui/gfx/gfx.gyp:gfx',
Expand Down
1 change: 1 addition & 0 deletions components/favicon/core/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ static_library("core") {
"//components/favicon_base",
"//components/history/core/browser",
"//components/keyed_service/core",
"//content/public/browser",
"//skia",
"//ui/base",
"//ui/gfx",
Expand Down
1 change: 1 addition & 0 deletions components/favicon/core/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ include_rules = [
"+components/bookmarks/browser",
"+components/history/core/browser",
"+components/keyed_service/core",
"+content/public/browser",
"+skia",
"+third_party/skia",
"+third_party/skia/include",
Expand Down
217 changes: 155 additions & 62 deletions components/favicon/core/large_icon_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,110 +4,203 @@

#include "components/favicon/core/large_icon_service.h"

#include "base/bind.h"
#include "base/logging.h"
#include "base/memory/ref_counted.h"
#include "base/memory/scoped_ptr.h"
#include "base/task_runner.h"
#include "base/threading/sequenced_worker_pool.h"
#include "components/favicon/core/favicon_service.h"
#include "components/favicon_base/fallback_icon_style.h"
#include "components/favicon_base/favicon_types.h"
#include "content/public/browser/browser_thread.h"
#include "skia/ext/image_operations.h"
#include "ui/gfx/codec/png_codec.h"
#include "ui/gfx/geometry/size.h"

namespace favicon {

LargeIconService::LargeIconService(FaviconService* favicon_service)
: favicon_service_(favicon_service) {
large_icon_types_.push_back(favicon_base::IconType::FAVICON);
large_icon_types_.push_back(favicon_base::IconType::TOUCH_ICON);
large_icon_types_.push_back(favicon_base::IconType::TOUCH_PRECOMPOSED_ICON);
namespace {

// Processes the bitmap data returned from the FaviconService as part of a
// LargeIconService request.
class LargeIconWorker : public base::RefCountedThreadSafe<LargeIconWorker> {
public:
LargeIconWorker(int min_source_size_in_pixel,
int desired_size_in_pixel,
favicon_base::LargeIconCallback callback,
scoped_refptr<base::TaskRunner> background_task_runner,
base::CancelableTaskTracker* tracker);

// Must run on the owner (UI) thread in production.
// Intermediate callback for GetLargeIconOrFallbackStyle(). Invokes
// ProcessIconOnBackgroundThread() so we do not perform complex image
// operations on the UI thread.
void OnIconLookupComplete(
const favicon_base::FaviconRawBitmapResult& bitmap_result);

private:
friend class base::RefCountedThreadSafe<LargeIconWorker>;

~LargeIconWorker();

// Must run on a background thread in production.
// Tries to resize |bitmap_result_| and pass the output to |callback_|. If
// that does not work, computes the icon fallback style and uses it to
// invoke |callback_|. This must be run on a background thread because image
// resizing and dominant color extraction can be expensive.
void ProcessIconOnBackgroundThread();

// Must run on a background thread in production.
// If |bitmap_result_| is square and large enough (>= |min_source_in_pixel_|),
// resizes it to |desired_size_in_pixel_| (but if |desired_size_in_pixel_| is
// 0 then don't resize). If successful, stores the resulting bitmap data
// into |resized_bitmap_result| and returns true.
bool ResizeLargeIconOnBackgroundThreadIfValid(
favicon_base::FaviconRawBitmapResult* resized_bitmap_result);

// Must run on the owner (UI) thread in production.
// Invoked when ProcessIconOnBackgroundThread() is done.
void OnIconProcessingComplete();

int min_source_size_in_pixel_;
int desired_size_in_pixel_;
favicon_base::LargeIconCallback callback_;
scoped_refptr<base::TaskRunner> background_task_runner_;
base::CancelableTaskTracker* tracker_;
favicon_base::FaviconRawBitmapResult bitmap_result_;
scoped_ptr<favicon_base::LargeIconResult> result_;

DISALLOW_COPY_AND_ASSIGN(LargeIconWorker);
};

LargeIconWorker::LargeIconWorker(
int min_source_size_in_pixel,
int desired_size_in_pixel,
favicon_base::LargeIconCallback 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),
callback_(callback),
background_task_runner_(background_task_runner),
tracker_(tracker) {
}

LargeIconService::~LargeIconService() {
LargeIconWorker::~LargeIconWorker() {
}

base::CancelableTaskTracker::TaskId
LargeIconService::GetLargeIconOrFallbackStyle(
const GURL& page_url,
int min_source_size_in_pixel,
int desired_size_in_pixel,
const favicon_base::LargeIconCallback& callback,
base::CancelableTaskTracker* tracker) {
DCHECK_LE(1, min_source_size_in_pixel);
DCHECK_LE(0, desired_size_in_pixel);
void LargeIconWorker::OnIconLookupComplete(
const favicon_base::FaviconRawBitmapResult& bitmap_result) {
bitmap_result_ = bitmap_result;
tracker_->PostTaskAndReply(
background_task_runner_.get(), FROM_HERE,
base::Bind(&LargeIconWorker::ProcessIconOnBackgroundThread, this),
base::Bind(&LargeIconWorker::OnIconProcessingComplete, this));
}

// TODO(beaudoin): For now this is just a wrapper around
// GetLargestRawFaviconForPageURL. Add the logic required to select the best
// possible large icon. Also add logic to fetch-on-demand when the URL of
// a large icon is known but its bitmap is not available.
return favicon_service_->GetLargestRawFaviconForPageURL(
page_url,
large_icon_types_,
std::max(min_source_size_in_pixel, desired_size_in_pixel),
base::Bind(&LargeIconService::RunLargeIconCallback,
base::Unretained(this), callback, min_source_size_in_pixel,
desired_size_in_pixel),
tracker);
void LargeIconWorker::ProcessIconOnBackgroundThread() {
favicon_base::FaviconRawBitmapResult resized_bitmap_result;
if (ResizeLargeIconOnBackgroundThreadIfValid(&resized_bitmap_result)) {
result_.reset(
new favicon_base::LargeIconResult(resized_bitmap_result));
} else {
// Failed to resize |bitmap_result_|, so compute fallback icon style.
scoped_ptr<favicon_base::FallbackIconStyle> fallback_icon_style(
new favicon_base::FallbackIconStyle());
if (bitmap_result_.is_valid()) {
favicon_base::SetDominantColorAsBackground(
bitmap_result_.bitmap_data, fallback_icon_style.get());
}
result_.reset(
new favicon_base::LargeIconResult(fallback_icon_style.release()));
}
}

bool LargeIconService::ResizeLargeIconIfValid(
int min_source_size_in_pixel,
int desired_size_in_pixel,
const favicon_base::FaviconRawBitmapResult& bitmap_result,
bool LargeIconWorker::ResizeLargeIconOnBackgroundThreadIfValid(
favicon_base::FaviconRawBitmapResult* resized_bitmap_result) {
// Require bitmap to be valid and square.
if (!bitmap_result.is_valid() ||
bitmap_result.pixel_size.width() != bitmap_result.pixel_size.height())
if (!bitmap_result_.is_valid() ||
bitmap_result_.pixel_size.width() != bitmap_result_.pixel_size.height())
return false;

// Require bitmap to be large enough. It's square, so just check width.
if (bitmap_result.pixel_size.width() < min_source_size_in_pixel)
if (bitmap_result_.pixel_size.width() < min_source_size_in_pixel_)
return false;

*resized_bitmap_result = bitmap_result;
*resized_bitmap_result = bitmap_result_;

// Special case: Can use |bitmap_result| as is.
if (desired_size_in_pixel == 0 ||
bitmap_result.pixel_size.width() == desired_size_in_pixel)
// Special case: Can use |bitmap_result_| as is.
if (desired_size_in_pixel_ == 0 ||
bitmap_result_.pixel_size.width() == desired_size_in_pixel_)
return true;

// Resize bitmap: decode PNG, resize, and re-encode PNG.
SkBitmap decoded_bitmap;
if (!gfx::PNGCodec::Decode(bitmap_result.bitmap_data->front(),
bitmap_result.bitmap_data->size(), &decoded_bitmap))
if (!gfx::PNGCodec::Decode(bitmap_result_.bitmap_data->front(),
bitmap_result_.bitmap_data->size(), &decoded_bitmap))
return false;

SkBitmap resized_bitmap = skia::ImageOperations::Resize(
decoded_bitmap, skia::ImageOperations::RESIZE_LANCZOS3,
desired_size_in_pixel, desired_size_in_pixel);
desired_size_in_pixel_, desired_size_in_pixel_);

std::vector<unsigned char> bitmap_data;
if (!gfx::PNGCodec::EncodeBGRASkBitmap(resized_bitmap, false, &bitmap_data))
return false;

resized_bitmap_result->pixel_size =
gfx::Size(desired_size_in_pixel, desired_size_in_pixel);
gfx::Size(desired_size_in_pixel_, desired_size_in_pixel_);
resized_bitmap_result->bitmap_data =
base::RefCountedBytes::TakeVector(&bitmap_data);
return true;
}

void LargeIconService::RunLargeIconCallback(
const favicon_base::LargeIconCallback& callback,
int min_source_size_in_pixel,
int desired_size_in_pixel,
const favicon_base::FaviconRawBitmapResult& bitmap_result) {
favicon_base::FaviconRawBitmapResult resized_bitmap_result;
if (ResizeLargeIconIfValid(min_source_size_in_pixel, desired_size_in_pixel,
bitmap_result, &resized_bitmap_result)) {
callback.Run(favicon_base::LargeIconResult(resized_bitmap_result));
return;
}
void LargeIconWorker::OnIconProcessingComplete() {
callback_.Run(*result_);
}

// Failed to resize |bitmap_result|, so compute fallback icon style.
favicon_base::LargeIconResult result(new favicon_base::FallbackIconStyle());
if (bitmap_result.is_valid()) {
favicon_base::SetDominantColorAsBackground(
bitmap_result.bitmap_data, result.fallback_icon_style.get());
}
callback.Run(result);
} // namespace

namespace favicon {

LargeIconService::LargeIconService(FaviconService* favicon_service)
: favicon_service_(favicon_service) {
large_icon_types_.push_back(favicon_base::IconType::FAVICON);
large_icon_types_.push_back(favicon_base::IconType::TOUCH_ICON);
large_icon_types_.push_back(favicon_base::IconType::TOUCH_PRECOMPOSED_ICON);
}

LargeIconService::~LargeIconService() {
}

base::CancelableTaskTracker::TaskId
LargeIconService::GetLargeIconOrFallbackStyle(
const GURL& page_url,
int min_source_size_in_pixel,
int desired_size_in_pixel,
const favicon_base::LargeIconCallback& callback,
base::CancelableTaskTracker* tracker) {
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, callback, GetBackgroundTaskRunner(), tracker);

// TODO(beaudoin): For now this is just a wrapper around
// GetLargestRawFaviconForPageURL. Add the logic required to select the best
// possible large icon. Also add logic to fetch-on-demand when the URL of
// a large icon is known but its bitmap is not available.
return favicon_service_->GetLargestRawFaviconForPageURL(
page_url, large_icon_types_, desired_size_in_pixel,
base::Bind(&LargeIconWorker::OnIconLookupComplete, worker),
tracker);
}

// Returns TaskRunner used to execute background task.
scoped_refptr<base::TaskRunner> LargeIconService::GetBackgroundTaskRunner() {
return content::BrowserThread::GetBlockingPool()
->GetTaskRunnerWithShutdownBehavior(
base::SequencedWorkerPool::SKIP_ON_SHUTDOWN);
}

} // namespace favicon
33 changes: 9 additions & 24 deletions components/favicon/core/large_icon_service.h
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2015 The Chromium Authors. All rights reserved.
// Copyright 2015 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.

Expand All @@ -13,8 +13,8 @@

class GURL;

namespace favicon_base {
struct FaviconRawBitmapResult;
namespace base {
class TaskRunner;
}

namespace favicon {
Expand Down Expand Up @@ -47,33 +47,18 @@ class LargeIconService : public KeyedService {
const favicon_base::LargeIconCallback& callback,
base::CancelableTaskTracker* tracker);

// Returns TaskRunner used to execute background task.
virtual scoped_refptr<base::TaskRunner> GetBackgroundTaskRunner();

private:
// For testing.
friend class TestLargeIconService;

// Resizes |bitmap_result| to |desired_size_in_pixel|x|desired_size_in_pixel|.
// Stores the resized bitmap data in |resized_bitmap_result| and returns true
// if successful.
bool ResizeLargeIconIfValid(
int min_source_size_in_pixel,
int desired_size_in_pixel,
const favicon_base::FaviconRawBitmapResult& bitmap_result,
favicon_base::FaviconRawBitmapResult* resized_bitmap_result);

// Intermediate callback for GetLargeIconOrFallbackStyle(). Tries to resize
// |bitmap_result| and pass the output to |callback|. If that does not work,
// computes the icon fallback style and uses it to invoke |callback|.
void RunLargeIconCallback(
const favicon_base::LargeIconCallback& callback,
int min_source_size_in_pixel,
int desired_size_in_pixel,
const favicon_base::FaviconRawBitmapResult& bitmap_result);

FaviconService* favicon_service_;

// A pre-populated list of the types of icon files to consider when looking
// for large icons. This is an optimization over populating an icon type
// vector on each request.
// 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
// request.
std::vector<int> large_icon_types_;

DISALLOW_COPY_AND_ASSIGN(LargeIconService);
Expand Down
10 changes: 7 additions & 3 deletions components/favicon/core/large_icon_service_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,9 @@ class MockFaviconService : public FaviconService {
}

private:
DISALLOW_COPY_AND_ASSIGN(MockFaviconService);

std::deque<favicon_base::FaviconRawBitmapResult> mock_result_queue_;

DISALLOW_COPY_AND_ASSIGN(MockFaviconService);
};

// This class provides access to LargeIconService internals.
Expand All @@ -96,6 +96,11 @@ class TestLargeIconService : public LargeIconService {
~TestLargeIconService() override {
}

// Using the current thread's task runner for testing.
scoped_refptr<base::TaskRunner> GetBackgroundTaskRunner() override {
return base::MessageLoop::current()->task_runner();
}

private:
DISALLOW_COPY_AND_ASSIGN(TestLargeIconService);
};
Expand Down Expand Up @@ -152,7 +157,6 @@ class LargeIconServiceTest : public testing::Test {
};

TEST_F(LargeIconServiceTest, SameSize) {

mock_favicon_service_->InjectResult(CreateTestBitmap(24, 24, kTestColor));
expected_bitmap_ = CreateTestBitmap(24, 24, kTestColor);
large_icon_service_->GetLargeIconOrFallbackStyle(
Expand Down

0 comments on commit 7f7237f

Please sign in to comment.