Restrict salient image size before storing

chrome/browser/enhanced_bookmarks/android/bookmark_image_service_android.cc

Index: chrome/browser/enhanced_bookmarks/android/bookmark_image_service_android.cc
diff --git a/chrome/browser/enhanced_bookmarks/android/bookmark_image_service_android.cc b/chrome/browser/enhanced_bookmarks/android/bookmark_image_service_android.cc
index 5671c783b2578bf443d94d567e3a09454003bfe0..c3f5c097526827315043b46fe7d8b23e6662a395 100644
--- a/chrome/browser/enhanced_bookmarks/android/bookmark_image_service_android.cc
+++ b/chrome/browser/enhanced_bookmarks/android/bookmark_image_service_android.cc
@@ -18,7 +18,10 @@
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/referrer.h"
 #include "net/base/load_flags.h"
+#include "skia/ext/image_operations.h"
 #include "ui/base/resource/resource_bundle.h"
+#include "ui/base/resource/resource_bundle.h"
+#include "ui/gfx/android/device_display_info.h"
 #include "ui/gfx/image/image_skia.h"
 
 using content::Referrer;
@@ -33,6 +36,28 @@ BookmarkImageServiceAndroid::BookmarkImageServiceAndroid(
           EnhancedBookmarkModelFactory::GetForBrowserContext(browserContext),
           make_scoped_refptr(content::BrowserThread::GetBlockingPool())),
       browser_context_(browserContext) {
+  // The images we're saving will be used locally. So it's wasteful to store
+  // images larger than the device resolution.
+  gfx::DeviceDisplayInfo display_info;
+  int max_length = std::max(display_info.GetPhysicalDisplayWidth(),
+                            display_info.GetPhysicalDisplayHeight());
+  // GetPhysicalDisplay*() returns 0 for pre-JB MR1. If so, fall back to the
+  // second best option we have.
+  if (max_length == 0) {
+    max_length = std::max(display_info.GetDisplayWidth(),
+                          display_info.GetDisplayHeight());
+  }
+  max_length /= display_info.GetDIPScale();
+  // For tablet, having an image larger than 600dip does not make sense, because
+  // we are showing it inside of a dialog, which only takes about 3/4 the
+  // display width. This restriction will not affect phones (we defined phones
+  // to be some devices having one dimension less than 600dip).
+  max_length = std::min(max_length, 600);
+  DCHECK(max_length > 0);
+  max_size_.reset(new gfx::Size(max_length, max_length));
+}
+
+BookmarkImageServiceAndroid::~BookmarkImageServiceAndroid() {
 }
 
 void BookmarkImageServiceAndroid::RetrieveSalientImage(
@@ -45,7 +70,7 @@ void BookmarkImageServiceAndroid::RetrieveSalientImage(
       enhanced_bookmark_model_->bookmark_model()
           ->GetMostRecentlyAddedUserNodeForURL(page_url);
   if (!bookmark || !image_url.is_valid()) {
-    ProcessNewImage(page_url, update_bookmark, gfx::Image(), image_url);
+    ProcessNewImage(page_url, update_bookmark, image_url, gfx::Image());
     return;
   }
 
@@ -184,6 +209,30 @@ void BookmarkImageServiceAndroid::RetrieveSalientImageFromContextCallback(
                        referrer_policy, update_bookmark);
 }
 
+gfx::Image BookmarkImageServiceAndroid::ResizeImage(gfx::Image image) {
+  DCHECK(!content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+
+  gfx::Image resized_uimage = image;

[Kibeom Kim (inactive), 2015/02/12 23:24:15]

Q: what does |uimage| stand for?

[Ian Wen, 2015/02/13 00:52:41]

Typo. Done.

+  if (max_size_->width() > 0 && max_size_->height() > 0 &&
+      (image.Width() > max_size_->width() ||
+       image.Height() > max_size_->height())) {
+    float resize_ratio =
+        std::min(((float)max_size_->width()) / image.Width(),
+                 ((float)max_size_->height()) / image.Height());
+    // +0.5f is for correct rounding. Without it, it's possible that dest_width
+    // is smaller than max_size_.Width() by one.
+    int dest_width = static_cast<int>(resize_ratio * image.Width() + 0.5f);
+    int dest_height = static_cast<int>(resize_ratio * image.Height() + 0.5f);

[Kibeom Kim (inactive), 2015/02/12 23:24:15]

Eventually, I think it's good to share computing dest_* size logic and the above
max_size_ logic with ios by putting at BookmarkImageService. I'm OK with leaving
here for now since there will be a follow up ios CL.

+
+    resized_uimage =
+        gfx::Image::CreateFrom1xBitmap(skia::ImageOperations::Resize(
+            image.AsBitmap(), skia::ImageOperations::RESIZE_BEST, dest_width,
+            dest_height));
+    resized_uimage.AsImageSkia().MakeThreadSafe();
+  }
+  return resized_uimage;
+}
+
 void BookmarkImageServiceAndroid::BitmapFetcherHandler::Start(
     content::BrowserContext* browser_context,
     const std::string& referrer,
@@ -209,7 +258,7 @@ void BookmarkImageServiceAndroid::BitmapFetcherHandler::OnFetchComplete(
     imageSkia.MakeThreadSafe();
     image = gfx::Image(imageSkia);
   }
-  service_->ProcessNewImage(page_url_, update_bookmark_, image, url);
+  service_->ProcessNewImage(page_url_, update_bookmark_, url, image);
 
   delete this;
 }