Bug 1390356 - review: getMetadataSlow -> getImageUrl in Item interface. r=mcomella draft
authorMichael Comella <michael.l.comella@gmail.com>
Wed, 23 Aug 2017 13:43:59 -0700
changeset 651517 6a004bb45a1d88d8f0c028a5304042a31eca52d3
parent 646947 a0ce0cb638a985a7c41ecac89014e9b4e22c70a4
child 651538 b56f134a93012d37201ca00ca4a7ae8aa3926492
child 651605 e8c460efd040aba4cf9b4aedb378e0f7fcc96040
push id75758
push usermichael.l.comella@gmail.com
push dateWed, 23 Aug 2017 20:44:41 +0000
reviewersmcomella
bugs1390356
milestone57.0a1
Bug 1390356 - review: getMetadataSlow -> getImageUrl in Item interface. r=mcomella MozReview-Commit-ID: 8pFmBQf4fzp
mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/menu/BottomSheetContextMenu.java
mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/model/Highlight.java
mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/model/Item.java
mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/model/TopSite.java
mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/stream/HighlightItem.java
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/menu/BottomSheetContextMenu.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/menu/BottomSheetContextMenu.java
@@ -84,17 +84,17 @@ import java.net.URISyntaxException;
         final StreamOverridablePageIconLayout pageIconLayout =
                 (StreamOverridablePageIconLayout) content.findViewById(R.id.page_icon_layout);
         final ViewGroup.LayoutParams layoutParams = pageIconLayout.getLayoutParams();
         layoutParams.width = tilesWidth;
         layoutParams.height = tilesHeight;
         pageIconLayout.setLayoutParams(layoutParams);
 
         // We're matching the specific icon behavior for highlights and top sites.
-        final String overrideIconURL = !shouldOverrideIconWithImageProvider ? null : item.getMetadataSlow().getImageUrl();
+        final String overrideIconURL = !shouldOverrideIconWithImageProvider ? null : item.getImageUrl();
         pageIconLayout.updateIcon(item.getUrl(), overrideIconURL);
 
         navigationView = (NavigationView) content.findViewById(R.id.menu);
         navigationView.setNavigationItemSelectedListener(this);
 
         super.postInit();
     }
 
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/model/Highlight.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/model/Highlight.java
@@ -134,16 +134,31 @@ public class Highlight implements Item {
     public Metadata getMetadataSlow() {
         if (metadata == null) {
             metadata = new Metadata(metadataJSON);
         }
         return metadata;
     }
 
     /**
+     * Returns the image URL associated with this Highlight.
+     *
+     * This implementation may be slow: see {@link #getMetadataSlow()}.
+     *
+     * @return the image URL, or the empty String if there is none.
+     */
+    @NonNull
+    @Override
+    public String getImageUrl() {
+        final Metadata metadata = getMetadataSlow();
+        final String imageUrl = metadata.getImageUrl();
+        return imageUrl != null ? imageUrl : "";
+    }
+
+    /**
      * Returns the image url in the highlight's metadata. This value does not provide valid image url but is
      * consistent across invocations and can be used to compare against other Highlight's fast image urls.
      * See {@link #getMetadataSlow()} for a description of why we use this method.
      *
      * To get a valid image url (at a performance penalty), use {@link #getMetadataSlow()}
      * {@link #getMetadataSlow()} & {@link Metadata#getImageUrl()}.
      *
      * Note that this explanation is dependent on the implementation of {@link #initFastImageURL(String)}.
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/model/Item.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/model/Item.java
@@ -7,23 +7,24 @@ import android.support.annotation.Nullab
  * Shared interface for activity stream item models.
  */
 public interface Item {
     String getTitle();
 
     String getUrl();
 
     /**
-     * Returns the metadata associated with this stream item.
+     * Returns the image URL associated with this stream item.
      *
-     * This operation could be slow in some implementations (see {@link Highlight#getMetadataSlow()}), hence the name.
-     * imo, it is better to expose this possibility in the interface for all implementations rather than hide this fact.
+     * Some implementations may be slow due to lazy loading: see {@link Highlight#getImageUrl()}.
+     *
+     * @return the image URL, or the empty String when there is none.
      */
     @NonNull
-    Metadata getMetadataSlow();
+    String getImageUrl();
 
     /**
      * @return True if the item is bookmarked, false otherwise. Might return 'null' if the bookmark
      *         state is unknown and the database needs to be asked whether the URL is bookmarked.
      */
     @Nullable
     Boolean isBookmarked();
 
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/model/TopSite.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/model/TopSite.java
@@ -70,20 +70,21 @@ public class TopSite implements Item {
     public int getType() {
         return type;
     }
 
     public Boolean isPinned() {
         return isPinned;
     }
 
+    @NonNull
     @Override
-    @NonNull
-    public Metadata getMetadataSlow() {
-        return metadata;
+    public String getImageUrl() {
+        final String imageUrl = metadata.getImageUrl();
+        return imageUrl != null ? imageUrl : "";
     }
 
     @Override
     public void updateBookmarked(boolean bookmarked) {
         this.isBookmarked = bookmarked;
     }
 
     @Override
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/stream/HighlightItem.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/stream/HighlightItem.java
@@ -106,17 +106,17 @@ public class HighlightItem extends Strea
 
         ViewGroup.LayoutParams layoutParams = pageIconLayout.getLayoutParams();
         layoutParams.width = tilesWidth;
         layoutParams.height = (int) Math.floor(tilesWidth * SIZE_RATIO);
         pageIconLayout.setLayoutParams(layoutParams);
 
         updateUiForSource(highlight.getSource());
         updatePageDomain();
-        pageIconLayout.updateIcon(highlight.getUrl(), highlight.getMetadataSlow().getImageUrl());
+        pageIconLayout.updateIcon(highlight.getUrl(), highlight.getImageUrl());
     }
 
     private void updateUiForSource(Utils.HighlightSource source) {
         switch (source) {
             case BOOKMARKED:
                 pageSourceView.setText(R.string.activity_stream_highlight_label_bookmarked);
                 pageSourceView.setVisibility(View.VISIBLE);
                 pageSourceIconView.setImageResource(R.drawable.ic_as_bookmarked);