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

chrome/browser/manifest/manifest_icon_downloader.cc

 // Copyright 2015 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/browser/manifest/manifest_icon_downloader.h"
 
 #include <limits>
 
 #include "chrome/browser/manifest/manifest_icon_selector.h"
 #include "content/public/browser/browser_thread.h"
@@ skipping 108 matching lines @@
 
     if (best_delta > 0 && delta < 0)
       continue;
 
     if ((best_delta > 0 && delta < best_delta) ||
         (best_delta < 0 && delta > best_delta && delta >= max_negative_delta)) {
       best_index = i;
       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;
 }