Break out manifest icon logic from ShortcutHelper

chrome/browser/android/manifest_icon_selector.h

Index: chrome/browser/android/manifest_icon_selector.h
diff --git a/chrome/browser/android/manifest_icon_selector.h b/chrome/browser/android/manifest_icon_selector.h
new file mode 100644
index 0000000000000000000000000000000000000000..0f076f4b3d9b24a5a595e08aa5b984477fcef624
--- /dev/null
+++ b/chrome/browser/android/manifest_icon_selector.h
@@ -0,0 +1,63 @@
+// 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.
+
+#ifndef CHROME_BROWSER_ANDROID_MANIFEST_ICON_SELECTOR_H_
+#define CHROME_BROWSER_ANDROID_MANIFEST_ICON_SELECTOR_H_
+
+#include "base/basictypes.h"
+#include "content/public/common/manifest.h"
+#include "url/gurl.h"
+
+namespace content {
+class WebContents;
+}  // namespace content
+
+namespace IPC {
+class Message;
+}  // namespace IPC
+
+// Selects the icon most closely matching the size constraints.  This follows
+// very basic heuristics -- improvements are welcome.
+class ManifestIconSelector {
+ public:
+  // Runs the algorithm to find the best matching icon in the icons listed in
+  // the Manifest.
+  // Returns the icon url if a suitable icon is found. An empty URL otherwise.

[mlamouri (slow - plz ping), 2015/02/03 17:14:14]

Could you point that the preferred size is in dp, not in px. It will hopefully
reduce the chances of someone passing the wrong value there.

[gone, 2015/02/03 19:25:06]

Done.

+  static GURL FindBestMatchingIcon(
+      const std::vector<content::Manifest::Icon>& icons,
+      float preferred_size_in_dp,
+      content::WebContents* web_contents);

[mlamouri (slow - plz ping), 2015/02/03 17:14:14]

It seems unfortunate to pass a WebContents when we actually want to know the
density. I understand that we usually know about the WebContents and it allows
that code to do the logic but maybe we can make it pass a screen instead?
Reducing the dependency would be good. Typically, it would allow testing it
without a web context.

[gone, 2015/02/03 19:25:06]

Passing in just the screen now.

+
+ private:
+  ManifestIconSelector() {}
+  virtual ~ManifestIconSelector() {}
+
+  // Returns an array containing the items in |icons| without the unsupported
+  // image MIME types.
+  static std::vector<content::Manifest::Icon> FilterIconsByType(
+      const std::vector<content::Manifest::Icon>& icons);
+
+  // Runs an algorithm only based on icon declared sizes. It will try to find
+  // size that is the closest to preferred_icon_size_in_px_ but bigger than
+  // preferred_icon_size_in_px_ if possible.
+  // Returns the icon url if a suitable icon is found. An empty URL otherwise.
+  static GURL FindBestMatchingIcon(
+      const std::vector<content::Manifest::Icon>& icons,
+      float density,
+      int preferred_icon_size_in_px);

[mlamouri (slow - plz ping), 2015/02/03 17:14:14]

If you want to pass |preferred_icon_size_in_px| to all the private static
method, you should move them to the .cc file and make them anonymous. Otherwise,
make |preferred_icon_size_in_px| a member of the class and make those method not
static, you can create an instance of the class in |FindBestMatchingIcon|.

[gone, 2015/02/03 19:25:05]

Done.

+
+  // Returns whether the |preferred_icon_size_in_px| is in the given |sizes|.
+  static bool IconSizesContainsPreferredSize(
+      const std::vector<gfx::Size>& sizes,
+      int preferred_icon_size_in_px);
+
+  // Returns whether the 'any' (ie. gfx::Size(0,0)) is in the given |sizes|.
+  static bool IconSizesContainsAny(const std::vector<gfx::Size>& sizes);
+
+  friend class ManifestIconSelectorTest;
+
+  DISALLOW_COPY_AND_ASSIGN(ManifestIconSelector);
+};
+
+#endif  // CHROME_BROWSER_ANDROID_MANIFEST_ICON_SELECTOR_H_