Large wallpaper decoding in utility process

chrome/utility/chrome_content_utility_client.cc

 // Copyright (c) 2012 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/utility/chrome_content_utility_client.h"
 
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/memory/ref_counted.h"
 #include "base/time/time.h"
 #include "chrome/common/chrome_utility_messages.h"
 #include "chrome/common/safe_browsing/zip_analyzer.h"
 #include "chrome/utility/chrome_content_utility_ipc_whitelist.h"
 #include "chrome/utility/utility_message_handler.h"
 #include "chrome/utility/web_resource_unpacker.h"
 #include "content/public/child/image_decoder_utils.h"
 #include "content/public/common/content_switches.h"
 #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"
 #include "ui/gfx/size.h"
 
 #if !defined(OS_ANDROID)
 #include "chrome/utility/profile_import_handler.h"
 #endif
 
 #if defined(OS_WIN)
@@ skipping 134 matching lines @@
 #endif
 
 #if defined(ENABLE_MDNS)
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kUtilityProcessEnableMDns)) {
     local_discovery::ServiceDiscoveryMessageHandler::PreSandboxStartup();
   }
 #endif  // ENABLE_MDNS
 }
 
+// Compute size of ChromeUtilityHostMsg_DecodeImage_Succeeded message with
+// image argument, and compare with maximum IPC message size
+size_t IsMessageTooBig(const SkBitmap& image, int width, int height) {
+  // Slightly different image size formula from SkImageInfo::getSafeSize64()
+  int64_t msg_size = sk_64_mul(height, width * image.bytesPerPixel());
+  msg_size += sizeof(ChromeUtilityHostMsg_DecodeImage_Succeeded);
+  return msg_size >= static_cast<int64_t>(IPC::Channel::kMaximumMessageSize);
+}
+
 // static
-void ChromeContentUtilityClient::DecodeImage(
-    const std::vector<unsigned char>& encoded_data) {
-  const SkBitmap& decoded_image = content::DecodeImage(&encoded_data[0],
+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());

[bshe, 2014/08/29 19:56:26]

nit: indent

[Greg Levin, 2014/09/04 16:39:19]

Done.

+
+  // If decoded image is too large for IPC message, shrink it by halves

[Tom Sepez, 2014/08/29 19:47:13]

looks like using SkBitmap::computeSize64() would avoid these calculations. And
if IsMessageTooBig took a size_t (or an int64_t to be consistent with Sk's
unfortunate use of signed types), it could avoid multiplying, too ...

[bshe, 2014/08/29 19:56:26]

Ideally, I think we should shrink to best fit. Not just shrink by halves.
This can avoid the over shrunk problem when the image is just slightly over the
limit. Do you mind to leave a TODO.

[Greg Levin, 2014/09/04 16:39:18]

Rewrote size checking logic, hopefully addressed all concerns here

[Greg Levin, 2014/09/04 16:39:19]

Discussed off-line, decided shrinking by halves was okay (right?).  Added
comment to code.

+  int width = decoded_image.width();
+  int height = decoded_image.height();
+  if (shrink_to_fit && IsMessageTooBig(decoded_image, width, height)) {
+    do {

[Tom Sepez, 2014/08/29 19:47:13]

A while loop may get rid of having to call IsMessageTooBig from two places in
this function.

[Greg Levin, 2014/09/04 16:39:18]

Done.

+      width = width/2;

[Tom Sepez, 2014/08/29 19:47:13]

... having done the above, then in this loop, you'd divide
SkBitmap::computeSize64() by 4 on each pass (since you're havling in each
dimension), keeping track of iterations ...

[Greg Levin, 2014/09/04 16:39:18]

Done.

+      height = height/2;

[bshe, 2014/08/29 19:56:26]

nit: width /= 2 or width = width / 2;. same for height.

[Greg Levin, 2014/09/04 16:39:18]

Replaced division with bit shifts

+    } while (IsMessageTooBig(decoded_image, width, height));
+    decoded_image = skia::ImageOperations::Resize(

[Tom Sepez, 2014/08/29 19:47:13]

Lastly, you can extract and divide width, height appropriately.

[Greg Levin, 2014/09/04 16:39:19]

Done.

+        decoded_image, skia::ImageOperations::RESIZE_LANCZOS3,
+        width, height);
+  }
+
+  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);
+

[bshe, 2014/08/29 19:56:26]

Add an error message for over IPC limit case would be nice.

[Greg Levin, 2014/09/04 16:39:19]

Improved error handling and LOG'ed error

   if (decoded_image.empty()) {
     Send(new ChromeUtilityHostMsg_DecodeImage_Failed());
   } else {
     Send(new ChromeUtilityHostMsg_DecodeImage_Succeeded(decoded_image));
   }
   ReleaseProcessIfNeeded();
 }
 
 void ChromeContentUtilityClient::OnUnpackWebResource(
     const std::string& resource_data) {
   // Parse json data.
   // TODO(mrc): Add the possibility of a template that controls parsing, and
   // the ability to download and verify images.
   WebResourceUnpacker unpacker(resource_data);
   if (unpacker.Run()) {
     Send(new ChromeUtilityHostMsg_UnpackWebResource_Succeeded(
         *unpacker.parsed_json()));
   } else {
     Send(new ChromeUtilityHostMsg_UnpackWebResource_Failed(
         unpacker.error_message()));
   }
 
   ReleaseProcessIfNeeded();
 }
 
 void ChromeContentUtilityClient::OnDecodeImage(
-    const std::vector<unsigned char>& encoded_data) {
-  DecodeImage(encoded_data);
+    const std::vector<unsigned char>& encoded_data, bool shrink_to_fit) {
+  DecodeImageAndSend(encoded_data, shrink_to_fit);
 }
 
 #if defined(OS_CHROMEOS)
 void ChromeContentUtilityClient::OnCreateZipFile(
     const base::FilePath& src_dir,
     const std::vector<base::FilePath>& src_relative_paths,
     const base::FileDescriptor& dest_fd) {
   bool succeeded = true;
 
   // Check sanity of source relative paths. Reject if path is absolute or
@@ skipping 91 matching lines @@
     const std::string& mime_type, int64 total_size, bool get_attached_images) {
   // Only one IPCDataSource may be created and added to the list of handlers.
   metadata::IPCDataSource* source = new metadata::IPCDataSource(total_size);
   handlers_.push_back(source);
 
   metadata::MediaMetadataParser* parser = new metadata::MediaMetadataParser(
       source, mime_type, get_attached_images);
   parser->Start(base::Bind(&FinishParseMediaMetadata, base::Owned(parser)));
 }
 #endif