Commit 4ef72fa4 authored by jiayl's avatar jiayl Committed by Commit bot

Revert of Large wallpaper decoding in utility process (patchset #5 id:190001...

Revert of Large wallpaper decoding in utility process (patchset #5 id:190001 of https://codereview.chromium.org/482163002/)

Reason for revert:
Speculative revert for unit_tests failures:

http://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%20Tests%20%28dbg%29%281%29/builds/31845/steps/unit_tests/logs/DecodeImageSizeLimit

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

TBR=dcheng@chromium.org,bshe@chromium.org,skuhne@chromium.org,sky@chromium.org,tsepez@chromium.org,yoz@chromium.org,glevin@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=366825

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

Cr-Commit-Position: refs/heads/master@{#296457}
parent 0106c710
......@@ -56,8 +56,6 @@ class WallpaperFunctionBase::UnsafeWallpaperDecoder
CHECK(chromeos::LoginState::Get()->IsUserLoggedIn());
unsafe_image_decoder_ = new ImageDecoder(this, image_data,
ImageDecoder::DEFAULT_CODEC);
unsafe_image_decoder_->set_shrink_to_fit(true);
scoped_refptr<base::MessageLoopProxy> task_runner =
BrowserThread::GetMessageLoopProxyForThread(BrowserThread::UI);
unsafe_image_decoder_->Start(task_runner);
......
......@@ -114,8 +114,7 @@ void WebstoreInstallHelper::OnURLFetchComplete(
void WebstoreInstallHelper::StartFetchedImageDecode() {
CHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
CHECK(utility_host_.get());
utility_host_->Send(new ChromeUtilityMsg_DecodeImage(fetched_icon_data_,
false));
utility_host_->Send(new ChromeUtilityMsg_DecodeImage(fetched_icon_data_));
}
......
......@@ -19,8 +19,7 @@ ImageDecoder::ImageDecoder(Delegate* delegate,
: delegate_(delegate),
image_data_(image_data.begin(), image_data.end()),
image_codec_(image_codec),
task_runner_(NULL),
shrink_to_fit_(false) {
task_runner_(NULL) {
}
ImageDecoder::~ImageDecoder() {}
......@@ -65,7 +64,6 @@ void ImageDecoder::DecodeImageInSandbox(
utility_process_host->Send(
new ChromeUtilityMsg_RobustJPEGDecodeImage(image_data));
} else {
utility_process_host->Send(
new ChromeUtilityMsg_DecodeImage(image_data, shrink_to_fit_));
utility_process_host->Send(new ChromeUtilityMsg_DecodeImage(image_data));
}
}
......@@ -52,7 +52,6 @@ class ImageDecoder : public content::UtilityProcessHostClient {
}
void set_delegate(Delegate* delegate) { delegate_ = delegate; }
void set_shrink_to_fit(bool shrink_to_fit) { shrink_to_fit_ = shrink_to_fit; }
private:
// It's a reference counted object, so destructor is private.
......@@ -72,7 +71,6 @@ class ImageDecoder : public content::UtilityProcessHostClient {
std::vector<unsigned char> image_data_;
const ImageCodec image_codec_;
scoped_refptr<base::SequencedTaskRunner> task_runner_;
bool shrink_to_fit_; // if needed for IPC msg size limit
DISALLOW_COPY_AND_ASSIGN(ImageDecoder);
};
......
......@@ -1382,7 +1382,6 @@
'test/data/unit/framework_unittest.gtestjs',
'test/logging/win/mof_data_parser_unittest.cc',
'tools/convert_dict/convert_dict_unittest.cc',
'utility/chrome_content_utility_client_unittest.cc',
'utility/cloud_print/pwg_encoder_unittest.cc',
'utility/extensions/unpacker_unittest.cc',
'utility/image_writer/image_writer_unittest.cc',
......
......@@ -65,9 +65,8 @@ IPC_MESSAGE_CONTROL1(ChromeUtilityMsg_UnpackWebResource,
std::string /* JSON data */)
// Tell the utility process to decode the given image data.
IPC_MESSAGE_CONTROL2(ChromeUtilityMsg_DecodeImage,
std::vector<unsigned char> /* encoded image contents */,
bool /* shrink image if needed for IPC msg limit */)
IPC_MESSAGE_CONTROL1(ChromeUtilityMsg_DecodeImage,
std::vector<unsigned char>) // encoded image contents
// Tell the utility process to decode the given JPEG image data with a robust
// libjpeg codec.
......
......@@ -18,8 +18,6 @@
#include "content/public/utility/utility_thread.h"
#include "courgette/courgette.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/zlib/google/zip.h"
#include "ui/gfx/codec/jpeg_codec.h"
......@@ -175,43 +173,11 @@ void ChromeContentUtilityClient::PreSandboxStartup() {
}
// static
SkBitmap ChromeContentUtilityClient::DecodeImage(
const std::vector<unsigned char>& encoded_data, bool shrink_to_fit) {
SkBitmap decoded_image = content::DecodeImage(&encoded_data[0],
gfx::Size(),
encoded_data.size());
int64_t max_msg_size = IPC::Channel::kMaximumMessageSize;
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_msg_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);
void ChromeContentUtilityClient::DecodeImage(
const std::vector<unsigned char>& encoded_data) {
const SkBitmap& decoded_image = content::DecodeImage(&encoded_data[0],
gfx::Size(),
encoded_data.size());
if (decoded_image.empty()) {
Send(new ChromeUtilityHostMsg_DecodeImage_Failed());
} else {
......@@ -238,8 +204,8 @@ void ChromeContentUtilityClient::OnUnpackWebResource(
}
void ChromeContentUtilityClient::OnDecodeImage(
const std::vector<unsigned char>& encoded_data, bool shrink_to_fit) {
DecodeImageAndSend(encoded_data, shrink_to_fit);
const std::vector<unsigned char>& encoded_data) {
DecodeImage(encoded_data);
}
#if defined(OS_CHROMEOS)
......
......@@ -32,16 +32,12 @@ class ChromeContentUtilityClient : public content::ContentUtilityClient {
static void PreSandboxStartup();
// Shared with extensions::ExtensionsHandler.
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 DecodeImage(const std::vector<unsigned char>& encoded_data);
private:
// IPC message handlers.
void OnUnpackWebResource(const std::string& resource_data);
void OnDecodeImage(const std::vector<unsigned char>& encoded_data,
bool shrink_to_fit);
void OnDecodeImage(const std::vector<unsigned char>& encoded_data);
void OnRobustJPEGDecodeImage(
const std::vector<unsigned char>& encoded_data);
......
// 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 > kMaximumMessageSize
TEST_F(ChromeContentUtilityClientTest, DecodeImageSizeLimit) {
// 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(IPC::Channel::kMaximumMessageSize/(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 - 20,
max_height_for_msg + 20,
2 * max_height_for_msg + 20 };
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>(IPC::Channel::kMaximumMessageSize));
// 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
}
}
......@@ -182,7 +182,7 @@ void ExtensionsHandler::OnDecodeImageBase64(
decoded_vector[i] = static_cast<unsigned char>(decoded_string[i]);
}
ChromeContentUtilityClient::DecodeImageAndSend(decoded_vector, false);
ChromeContentUtilityClient::DecodeImage(decoded_vector);
}
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