Bug 1400397: Use Icons if overridePageURL fails to load. r=liuche
Icons apparently doesn't fade images, however, so it looks bad. Also, we try to
request the image each time we bind, so scrolling up and down will create
additional pop-in, which also sucks.
MozReview-Commit-ID: 246pokTMFl7
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/stream/StreamOverridablePageIconLayout.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/stream/StreamOverridablePageIconLayout.java
@@ -9,24 +9,26 @@ import android.net.Uri;
import android.support.annotation.NonNull;
import android.support.annotation.Nullable;
import android.text.TextUtils;
import android.util.AttributeSet;
import android.view.LayoutInflater;
import android.view.View;
import android.widget.FrameLayout;
import android.widget.ImageView;
+import com.squareup.picasso.Callback;
import com.squareup.picasso.Picasso;
import org.mozilla.gecko.R;
import org.mozilla.gecko.icons.IconCallback;
import org.mozilla.gecko.icons.IconResponse;
import org.mozilla.gecko.icons.Icons;
import org.mozilla.gecko.util.NetworkUtils;
import org.mozilla.gecko.widget.FaviconView;
+import java.lang.ref.WeakReference;
import java.util.concurrent.Future;
/**
* A layout that represents page icons in Activity Stream, which can be overridden with a custom URL.
*
* Under the hood, it switches between multiple icon views because favicons (in FaviconView)
* are handled differently from other types of page images.
*
@@ -66,27 +68,30 @@ public class StreamOverridablePageIconLa
setUIMode(UIMode.NONFAVICON_IMAGE);
// TODO (bug 1322501): Optimization: since we've already navigated to these pages, there's a chance
// Gecko has the image in its cache: we should try to get it first before making this network request.
Picasso.with(getContext())
.load(Uri.parse(overrideImageURL))
.fit()
.centerCrop()
- .into(imageView);
+ .into(imageView, new OnErrorUsePageURLCallback(this, pageURL));
} else {
- setUIMode(UIMode.FAVICON_IMAGE);
+ setFaviconImage(pageURL);
+ }
+ }
- ongoingFaviconLoad = Icons.with(getContext())
- .pageUrl(pageURL)
- .skipNetwork()
- .forActivityStream()
- .build()
- .execute(this);
- }
+ private void setFaviconImage(@NonNull final String pageURL) {
+ setUIMode(UIMode.FAVICON_IMAGE);
+ ongoingFaviconLoad = Icons.with(getContext())
+ .pageUrl(pageURL)
+ .skipNetwork()
+ .forActivityStream()
+ .build()
+ .execute(this);
}
@Override
public void onIconResponse(final IconResponse response) {
faviconView.updateImage(response);
}
private void setUIMode(final UIMode uiMode) {
@@ -115,9 +120,34 @@ public class StreamOverridablePageIconLa
}
}
private void initViews() {
faviconView = (FaviconView) findViewById(R.id.favicon_view);
imageView = (ImageView) findViewById(R.id.image_view);
setUIMode(UIMode.FAVICON_IMAGE); // set in code to ensure state is consistent.
}
+
+ private static class OnErrorUsePageURLCallback implements Callback {
+ private final WeakReference<StreamOverridablePageIconLayout> layoutWeakReference;
+ private final String pageURL;
+
+ private OnErrorUsePageURLCallback(final StreamOverridablePageIconLayout layoutWeakReference,
+ @NonNull final String pageURL) {
+ this.layoutWeakReference = new WeakReference<>(layoutWeakReference);
+ this.pageURL = pageURL;
+ }
+
+ @Override
+ public void onSuccess() { /* Picasso sets the image, nothing to do. */ }
+
+ @Override
+ public void onError() {
+ // I'm slightly concerned that cancelPendingRequests could get called during
+ // this Picasso -> Icons request chain and we'll get bugs where favicons don't
+ // appear correctly. However, we're already in an unexpected error case so it's
+ // probably not worth worrying about.
+ final StreamOverridablePageIconLayout layout = layoutWeakReference.get();
+ if (layout == null) { return; }
+ layout.setFaviconImage(pageURL);
+ }
+ }
}
\ No newline at end of file