[Android] Fixed default bookmark shortcut icon drawing.

chrome/android/java/src/org/chromium/chrome/browser/BookmarkUtils.java

Index: chrome/android/java/src/org/chromium/chrome/browser/BookmarkUtils.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/BookmarkUtils.java b/chrome/android/java/src/org/chromium/chrome/browser/BookmarkUtils.java
index fe43e3be164b80a0b7cc877c0bb0a1e781c4376d..726db69d8feb024bd3c80be598a64e7dbc8f085a 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/BookmarkUtils.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/BookmarkUtils.java
@@ -14,6 +14,7 @@ import android.graphics.LinearGradient;
 import android.graphics.Paint;
 import android.graphics.Path;
 import android.graphics.PorterDuff;
+import android.graphics.PorterDuffColorFilter;
 import android.graphics.PorterDuffXfermode;
 import android.graphics.Rect;
 import android.graphics.RectF;
@@ -102,11 +103,9 @@ public class BookmarkUtils {
             if (favicon.getWidth() >= smallestSide / 2 && favicon.getHeight() >= smallestSide / 2) {
                 drawTouchIconToCanvas(context, favicon, canvas, iconBounds);
             } else {
-                Bitmap icon = getIconBackground(context, iconDensity);
-                if (icon != null) {
-                    canvas.drawBitmap(icon, null, iconBounds, new Paint(Paint.ANTI_ALIAS_FLAG));
-                }
-                drawFaviconToCanvas(context, favicon, canvas, iconBounds, rValue, gValue, bValue);
+                drawDocumentIconToCanvas(context, canvas, iconBounds, iconDensity,
+                        Color.rgb(rValue, gValue, bValue));
+                drawFaviconToCanvas(context, favicon, canvas, iconBounds);
             }
             canvas.setBitmap(null);
         } catch (OutOfMemoryError e) {
@@ -115,15 +114,6 @@ public class BookmarkUtils {
         return bitmap;
     }
 
-    /**
-     * Get the icon background asset.
-     * @param context Context used to create the intent.
-     * @return Bitmap favicon background asset.
-     */
-    private static Bitmap getIconBackground(Context context, int density) {
-        return getBitmapFromResourceId(context, R.mipmap.homescreen_bg, density);
-    }
-
     private static Bitmap getBitmapFromResourceId(Context context, int id, int density) {

[newt (away), 2013/05/30 22:12:01]

we might not need this anymore. you can just load the image normally (i.e.
context.getResources().getDrawable(R.mipmap.xyz)) from the mipmap folder, right?

[Kibeom Kim (inactive), 2013/05/31 01:23:52]

(offline talk & Romain's reply summary: res/mipmap/ is just res/drawables/ with
guaranteed not stripping, so we still need getDrawableForDensity(...) )

Done.

         Drawable drawable = null;
         if (Build.VERSION.SDK_INT >= SDK_VERSION_FOR_ACCESS_TO_METHODS) {
@@ -169,77 +159,10 @@ public class BookmarkUtils {
      * @param favicon favicon bitmap.
      * @param canvas Canvas that holds the favicon.
      * @param iconBounds Rectangle bounds needed to create the homescreen favicon.
-     * @param rValue Red component of the dominant favicon color.
-     * @param gValue Green component of the dominant favicon color.
-     * @param bValue Blue component of the dominant favicon color.
      */
     private static void drawFaviconToCanvas(Context context, Bitmap favicon,
-            Canvas canvas, Rect iconBounds, int rValue, int gValue, int bValue) {
-        int colorStripWidth = context.getResources().getDimensionPixelSize(
-                R.dimen.favicon_colorstrip_width);
-        int colorStripHeight = context.getResources().getDimensionPixelSize(
-                R.dimen.favicon_colorstrip_height);
-        int colorStripPadding = context.getResources().getDimensionPixelSize(
-                R.dimen.favicon_colorstrip_padding);
-        int colorStripCornerRadii = context.getResources().getDimensionPixelSize(
-                R.dimen.favicon_colorstrip_corner_radii);
-        int foldSize = context.getResources().getDimensionPixelSize(R.dimen.favicon_fold_size);
-        int foldCornerRadii = context.getResources().getDimensionPixelSize(
-                R.dimen.favicon_fold_corner_radii);
-        int foldBorder = context.getResources().getDimensionPixelSize(R.dimen.favicon_fold_border);
-        int foldShadow = context.getResources().getDimensionPixelSize(R.dimen.favicon_fold_shadow);
+            Canvas canvas, Rect iconBounds) {
         int faviconSize = context.getResources().getDimensionPixelSize(R.dimen.favicon_size);

[newt (away), 2013/05/30 22:12:01]

I'd calculate faviconSize from the canvas's size, instead of loading a
dimension.

[Kibeom Kim (inactive), 2013/05/31 01:23:52]

Done.

-
-        float x1 = iconBounds.exactCenterX() - (colorStripWidth / 2.0f);
-        float x2 = x1 + colorStripWidth;
-        float y1 = iconBounds.height() - colorStripPadding;
-        float y2 = y1 - colorStripHeight;
-        int faviconColorAlpha100 = Color.argb(255, rValue, gValue, bValue);
-        Paint stripPaint = new Paint(Paint.ANTI_ALIAS_FLAG);
-        stripPaint.setColor(faviconColorAlpha100);
-        stripPaint.setStyle(Paint.Style.FILL);
-        Path stripPath = new Path();
-        stripPath.moveTo(x1, y2);
-        stripPath.lineTo(x2, y2);
-        stripPath.lineTo(x2, y1 - colorStripCornerRadii);
-        stripPath.arcTo(new RectF(x2 - colorStripCornerRadii, y1 - colorStripCornerRadii, x2, y1),
-                0, 90);
-        stripPath.lineTo(x1 + colorStripCornerRadii, y1);
-        stripPath.arcTo(new RectF(x1, y1 - colorStripCornerRadii, x1 + colorStripCornerRadii, y1),
-                90, 90);
-        stripPath.lineTo(x1, y2);
-        canvas.drawPath(stripPath, stripPaint);
-
-        float ax, ay, bx, by, cx, cy;
-        bx = ax = x2 - foldSize;
-        cx = x2;
-        ay = colorStripPadding;
-        by = cy = colorStripPadding + foldSize;
-        int blackAlpha10 = Color.argb((int) (255 * 0.10f), 0, 0, 0);
-
-        Paint foldShadowPaint = new Paint(Paint.ANTI_ALIAS_FLAG);
-        foldShadowPaint.setColor(Color.WHITE);
-        foldShadowPaint.setShadowLayer(1, 0, foldShadow, blackAlpha10);
-        int faviconColorAlpha60 = Color.argb((int) (255 * 0.60f), rValue, gValue, bValue);
-        Paint foldGradientPaint = new Paint(Paint.ANTI_ALIAS_FLAG);
-        foldGradientPaint.setShader(new LinearGradient(x2 - foldSize / 2.0f,
-                colorStripPadding + foldSize / 2.0f, x2 - foldSize, colorStripPadding + foldSize,
-                faviconColorAlpha60, faviconColorAlpha100,
-                android.graphics.Shader.TileMode.CLAMP));
-        Path foldPath = new Path();
-        foldPath.moveTo(ax, ay);
-        foldPath.lineTo(cx, cy);
-        foldPath.lineTo(bx + foldCornerRadii, by);
-        foldPath.arcTo(new RectF(bx, by - foldCornerRadii, bx + foldCornerRadii, by), 90, 90);
-        foldPath.lineTo(ax, ay);
-        canvas.drawPath(foldPath, foldShadowPaint);
-        canvas.drawPath(foldPath, foldGradientPaint);
-        int faviconColorAlpha80 = Color.argb((int) (255 * 0.80f), rValue, gValue, bValue);
-        Paint foldBorderPaint = new Paint(Paint.ANTI_ALIAS_FLAG);
-        foldBorderPaint.setColor(faviconColorAlpha80);
-        foldBorderPaint.setStyle(Paint.Style.STROKE);
-        foldBorderPaint.setStrokeWidth(foldBorder);
-        canvas.drawPath(foldPath, foldBorderPaint);
         try {
             Bitmap scaledFavicon = Bitmap.createScaledBitmap(favicon, faviconSize, faviconSize,
                     true);
@@ -250,4 +173,29 @@ public class BookmarkUtils {
             Log.w(TAG, "OutOfMemoryError while trying to draw bitmap on canvas.");
         }
     }
+
+    /**
+     * Draw document icon to canvas.
+     * @param context     Context used to get bitmap resources.
+     * @param canvas      canvas Canvas that holds the document icon.

[newt (away), 2013/05/30 22:12:01]

"canvas canvas" typo

[Kibeom Kim (inactive), 2013/05/31 01:23:52]

Done.

+     * @param iconBounds  Rectangle bounds needed to create the homescreen icon.

[newt (away), 2013/05/30 22:12:01]

explain this?

[Kibeom Kim (inactive), 2013/05/31 01:23:52]

Removed iconBounds as an argument because that's redundant information. Instead
we can use the canvas size.

Done.

+     * @param iconDensity Density information to get bitmap resources.

[newt (away), 2013/05/30 22:12:01]

do we even care about the density of the current screen? previously, we were
using this and actually loading the wrong size image assets...

I think you should just load the resources from the mipmap folder (without
specifying a density). Does that work?

[Kibeom Kim (inactive), 2013/05/31 01:23:52]

(talked offline)

Done.

+     * @param color       Color for the document icon's folding and the bottom strip.
+     * @return            True if the document icon is drawn, false otherwise.
+     */
+    private static boolean drawDocumentIconToCanvas(

[newt (away), 2013/05/30 22:12:01]

maybe "drawWidgetBackground", or at least explain what "document icon" means in
the comment above. In general, a comment that equals the method name is
superfluous :)

[Kibeom Kim (inactive), 2013/05/31 01:23:52]

Done.

+            Context context, Canvas canvas, Rect iconBounds, int iconDensity, int color) {
+        Bitmap icon = getBitmapFromResourceId(context, R.mipmap.homescreen_bg, iconDensity);

[newt (away), 2013/05/30 22:12:01]

can you change the image names from homescreen_bg to bookmark_widget_bg.png and
bookmark_widget_highlights.png or something sensible like that?

[Kibeom Kim (inactive), 2013/05/31 01:23:52]

Done.

+        Bitmap icon_overlay =
+                getBitmapFromResourceId(context, R.mipmap.homescreen_bg_overlay, iconDensity);
+        if (icon == null || icon_overlay == null) {
+            Log.w(TAG, "Can't load R.mipmap.homescreen_bg or R.mipmap.homescreen_bg_overlay");
+            return false;
+        }
+        Paint paint = new Paint(Paint.ANTI_ALIAS_FLAG);

[newt (away), 2013/05/30 22:12:01]

anti-aliasing shouldn't have an effect when you're drawing a bitmap, though if
you're scaling the bitmap you should use the FILTER_BITMAP_FLAG.

[Kibeom Kim (inactive), 2013/05/31 01:23:52]

Done. Changed to FILTER_BITMAP_FLAG.

if icon&icon_overlay's size is identical to iconBounds, which is
getLauncherLargeIconSize() in this context, then it won't be scaled. And because
our asset's icon size matches getLauncherLargeIconSize(), it won't be scaled.
But I think it's safe to assume that it might be scaled because we don't have
control over getLauncherLargeIconSize() and Android might decide to change that
for some situations.

+        canvas.drawBitmap(icon, null, iconBounds, paint);
+        paint.setColorFilter(new PorterDuffColorFilter(color, PorterDuff.Mode.SRC_IN));
+        canvas.drawBitmap(icon_overlay, null, iconBounds, paint);
+        return true;
+    }
 }