Revert "[blink/renderer] Fix image download logic in renderer to filter instead"
This reverts commit 45351855e193fbb7caee43ab2da8178a490a41d6.
Reason for revert: likely causes crashes on perf bots (crbug/1352191)
Original change's description:
> [blink/renderer] Fix image download logic in renderer to filter instead
> of resize
>
> This CL introduces logic in blink/renderer to filter out images instead
> of resizing the smallest image in case no images are found about a
> specific max size. This also moves the filter logic into the callsites
> instead of renderer, so that this is handler on a use-case basis.
>
> Doc: go/blink_image
>
> Bug: 1176678, 1348264
> Change-Id: If1cae73c5f5bfb2146156a6872dac5d4b41aa577
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3781583
> Commit-Queue: Dibyajyoti Pal <[email protected]>
> Reviewed-by: Daniel Cheng <[email protected]>
> Reviewed-by: Dominick Ng <[email protected]>
> Reviewed-by: Avi Drissman <[email protected]>
> Reviewed-by: Nate Fischer <[email protected]>
> Reviewed-by: Scott Violet <[email protected]>
> Reviewed-by: Yaron Friedman <[email protected]>
> Cr-Commit-Position: refs/heads/main@{#1033659}
Bug: 1176678, 1348264
Bug: 1352191
Change-Id: I50dc02aa3411f9e921dda59bad5f7da3daa55406
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3825876
Bot-Commit: Rubber Stamper <[email protected]>
Reviewed-by: Dibyajyoti Pal <[email protected]>
Auto-Submit: Eric Seckler <[email protected]>
Reviewed-by: Avi Drissman <[email protected]>
Owners-Override: Avi Drissman <[email protected]>
Commit-Queue: Avi Drissman <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1034081}
diff --git a/android_webview/browser/icon_helper.cc b/android_webview/browser/icon_helper.cc
index 8247e795..7e3b3b1 100644
--- a/android_webview/browser/icon_helper.cc
+++ b/android_webview/browser/icon_helper.cc
@@ -4,8 +4,6 @@
#include "android_webview/browser/icon_helper.h"
-#include <vector>
-
#include "base/bind.h"
#include "base/callback.h"
#include "base/check_op.h"
@@ -18,7 +16,6 @@
#include "third_party/blink/public/mojom/favicon/favicon_url.mojom.h"
#include "third_party/skia/include/core/SkBitmap.h"
#include "ui/gfx/geometry/size.h"
-#include "ui/gfx/image/image_util.h"
using content::BrowserThread;
using content::WebContents;
@@ -55,13 +52,7 @@
return;
}
- std::vector<SkBitmap> filtered_images;
- std::vector<gfx::Size> filtered_bitmap_sizes;
- gfx::FilterAndResizeImagesForMaximalSize(
- bitmaps, kLargestIconSize, filtered_images, filtered_bitmap_sizes);
- DCHECK_EQ(filtered_images.size(), filtered_bitmap_sizes.size());
-
- if (filtered_images.size() == 0) {
+ if (bitmaps.size() == 0) {
return;
}
@@ -71,13 +62,13 @@
if (listener_) {
std::vector<size_t> best_indices;
- SelectFaviconFrameIndices(filtered_bitmap_sizes,
+ SelectFaviconFrameIndices(original_bitmap_sizes,
std::vector<int>(1U, kLargestIconSize),
&best_indices, nullptr);
listener_->OnReceivedIcon(
image_url,
- filtered_images[best_indices.size() == 0 ? 0 : best_indices.front()]);
+ bitmaps[best_indices.size() == 0 ? 0 : best_indices.front()]);
}
}
@@ -98,10 +89,10 @@
}
web_contents()->DownloadImage(
candidate->icon_url,
- true, // Is a favicon
- gfx::Size(), // No preferred size
- 0, // Max bitmap size - 0 means unlimited
- false, // Normal cache policy
+ true, // Is a favicon
+ gfx::Size(), // No preferred size
+ kLargestIconSize, // Max bitmap size
+ false, // Normal cache policy
base::BindOnce(&IconHelper::DownloadFaviconCallback,
base::Unretained(this)));
break;
diff --git a/components/favicon/content/content_favicon_driver.cc b/components/favicon/content/content_favicon_driver.cc
index f185a4f..13025e8 100644
--- a/components/favicon/content/content_favicon_driver.cc
+++ b/components/favicon/content/content_favicon_driver.cc
@@ -96,15 +96,15 @@
}
int ContentFaviconDriver::DownloadImage(const GURL& url,
- int preferred_size,
+ int max_image_size,
ImageDownloadCallback callback) {
bool bypass_cache = (bypass_cache_page_url_ == GetActiveURL());
bypass_cache_page_url_ = GURL();
- const gfx::Size preferred_size_constraints(preferred_size, preferred_size);
- return web_contents()->DownloadImage(url, true, preferred_size_constraints,
- /*max_image_size = */ 0, bypass_cache,
- std::move(callback));
+ const gfx::Size preferred_size(max_image_size, max_image_size);
+ return web_contents()->DownloadImage(url, true, preferred_size,
+ /*max_bitmap_size=*/max_image_size,
+ bypass_cache, std::move(callback));
}
void ContentFaviconDriver::DownloadManifest(const GURL& url,
diff --git a/components/favicon/content/content_favicon_driver.h b/components/favicon/content/content_favicon_driver.h
index 6cf0a23..000ac24 100644
--- a/components/favicon/content/content_favicon_driver.h
+++ b/components/favicon/content/content_favicon_driver.h
@@ -74,7 +74,7 @@
// FaviconHandler::Delegate implementation.
int DownloadImage(const GURL& url,
- int preferred_size,
+ int max_image_size,
ImageDownloadCallback callback) override;
void DownloadManifest(const GURL& url,
ManifestDownloadCallback callback) override;
diff --git a/components/favicon/core/favicon_handler.cc b/components/favicon/core/favicon_handler.cc
index 2bea93f..56f54be5 100644
--- a/components/favicon/core/favicon_handler.cc
+++ b/components/favicon/core/favicon_handler.cc
@@ -22,7 +22,6 @@
#include "components/favicon_base/favicon_util.h"
#include "components/favicon_base/select_favicon_frames.h"
#include "skia/ext/image_operations.h"
-#include "third_party/skia/include/core/SkBitmap.h"
#include "ui/gfx/codec/png_codec.h"
#include "ui/gfx/image/image_skia.h"
#include "ui/gfx/image/image_util.h"
@@ -714,8 +713,9 @@
image_download_request_.Reset(
base::BindOnce(&FaviconHandler::OnDidDownloadFavicon,
base::Unretained(this), icon_type));
- // The maximal icon size is passed in to set the preferred size for vector
- // images. See FaviconHandler::Delegate::DownloadImage() for more info.
+ // A max bitmap size is specified to avoid receiving huge bitmaps in
+ // OnDidDownloadFavicon(). See FaviconDriver::StartDownload()
+ // for more details about the max bitmap size.
const int download_id = delegate_->DownloadImage(
image_url, GetMaximalIconSize(handler_type_, !manifest_url_.is_empty()),
image_download_request_.callback());
diff --git a/components/favicon/core/favicon_handler.h b/components/favicon/core/favicon_handler.h
index 7e2dc5f..2be71b2 100644
--- a/components/favicon/core/favicon_handler.h
+++ b/components/favicon/core/favicon_handler.h
@@ -93,14 +93,12 @@
// is called with the results. Returns the unique id of the download
// request, which will also be passed to the callback. In case of error, 0
// is returned and no callback will be called.
- // For vector images, |preferred_size| will serve as a viewport into which
- // the image will be rendered. This would usually be the dimensions of the
- // rectangle where the bitmap will be rendered. If |preferred_size| is
- // empty, any existing intrinsic dimensions of the image will be used. All
- // images are downloaded (without filtering by size), and all filtering is
- // handled by the utility functions inside select_favicon_frames.cc.
+ // Bitmaps with pixel sizes larger than |max_bitmap_size| are filtered out
+ // from the bitmap results. If there are no bitmap results <=
+ // |max_bitmap_size|, the smallest bitmap is resized to |max_bitmap_size|
+ // and is the only result. A |max_bitmap_size| of 0 means unlimited.
virtual int DownloadImage(const GURL& url,
- int preferred_size,
+ int max_image_size,
ImageDownloadCallback callback) = 0;
// Downloads a WebManifest and returns the favicons listed there.
diff --git a/components/favicon/core/favicon_handler_unittest.cc b/components/favicon/core/favicon_handler_unittest.cc
index 5736a55..7530efa 100644
--- a/components/favicon/core/favicon_handler_unittest.cc
+++ b/components/favicon/core/favicon_handler_unittest.cc
@@ -134,12 +134,21 @@
// Implementation of FaviconHalder::Delegate's DownloadImage(). If a given
// URL is not known (i.e. not previously added via Add()), it produces 404s.
int DownloadImage(const GURL& url,
- int preferred_size,
+ int max_image_size,
FaviconHandler::Delegate::ImageDownloadCallback callback) {
downloads_->push_back(url);
Response response = responses_[url];
DCHECK_EQ(response.bitmaps.size(), response.original_bitmap_sizes.size());
+ // Apply maximum image size.
+ for (size_t i = 0; i < response.bitmaps.size(); ++i) {
+ if (response.bitmaps[i].width() > max_image_size ||
+ response.bitmaps[i].height() > max_image_size) {
+ response.bitmaps[i] = skia::ImageOperations::Resize(
+ response.bitmaps[i], skia::ImageOperations::RESIZE_LANCZOS3,
+ max_image_size, max_image_size);
+ }
+ }
int download_id = next_download_id_++;
base::OnceClosure bound_callback = base::BindOnce(
@@ -290,9 +299,9 @@
}
int DownloadImage(const GURL& url,
- int preferred_size,
+ int max_image_size,
ImageDownloadCallback callback) override {
- return fake_image_downloader_.DownloadImage(url, preferred_size,
+ return fake_image_downloader_.DownloadImage(url, max_image_size,
std::move(callback));
}
@@ -1718,7 +1727,7 @@
EXPECT_THAT(delegate_.downloads(), ElementsAre(kIconURL2));
}
-TEST_F(FaviconHandlerTest, TestFaviconNotScaledButSelectedCorrectly) {
+TEST_F(FaviconHandlerTest, TestFaviconWasScaledAfterDownload) {
const int kMaximalSize = FaviconHandler::GetMaximalIconSize(
FaviconDriverObserver::NON_TOUCH_LARGEST,
/*candidates_from_web_manifest=*/false);
@@ -1735,10 +1744,10 @@
SK_ColorBLUE);
// Verify the best bitmap was selected (although smaller than |kIconURL2|)
- // and that it was downloaded without any scaling.
+ // and that it was scaled down to |kMaximalSize|.
EXPECT_CALL(delegate_,
OnFaviconUpdated(_, _, kIconURL1, _,
- ImageSizeIs(kOriginalSize1, kOriginalSize1)));
+ ImageSizeIs(kMaximalSize, kMaximalSize)));
RunHandlerWithCandidates(
FaviconDriverObserver::NON_TOUCH_LARGEST,
diff --git a/components/favicon/ios/web_favicon_driver.h b/components/favicon/ios/web_favicon_driver.h
index 9657294..84bbb2a 100644
--- a/components/favicon/ios/web_favicon_driver.h
+++ b/components/favicon/ios/web_favicon_driver.h
@@ -41,7 +41,7 @@
// FaviconHandler::Delegate implementation.
int DownloadImage(const GURL& url,
- int preferred_size,
+ int max_image_size,
ImageDownloadCallback callback) override;
void DownloadManifest(const GURL& url,
ManifestDownloadCallback callback) override;
diff --git a/components/favicon/ios/web_favicon_driver.mm b/components/favicon/ios/web_favicon_driver.mm
index 150c776..00ca6e83 100644
--- a/components/favicon/ios/web_favicon_driver.mm
+++ b/components/favicon/ios/web_favicon_driver.mm
@@ -48,7 +48,7 @@
}
int WebFaviconDriver::DownloadImage(const GURL& url,
- int preferred_size,
+ int max_image_size,
ImageDownloadCallback callback) {
static int downloaded_image_count = 0;
int local_download_id = ++downloaded_image_count;
@@ -65,10 +65,7 @@
std::vector<SkBitmap> frames;
std::vector<gfx::Size> sizes;
if (data) {
- // From FaviconHandler::ScheduleImageDownload, the preferred_size
- // contains the maximal size while the max_image_size is set to 0 to
- // allow all files to be downloaded by the blink renderer.
- frames = skia::ImageDataToSkBitmapsWithMaxSize(data, preferred_size);
+ frames = skia::ImageDataToSkBitmapsWithMaxSize(data, max_image_size);
for (const auto& frame : frames) {
sizes.push_back(gfx::Size(frame.width(), frame.height()));
}
diff --git a/components/webapps/browser/installable/installable_manager.cc b/components/webapps/browser/installable/installable_manager.cc
index c85ce7a..8af8c3d6 100644
--- a/components/webapps/browser/installable/installable_manager.cc
+++ b/components/webapps/browser/installable/installable_manager.cc
@@ -922,12 +922,14 @@
? kMinimumScreenshotSizeInPx
: std::max(url->image.sizes[0].width(),
url->image.sizes[0].height());
- // Passing in kMaximumScreenshotSizeInPx ensures that screenshots above that
- // size are filtered out by the blink image_downloader implementation.
- // Thus, we no longer need them to be resized in OnScreenshotFetched().
+ // Do not pass in a maximum icon size so that screenshots larger than
+ // kMaximumScreenshotSizeInPx are not downscaled to the maximum size by
+ // `ManifestIconDownloader::Download`. Screenshots with size larger than
+ // kMaximumScreenshotSizeInPx get filtered out by OnScreenshotFetched.
bool can_download = content::ManifestIconDownloader::Download(
GetWebContents(), url->image.src, ideal_size_in_px,
- kMinimumScreenshotSizeInPx, kMaximumScreenshotSizeInPx,
+ kMinimumScreenshotSizeInPx,
+ /*maximum_icon_size_in_px=*/0,
base::BindOnce(&InstallableManager::OnScreenshotFetched,
weak_factory_.GetWeakPtr(), url->image.src),
/*square_only=*/false);
@@ -969,7 +971,12 @@
auto iter = downloaded_screenshots_.find(url->image.src);
if (iter == downloaded_screenshots_.end())
continue;
+
auto screenshot = iter->second;
+ if (screenshot.dimensions().width() > kMaximumScreenshotSizeInPx ||
+ screenshot.dimensions().height() > kMaximumScreenshotSizeInPx) {
+ continue;
+ }
// Screenshots must have the same aspect ratio. Cross-multiplying
// dimensions checks portrait vs landscape mode (1:2 vs 2:1 for instance).
diff --git a/content/browser/media/session/media_session_impl.cc b/content/browser/media/session/media_session_impl.cc
index 6c7be7d..2096290 100644
--- a/content/browser/media/session/media_session_impl.cc
+++ b/content/browser/media/session/media_session_impl.cc
@@ -7,12 +7,10 @@
#include <algorithm>
#include <memory>
#include <utility>
-#include <vector>
#include "base/bind.h"
#include "base/containers/contains.h"
#include "base/cxx17_backports.h"
-#include "base/numerics/safe_conversions.h"
#include "base/ranges/algorithm.h"
#include "base/strings/string_util.h"
#include "base/time/time.h"
@@ -41,9 +39,7 @@
#include "services/media_session/public/mojom/audio_focus.mojom.h"
#include "third_party/blink/public/mojom/favicon/favicon_url.mojom.h"
#include "third_party/blink/public/strings/grit/blink_strings.h"
-#include "third_party/skia/include/core/SkBitmap.h"
#include "ui/gfx/favicon_size.h"
-#include "ui/gfx/image/image_util.h"
#if BUILDFLAG(IS_ANDROID)
#include "content/browser/media/session/media_session_android.h"
@@ -869,25 +865,17 @@
const GURL& image_url,
const std::vector<SkBitmap>& bitmaps,
const std::vector<gfx::Size>& sizes) {
- DCHECK_EQ(bitmaps.size(), sizes.size());
-
- std::vector<SkBitmap> filtered_images;
- std::vector<gfx::Size> filtered_bitmap_sizes;
- gfx::FilterAndResizeImagesForMaximalSize(
- bitmaps, base::checked_cast<uint32_t>(desired_size_px), filtered_images,
- filtered_bitmap_sizes);
- DCHECK_EQ(filtered_images.size(), filtered_bitmap_sizes.size());
-
+ DCHECK(bitmaps.size() == sizes.size());
SkBitmap image;
double best_image_score = 0.0;
- // Rank |filtered_bitmap_sizes| and |filtered_images| using MediaImageManager.
- for (size_t i = 0; i < filtered_images.size(); i++) {
+ // Rank |sizes| and |bitmaps| using MediaImageManager.
+ for (size_t i = 0; i < bitmaps.size(); i++) {
double image_score = media_session::MediaImageManager::GetImageSizeScore(
- minimum_size_px, desired_size_px, filtered_bitmap_sizes.at(i));
+ minimum_size_px, desired_size_px, sizes.at(i));
if (image_score > best_image_score)
- image = filtered_images.at(i);
+ image = bitmaps.at(i);
}
// If the image is the wrong color type then we should convert it.
@@ -1334,7 +1322,7 @@
const gfx::Size preferred_size(desired_size_px, desired_size_px);
web_contents()->DownloadImage(
image.src, false /* is_favicon */, preferred_size,
- /*max_bitmap_size=*/0, false /* bypass_cache */,
+ desired_size_px /* max_bitmap_size */, false /* bypass_cache */,
base::BindOnce(&MediaSessionImpl::OnImageDownloadComplete,
base::Unretained(this),
mojo::WrapCallbackWithDefaultInvokeIfNotRun(
diff --git a/content/browser/web_contents/web_contents_android.cc b/content/browser/web_contents/web_contents_android.cc
index e7ccd06..4d57232 100644
--- a/content/browser/web_contents/web_contents_android.cc
+++ b/content/browser/web_contents/web_contents_android.cc
@@ -20,8 +20,6 @@
#include "base/lazy_instance.h"
#include "base/logging.h"
#include "base/metrics/user_metrics.h"
-#include "base/numerics/safe_conversions.h"
-#include "base/task/thread_pool.h"
#include "base/threading/scoped_blocking_call.h"
#include "content/browser/android/java/gin_java_bridge_dispatcher_host.h"
#include "content/browser/media/media_web_contents_observer.h"
@@ -37,9 +35,7 @@
#include "content/public/browser/render_widget_host.h"
#include "content/public/browser/web_contents.h"
#include "content/public/common/content_switches.h"
-#include "skia/ext/image_operations.h"
#include "third_party/blink/public/mojom/input/input_handler.mojom-blink.h"
-#include "third_party/skia/include/core/SkBitmap.h"
#include "ui/accessibility/ax_assistant_structure.h"
#include "ui/accessibility/ax_node_data.h"
#include "ui/accessibility/ax_tree_update.h"
@@ -49,7 +45,6 @@
#include "ui/gfx/android/java_bitmap.h"
#include "ui/gfx/geometry/point.h"
#include "ui/gfx/geometry/rect.h"
-#include "ui/gfx/image/image_util.h"
#include "url/android/gurl_android.h"
#include "url/gurl.h"
@@ -726,19 +721,13 @@
jint max_bitmap_size,
jboolean bypass_cache,
const base::android::JavaParamRef<jobject>& jcallback) {
- ScopedJavaGlobalRef<jobject> j_callback;
- j_callback.Reset(env, jcallback);
- // The max_image_size of 0 being passed to web_contents_->DownloadImage()
- // ensures that all images are fetched by the renderer before being
- // resized/filtered out in WebContentsAndroid::OnFinishDownloadImage()
- // using the max_bitmap_size in a separate threadpool.
const gfx::Size preferred_size;
return web_contents_->DownloadImage(
*url::GURLAndroid::ToNativeGURL(env, jurl), is_fav_icon, preferred_size,
- /*max_image_size=*/0, bypass_cache,
+ max_bitmap_size, bypass_cache,
base::BindOnce(&WebContentsAndroid::OnFinishDownloadImage,
- weak_factory_.GetWeakPtr(), j_callback,
- base::checked_cast<uint32_t>(max_bitmap_size)));
+ weak_factory_.GetWeakPtr(), obj_,
+ ScopedJavaGlobalRef<jobject>(env, jcallback)));
}
void WebContentsAndroid::SetHasPersistentVideo(JNIEnv* env, jboolean value) {
@@ -782,45 +771,13 @@
}
void WebContentsAndroid::OnFinishDownloadImage(
- const base::android::ScopedJavaGlobalRef<jobject>& callback,
- uint32_t max_image_size,
- int id,
- int http_status_code,
- const GURL& url,
- const std::vector<SkBitmap>& bitmaps,
- const std::vector<gfx::Size>& sizes) {
- std::vector<SkBitmap> filtered_images;
- std::vector<gfx::Size> filtered_bitmap_sizes;
-
- base::ThreadPool::PostTaskAndReply(
- FROM_HERE, {base::TaskPriority::USER_VISIBLE},
- base::BindOnce(&WebContentsAndroid::FilterOrResizeReceivedImages,
- weak_factory_.GetWeakPtr(), bitmaps, max_image_size,
- std::ref(filtered_images),
- std::ref(filtered_bitmap_sizes)),
- base::BindOnce(&WebContentsAndroid::ConvertToJavaBitmap,
- weak_factory_.GetWeakPtr(), obj_, callback, id,
- http_status_code, url, filtered_images,
- filtered_bitmap_sizes));
-}
-
-void WebContentsAndroid::FilterOrResizeReceivedImages(
- const std::vector<SkBitmap>& bitmaps,
- uint32_t max_image_size,
- std::vector<SkBitmap>& filtered_images,
- std::vector<gfx::Size>& filtered_bitmap_sizes) {
- gfx::FilterAndResizeImagesForMaximalSize(
- bitmaps, max_image_size, filtered_images, filtered_bitmap_sizes);
-}
-
-void WebContentsAndroid::ConvertToJavaBitmap(
const JavaRef<jobject>& obj,
const JavaRef<jobject>& callback,
int id,
int http_status_code,
const GURL& url,
- const std::vector<SkBitmap>& filtered_images,
- const std::vector<gfx::Size>& filtered_bitmap_sizes) {
+ const std::vector<SkBitmap>& bitmaps,
+ const std::vector<gfx::Size>& sizes) {
JNIEnv* env = base::android::AttachCurrentThread();
ScopedJavaLocalRef<jobject> jbitmaps =
Java_WebContentsImpl_createBitmapList(env);
@@ -828,15 +785,14 @@
Java_WebContentsImpl_createSizeList(env);
ScopedJavaLocalRef<jobject> jurl = url::GURLAndroid::FromNativeGURL(env, url);
- DCHECK_EQ(filtered_images.size(), filtered_bitmap_sizes.size());
- for (const SkBitmap& bitmap : filtered_images) {
- // WARNING: converting to java bitmaps results in duplicate memory
+ for (const SkBitmap& bitmap : bitmaps) {
+ // WARNING: convering to java bitmaps results in duplicate memory
// allocations, which increases the chance of OOMs if DownloadImage() is
// misused.
ScopedJavaLocalRef<jobject> jbitmap = gfx::ConvertToJavaBitmap(bitmap);
Java_WebContentsImpl_addToBitmapList(env, jbitmaps, jbitmap);
}
- for (const gfx::Size& size : filtered_bitmap_sizes) {
+ for (const gfx::Size& size : sizes) {
Java_WebContentsImpl_createSizeAndAddToList(env, jsizes, size.width(),
size.height());
}
diff --git a/content/browser/web_contents/web_contents_android.h b/content/browser/web_contents/web_contents_android.h
index f28c0d19..2b278693 100644
--- a/content/browser/web_contents/web_contents_android.h
+++ b/content/browser/web_contents/web_contents_android.h
@@ -215,26 +215,13 @@
void RemoveDestructionObserver(DestructionObserver* observer);
private:
- void OnFinishDownloadImage(
- const base::android::ScopedJavaGlobalRef<jobject>& jcallback,
- uint32_t max_image_size,
- int id,
- int http_status_code,
- const GURL& url,
- const std::vector<SkBitmap>& bitmaps,
- const std::vector<gfx::Size>& sizes);
- void FilterOrResizeReceivedImages(
- const std::vector<SkBitmap>& bitmaps,
- uint32_t max_image_size,
- std::vector<SkBitmap>& filtered_images,
- std::vector<gfx::Size>& filtered_bitmap_sizes);
- void ConvertToJavaBitmap(const base::android::JavaRef<jobject>& obj,
- const base::android::JavaRef<jobject>& callback,
- int id,
- int http_status_code,
- const GURL& url,
- const std::vector<SkBitmap>& filtered_images,
- const std::vector<gfx::Size>& filtered_bitmap_sizes);
+ void OnFinishDownloadImage(const base::android::JavaRef<jobject>& obj,
+ const base::android::JavaRef<jobject>& callback,
+ int id,
+ int http_status_code,
+ const GURL& url,
+ const std::vector<SkBitmap>& bitmaps,
+ const std::vector<gfx::Size>& sizes);
void SelectAroundCaretAck(blink::mojom::SelectAroundCaretResultPtr result);
// Walks over the AXTreeUpdate and creates a light weight snapshot.
void AXTreeSnapshotCallback(
diff --git a/content/public/browser/web_contents.h b/content/public/browser/web_contents.h
index 2ee2f2a..c12e9514 100644
--- a/content/public/browser/web_contents.h
+++ b/content/public/browser/web_contents.h
@@ -1096,8 +1096,8 @@
// be called with the bitmaps received from the renderer.
// If |is_favicon| is true, the cookies are not sent and not accepted during
// download.
- // Bitmaps with pixel sizes larger than |max_bitmap_size| are filtered out
- // from the bitmap results.
+ // If there are no bitmap results <= |max_bitmap_size|, the smallest bitmap
+ // is resized to |max_bitmap_size| and is the only result.
// A |max_bitmap_size| of 0 means unlimited.
// For vector images, |preferred_size| will serve as a viewport into which
// the image will be rendered. This would usually be the dimensions of the
diff --git a/third_party/blink/renderer/modules/image_downloader/image_downloader_impl.cc b/third_party/blink/renderer/modules/image_downloader/image_downloader_impl.cc
index e30f7ae5..db9b9d38 100644
--- a/third_party/blink/renderer/modules/image_downloader/image_downloader_impl.cc
+++ b/third_party/blink/renderer/modules/image_downloader/image_downloader_impl.cc
@@ -8,7 +8,6 @@
#include "base/bind.h"
#include "base/check.h"
-#include "base/numerics/safe_conversions.h"
#include "skia/ext/image_operations.h"
#include "third_party/blink/public/mojom/fetch/fetch_api_request.mojom-blink.h"
#include "third_party/blink/public/platform/interface_registry.h"
@@ -59,12 +58,29 @@
return DecodeImageData(data, mime_type, preferred_size);
}
-// Images are allowed only if the following criteria is true:
-// If |max_image_size| == 0, denoting that no upper limit is provided for image
-// size.
-// If size of image (width and height) is <= max_image_size.
-// For all other cases, the image is filtered out.
-void FilterImagesBasedOnMaximalSize(
+// Proportionally resizes the |image| to fit in a box of size
+// |max_image_size|.
+SkBitmap ResizeImage(const SkBitmap& image, uint32_t max_image_size) {
+ if (max_image_size == 0)
+ return image;
+ uint32_t max_dimension = std::max(image.width(), image.height());
+ if (max_dimension <= max_image_size)
+ return image;
+ // Proportionally resize the minimal image to fit in a box of size
+ // max_image_size.
+ return skia::ImageOperations::Resize(
+ image, skia::ImageOperations::RESIZE_BEST,
+ static_cast<uint32_t>(image.width()) * max_image_size / max_dimension,
+ static_cast<uint32_t>(image.height()) * max_image_size / max_dimension);
+}
+
+// Filters the array of bitmaps, removing all images that do not fit in a box of
+// size |max_image_size|. Returns the result if it is not empty. Otherwise,
+// find the smallest image in the array and resize it proportionally to fit
+// in a box of size |max_image_size|.
+// Sets |original_image_sizes| to the sizes of |images| before resizing. Both
+// output vectors are guaranteed to have the same size.
+void FilterAndResizeImagesForMaximalSize(
const WTF::Vector<SkBitmap>& unfiltered,
uint32_t max_image_size,
WTF::Vector<SkBitmap>* images,
@@ -75,14 +91,38 @@
if (unfiltered.IsEmpty())
return;
- for (const SkBitmap& image : unfiltered) {
- if ((max_image_size == 0) ||
- (base::checked_cast<uint32_t>(image.width()) <= max_image_size &&
- base::checked_cast<uint32_t>(image.height()) <= max_image_size)) {
+ if (max_image_size == 0)
+ max_image_size = std::numeric_limits<uint32_t>::max();
+
+ const SkBitmap* min_image = nullptr;
+ uint32_t min_image_size = std::numeric_limits<uint32_t>::max();
+ // Filter the images by |max_image_size|, and also identify the smallest image
+ // in case all the images are bigger than |max_image_size|.
+ for (auto* it = unfiltered.begin(); it != unfiltered.end(); ++it) {
+ const SkBitmap& image = *it;
+ uint32_t current_size = std::max(it->width(), it->height());
+ if (current_size < min_image_size) {
+ min_image = ℑ
+ min_image_size = current_size;
+ }
+ if (static_cast<uint32_t>(image.width()) <= max_image_size &&
+ static_cast<uint32_t>(image.height()) <= max_image_size) {
images->push_back(image);
original_image_sizes->push_back(gfx::Size(image.width(), image.height()));
}
}
+ DCHECK(min_image);
+ if (images->size())
+ return;
+ // Proportionally resize the minimal image to fit in a box of size
+ // |max_image_size|.
+ SkBitmap resized = ResizeImage(*min_image, max_image_size);
+ // Drop null or empty SkBitmap.
+ if (resized.drawsNothing())
+ return;
+ images->push_back(resized);
+ original_image_sizes->push_back(
+ gfx::Size(min_image->width(), min_image->height()));
}
} // namespace
@@ -163,8 +203,8 @@
const WTF::Vector<SkBitmap>& images) {
WTF::Vector<SkBitmap> result_images;
WTF::Vector<gfx::Size> result_original_image_sizes;
- FilterImagesBasedOnMaximalSize(images, max_image_size, &result_images,
- &result_original_image_sizes);
+ FilterAndResizeImagesForMaximalSize(images, max_image_size, &result_images,
+ &result_original_image_sizes);
DCHECK_EQ(result_images.size(), result_original_image_sizes.size());
diff --git a/ui/gfx/image/image_util.cc b/ui/gfx/image/image_util.cc
index bb49ea5..63305ce 100644
--- a/ui/gfx/image/image_util.cc
+++ b/ui/gfx/image/image_util.cc
@@ -8,17 +8,13 @@
#include <algorithm>
#include <memory>
-#include <vector>
-#include "base/check.h"
#include "base/cxx17_backports.h"
-#include "base/numerics/safe_conversions.h"
#include "build/build_config.h"
#include "skia/ext/image_operations.h"
#include "third_party/skia/include/core/SkBitmap.h"
#include "ui/gfx/codec/jpeg_codec.h"
#include "ui/gfx/codec/webp_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_skia_rep.h"
@@ -162,61 +158,4 @@
*right = bitmap.width() - 1 - x;
}
-SkBitmap ResizeImageToLargestSize(const SkBitmap& image,
- uint32_t max_image_size) {
- uint32_t max_dimension = std::max(image.width(), image.height());
- if (max_dimension <= max_image_size)
- return image;
- // Proportionally resize the minimal image to fit in a box of size
- // max_image_size.
- return skia::ImageOperations::Resize(
- image, skia::ImageOperations::RESIZE_BEST,
- base::checked_cast<uint32_t>(image.width()) * max_image_size /
- max_dimension,
- base::checked_cast<uint32_t>(image.height()) * max_image_size /
- max_dimension);
-}
-
-void FilterAndResizeImagesForMaximalSize(
- const std::vector<SkBitmap>& images,
- uint32_t max_image_size,
- std::vector<SkBitmap>& filtered_images,
- std::vector<gfx::Size>& filtered_image_sizes) {
- filtered_images.clear();
- filtered_image_sizes.clear();
-
- if (images.empty())
- return;
-
- const SkBitmap* min_image = nullptr;
- uint32_t min_image_size = std::numeric_limits<uint32_t>::max();
- // Filter the images by |max_image_size|, and also identify the smallest
- // image in case all the images are bigger than |max_image_size|.
- for (const SkBitmap& image : images) {
- uint32_t current_size = std::max(image.width(), image.height());
- if (current_size < min_image_size) {
- min_image = ℑ
- min_image_size = current_size;
- }
- if (base::checked_cast<uint32_t>(image.width()) <= max_image_size &&
- base::checked_cast<uint32_t>(image.height()) <= max_image_size) {
- filtered_images.emplace_back(image);
- filtered_image_sizes.emplace_back(
- gfx::Size(image.width(), image.height()));
- }
- }
- if (!filtered_images.empty())
- return;
- // Proportionally resize the minimal image to fit in a box of size
- // |max_image_size|.
- DCHECK(min_image);
- SkBitmap resized = ResizeImageToLargestSize(*min_image, max_image_size);
- // Drop null or empty SkBitmap.
- if (resized.drawsNothing())
- return;
- filtered_images.emplace_back(resized);
- filtered_image_sizes.emplace_back(
- gfx::Size(min_image->width(), min_image->height()));
-}
-
} // namespace gfx
diff --git a/ui/gfx/image/image_util.h b/ui/gfx/image/image_util.h
index 140403e50..aa954a2 100644
--- a/ui/gfx/image/image_util.h
+++ b/ui/gfx/image/image_util.h
@@ -9,8 +9,6 @@
#include <vector>
-#include "third_party/skia/include/core/SkBitmap.h"
-#include "ui/gfx/geometry/size.h"
#include "ui/gfx/gfx_export.h"
namespace gfx {
@@ -76,24 +74,6 @@
int max_height,
int max_area);
-// Proportionally resizes the |image| to fit in a box of size
-// |max_image_size|. If the |image| already fits, it is
-// returned without any resizing.
-GFX_EXPORT SkBitmap ResizeImageToLargestSize(const SkBitmap& image,
- uint32_t max_image_size);
-
-// Filters the array of bitmaps, removing all images that do not fit in a box of
-// size |max_image_size|. Returns the result if it is not empty. Otherwise,
-// find the smallest image in the array and resize it proportionally to fit
-// in a box of size |max_image_size|.
-// Sets |filtered_image_sizes| to the sizes of |filtered_images| before
-// resizing. Both output vectors are guaranteed to have the same size.
-GFX_EXPORT void FilterAndResizeImagesForMaximalSize(
- const std::vector<SkBitmap>& images,
- uint32_t max_image_size,
- std::vector<SkBitmap>& filtered_images,
- std::vector<gfx::Size>& filtered_image_sizes);
-
} // namespace gfx
#endif // UI_GFX_IMAGE_IMAGE_UTIL_H_
diff --git a/ui/gfx/image/image_util_unittest.cc b/ui/gfx/image/image_util_unittest.cc
index 5244772..0ef65fd 100644
--- a/ui/gfx/image/image_util_unittest.cc
+++ b/ui/gfx/image/image_util_unittest.cc
@@ -14,17 +14,6 @@
#include "ui/gfx/image/image_unittest_util.h"
#include "ui/gfx/image/resize_image_dimensions.h"
-namespace {
-
-SkBitmap CreateRandomImage(int size, SkColor color) {
- SkBitmap bitmap;
- bitmap.allocN32Pixels(size, size);
- bitmap.eraseColor(color);
- return bitmap;
-}
-
-} // namespace
-
TEST(ImageUtilTest, JPEGEncodeAndDecode) {
gfx::Image original = gfx::test::CreateImage(100, 100);
@@ -168,70 +157,3 @@
EXPECT_EQ(resized_image.Width(), 400);
EXPECT_EQ(resized_image.Height(), 400);
}
-
-TEST(ImageUtilTest, NoFilterNoResize) {
- std::vector<SkBitmap> previous_images;
- previous_images.push_back(CreateRandomImage(1, SK_ColorBLACK));
- previous_images.push_back(CreateRandomImage(2, SK_ColorGRAY));
- previous_images.push_back(CreateRandomImage(3, SK_ColorBLUE));
-
- std::vector<SkBitmap> filtered_images;
- std::vector<gfx::Size> filtered_sizes;
- gfx::FilterAndResizeImagesForMaximalSize(
- previous_images, /*max_image_size=*/5, filtered_images, filtered_sizes);
-
- // No image gets filtered.
- EXPECT_EQ(filtered_images.size(), previous_images.size());
- EXPECT_EQ(3u, filtered_sizes.size());
- EXPECT_EQ(SK_ColorBLACK, filtered_images.at(0).getColor(0, 0));
- EXPECT_EQ(SK_ColorGRAY, filtered_images.at(1).getColor(1, 1));
- EXPECT_EQ(SK_ColorBLUE, filtered_images.at(2).getColor(2, 2));
-}
-
-TEST(ImageUtilTest, FilterImageNoResize) {
- std::vector<SkBitmap> previous_images;
- previous_images.push_back(CreateRandomImage(1, SK_ColorBLACK));
- previous_images.push_back(CreateRandomImage(3, SK_ColorGRAY));
- previous_images.push_back(CreateRandomImage(4, SK_ColorBLUE));
-
- std::vector<SkBitmap> filtered_images;
- std::vector<gfx::Size> filtered_sizes;
- gfx::FilterAndResizeImagesForMaximalSize(
- previous_images, /*max_image_size=*/2, filtered_images, filtered_sizes);
-
- // No image gets filtered.
- EXPECT_EQ(1u, filtered_images.size());
- EXPECT_EQ(1u, filtered_sizes.size());
- EXPECT_EQ(SK_ColorBLACK, filtered_images.at(0).getColor(0, 0));
- // Verify grey and blue SkBitmaps are not in the filtered image.
- EXPECT_NE(SK_ColorGRAY, filtered_images.at(0).getColor(0, 0));
- EXPECT_NE(SK_ColorBLUE, filtered_images.at(0).getColor(0, 0));
-}
-
-TEST(ImageUtilTest, AllFilterOnlyResize) {
- std::vector<SkBitmap> previous_images;
- previous_images.push_back(CreateRandomImage(6, SK_ColorBLACK));
- previous_images.push_back(CreateRandomImage(5, SK_ColorGRAY));
- previous_images.push_back(CreateRandomImage(4, SK_ColorBLUE));
-
- std::vector<SkBitmap> filtered_images;
- std::vector<gfx::Size> filtered_sizes;
- gfx::FilterAndResizeImagesForMaximalSize(
- previous_images, /*max_image_size=*/3, filtered_images, filtered_sizes);
-
- // Only 1 image gets resized, and it is the smallest one (the blue one).
- // All other images are not filtered.
- EXPECT_EQ(1u, filtered_images.size());
- EXPECT_EQ(1u, filtered_sizes.size());
- // Verify that resizing happens to proper size.
- EXPECT_EQ(3, filtered_images.at(0).dimensions().width());
- EXPECT_EQ(3, filtered_images.at(0).dimensions().height());
- // Original sizes.
- EXPECT_EQ(4, filtered_sizes.at(0).width());
- EXPECT_EQ(4, filtered_sizes.at(0).height());
- // Verify grey and black SkBitmaps are not in the filtered image.
- // Only blue image is filtered.
- EXPECT_EQ(SK_ColorBLUE, filtered_images.at(0).getColor(0, 0));
- EXPECT_NE(SK_ColorGRAY, filtered_images.at(0).getColor(0, 0));
- EXPECT_NE(SK_ColorBLACK, filtered_images.at(0).getColor(0, 0));
-}
