Download Home : Fixed thumbnail mis-alignment issue on screen orientation change

This CL fixes the following issues :
1 - AspectRatioFrameLayout doesn't re-adjust its bounds after orientation change.
2 - We don't set the correct matrix for tiny images after view is resized.
3 - Some cleanup for the bitmap resize code.

Bug : 903634

Change-Id: I547ff13b6bb1bd8832966c30cf44900346c0d85f
Reviewed-on: https://chromium-review.googlesource.com/c/1328227
Commit-Queue: Shakti Sahu <shaktisahu@chromium.org>
Reviewed-by: Xing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607038}

chrome/android/java/src/org/chromium/chrome/browser/download/home/list/holder/OfflineItemViewHolder.java

@@ -26,8 +26,6 @@ import org.chromium.components.offline_items_collection.OfflineItemVisuals;
  * Helper that supports all typical actions for OfflineItems.
  */
 class OfflineItemViewHolder extends ListItemViewHolder implements ListMenuButton.Delegate {
-    private static final float IMAGE_VIEW_MAX_SCALE_FACTOR = 4.f;
-
     /** The {@link View} that visually represents the selected state of this list item. */
     protected final SelectionView mSelectionView;
 
@@ -91,13 +89,10 @@ class OfflineItemViewHolder extends ListItemViewHolder implements ListMenuButton
 
         // Push 'thumbnail' state.
         if (mThumbnail != null) {
+            mThumbnail.setImageResizer(new BitmapResizer(mThumbnail));
             mThumbnail.setAsyncImageDrawable((consumer, width, height) -> {
                 return properties.get(ListProperties.PROVIDER_VISUALS)
                         .getVisuals(offlineItem, width, height, (id, visuals) -> {
-                            Matrix matrix = upscaleBitmapIfNecessary(mThumbnail, visuals);
-                            mThumbnail.setImageMatrix(matrix);
-                            mThumbnail.setScaleType(matrix == null ? ImageView.ScaleType.CENTER_CROP
-                                                                   : ImageView.ScaleType.MATRIX);
                             consumer.onResult(onThumbnailRetrieved(visuals));
                         });
             }, offlineItem.id);
@@ -150,33 +145,57 @@ class OfflineItemViewHolder extends ListItemViewHolder implements ListMenuButton
                 != properties.get(ListProperties.SELECTION_MODE_ACTIVE);
     }
 
-    private Matrix upscaleBitmapIfNecessary(ImageView view, OfflineItemVisuals visuals) {
-        Bitmap bitmap = visuals == null ? null : visuals.icon;
-        if (bitmap == null) return null;
+    /**
+     * A class that sets the correct image matrix on the given {@link ImageView} depending on the
+     * size of the bitmap.
+     */
+    private static class BitmapResizer implements AsyncImageView.ImageResizer {
+        private static final float IMAGE_VIEW_MAX_SCALE_FACTOR = 4.f;
 
-        float scale = computeScaleFactor(view, bitmap);
-        if (scale <= 1.f) return null;
+        private ImageView mImageView;
 
-        // Compute the required matrix to scale and center the bitmap.
-        Matrix matrix = new Matrix();
-        matrix.setScale(scale, scale);
-        matrix.postTranslate((view.getWidth() - scale * bitmap.getWidth()) * 0.5f,
-                (view.getHeight() - scale * bitmap.getHeight()) * 0.5f);
-        return matrix;
-    }
+        /** Constructor. */
+        public BitmapResizer(ImageView imageView) {
+            mImageView = imageView;
+        }
 
-    /**
-     * Computes a scale factor for the bitmap if the bitmap is too small compared to the view
-     * dimensions. The scaled bitmap will be centered inside the view. No scaling if the dimensions
-     * are comparable.
-     */
-    private float computeScaleFactor(ImageView view, Bitmap bitmap) {
-        float widthRatio = (float) view.getWidth() / bitmap.getWidth();
-        float heightRatio = (float) view.getHeight() / bitmap.getHeight();
+        @Override
+        public void maybeResizeImage(Drawable drawable) {
+            if (!(drawable instanceof BitmapDrawable)) return;
 
-        if (Math.max(widthRatio, heightRatio) < IMAGE_VIEW_MAX_SCALE_FACTOR) return 1.f;
+            Matrix matrix = upscaleBitmapIfNecessary(((BitmapDrawable) drawable).getBitmap());
+            mImageView.setImageMatrix(matrix);
+            mImageView.setScaleType(
+                    matrix == null ? ImageView.ScaleType.CENTER_CROP : ImageView.ScaleType.MATRIX);
+        }
 
-        float minRequiredScale = Math.min(widthRatio, heightRatio);
-        return Math.min(minRequiredScale, IMAGE_VIEW_MAX_SCALE_FACTOR);
+        private Matrix upscaleBitmapIfNecessary(Bitmap bitmap) {
+            if (bitmap == null) return null;
+
+            float scale = computeScaleFactor(bitmap);
+            if (scale <= 1.f) return null;
+
+            // Compute the required matrix to scale and center the bitmap.
+            Matrix matrix = new Matrix();
+            matrix.setScale(scale, scale);
+            matrix.postTranslate((mImageView.getWidth() - scale * bitmap.getWidth()) * 0.5f,
+                    (mImageView.getHeight() - scale * bitmap.getHeight()) * 0.5f);
+            return matrix;
+        }
+
+        /**
+         * Computes a scale factor for the bitmap if the bitmap is too small compared to the view
+         * dimensions. The scaled bitmap will be centered inside the view. No scaling if the
+         * dimensions are comparable.
+         */
+        private float computeScaleFactor(Bitmap bitmap) {
+            float widthRatio = (float) mImageView.getWidth() / bitmap.getWidth();
+            float heightRatio = (float) mImageView.getHeight() / bitmap.getHeight();
+
+            if (Math.max(widthRatio, heightRatio) < IMAGE_VIEW_MAX_SCALE_FACTOR) return 1.f;
+
+            float minRequiredScale = Math.min(widthRatio, heightRatio);
+            return Math.min(minRequiredScale, IMAGE_VIEW_MAX_SCALE_FACTOR);
+        }
     }
 }

chrome/android/java/src/org/chromium/chrome/browser/download/home/list/view/AspectRatioFrameLayout.java

@@ -71,8 +71,8 @@ public class AspectRatioFrameLayout extends FrameLayout {
             if (!(view.getLayoutParams() instanceof LayoutParams)) continue;
             LayoutParams params = (LayoutParams) view.getLayoutParams();
 
-            params.mRestorableWidth = params.width;
-            params.mRestorableHeight = params.height;
+            params.mRestorableWidth = params.mOriginalWidth;
+            params.mRestorableHeight = params.mOriginalHeight;
 
             float aspectRatio = params.aspectRatio;
             if (aspectRatio <= 0.f) continue;
@@ -124,6 +124,8 @@ public class AspectRatioFrameLayout extends FrameLayout {
         private boolean mOverrodeHeight;
         private int mRestorableWidth;
         private int mRestorableHeight;
+        private int mOriginalWidth;
+        private int mOriginalHeight;
 
         public LayoutParams(Context context, AttributeSet attrs) {
             super(context, attrs);
@@ -157,6 +159,8 @@ public class AspectRatioFrameLayout extends FrameLayout {
             // Do not throw errors if width or height aren't supplied.
             width = a.getLayoutDimension(widthAttr, 0);
             height = a.getLayoutDimension(heightAttr, 0);
+            mOriginalWidth = width;
+            mOriginalHeight = height;
         }
     }
-}
\ No newline at end of file
+}

chrome/android/java/src/org/chromium/chrome/browser/download/home/list/view/AsyncImageView.java

@@ -37,10 +37,20 @@ public class AsyncImageView extends ForegroundRoundedCornerImageView {
         Runnable get(Callback<Drawable> consumer, int widthPx, int heightPx);
     }
 
+    /** Provides the callers an opportunity to override the image size before it is drawn. */
+    public interface ImageResizer {
+        /**
+         * Called by the {@link AsyncImageView} before drawing to the screen.
+         * @param drawable The {@link Drawable} to be drawn.
+         */
+        void maybeResizeImage(Drawable drawable);
+    }
+
     private Drawable mUnavailableDrawable;
     private Drawable mWaitingDrawable;
 
     private Factory mFactory;
+    private ImageResizer mImageResizer;
 
     private Runnable mCancelable;
     private boolean mWaitingForResponse;
@@ -115,6 +125,13 @@ public class AsyncImageView extends ForegroundRoundedCornerImageView {
         if (mWaitingForResponse) setForegroundDrawableCompat(mWaitingDrawable);
     }
 
+    /**
+     * @param resizer Sets a {@link ImageResizer} to use when drawing the image to the screen.
+     */
+    public void setImageResizer(ImageResizer resizer) {
+        mImageResizer = resizer;
+    }
+
     // RoundedCornerImageView implementation.
     @Override
     public void setImageDrawable(Drawable drawable) {
@@ -122,6 +139,7 @@ public class AsyncImageView extends ForegroundRoundedCornerImageView {
         // to something else.
         cancelPreviousDrawableRequest();
 
+        if (mImageResizer != null) mImageResizer.maybeResizeImage(drawable);
         setForegroundDrawableCompat(null);
         super.setImageDrawable(drawable);
     }
@@ -134,6 +152,13 @@ public class AsyncImageView extends ForegroundRoundedCornerImageView {
         retrieveDrawableIfNeeded();
     }
 
+    @Override
+    protected void onSizeChanged(int width, int height, int oldWidth, int oldHeight) {
+        super.onSizeChanged(width, height, oldWidth, oldHeight);
+        if (width == oldWidth && height == oldHeight) return;
+        if (mImageResizer != null) mImageResizer.maybeResizeImage(getDrawable());
+    }
+
     private void setAsyncImageDrawableResponse(Drawable drawable, Object identifier) {
         // If we ended up swapping out the identifier and somehow this request didn't cancel ignore
         // the response.  This does a direct == comparison instead of .equals() because any new