Update AddToHomescreenDataFetcher to accept badge icon for WebAPK.

chrome/browser/android/webapps/add_to_homescreen_data_fetcher.h

Index: chrome/browser/android/webapps/add_to_homescreen_data_fetcher.h
diff --git a/chrome/browser/android/webapps/add_to_homescreen_data_fetcher.h b/chrome/browser/android/webapps/add_to_homescreen_data_fetcher.h
index 20de87adca0e94c373a626207b7e892e97e929f8..566afe54cc8abaed4ca5809377f7d0c9add4fcd6 100644
--- a/chrome/browser/android/webapps/add_to_homescreen_data_fetcher.h
+++ b/chrome/browser/android/webapps/add_to_homescreen_data_fetcher.h
@@ -59,10 +59,11 @@ class AddToHomescreenDataFetcher
 
     // Called when all the data needed to create a shortcut is available.
     virtual void OnDataAvailable(const ShortcutInfo& info,
-                                 const SkBitmap& icon) = 0;
+                                 const SkBitmap& icon,

[dominickn, 2017/02/02 19:16:50]

primary_icon?
badge_icon?

[F, 2017/02/02 21:05:51]

Done.

+                                 const SkBitmap& badge) = 0;
 
-    protected:
-     virtual ~Observer() {}
+   protected:
+    virtual ~Observer() {}
   };
 
   // Initialize the fetcher by requesting the information about the page from
@@ -73,6 +74,8 @@ class AddToHomescreenDataFetcher
                              int minimum_icon_size_in_px,
                              int ideal_splash_image_size_in_px,
                              int minimum_splash_image_size_in_px,
+                             int ideal_badge_size_in_px,
+                             int minimum_badge_size_in_px,
                              bool check_webapk_compatible,
                              Observer* observer);
 
@@ -125,6 +128,7 @@ class AddToHomescreenDataFetcher
 
   // The icon must only be set on the UI thread for thread safety.
   SkBitmap shortcut_icon_;

[dominickn, 2017/02/02 19:16:50]

primary_icon_?
badge_icon?

[F, 2017/02/02 21:05:51]

Done.

[dominickn, 2017/02/02 21:17:11]

I don't think we need the shortcut_ prefix here - primary_icon_ and badge_icon_
should suffice (and make some methods fit on one line). E.g. see
splash_screen_url_

[F, 2017/02/02 22:30:42]

Done.

+  SkBitmap shortcut_badge_;
   ShortcutInfo shortcut_info_;
   GURL splash_screen_url_;
 
@@ -135,6 +139,8 @@ class AddToHomescreenDataFetcher
   const int minimum_icon_size_in_px_;
   const int ideal_splash_image_size_in_px_;
   const int minimum_splash_image_size_in_px_;
+  const int ideal_badge_size_in_px_;

[dominickn, 2017/02/02 19:16:50]

These appear to always be set to the same value. Can we just have one member?

A corollary is that it seems to me that we don't have this in-between zone for
badge icons: the minimal and ideal sizes are always the same. If that's the
case, why don't we just have one size parameter passed into InstallableManager
as well?

[F, 2017/02/02 21:05:51]

My understanding is that ShortcutHelper wraps the device-related information
while InstallableManager is more on the logic side. So I think
InstallableManager should allow setting a minimal value, though right now we
decide to provide the same value for both minimal and ideal.

This way, setting the values is AddToHomescreenManager's concern, and data
fetcher & InstallableManager provide a more generic interface.

WDUT?

[dominickn, 2017/02/02 21:17:11]

If we're going to provide the same value for now, we should just have one value.
Later on if we decide to have different values we can have the separate
variables (I doubt that we'll ever have a minimal badge icon size different from
the ideal).

I'm okay with you leaving InstallableManager alone, but in this class here I at
least think having one badge_size_in_px_ should suffice.

[F, 2017/02/02 22:30:42]

Done.

+  const int minimum_badge_size_in_px_;
 
   // Indicates whether to check WebAPK compatibility.
   bool check_webapk_compatibility_;