Bug 1341275: Add mShouldShowImage flag to FaviconView. r=sebastian
I verified this fixed the issue by:
- Attaching a debugger and checking against the STR call order (comment 3).
- Ensuring the favicons in the SearchEngineRow looked correct
MozReview-Commit-ID: GJeh3FLim36
--- a/mobile/android/base/java/org/mozilla/gecko/widget/FaviconView.java
+++ b/mobile/android/base/java/org/mozilla/gecko/widget/FaviconView.java
@@ -31,16 +31,33 @@ import java.lang.ref.WeakReference;
* Displays solid colour background around Favicon to fill space not occupied by the icon. Colour
* selected is the dominant colour of the provided Favicon.
*/
public class FaviconView extends ImageView {
// Default x/y-radius of the oval used to round the corners of the background (dp)
private static final int DEFAULT_CORNER_RADIUS_DP = 2;
+ /**
+ * True if we're capable of, and want to, display an image, false otherwise. This acts as a switch: if we
+ * don't want to show an image, most other state will be ignored because we don't need it to draw.
+ *
+ * We use this flag, rather than zeroing all relevant state, because if update*Image is called before
+ * onSizeChanged (during init), we would not be able to show an image and thus zero all state. When
+ * onSizeChanged is called, the state has been zeroed and we would not be able to show the image properly
+ * (bug 1341275).
+ *
+ * One notable exception is for ImageView's internal state via setImageBitmap/Drawable, which must be set to
+ * null if we don't want to draw anything.
+ *
+ * By default, we can't show an image because we don't have a size yet (onSizeChanged) or an image (update*Image),
+ * hence false.
+ */
+ private boolean mShouldShowImage = false;
+
private Bitmap mIconBitmap;
// Reference to the unscaled bitmap, if any, to prevent repeated assignments of the same bitmap
// to the view from causing repeated rescalings (Some of the callers do this)
private Bitmap mUnscaledBitmap;
// Flag indicating if the most recently assigned image is considered likely to need scaling.
private boolean mScalingExpected;
@@ -105,41 +122,45 @@ public class FaviconView extends ImageVi
mBackgroundRect.right = w;
mBackgroundRect.bottom = h;
formatImage();
}
@Override
public void onDraw(Canvas canvas) {
- if (isDominantBorderEnabled) {
+ if (mShouldShowImage && isDominantBorderEnabled) {
sBackgroundPaint.setColor(mDominantColor);
if (areRoundCornersEnabled) {
canvas.drawRoundRect(mBackgroundRect, mBackgroundCornerRadius, mBackgroundCornerRadius, sBackgroundPaint);
} else {
canvas.drawRect(mBackgroundRect, sBackgroundPaint);
}
}
+ // If mShouldShowImage == false, setImageDrawable should be null & we are safe to call super.
super.onDraw(canvas);
}
/**
* Formats the image for display, if the prerequisite data are available. Upscales tiny Favicons to
* normal sized ones, replaces null bitmaps with the default Favicon, and fills all remaining space
* in this view with the coloured background.
*/
private void formatImage() {
- // We're waiting for both onSizeChanged and updateImage to be called before scaling.
- if (mIconBitmap == null || getWidth() == 0 || getHeight() == 0) {
+ // Both onSizeChanged and updateImage have to be called before an image can be shown.
+ final boolean canImageBeShown = (mIconBitmap != null && getWidth() != 0 && getHeight() != 0);
+ if (!canImageBeShown) {
showNoImage();
return;
}
+ mShouldShowImage = true;
+
if (mScalingExpected && getWidth() != mIconBitmap.getWidth()) {
scaleBitmap();
// Don't scale the image every time something changes.
mScalingExpected = false;
}
// In original, there is no round corners if FaviconView has bitmap icon. But the new design
// needs round corners all the time, so we use RoundedBitmapDrawableFactory to create round corners.
@@ -200,18 +221,18 @@ public class FaviconView extends ImageVi
mDominantColor = response.getColor();
mScalingExpected = allowScaling;
// Possibly update the display.
formatImage();
}
private void showNoImage() {
+ mShouldShowImage = false;
setImageDrawable(null);
- mDominantColor = Color.TRANSPARENT;
}
/**
* Clear image and background shown by this view.
*/
public void clearImage() {
showNoImage();
mUnscaledBitmap = null;