Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(40)

Unified Diff: chrome/browser/manifest/manifest_icon_selector.cc

Issue 2662103002: Refactor ManifestIconSelector and update it for Manifest.icon.purpose (Closed)
Patch Set: Addressing comments Created 3 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/manifest/manifest_icon_selector.cc
diff --git a/chrome/browser/manifest/manifest_icon_selector.cc b/chrome/browser/manifest/manifest_icon_selector.cc
index 139511b750e1cf8c17314f8a5ee703b02105c065..3093224b1f9d0090ac6a9f044d8e4e31e053d62d 100644
--- a/chrome/browser/manifest/manifest_icon_selector.cc
+++ b/chrome/browser/manifest/manifest_icon_selector.cc
@@ -4,141 +4,79 @@
#include "chrome/browser/manifest/manifest_icon_selector.h"
-#include <stddef.h>
-
-#include <algorithm>
-#include <cmath>
#include <limits>
+#include "base/stl_util.h"
#include "base/strings/utf_string_conversions.h"
#include "components/mime_util/mime_util.h"
-#include "content/public/browser/web_contents.h"
-using content::Manifest;
+// static
+GURL ManifestIconSelector::FindBestMatchingIcon(
+ const std::vector<content::Manifest::Icon>& icons,
+ int ideal_icon_size_in_px,
+ int minimum_icon_size_in_px,
+ content::Manifest::Icon::IconPurpose purpose) {
+ DCHECK(minimum_icon_size_in_px <= ideal_icon_size_in_px);
-ManifestIconSelector::ManifestIconSelector(int ideal_icon_size_in_px,
- int minimum_icon_size_in_px)
- : ideal_icon_size_in_px_(ideal_icon_size_in_px),
- minimum_icon_size_in_px_(minimum_icon_size_in_px) {
-}
+ // Icon with exact matching size has priority over icon with size "any", which
+ // has priority over icon with closest matching size.
+ int latest_size_any_index = -1;
+ int closest_size_match_index = -1;
+ int best_delta_in_size = std::numeric_limits<int>::min();
-bool ManifestIconSelector::IconSizesContainsPreferredSize(
- const std::vector<gfx::Size>& sizes) const {
- for (size_t i = 0; i < sizes.size(); ++i) {
- if (sizes[i].height() != sizes[i].width())
- continue;
- if (sizes[i].width() == ideal_icon_size_in_px_)
- return true;
- }
-
- return false;
-}
+ for (size_t i = 0; i < icons.size(); ++i) {
+ const auto& icon = icons[i];
-bool ManifestIconSelector::IconSizesContainsBiggerThanMinimumSize(
- const std::vector<gfx::Size>& sizes) const {
- for (size_t i = 0; i < sizes.size(); ++i) {
- if (sizes[i].height() != sizes[i].width())
+ // Check for supported image MIME types.
+ if (!icon.type.empty() &&
+ !mime_util::IsSupportedImageMimeType(base::UTF16ToUTF8(icon.type))) {
continue;
- if (sizes[i].width() >= minimum_icon_size_in_px_)
- return true;
- }
- return false;
-}
+ }
-int ManifestIconSelector::FindClosestIconToIdealSize(
- const std::vector<content::Manifest::Icon>& icons) const {
- int best_index = -1;
- int best_delta = std::numeric_limits<int>::min();
+ // Check for icon purpose.
+ if (!base::ContainsValue(icon.purpose, purpose))
+ continue;
- for (size_t i = 0; i < icons.size(); ++i) {
- const std::vector<gfx::Size>& sizes = icons[i].sizes;
- for (size_t j = 0; j < sizes.size(); ++j) {
- if (sizes[j].height() != sizes[j].width())
- continue;
- int delta = sizes[j].width() - ideal_icon_size_in_px_;
- if (delta == 0)
- return i;
- if (best_delta > 0 && delta < 0)
+ // Check for size constraints.
+ for (const gfx::Size& size : icon.sizes) {
+ // Check for size "any". Return this icon if no better one is found.
+ if (size.IsEmpty()) {
+ latest_size_any_index = i;
continue;
- if ((best_delta > 0 && delta < best_delta) ||
- (best_delta < 0 && delta > best_delta)) {
- best_index = i;
- best_delta = delta;
}
- }
- }
-
- return best_index;
-}
-int ManifestIconSelector::FindBestMatchingIcon(
- const std::vector<content::Manifest::Icon>& icons) const {
- int best_index = -1;
-
- // The first pass is to find the ideal icon - one with the exact right size.
- for (size_t i = 0; i < icons.size(); ++i) {
- if (IconSizesContainsPreferredSize(icons[i].sizes))
- return i;
-
- // If there is an icon size 'any', keep it on the side and only use it if
- // nothing better is found.
- if (IconSizesContainsAny(icons[i].sizes))
- best_index = i;
- }
- if (best_index != -1)
- return best_index;
-
- // The last pass will try to find the smallest icon larger than the ideal
- // size, or the largest icon smaller than the ideal size.
- best_index = FindClosestIconToIdealSize(icons);
-
- if (best_index != -1 &&
- IconSizesContainsBiggerThanMinimumSize(icons[best_index].sizes))
- return best_index;
+ // Check for squareness.
+ if (size.width() != size.height())
+ continue;
- return -1;
-}
+ // Check for minimum size.
+ if (size.width() < minimum_icon_size_in_px)
+ continue;
+ // Check for ideal size. Return this icon immediately.
+ if (size.width() == ideal_icon_size_in_px)
+ return icon.src;
-// static
-bool ManifestIconSelector::IconSizesContainsAny(
- const std::vector<gfx::Size>& sizes) {
- for (size_t i = 0; i < sizes.size(); ++i) {
- if (sizes[i].IsEmpty())
- return true;
- }
- return false;
-}
+ // Check for closest match.
+ int delta = size.width() - ideal_icon_size_in_px;
-// static
-std::vector<Manifest::Icon> ManifestIconSelector::FilterIconsByType(
- const std::vector<content::Manifest::Icon>& icons) {
- std::vector<Manifest::Icon> result;
+ // Smallest icon larger than ideal size has priority over largest icon
+ // smaller than ideal size.
+ if (best_delta_in_size > 0 && delta < 0)
+ continue;
- for (size_t i = 0; i < icons.size(); ++i) {
- if (icons[i].type.empty() ||
- mime_util::IsSupportedImageMimeType(base::UTF16ToUTF8(icons[i].type))) {
- result.push_back(icons[i]);
+ if ((best_delta_in_size > 0 && delta < best_delta_in_size) ||
+ (best_delta_in_size < 0 && delta > best_delta_in_size)) {
+ closest_size_match_index = i;
+ best_delta_in_size = delta;
+ }
}
}
- return result;
-}
-
-// static
-GURL ManifestIconSelector::FindBestMatchingIcon(
- const std::vector<Manifest::Icon>& unfiltered_icons,
- const int ideal_icon_size_in_px,
- const int minimum_icon_size_in_px) {
- DCHECK(minimum_icon_size_in_px <= ideal_icon_size_in_px);
-
- std::vector<Manifest::Icon> icons =
- ManifestIconSelector::FilterIconsByType(unfiltered_icons);
-
- ManifestIconSelector selector(ideal_icon_size_in_px,
- minimum_icon_size_in_px);
- int index = selector.FindBestMatchingIcon(icons);
- if (index == -1)
+ if (latest_size_any_index != -1)
+ return icons[latest_size_any_index].src;
+ else if (closest_size_match_index != -1)
+ return icons[closest_size_match_index].src;
+ else
return GURL();
- return icons[index].src;
}
« no previous file with comments | « chrome/browser/manifest/manifest_icon_selector.h ('k') | chrome/browser/manifest/manifest_icon_selector_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698