manifest: allow the icon downloader to choose non-square icons

chrome/browser/manifest/manifest_icon_downloader.cc

Index: chrome/browser/manifest/manifest_icon_downloader.cc
diff --git a/chrome/browser/manifest/manifest_icon_downloader.cc b/chrome/browser/manifest/manifest_icon_downloader.cc
index 34ff954d39e64fe15fe901ff85c973b2b19ba0da..64f689f9f4c6b8d8b1f619f12b306eef1c54d888 100644
--- a/chrome/browser/manifest/manifest_icon_downloader.cc
+++ b/chrome/browser/manifest/manifest_icon_downloader.cc
@@ -126,5 +126,26 @@ int ManifestIconDownloader::FindClosestBitmapIndex(
       best_delta = delta;
     }
   }
+
+  if (best_index != -1)
+    return best_index;
+
+  // There was no square icon of a correct size found. Try to find the most
+  // square possible icon which has both dimensions greater than the minimum
+  // size.

[mlamouri (slow - plz ping), 2015/09/18 13:41:59]

Very quick comment: maybe you could use a ratio instead of a difference? It
wouldn't change the code that much (ie. compute heght/width ratio instead of
height - width) and it would be way more flexible because the ratio could be
re-used if we want to generalise that code to pick non-square icons as preferred
icon (as in, we could try to find the closest ratio).

[Lalit Maganti, 2015/09/18 14:06:46]

Problem with using a ratio is that the code becomes a lot messier because floats
come into the equation. So you have to deal with floating point arithmetic
issues like division by 0, 0/0 and stuff. Of course a zero check can be added at
the start to make sure that this doesn't happen but it makes the code seem much
more cryptic than it is plus we have to include int -> float casting.

[gone, 2015/09/18 18:46:41]

I agree with Mounir about relying on ratios; the differences are likely to grow
for larger icons but can provide have a better quality for the same ratio.  I
don't know how you could end up with a 0 width or height after line 141; if they
are smaller than the minimum size, they get thrown out.

[Lalit Maganti, 2015/09/21 09:52:34]

Done.

+  int best_dimension_difference = std::numeric_limits<int>::max();
+  for (size_t i = 0; i < bitmaps.size(); ++i) {
+    if (bitmaps[i].height() < minimum_icon_size_in_px ||
+        bitmaps[i].width() < minimum_icon_size_in_px) {
+      continue;
+    }
+
+    int dimension_difference = abs(bitmaps[i].height() - bitmaps[i].width());
+    if (dimension_difference < best_dimension_difference) {
+      best_index = i;
+      best_dimension_difference = dimension_difference;
+    }
+  }
+
   return best_index;
 }