Commit 30ee1086 authored by tschumann's avatar tschumann Committed by Commit bot

Specify desired_image_size when decoding image files for the NTP Tile icons.

Currently, the size (0,0) is hard-coded which makes the decoder prefer small
images in case multiple sizes are available -- e.g. in ICO files.
However, this resolution is too low for NTP tiles and hence these tiles render
as "scrabble" tiles.

I looked at popular sites sets of 8 countries (especially EM countries), and the
number of nicely rendered icons increased from 51 to 57 (out of 64). Here are
the details:
https://docs.google.com/document/d/1eNCDcsVkBEzG2hUm8uPh9RgMRpM3NvS9C0VhO_gpCJM

Unfortunately, this change requires drilling through a number of layers.
ImageFetcher (components/image_fetcher/image_fetcher_impl.h) is used by 6
different clients which is why I added a setter method on that class -- also
because we already do this with other parameters.

Review-Url: https://codereview.chromium.org/2715153006
Cr-Commit-Position: refs/heads/master@{#454341}
parent 08eddf09
......@@ -17,6 +17,7 @@
#include "components/wallpaper/wallpaper_layout.h"
#include "content/public/browser/browser_thread.h"
#include "ui/gfx/codec/png_codec.h"
#include "ui/gfx/geometry/size.h"
#include "ui/gfx/image/image.h"
#include "ui/gfx/image/image_skia.h"
#include "ui/gfx/image/image_util.h"
......@@ -109,7 +110,8 @@ void ArcWallpaperService::OnInstanceClosed() {
void ArcWallpaperService::SetWallpaper(const std::vector<uint8_t>& data) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
ImageDecoder::Cancel(this);
ImageDecoder::StartWithOptions(this, data, ImageDecoder::DEFAULT_CODEC, true);
ImageDecoder::StartWithOptions(this, data, ImageDecoder::DEFAULT_CODEC, true,
gfx::Size());
}
void ArcWallpaperService::GetWallpaper(const GetWallpaperCallback& callback) {
......
......@@ -75,6 +75,7 @@ class MockImageFetcher : public image_fetcher::ImageFetcher {
base::Callback<void(const std::string&, const gfx::Image&)>));
MOCK_METHOD1(SetImageFetcherDelegate, void(ImageFetcherDelegate*));
MOCK_METHOD1(SetDataUseServiceName, void(DataUseServiceName));
MOCK_METHOD1(SetDesiredImageFrameSize, void(const gfx::Size&));
private:
DISALLOW_COPY_AND_ASSIGN(MockImageFetcher);
......
......@@ -16,6 +16,7 @@
#include "services/image_decoder/public/cpp/decode.h"
#include "services/service_manager/public/cpp/connector.h"
#include "third_party/skia/include/core/SkBitmap.h"
#include "ui/gfx/geometry/size.h"
namespace {
......@@ -58,6 +59,7 @@ void DecodeImage(
std::vector<uint8_t> image_data,
image_decoder::mojom::ImageCodec codec,
bool shrink_to_fit,
const gfx::Size& desired_image_frame_size,
const image_decoder::mojom::ImageDecoder::DecodeImageCallback& callback,
scoped_refptr<base::SequencedTaskRunner> callback_task_runner) {
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
......@@ -68,9 +70,9 @@ void DecodeImage(
BindToBrowserConnector(std::move(connector_request));
image_decoder::Decode(connector.get(), image_data, codec, shrink_to_fit,
kMaxImageSizeInBytes,
base::Bind(&RunDecodeCallbackOnTaskRunner,
callback, callback_task_runner));
kMaxImageSizeInBytes, desired_image_frame_size,
base::Bind(&RunDecodeCallbackOnTaskRunner, callback,
callback_task_runner));
}
} // namespace
......@@ -100,7 +102,8 @@ ImageDecoder* ImageDecoder::GetInstance() {
// static
void ImageDecoder::Start(ImageRequest* image_request,
std::vector<uint8_t> image_data) {
StartWithOptions(image_request, std::move(image_data), DEFAULT_CODEC, false);
StartWithOptions(image_request, std::move(image_data), DEFAULT_CODEC, false,
gfx::Size());
}
// static
......@@ -114,9 +117,11 @@ void ImageDecoder::Start(ImageRequest* image_request,
void ImageDecoder::StartWithOptions(ImageRequest* image_request,
std::vector<uint8_t> image_data,
ImageCodec image_codec,
bool shrink_to_fit) {
bool shrink_to_fit,
const gfx::Size& desired_image_frame_size) {
ImageDecoder::GetInstance()->StartWithOptionsImpl(
image_request, std::move(image_data), image_codec, shrink_to_fit);
image_request, std::move(image_data), image_codec, shrink_to_fit,
desired_image_frame_size);
}
// static
......@@ -126,15 +131,17 @@ void ImageDecoder::StartWithOptions(ImageRequest* image_request,
bool shrink_to_fit) {
StartWithOptions(image_request,
std::vector<uint8_t>(image_data.begin(), image_data.end()),
image_codec, shrink_to_fit);
image_codec, shrink_to_fit, gfx::Size());
}
ImageDecoder::ImageDecoder() : image_request_id_counter_(0) {}
void ImageDecoder::StartWithOptionsImpl(ImageRequest* image_request,
std::vector<uint8_t> image_data,
ImageCodec image_codec,
bool shrink_to_fit) {
void ImageDecoder::StartWithOptionsImpl(
ImageRequest* image_request,
std::vector<uint8_t> image_data,
ImageCodec image_codec,
bool shrink_to_fit,
const gfx::Size& desired_image_frame_size) {
DCHECK(image_request);
DCHECK(image_request->task_runner());
......@@ -167,7 +174,8 @@ void ImageDecoder::StartWithOptionsImpl(ImageRequest* image_request,
content::BrowserThread::PostTask(
content::BrowserThread::IO, FROM_HERE,
base::Bind(&DecodeImage, base::Passed(&image_data), codec, shrink_to_fit,
callback, make_scoped_refptr(image_request->task_runner())));
desired_image_frame_size, callback,
make_scoped_refptr(image_request->task_runner())));
}
// static
......
......@@ -15,6 +15,10 @@
#include "base/sequenced_task_runner.h"
#include "base/synchronization/lock.h"
namespace gfx {
class Size;
} // namespace gfx
class SkBitmap;
// This is a helper class for decoding images safely in a sandboxed service. To
......@@ -83,10 +87,16 @@ class ImageDecoder {
// Starts asynchronous image decoding. Once finished, the callback will be
// posted back to image_request's |task_runner_|.
// For images with multiple frames (e.g. ico files), a frame with a size as
// close as possible to |desired_image_frame_size| is chosen (tries to take
// one in larger size if there's no precise match). Passing gfx::Size() as
// |desired_image_frame_size| is also supported and will result in chosing the
// smallest available size.
static void StartWithOptions(ImageRequest* image_request,
std::vector<uint8_t> image_data,
ImageCodec image_codec,
bool shrink_to_fit);
bool shrink_to_fit,
const gfx::Size& desired_image_frame_size);
// Deprecated. Use std::vector<uint8_t> version to avoid an extra copy.
static void StartWithOptions(ImageRequest* image_request,
const std::string& image_data,
......@@ -106,7 +116,8 @@ class ImageDecoder {
void StartWithOptionsImpl(ImageRequest* image_request,
std::vector<uint8_t> image_data,
ImageCodec image_codec,
bool shrink_to_fit);
bool shrink_to_fit,
const gfx::Size& desired_image_frame_size);
void CancelImpl(ImageRequest* image_request);
......
......@@ -178,7 +178,7 @@ IN_PROC_BROWSER_TEST_F(ImageDecoderBrowserTest, BasicDecodeWithOptionsString) {
ImageDecoder::StartWithOptions(&test_request,
std::string(data.begin(), data.end()),
ImageDecoder::ROBUST_PNG_CODEC,
false /* shrink_to_fit */);
/*shrink_to_fit=*/false);
runner->Run();
EXPECT_TRUE(test_request.decode_succeeded());
}
......@@ -187,9 +187,9 @@ IN_PROC_BROWSER_TEST_F(ImageDecoderBrowserTest, RobustJpegCodecWithJpegData) {
scoped_refptr<content::MessageLoopRunner> runner =
new content::MessageLoopRunner;
TestImageRequest test_request(runner->QuitClosure());
ImageDecoder::StartWithOptions(&test_request, GetValidJpgData(),
ImageDecoder::ROBUST_JPEG_CODEC,
false /* shrink_to_fit */);
ImageDecoder::StartWithOptions(
&test_request, GetValidJpgData(), ImageDecoder::ROBUST_JPEG_CODEC,
/*shrink_to_fit=*/false, /*desired_image_frame_size=*/gfx::Size());
runner->Run();
EXPECT_TRUE(test_request.decode_succeeded());
}
......@@ -198,9 +198,9 @@ IN_PROC_BROWSER_TEST_F(ImageDecoderBrowserTest, RobustJpegCodecWithPngData) {
scoped_refptr<content::MessageLoopRunner> runner =
new content::MessageLoopRunner;
TestImageRequest test_request(runner->QuitClosure());
ImageDecoder::StartWithOptions(&test_request, GetValidPngData(),
ImageDecoder::ROBUST_JPEG_CODEC,
false /* shrink_to_fit */);
ImageDecoder::StartWithOptions(
&test_request, GetValidPngData(), ImageDecoder::ROBUST_JPEG_CODEC,
/*shrink_to_fit=*/false, /*desired_image_frame_size=*/gfx::Size());
runner->Run();
// Should fail with PNG data because only JPEG data is allowed.
EXPECT_FALSE(test_request.decode_succeeded());
......@@ -210,9 +210,9 @@ IN_PROC_BROWSER_TEST_F(ImageDecoderBrowserTest, RobustPngCodecWithPngData) {
scoped_refptr<content::MessageLoopRunner> runner =
new content::MessageLoopRunner;
TestImageRequest test_request(runner->QuitClosure());
ImageDecoder::StartWithOptions(&test_request, GetValidPngData(),
ImageDecoder::ROBUST_PNG_CODEC,
false /* shrink_to_fit */);
ImageDecoder::StartWithOptions(
&test_request, GetValidPngData(), ImageDecoder::ROBUST_PNG_CODEC,
/*shrink_to_fit=*/false, /*desired_image_frame_size=*/gfx::Size());
runner->Run();
EXPECT_TRUE(test_request.decode_succeeded());
}
......@@ -221,9 +221,9 @@ IN_PROC_BROWSER_TEST_F(ImageDecoderBrowserTest, RobustPngCodecWithJpegData) {
scoped_refptr<content::MessageLoopRunner> runner =
new content::MessageLoopRunner;
TestImageRequest test_request(runner->QuitClosure());
ImageDecoder::StartWithOptions(&test_request, GetValidJpgData(),
ImageDecoder::ROBUST_PNG_CODEC,
false /* shrink_to_fit */);
ImageDecoder::StartWithOptions(
&test_request, GetValidJpgData(), ImageDecoder::ROBUST_PNG_CODEC,
/*shrink_to_fit=*/false, /*desired_image_frame_size=*/gfx::Size());
runner->Run();
// Should fail with JPEG data because only PNG data is allowed.
EXPECT_FALSE(test_request.decode_succeeded());
......
......@@ -7,6 +7,7 @@
#include "base/callback.h"
#include "chrome/browser/search/suggestions/image_decoder_impl.h"
#include "ui/gfx/geometry/size.h"
#include "ui/gfx/image/image.h"
namespace suggestions {
......@@ -65,11 +66,16 @@ ImageDecoderImpl::~ImageDecoderImpl() {}
void ImageDecoderImpl::DecodeImage(
const std::string& image_data,
const gfx::Size& desired_image_frame_size,
const image_fetcher::ImageDecodedCallback& callback) {
std::unique_ptr<DecodeImageRequest> decode_image_request(
new DecodeImageRequest(this, callback));
::ImageDecoder::Start(decode_image_request.get(), image_data);
::ImageDecoder::StartWithOptions(
decode_image_request.get(),
std::vector<uint8_t>(image_data.begin(), image_data.end()),
::ImageDecoder::DEFAULT_CODEC,
/*shrink_to_fit=*/false, desired_image_frame_size);
decode_image_requests_.push_back(std::move(decode_image_request));
}
......
......@@ -23,6 +23,7 @@ class ImageDecoderImpl : public image_fetcher::ImageDecoder {
void DecodeImage(
const std::string& image_data,
const gfx::Size& desired_image_frame_size,
const image_fetcher::ImageDecodedCallback& callback) override;
private:
......
......@@ -19,6 +19,7 @@ static_library("image_fetcher") {
"//base",
"//components/data_use_measurement/core",
"//net",
"//ui/gfx/geometry",
"//url",
]
}
......
include_rules = [
"+components/data_use_measurement/core",
"+net",
"+ui/gfx/geometry",
"+url",
]
......@@ -12,6 +12,7 @@
namespace gfx {
class Image;
class Size;
} // namespace gfx
namespace image_fetcher {
......@@ -27,7 +28,13 @@ class ImageDecoder {
// Decodes the passed |image_data| and runs the given callback. The callback
// is run even if decoding the image fails. In case an error occured during
// decoding the image an empty image is passed to the callback.
// For images with multiple frames (e.g. ico files), a frame with a size as
// close as possible to |desired_image_frame_size| is chosen (tries to take
// one in larger size if there's no precise match). Passing gfx::Size() as
// |desired_image_frame_size| is also supported and will result in chosing the
// smallest available size.
virtual void DecodeImage(const std::string& image_data,
const gfx::Size& desired_image_frame_size,
const ImageDecodedCallback& callback) = 0;
private:
......
......@@ -15,6 +15,7 @@
namespace gfx {
class Image;
class Size;
}
namespace image_fetcher {
......@@ -34,6 +35,14 @@ class ImageFetcher {
virtual void SetDataUseServiceName(
DataUseServiceName data_use_service_name) = 0;
// Sets the desired size for images with multiple frames (like .ico files).
// By default, the image fetcher choses smaller images. Override to choose a
// frame with a size as close as possible to |size| (trying to take one in
// larger size if there's no precise match). Passing gfx::Size() as
// |size| is also supported and will result in chosing the smallest available
// size.
virtual void SetDesiredImageFrameSize(const gfx::Size& size) = 0;
// An empty gfx::Image will be returned to the callback in case the image
// could not be fetched.
virtual void StartOrQueueNetworkRequest(
......
......@@ -39,6 +39,10 @@ void ImageFetcherImpl::SetDataUseServiceName(
image_data_fetcher_->SetDataUseServiceName(data_use_service_name);
}
void ImageFetcherImpl::SetDesiredImageFrameSize(const gfx::Size& size) {
desired_image_frame_size_ = size;
}
void ImageFetcherImpl::StartOrQueueNetworkRequest(
const std::string& id,
const GURL& image_url,
......@@ -72,10 +76,9 @@ void ImageFetcherImpl::OnImageURLFetched(const GURL& image_url,
delegate_->OnImageDataFetched(it->second.id, image_data);
}
image_decoder_->DecodeImage(
image_data,
base::Bind(&ImageFetcherImpl::OnImageDecoded,
base::Unretained(this), image_url));
image_decoder_->DecodeImage(image_data, desired_image_frame_size_,
base::Bind(&ImageFetcherImpl::OnImageDecoded,
base::Unretained(this), image_url));
}
void ImageFetcherImpl::OnImageDecoded(const GURL& image_url,
......
......@@ -16,6 +16,7 @@
#include "components/image_fetcher/image_data_fetcher.h"
#include "components/image_fetcher/image_decoder.h"
#include "components/image_fetcher/image_fetcher.h"
#include "ui/gfx/geometry/size.h"
#include "url/gurl.h"
namespace gfx {
......@@ -44,6 +45,8 @@ class ImageFetcherImpl : public image_fetcher::ImageFetcher {
// Sets a service name against which to track data usage.
void SetDataUseServiceName(DataUseServiceName data_use_service_name) override;
void SetDesiredImageFrameSize(const gfx::Size& size) override;
void StartOrQueueNetworkRequest(
const std::string& id,
const GURL& image_url,
......@@ -83,9 +86,10 @@ class ImageFetcherImpl : public image_fetcher::ImageFetcher {
// creating callbacks that are passed to the ImageDecoder.
void OnImageDecoded(const GURL& image_url, const gfx::Image& image);
ImageFetcherDelegate* delegate_;
gfx::Size desired_image_frame_size_;
scoped_refptr<net::URLRequestContextGetter> url_request_context_;
std::unique_ptr<ImageDecoder> image_decoder_;
......
......@@ -31,6 +31,7 @@
#include "components/prefs/pref_registry_simple.h"
#include "components/prefs/pref_service.h"
#include "components/strings/grit/components_strings.h"
#include "ui/gfx/geometry/size.h"
#include "ui/gfx/image/image.h"
namespace ntp_snippets {
......@@ -216,8 +217,11 @@ void CachedImageFetcher::OnImageFetchedFromDatabase(
// |image_decoder_| is null in tests.
if (image_decoder_ && !data.empty()) {
image_decoder_->DecodeImage(
data, base::Bind(&CachedImageFetcher::OnImageDecodedFromDatabase,
base::Unretained(this), callback, suggestion_id, url));
data,
// We're not dealing with multi-frame images.
/*desired_image_frame_size=*/gfx::Size(),
base::Bind(&CachedImageFetcher::OnImageDecodedFromDatabase,
base::Unretained(this), callback, suggestion_id, url));
return;
}
// Fetching from the DB failed; start a network fetch.
......
......@@ -50,6 +50,7 @@
#include "net/url_request/url_request_test_util.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "ui/gfx/geometry/size.h"
#include "ui/gfx/image/image.h"
#include "ui/gfx/image/image_unittest_util.h"
......@@ -340,6 +341,7 @@ class MockImageFetcher : public ImageFetcher {
public:
MOCK_METHOD1(SetImageFetcherDelegate, void(ImageFetcherDelegate*));
MOCK_METHOD1(SetDataUseServiceName, void(DataUseServiceName));
MOCK_METHOD1(SetDesiredImageFrameSize, void(const gfx::Size&));
MOCK_METHOD3(
StartOrQueueNetworkRequest,
void(const std::string&,
......@@ -353,6 +355,7 @@ class FakeImageDecoder : public image_fetcher::ImageDecoder {
~FakeImageDecoder() override = default;
void DecodeImage(
const std::string& image_data,
const gfx::Size& desired_image_frame_size,
const image_fetcher::ImageDecodedCallback& callback) override {
callback.Run(decoded_image_);
}
......
......@@ -11,6 +11,7 @@
#include "components/favicon_base/favicon_types.h"
#include "components/favicon_base/favicon_util.h"
#include "components/image_fetcher/image_fetcher.h"
#include "ui/gfx/geometry/size.h"
#include "ui/gfx/image/image.h"
#include "url/gurl.h"
......@@ -37,6 +38,8 @@ IconCacherImpl::IconCacherImpl(
image_fetcher_(std::move(image_fetcher)) {
image_fetcher_->SetDataUseServiceName(
data_use_measurement::DataUseUserData::NTP_TILES);
// For images with multiple frames, prefer one of size 128x128px.
image_fetcher_->SetDesiredImageFrameSize(gfx::Size(128, 128));
}
IconCacherImpl::~IconCacherImpl() = default;
......
......@@ -39,6 +39,7 @@ class MockImageFetcher : public image_fetcher::ImageFetcher {
const GURL& image_url,
base::Callback<void(const std::string& id,
const gfx::Image& image)> callback));
MOCK_METHOD1(SetDesiredImageFrameSize, void(const gfx::Size&));
};
class IconCacherTest : public ::testing::Test {
......@@ -118,6 +119,7 @@ TEST_F(IconCacherTest, LargeCached) {
EXPECT_CALL(*image_fetcher_,
SetDataUseServiceName(
data_use_measurement::DataUseUserData::NTP_TILES));
EXPECT_CALL(*image_fetcher_, SetDesiredImageFrameSize(gfx::Size(128, 128)));
EXPECT_CALL(done, Call(false)).WillOnce(Quit(&loop));
}
PreloadIcon(site_.url, site_.large_icon_url, favicon_base::TOUCH_ICON, 128,
......@@ -138,6 +140,7 @@ TEST_F(IconCacherTest, LargeNotCachedAndFetchSucceeded) {
EXPECT_CALL(*image_fetcher_,
SetDataUseServiceName(
data_use_measurement::DataUseUserData::NTP_TILES));
EXPECT_CALL(*image_fetcher_, SetDesiredImageFrameSize(gfx::Size(128, 128)));
EXPECT_CALL(*image_fetcher_,
StartOrQueueNetworkRequest(_, site_.large_icon_url, _))
.WillOnce(PassFetch(128, 128));
......@@ -161,6 +164,7 @@ TEST_F(IconCacherTest, SmallNotCachedAndFetchSucceeded) {
EXPECT_CALL(*image_fetcher_,
SetDataUseServiceName(
data_use_measurement::DataUseUserData::NTP_TILES));
EXPECT_CALL(*image_fetcher_, SetDesiredImageFrameSize(gfx::Size(128, 128)));
EXPECT_CALL(*image_fetcher_,
StartOrQueueNetworkRequest(_, site_.favicon_url, _))
.WillOnce(PassFetch(128, 128));
......@@ -182,6 +186,7 @@ TEST_F(IconCacherTest, LargeNotCachedAndFetchFailed) {
EXPECT_CALL(*image_fetcher_,
SetDataUseServiceName(
data_use_measurement::DataUseUserData::NTP_TILES));
EXPECT_CALL(*image_fetcher_, SetDesiredImageFrameSize(gfx::Size(128, 128)));
EXPECT_CALL(*image_fetcher_,
StartOrQueueNetworkRequest(_, site_.large_icon_url, _))
.WillOnce(FailFetch());
......
......@@ -19,6 +19,7 @@
#include "components/suggestions/proto/suggestions.pb.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "ui/gfx/geometry/size.h"
#include "ui/gfx/image/image.h"
#include "ui/gfx/image/image_skia.h"
#include "url/gurl.h"
......@@ -53,6 +54,7 @@ class MockImageFetcher : public ImageFetcher {
const gfx::Image&)>));
MOCK_METHOD1(SetImageFetcherDelegate, void(ImageFetcherDelegate*));
MOCK_METHOD1(SetDataUseServiceName, void(DataUseServiceName));
MOCK_METHOD1(SetDesiredImageFrameSize, void(const gfx::Size&));
};
class ImageManagerTest : public testing::Test {
......
......@@ -13,6 +13,7 @@
#include "base/memory/weak_ptr.h"
#import "components/image_fetcher/ios/webp_decoder.h"
#include "ios/web/public/web_thread.h"
#include "ui/gfx/geometry/size.h"
#include "ui/gfx/image/image.h"
#if !defined(__has_feature) || !__has_feature(objc_arc)
......@@ -26,8 +27,11 @@ class IOSImageDecoderImpl : public image_fetcher::ImageDecoder {
explicit IOSImageDecoderImpl(scoped_refptr<base::TaskRunner> task_runner);
~IOSImageDecoderImpl() override;
// Note, that |desired_image_frame_size| is not supported
// (http://crbug/697596).
void DecodeImage(
const std::string& image_data,
const gfx::Size& desired_image_frame_size,
const image_fetcher::ImageDecodedCallback& callback) override;
private:
......@@ -55,6 +59,7 @@ IOSImageDecoderImpl::~IOSImageDecoderImpl() {}
void IOSImageDecoderImpl::DecodeImage(
const std::string& image_data,
const gfx::Size& desired_image_frame_size,
const image_fetcher::ImageDecodedCallback& callback) {
// Convert the |image_data| std::string to an NSData buffer.
// The data is copied as it may have to outlive the caller in
......
......@@ -13,6 +13,7 @@
#include "base/threading/thread_task_runner_handle.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "testing/platform_test.h"
#include "ui/gfx/geometry/size.h"
#include "ui/gfx/image/image.h"
#if !defined(__has_feature) || !__has_feature(objc_arc)
......@@ -82,8 +83,9 @@ TEST_F(IOSImageDecoderImplTest, JPGImage) {
std::string image_data =
std::string(reinterpret_cast<char*>(kJPGImage), sizeof(kJPGImage));
ios_image_decoder_impl_->DecodeImage(
image_data, base::Bind(&IOSImageDecoderImplTest::OnImageDecoded,
base::Unretained(this)));
image_data, gfx::Size(),
base::Bind(&IOSImageDecoderImplTest::OnImageDecoded,
base::Unretained(this)));
pool_->FlushForTesting();
base::RunLoop().RunUntilIdle();
......@@ -97,8 +99,9 @@ TEST_F(IOSImageDecoderImplTest, WebpImage) {
std::string image_data =
std::string(reinterpret_cast<char*>(kWEBPImage), sizeof(kWEBPImage));
ios_image_decoder_impl_->DecodeImage(
image_data, base::Bind(&IOSImageDecoderImplTest::OnImageDecoded,
base::Unretained(this)));
image_data, gfx::Size(),
base::Bind(&IOSImageDecoderImplTest::OnImageDecoded,
base::Unretained(this)));
pool_->FlushForTesting();
base::RunLoop().RunUntilIdle();
......
......@@ -13,7 +13,6 @@
#include "mojo/public/cpp/bindings/strong_binding.h"
#include "skia/ext/image_operations.h"
#include "third_party/skia/include/core/SkBitmap.h"
#include "ui/gfx/geometry/size.h"
#if defined(OS_CHROMEOS)
#include "ui/gfx/chromeos/codec/jpeg_codec_robust_slow.h"
......@@ -38,6 +37,7 @@ void ImageDecoderImpl::DecodeImage(const std::vector<uint8_t>& encoded_data,
mojom::ImageCodec codec,
bool shrink_to_fit,
int64_t max_size_in_bytes,
const gfx::Size& desired_image_frame_size,
const DecodeImageCallback& callback) {
if (encoded_data.size() == 0) {
callback.Run(SkBitmap());
......@@ -67,7 +67,7 @@ void ImageDecoderImpl::DecodeImage(const std::vector<uint8_t>& encoded_data,
#endif // defined(OS_CHROMEOS)
if (codec == mojom::ImageCodec::DEFAULT) {
decoded_image = content::DecodeImage(
encoded_data.data(), gfx::Size(), encoded_data.size());
encoded_data.data(), desired_image_frame_size, encoded_data.size());
}
if (!decoded_image.isNull()) {
......
......@@ -10,6 +10,7 @@
#include "base/macros.h"
#include "services/image_decoder/public/interfaces/image_decoder.mojom.h"
#include "services/service_manager/public/cpp/service_context_ref.h"
#include "ui/gfx/geometry/size.h"
namespace image_decoder {
......@@ -20,12 +21,12 @@ class ImageDecoderImpl : public mojom::ImageDecoder {
~ImageDecoderImpl() override;
// Overridden from mojom::ImageDecoder:
void DecodeImage(
const std::vector<uint8_t>& encoded_data,
mojom::ImageCodec codec,
bool shrink_to_fit,
int64_t max_size_in_bytes,
const DecodeImageCallback& callback) override;
void DecodeImage(const std::vector<uint8_t>& encoded_data,
mojom::ImageCodec codec,
bool shrink_to_fit,
int64_t max_size_in_bytes,
const gfx::Size& desired_image_frame_size,
const DecodeImageCallback& callback) override;
private:
const std::unique_ptr<service_manager::ServiceContextRef> service_ref_;
......
......@@ -53,6 +53,7 @@ class Request {
void DecodeImage(const std::vector<unsigned char>& image, bool shrink) {
decoder_->DecodeImage(
image, mojom::ImageCodec::DEFAULT, shrink, kTestMaxImageSize,
gfx::Size(), // Take the smallest frame (there's only one frame).
base::Bind(&Request::OnRequestDone, base::Unretained(this)));
}
......
......@@ -35,6 +35,7 @@ void Decode(service_manager::Connector* connector,
mojom::ImageCodec codec,
bool shrink_to_fit,
uint64_t max_size_in_bytes,
const gfx::Size& desired_image_frame_size,
const mojom::ImageDecoder::DecodeImageCallback& callback) {
mojom::ImageDecoderPtr decoder;
connector->BindInterface(mojom::kServiceName, &decoder);
......@@ -43,6 +44,7 @@ void Decode(service_manager::Connector* connector,
mojom::ImageDecoder* raw_decoder = decoder.get();
raw_decoder->DecodeImage(
encoded_bytes, codec, shrink_to_fit, max_size_in_bytes,
desired_image_frame_size,
base::Bind(&OnDecodeImage, base::Passed(&decoder), callback));
}
......
......@@ -11,6 +11,10 @@
#include "services/image_decoder/public/interfaces/image_decoder.mojom.h"
namespace gfx {
class Size;
}
namespace service_manager {
class Connector;
}
......@@ -19,15 +23,21 @@ namespace image_decoder {
const uint64_t kDefaultMaxSizeInBytes = 128 * 1024 * 1024;
// Helper function to decode an image via the image_decoder service. Upon
// completion, |callback| is invoked on the calling thread TaskRunner with an
// SkBitmap argument. The SkBitmap will be null on failure and non-null on
// Helper function to decode an image via the image_decoder service. For images
// with multiple frames (e.g. ico files), a frame with a size as close as
// possible to |desired_image_frame_size| is chosen (tries to take one in larger
// size if there's no precise match). Passing gfx::Size() as
// |desired_image_frame_size| is also supported and will result in chosing the
// smallest available size.
// Upon completion, |callback| is invoked on the calling thread TaskRunner with
// an SkBitmap argument. The SkBitmap will be null on failure and non-null on
// success.
void Decode(service_manager::Connector* connector,
const std::vector<uint8_t>& encoded_bytes,
mojom::ImageCodec codec,
bool shrink_to_fit,
uint64_t max_size_in_bytes,
const gfx::Size& desired_image_frame_size,
const mojom::ImageDecoder::DecodeImageCallback& callback);
} // namespace image_decoder
......
......@@ -12,6 +12,7 @@ mojom("interfaces") {
public_deps = [
":constants",
"//skia/public/interfaces",
"//ui/gfx/geometry/mojo",
]
}
......
......@@ -5,6 +5,7 @@
module image_decoder.mojom;
import "skia/public/interfaces/bitmap.mojom";
import "ui/gfx/geometry/mojo/geometry.mojom";
enum ImageCodec {
DEFAULT,
......@@ -23,6 +24,6 @@ interface ImageDecoder {
// |max_size_in_bytes| and |shrink_to_fit| is false, this is treated as a
// decoding failure and the |decoded_image| response is null.