Commit b18f4aa7 authored by glevin's avatar glevin Committed by Commit bot

Handle large wallpaper decoding in utility process:

Images over 32M pixels exceed IPC message limit size.  In ChromeContentUtilityClient::DecodeImage(), cut image dimensions in half to minimize quality loss and allow decoded image to be returned via IPC message.

BUG=366825
TEST=ChromeContentUtilityClientTest.DecodeImageSizeLimit; Set ChromeOS wallpaper using JPGs attached to bug

Committed: https://crrev.com/d9d0e601c4c45ec8f2fb1d148e68458d9baa4e24
Cr-Commit-Position: refs/heads/master@{#296407}

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

Cr-Commit-Position: refs/heads/master@{#296939}
parent 0713c994
...@@ -56,6 +56,8 @@ class WallpaperFunctionBase::UnsafeWallpaperDecoder ...@@ -56,6 +56,8 @@ class WallpaperFunctionBase::UnsafeWallpaperDecoder
CHECK(chromeos::LoginState::Get()->IsUserLoggedIn()); CHECK(chromeos::LoginState::Get()->IsUserLoggedIn());
unsafe_image_decoder_ = new ImageDecoder(this, image_data, unsafe_image_decoder_ = new ImageDecoder(this, image_data,
ImageDecoder::DEFAULT_CODEC); ImageDecoder::DEFAULT_CODEC);
unsafe_image_decoder_->set_shrink_to_fit(true);
scoped_refptr<base::MessageLoopProxy> task_runner = scoped_refptr<base::MessageLoopProxy> task_runner =
BrowserThread::GetMessageLoopProxyForThread(BrowserThread::UI); BrowserThread::GetMessageLoopProxyForThread(BrowserThread::UI);
unsafe_image_decoder_->Start(task_runner); unsafe_image_decoder_->Start(task_runner);
......
...@@ -114,7 +114,8 @@ void WebstoreInstallHelper::OnURLFetchComplete( ...@@ -114,7 +114,8 @@ void WebstoreInstallHelper::OnURLFetchComplete(
void WebstoreInstallHelper::StartFetchedImageDecode() { void WebstoreInstallHelper::StartFetchedImageDecode() {
CHECK(BrowserThread::CurrentlyOn(BrowserThread::IO)); CHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
CHECK(utility_host_.get()); CHECK(utility_host_.get());
utility_host_->Send(new ChromeUtilityMsg_DecodeImage(fetched_icon_data_)); utility_host_->Send(new ChromeUtilityMsg_DecodeImage(fetched_icon_data_,
false));
} }
......
...@@ -19,7 +19,8 @@ ImageDecoder::ImageDecoder(Delegate* delegate, ...@@ -19,7 +19,8 @@ ImageDecoder::ImageDecoder(Delegate* delegate,
: delegate_(delegate), : delegate_(delegate),
image_data_(image_data.begin(), image_data.end()), image_data_(image_data.begin(), image_data.end()),
image_codec_(image_codec), image_codec_(image_codec),
task_runner_(NULL) { task_runner_(NULL),
shrink_to_fit_(false) {
} }
ImageDecoder::~ImageDecoder() {} ImageDecoder::~ImageDecoder() {}
...@@ -64,6 +65,7 @@ void ImageDecoder::DecodeImageInSandbox( ...@@ -64,6 +65,7 @@ void ImageDecoder::DecodeImageInSandbox(
utility_process_host->Send( utility_process_host->Send(
new ChromeUtilityMsg_RobustJPEGDecodeImage(image_data)); new ChromeUtilityMsg_RobustJPEGDecodeImage(image_data));
} else { } else {
utility_process_host->Send(new ChromeUtilityMsg_DecodeImage(image_data)); utility_process_host->Send(
new ChromeUtilityMsg_DecodeImage(image_data, shrink_to_fit_));
} }
} }
...@@ -52,6 +52,7 @@ class ImageDecoder : public content::UtilityProcessHostClient { ...@@ -52,6 +52,7 @@ class ImageDecoder : public content::UtilityProcessHostClient {
} }
void set_delegate(Delegate* delegate) { delegate_ = delegate; } void set_delegate(Delegate* delegate) { delegate_ = delegate; }
void set_shrink_to_fit(bool shrink_to_fit) { shrink_to_fit_ = shrink_to_fit; }
private: private:
// It's a reference counted object, so destructor is private. // It's a reference counted object, so destructor is private.
...@@ -71,6 +72,7 @@ class ImageDecoder : public content::UtilityProcessHostClient { ...@@ -71,6 +72,7 @@ class ImageDecoder : public content::UtilityProcessHostClient {
std::vector<unsigned char> image_data_; std::vector<unsigned char> image_data_;
const ImageCodec image_codec_; const ImageCodec image_codec_;
scoped_refptr<base::SequencedTaskRunner> task_runner_; scoped_refptr<base::SequencedTaskRunner> task_runner_;
bool shrink_to_fit_; // if needed for IPC msg size limit
DISALLOW_COPY_AND_ASSIGN(ImageDecoder); DISALLOW_COPY_AND_ASSIGN(ImageDecoder);
}; };
......
...@@ -1382,6 +1382,7 @@ ...@@ -1382,6 +1382,7 @@
'test/data/unit/framework_unittest.gtestjs', 'test/data/unit/framework_unittest.gtestjs',
'test/logging/win/mof_data_parser_unittest.cc', 'test/logging/win/mof_data_parser_unittest.cc',
'tools/convert_dict/convert_dict_unittest.cc', 'tools/convert_dict/convert_dict_unittest.cc',
'utility/chrome_content_utility_client_unittest.cc',
'utility/cloud_print/pwg_encoder_unittest.cc', 'utility/cloud_print/pwg_encoder_unittest.cc',
'utility/extensions/unpacker_unittest.cc', 'utility/extensions/unpacker_unittest.cc',
'utility/image_writer/image_writer_unittest.cc', 'utility/image_writer/image_writer_unittest.cc',
......
...@@ -65,8 +65,9 @@ IPC_MESSAGE_CONTROL1(ChromeUtilityMsg_UnpackWebResource, ...@@ -65,8 +65,9 @@ IPC_MESSAGE_CONTROL1(ChromeUtilityMsg_UnpackWebResource,
std::string /* JSON data */) std::string /* JSON data */)
// Tell the utility process to decode the given image data. // Tell the utility process to decode the given image data.
IPC_MESSAGE_CONTROL1(ChromeUtilityMsg_DecodeImage, IPC_MESSAGE_CONTROL2(ChromeUtilityMsg_DecodeImage,
std::vector<unsigned char>) // encoded image contents std::vector<unsigned char> /* encoded image contents */,
bool /* shrink image if needed for IPC msg limit */)
// Tell the utility process to decode the given JPEG image data with a robust // Tell the utility process to decode the given JPEG image data with a robust
// libjpeg codec. // libjpeg codec.
......
...@@ -18,6 +18,8 @@ ...@@ -18,6 +18,8 @@
#include "content/public/utility/utility_thread.h" #include "content/public/utility/utility_thread.h"
#include "courgette/courgette.h" #include "courgette/courgette.h"
#include "courgette/third_party/bsdiff.h" #include "courgette/third_party/bsdiff.h"
#include "ipc/ipc_channel.h"
#include "skia/ext/image_operations.h"
#include "third_party/skia/include/core/SkBitmap.h" #include "third_party/skia/include/core/SkBitmap.h"
#include "third_party/zlib/google/zip.h" #include "third_party/zlib/google/zip.h"
#include "ui/gfx/codec/jpeg_codec.h" #include "ui/gfx/codec/jpeg_codec.h"
...@@ -70,6 +72,9 @@ void FinishParseMediaMetadata( ...@@ -70,6 +72,9 @@ void FinishParseMediaMetadata(
} // namespace } // namespace
int64_t ChromeContentUtilityClient::max_ipc_message_size_ =
IPC::Channel::kMaximumMessageSize;
ChromeContentUtilityClient::ChromeContentUtilityClient() ChromeContentUtilityClient::ChromeContentUtilityClient()
: filter_messages_(false) { : filter_messages_(false) {
#if !defined(OS_ANDROID) #if !defined(OS_ANDROID)
...@@ -173,11 +178,42 @@ void ChromeContentUtilityClient::PreSandboxStartup() { ...@@ -173,11 +178,42 @@ void ChromeContentUtilityClient::PreSandboxStartup() {
} }
// static // static
void ChromeContentUtilityClient::DecodeImage( SkBitmap ChromeContentUtilityClient::DecodeImage(
const std::vector<unsigned char>& encoded_data) { const std::vector<unsigned char>& encoded_data, bool shrink_to_fit) {
const SkBitmap& decoded_image = content::DecodeImage(&encoded_data[0], SkBitmap decoded_image = content::DecodeImage(&encoded_data[0],
gfx::Size(), gfx::Size(),
encoded_data.size()); encoded_data.size());
int64_t struct_size = sizeof(ChromeUtilityHostMsg_DecodeImage_Succeeded);
int64_t image_size = decoded_image.computeSize64();
int halves = 0;
while (struct_size + (image_size >> 2*halves) > max_ipc_message_size_)
halves++;
if (halves) {
if (shrink_to_fit) {
// If decoded image is too large for IPC message, shrink it by halves.
// This prevents quality loss, and should never overshrink on displays
// smaller than 3600x2400.
// TODO (Issue 416916): Instead of shrinking, return via shared memory
decoded_image = skia::ImageOperations::Resize(
decoded_image, skia::ImageOperations::RESIZE_LANCZOS3,
decoded_image.width() >> halves, decoded_image.height() >> halves);
} else {
// Image too big for IPC message, but caller didn't request resize;
// pre-delete image so DecodeImageAndSend() will send an error.
decoded_image.reset();
LOG(ERROR) << "Decoded image too large for IPC message";
}
}
return decoded_image;
}
// static
void ChromeContentUtilityClient::DecodeImageAndSend(
const std::vector<unsigned char>& encoded_data, bool shrink_to_fit){
SkBitmap decoded_image = DecodeImage(encoded_data, shrink_to_fit);
if (decoded_image.empty()) { if (decoded_image.empty()) {
Send(new ChromeUtilityHostMsg_DecodeImage_Failed()); Send(new ChromeUtilityHostMsg_DecodeImage_Failed());
} else { } else {
...@@ -204,8 +240,8 @@ void ChromeContentUtilityClient::OnUnpackWebResource( ...@@ -204,8 +240,8 @@ void ChromeContentUtilityClient::OnUnpackWebResource(
} }
void ChromeContentUtilityClient::OnDecodeImage( void ChromeContentUtilityClient::OnDecodeImage(
const std::vector<unsigned char>& encoded_data) { const std::vector<unsigned char>& encoded_data, bool shrink_to_fit) {
DecodeImage(encoded_data); DecodeImageAndSend(encoded_data, shrink_to_fit);
} }
#if defined(OS_CHROMEOS) #if defined(OS_CHROMEOS)
......
...@@ -32,12 +32,20 @@ class ChromeContentUtilityClient : public content::ContentUtilityClient { ...@@ -32,12 +32,20 @@ class ChromeContentUtilityClient : public content::ContentUtilityClient {
static void PreSandboxStartup(); static void PreSandboxStartup();
// Shared with extensions::ExtensionsHandler. // Shared with extensions::ExtensionsHandler.
static void DecodeImage(const std::vector<unsigned char>& encoded_data); static SkBitmap DecodeImage(const std::vector<unsigned char>& encoded_data,
bool shrink_to_fit);
static void DecodeImageAndSend(const std::vector<unsigned char>& encoded_data,
bool shrink_to_fit);
static void set_max_ipc_message_size_for_test(int64_t max_message_size) {
max_ipc_message_size_ = max_message_size;
}
private: private:
// IPC message handlers. // IPC message handlers.
void OnUnpackWebResource(const std::string& resource_data); void OnUnpackWebResource(const std::string& resource_data);
void OnDecodeImage(const std::vector<unsigned char>& encoded_data); void OnDecodeImage(const std::vector<unsigned char>& encoded_data,
bool shrink_to_fit);
void OnRobustJPEGDecodeImage( void OnRobustJPEGDecodeImage(
const std::vector<unsigned char>& encoded_data); const std::vector<unsigned char>& encoded_data);
...@@ -71,6 +79,8 @@ class ChromeContentUtilityClient : public content::ContentUtilityClient { ...@@ -71,6 +79,8 @@ class ChromeContentUtilityClient : public content::ContentUtilityClient {
bool filter_messages_; bool filter_messages_;
// A list of message_ids to filter. // A list of message_ids to filter.
std::set<int> message_id_whitelist_; std::set<int> message_id_whitelist_;
// Maximum IPC msg size (default to kMaximumMessageSize; override for testing)
static int64_t max_ipc_message_size_;
DISALLOW_COPY_AND_ASSIGN(ChromeContentUtilityClient); DISALLOW_COPY_AND_ASSIGN(ChromeContentUtilityClient);
}; };
......
// Copyright 2014 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 "chrome/common/chrome_utility_messages.h"
#include "chrome/utility/chrome_content_utility_client.h"
#include "ipc/ipc_channel.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "ui/gfx/codec/jpeg_codec.h"
namespace {
bool CreateJPEGImage(int width,
int height,
SkColor color,
std::vector<unsigned char>* output) {
SkBitmap bitmap;
bitmap.allocN32Pixels(width, height);
bitmap.eraseColor(color);
const int kQuality = 50;
if (!gfx::JPEGCodec::Encode(
static_cast<const unsigned char*>(bitmap.getPixels()),
gfx::JPEGCodec::FORMAT_SkBitmap,
width,
height,
bitmap.rowBytes(),
kQuality,
output)) {
LOG(ERROR) << "Unable to encode " << width << "x" << height << " bitmap";
return false;
}
return true;
}
} // namespace
typedef testing::Test ChromeContentUtilityClientTest;
// Test that DecodeImage() doesn't return image message > (max message size)
TEST_F(ChromeContentUtilityClientTest, DecodeImageSizeLimit) {
// Using actual limit generates 14000 x 9400 images, which causes the test to
// timeout. We test with a smaller limit for efficiency.
const size_t kTestMessageSize = IPC::Channel::kMaximumMessageSize / 1024;
ChromeContentUtilityClient::set_max_ipc_message_size_for_test(
kTestMessageSize);
// Approx max height for 3:2 image that will fit in IPC message;
// 1.5 for width/height ratio, 4 for bytes/pixel
int max_height_for_msg = sqrt(kTestMessageSize / (1.5 * 4));
int base_msg_size = sizeof(ChromeUtilityHostMsg_DecodeImage_Succeeded);
// Sizes which should trigger dimension-halving 0, 1 and 2 times
int heights[] = { max_height_for_msg - 10,
max_height_for_msg + 10,
2 * max_height_for_msg + 10 };
int widths[] = { heights[0] * 3 / 2, heights[1] * 3 / 2, heights[2] * 3 / 2 };
for (size_t i = 0; i < arraysize(heights); i++) {
std::vector<unsigned char> jpg;
CreateJPEGImage(widths[i], heights[i], SK_ColorRED, &jpg);
SkBitmap bitmap = ChromeContentUtilityClient::DecodeImage(jpg, true);
// Check that image has been shrunk appropriately
EXPECT_LT(bitmap.computeSize64() + base_msg_size,
static_cast<int64_t>(kTestMessageSize));
// Android does its own image shrinking for memory conservation deeper in
// the decode, so more specific tests here won't work.
#if !defined(OS_ANDROID)
EXPECT_EQ(widths[i] >> i, bitmap.width());
EXPECT_EQ(heights[i] >> i, bitmap.height());
// Check that if resize not requested and image exceeds IPC size limit,
// an empty image is returned
if (heights[i] > max_height_for_msg) {
SkBitmap empty_bmp = ChromeContentUtilityClient::DecodeImage(jpg, false);
EXPECT_TRUE(empty_bmp.empty());
}
#endif
}
}
...@@ -186,7 +186,7 @@ void ExtensionsHandler::OnDecodeImageBase64( ...@@ -186,7 +186,7 @@ void ExtensionsHandler::OnDecodeImageBase64(
decoded_vector[i] = static_cast<unsigned char>(decoded_string[i]); decoded_vector[i] = static_cast<unsigned char>(decoded_string[i]);
} }
ChromeContentUtilityClient::DecodeImage(decoded_vector); ChromeContentUtilityClient::DecodeImageAndSend(decoded_vector, false);
} }
void ExtensionsHandler::OnParseJSON(const std::string& json) { void ExtensionsHandler::OnParseJSON(const std::string& json) {
......
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