Bug 1380808 - Use a single data model, stop using resource ids as viewTypes. r?mcomella draft
authorChenxia Liu <liuche@mozilla.com>
Wed, 16 Aug 2017 14:10:43 -0700
changeset 652849 b25fd9e199b1151bb81c2f9ae8b8680029e7955e
parent 652622 2306e153fba9ca55726ffcce889eaca7a479c29f
child 652850 b0339dce6f690471a5cacedf4fc8d530f422d159
push id76170
push usercliu@mozilla.com
push dateFri, 25 Aug 2017 08:33:06 +0000
reviewersmcomella
bugs1380808
milestone57.0a1
Bug 1380808 - Use a single data model, stop using resource ids as viewTypes. r?mcomella MozReview-Commit-ID: IOcbCi6swo7 MozReview-Commit-ID: IGBWlzAD6N9
mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/StreamHighlightItemContextMenuListener.java
mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/StreamItemAnimator.java
mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/StreamRecyclerAdapter.java
mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/model/Highlight.java
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/StreamHighlightItemContextMenuListener.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/StreamHighlightItemContextMenuListener.java
@@ -9,10 +9,13 @@ import org.mozilla.gecko.activitystream.
 
 /**
  * Provides a method to open the context menu for a highlight item.
  *
  * I tried declaring this inside StreamRecyclerAdapter but I got cyclical inheritance warnings
  * (I don't understand why) so it's here instead.
  */
 public interface StreamHighlightItemContextMenuListener {
-    void openContextMenu(HighlightItem highlightItem, int actualPosition, @NonNull final String interactionExtra);
+    /**
+     * @param position position of item in RecyclerView
+     */
+    void openContextMenu(HighlightItem highlightItem, int position, @NonNull final String interactionExtra);
 }
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/StreamItemAnimator.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/StreamItemAnimator.java
@@ -12,17 +12,17 @@ import org.mozilla.gecko.activitystream.
 
 /**
  * We need our own item animator override to avoid default RV-style animations for certain panels.
  */
 public class StreamItemAnimator extends DefaultItemAnimator {
 
     @Override
     public boolean canReuseUpdatedViewHolder(@NonNull RecyclerView.ViewHolder viewHolder) {
-        if (viewHolder.getItemViewType() == TopPanel.LAYOUT_ID) {
+        if (viewHolder.getItemViewType() == StreamRecyclerAdapter.RowItemType.TOP_PANEL.getViewType()) {
             // The top panel doesn't ever change in size. We really don't want to reload it
             // because it has its own state (i.e. the ViewPager state gets lost, and we
             // also flicker when creating a new ViewHolder). Therefore we should try to
             // keep the existing TopPanel whenever possible.
             return true;
         }
 
         return super.canReuseUpdatedViewHolder(viewHolder);
--- a/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/StreamRecyclerAdapter.java
+++ b/mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/StreamRecyclerAdapter.java
@@ -22,123 +22,145 @@ import org.mozilla.gecko.activitystream.
 import org.mozilla.gecko.activitystream.homepanel.stream.HighlightItem;
 import org.mozilla.gecko.activitystream.homepanel.stream.HighlightsTitle;
 import org.mozilla.gecko.activitystream.homepanel.stream.StreamItem;
 import org.mozilla.gecko.activitystream.homepanel.stream.TopPanel;
 import org.mozilla.gecko.activitystream.homepanel.stream.WelcomePanel;
 import org.mozilla.gecko.util.StringUtils;
 import org.mozilla.gecko.widget.RecyclerViewClickSupport;
 
-import java.util.Collections;
 import java.util.EnumSet;
+import java.util.LinkedList;
 import java.util.List;
 
+/**
+ * The adapter for the Activity Stream panel.
+ *
+ * Every item is in a single adapter: Top Sites, Welcome panel, Highlights.
+ */
 public class StreamRecyclerAdapter extends RecyclerView.Adapter<StreamItem> implements RecyclerViewClickSupport.OnItemClickListener,
         RecyclerViewClickSupport.OnItemLongClickListener, StreamHighlightItemContextMenuListener {
 
     private static final String LOGTAG = StringUtils.safeSubstring("Gecko" + StreamRecyclerAdapter.class.getSimpleName(), 0, 23);
 
     private Cursor topSitesCursor;
+    private List<RowItem> recyclerViewModel; // List of item types backing this RecyclerView.
+
+    private final RowItemType[] FIXED_ROWS = {RowItemType.TOP_PANEL, RowItemType.WELCOME, RowItemType.HIGHLIGHTS_TITLE};
+    private static final int HIGHLIGHTS_OFFSET = 3; // Topsites, Welcome, Highlights Title
 
     private HomePager.OnUrlOpenListener onUrlOpenListener;
     private HomePager.OnUrlOpenInBackgroundListener onUrlOpenInBackgroundListener;
 
     private int tiles;
     private int tilesSize;
 
-    private List<Highlight> highlights;
+    public interface RowItem {
+        RowItemType getRowItemType();
+    }
+
+    public enum RowItemType {
+        TOP_PANEL (-2), // RecyclerView.NO_ID is -1, so start hard-coded stableIds at -2.
+        WELCOME (-3),
+        HIGHLIGHTS_TITLE (-4),
+        HIGHLIGHT_ITEM (-1); // There can be multiple Highlight Items so caller should handle as a special case.
+
+        public final int stableId;
+
+        RowItemType(int stableId) {
+            this.stableId = stableId;
+        }
+
+        int getViewType() {
+            return this.ordinal();
+        }
+    }
+
+    private static RowItem makeRowItemFromType(final RowItemType type) {
+        return new RowItem() {
+            @Override
+            public RowItemType getRowItemType() {
+                return type;
+            }
+        };
+    }
 
     public StreamRecyclerAdapter() {
         setHasStableIds(true);
-
-        highlights = Collections.emptyList();
+        recyclerViewModel = new LinkedList<>();
+        for (RowItemType type : FIXED_ROWS) {
+            recyclerViewModel.add(makeRowItemFromType(type));
+        }
     }
 
     void setOnUrlOpenListeners(HomePager.OnUrlOpenListener onUrlOpenListener, HomePager.OnUrlOpenInBackgroundListener onUrlOpenInBackgroundListener) {
         this.onUrlOpenListener = onUrlOpenListener;
         this.onUrlOpenInBackgroundListener = onUrlOpenInBackgroundListener;
     }
 
     public void setTileSize(int tiles, int tilesSize) {
         this.tilesSize = tilesSize;
         this.tiles = tiles;
 
         notifyDataSetChanged();
     }
 
     @Override
     public int getItemViewType(int position) {
-        if (position == 0) {
-            return TopPanel.LAYOUT_ID;
-        } else if (position == 1) {
-            return WelcomePanel.LAYOUT_ID;
-        } else if (position == 2) {
-            return HighlightsTitle.LAYOUT_ID;
-        } else if (position < getItemCount()) {
-            return HighlightItem.LAYOUT_ID;
-        } else {
+        if (position >= recyclerViewModel.size()) {
             throw new IllegalArgumentException("Requested position does not exist");
         }
+        return recyclerViewModel.get(position).getRowItemType().getViewType();
     }
 
     @Override
     public StreamItem onCreateViewHolder(ViewGroup parent, final int type) {
         final LayoutInflater inflater = LayoutInflater.from(parent.getContext());
 
-        if (type == TopPanel.LAYOUT_ID) {
-            return new TopPanel(inflater.inflate(type, parent, false), onUrlOpenListener, onUrlOpenInBackgroundListener);
-        } else if (type == WelcomePanel.LAYOUT_ID) {
-            return new WelcomePanel(inflater.inflate(type, parent, false), this);
-        } else if (type == HighlightItem.LAYOUT_ID) {
-            return new HighlightItem(inflater.inflate(type, parent, false), this);
-        } else if (type == HighlightsTitle.LAYOUT_ID) {
-            return new HighlightsTitle(inflater.inflate(type, parent, false));
+        if (type == RowItemType.TOP_PANEL.getViewType()) {
+            return new TopPanel(inflater.inflate(TopPanel.LAYOUT_ID, parent, false), onUrlOpenListener, onUrlOpenInBackgroundListener);
+        } else if (type == RowItemType.WELCOME.getViewType()) {
+            return new WelcomePanel(inflater.inflate(WelcomePanel.LAYOUT_ID, parent, false), this);
+        } else if (type == RowItemType.HIGHLIGHT_ITEM.getViewType()) {
+            return new HighlightItem(inflater.inflate(HighlightItem.LAYOUT_ID, parent, false), this);
+        } else if (type == RowItemType.HIGHLIGHTS_TITLE.getViewType()) {
+            return new HighlightsTitle(inflater.inflate(HighlightsTitle.LAYOUT_ID, parent, false));
         } else {
             throw new IllegalStateException("Missing inflation for ViewType " + type);
         }
     }
 
-    private int translatePositionToCursor(int position) {
-        if (getItemViewType(position) != HighlightItem.LAYOUT_ID) {
-            throw new IllegalArgumentException("Requested cursor position for invalid item");
-        }
-
-        // We have three blank panels at the top, hence remove that to obtain the cursor position
-        return position - 3;
+    private int getHighlightsOffsetFromRVPosition(int position) {
+        return position - HIGHLIGHTS_OFFSET;
     }
 
     @Override
     public void onBindViewHolder(StreamItem holder, int position) {
         int type = getItemViewType(position);
-
-        if (type == HighlightItem.LAYOUT_ID) {
-            final int actualPosition = translatePositionToCursor(position);
-
-            final Highlight highlight = highlights.get(actualPosition);
-
-            ((HighlightItem) holder).bind(highlight, actualPosition, tilesSize);
-        } else if (type == TopPanel.LAYOUT_ID) {
+        if (type == RowItemType.HIGHLIGHT_ITEM.getViewType()) {
+            final Highlight highlight = (Highlight) recyclerViewModel.get(position);
+            ((HighlightItem) holder).bind(highlight, position, tilesSize);
+        } else if (type == RowItemType.TOP_PANEL.getViewType()) {
             ((TopPanel) holder).bind(topSitesCursor, tiles, tilesSize);
         }
     }
 
     @Override
     public void onItemClicked(RecyclerView recyclerView, int position, View v) {
         if (!onItemClickIsValidHighlightItem(position)) {
             return;
         }
 
-        final int actualPosition = translatePositionToCursor(position);
-        final Highlight highlight = highlights.get(actualPosition);
+        final Highlight highlight = (Highlight) recyclerViewModel.get(position);
 
         ActivityStreamTelemetry.Extras.Builder extras = ActivityStreamTelemetry.Extras.builder()
                 .forHighlightSource(highlight.getSource())
                 .set(ActivityStreamTelemetry.Contract.SOURCE_TYPE, ActivityStreamTelemetry.Contract.TYPE_HIGHLIGHTS)
-                .set(ActivityStreamTelemetry.Contract.ACTION_POSITION, actualPosition)
-                .set(ActivityStreamTelemetry.Contract.COUNT, highlights.size());
+                .set(ActivityStreamTelemetry.Contract.ACTION_POSITION, getHighlightsOffsetFromRVPosition(position))
+                .set(ActivityStreamTelemetry.Contract.COUNT, recyclerViewModel.size() - FIXED_ROWS.length);
 
         Telemetry.sendUIEvent(
                 TelemetryContract.Event.LOAD_URL,
                 TelemetryContract.Method.LIST_ITEM,
                 extras.build()
         );
 
         // NB: This is hacky. We need to process telemetry data first, otherwise we run a risk of
@@ -149,23 +171,22 @@ public class StreamRecyclerAdapter exten
 
     @Override
     public boolean onItemLongClicked(final RecyclerView recyclerView, final int position, final View v) {
         if (!onItemClickIsValidHighlightItem(position)) {
             return false;
         }
 
         final HighlightItem highlightItem = (HighlightItem) recyclerView.getChildViewHolder(v);
-        final int actualPosition = translatePositionToCursor(position);
-        openContextMenu(highlightItem, actualPosition, ActivityStreamTelemetry.Contract.INTERACTION_LONG_CLICK);
+        openContextMenu(highlightItem, position, ActivityStreamTelemetry.Contract.INTERACTION_LONG_CLICK);
         return true;
     }
 
     private boolean onItemClickIsValidHighlightItem(final int position) {
-        if (getItemViewType(position) != HighlightItem.LAYOUT_ID) {
+        if (getItemViewType(position) != RowItemType.HIGHLIGHT_ITEM.getViewType()) {
             // Headers (containing topsites and/or the highlights title) do their own click handling as needed
             return false;
         }
 
         // The position this method receives is from RecyclerView.ViewHolder.getAdapterPosition, whose docs state:
         // "Note that if you've called notifyDataSetChanged(), until the next layout pass, the return value of this
         // method will be NO_POSITION."
         //
@@ -179,22 +200,22 @@ public class StreamRecyclerAdapter exten
             Log.w(LOGTAG, "onItemClicked: received NO_POSITION. Returning");
             return false;
         }
 
         return true;
     }
 
     @Override
-    public void openContextMenu(final HighlightItem highlightItem, final int actualPosition, @NonNull final String interactionExtra) {
-        final Highlight highlight = highlights.get(actualPosition);
+    public void openContextMenu(final HighlightItem highlightItem, final int position, @NonNull final String interactionExtra) {
+        final Highlight highlight = (Highlight) recyclerViewModel.get(position);
 
         ActivityStreamTelemetry.Extras.Builder extras = ActivityStreamTelemetry.Extras.builder()
                 .set(ActivityStreamTelemetry.Contract.SOURCE_TYPE, ActivityStreamTelemetry.Contract.TYPE_HIGHLIGHTS)
-                .set(ActivityStreamTelemetry.Contract.ACTION_POSITION, actualPosition)
+                .set(ActivityStreamTelemetry.Contract.ACTION_POSITION, position - HIGHLIGHTS_OFFSET)
                 .set(ActivityStreamTelemetry.Contract.INTERACTION, interactionExtra)
                 .forHighlightSource(highlight.getSource());
 
         ActivityStreamContextMenu.show(highlightItem.itemView.getContext(),
                 highlightItem.getContextMenuAnchor(),
                 extras,
                 ActivityStreamContextMenu.MenuMode.HIGHLIGHT,
                 highlight,
@@ -206,47 +227,35 @@ public class StreamRecyclerAdapter exten
                 TelemetryContract.Event.SHOW,
                 TelemetryContract.Method.CONTEXT_MENU,
                 extras.build()
         );
     }
 
     @Override
     public int getItemCount() {
-        // Number of highlights + Top Sites Panel + Welcome Panel + Highlights Title
-        return highlights.size() + 3;
+        return recyclerViewModel.size();
     }
 
     public void swapHighlights(List<Highlight> highlights) {
-        this.highlights = highlights;
-
+        recyclerViewModel = recyclerViewModel.subList(0, HIGHLIGHTS_OFFSET);
+        recyclerViewModel.addAll(highlights);
         notifyDataSetChanged();
     }
 
     public void swapTopSitesCursor(Cursor cursor) {
         this.topSitesCursor = cursor;
-
         notifyItemChanged(0);
     }
 
     @Override
     public long getItemId(int position) {
-        final int type = getItemViewType(position);
-
-        // RecyclerView.NO_ID is -1, so start our hard-coded IDs at -2.
-        switch (type) {
-            case TopPanel.LAYOUT_ID:
-                return -2;
-            case WelcomePanel.LAYOUT_ID:
-                return -3;
-            case HighlightsTitle.LAYOUT_ID:
-                return -4;
-            case HighlightItem.LAYOUT_ID:
-                final Highlight highlight = highlights.get(translatePositionToCursor(position));
-
-                // Highlights are always picked from recent history - So using the history id should
-                // give us a unique (positive) id.
-                return highlight.getHistoryId();
-            default:
-                throw new IllegalArgumentException("StreamItem with LAYOUT_ID=" + type + " not handled in getItemId()");
+        final int viewType = getItemViewType(position);
+        if (viewType == RowItemType.HIGHLIGHT_ITEM.getViewType()) {
+            // Highlights are always picked from recent history - So using the history id should
+            // give us a unique (positive) id.
+            final Highlight highlight = (Highlight) recyclerViewModel.get(position);
+            return highlight.getHistoryId();
+        } else {
+            return recyclerViewModel.get(position).getRowItemType().stableId;
         }
     }
 }
--- 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
@@ -5,26 +5,25 @@
 
 package org.mozilla.gecko.activitystream.homepanel.model;
 
 import android.database.Cursor;
 import android.support.annotation.NonNull;
 import android.support.annotation.Nullable;
 import android.support.annotation.VisibleForTesting;
 import android.text.TextUtils;
-import android.text.format.DateUtils;
 import org.mozilla.gecko.activitystream.Utils;
+import org.mozilla.gecko.activitystream.homepanel.StreamRecyclerAdapter;
 import org.mozilla.gecko.activitystream.ranking.HighlightCandidateCursorIndices;
 import org.mozilla.gecko.activitystream.ranking.HighlightsRanking;
-import org.mozilla.gecko.db.BrowserContract;
 
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
-public class Highlight implements Item {
+public class Highlight implements Item, StreamRecyclerAdapter.RowItem {
 
     /**
      * A pattern matching a json object containing the key "image_url" and extracting the value. afaik, these urls
      * are not encoded so it's entirely possible that the url will contain a quote and we will not extract the whole
      * url. However, given these are coming from websites providing favicon-like images, it's not likely a quote will
      * appear and since these urls are only being used to compare against one another (as imageURLs in Highlight items),
      * a partial URL may actually have the same behavior: good enough for me!
      */
@@ -84,16 +83,21 @@ public class Highlight implements Item {
         if (metadataJSON == null) {
             return null;
         }
 
         final Matcher matcher = pattern.matcher(metadataJSON);
         return matcher.find() ? matcher.group(1) : null;
     }
 
+    @Override
+    public StreamRecyclerAdapter.RowItemType getRowItemType() {
+        return StreamRecyclerAdapter.RowItemType.HIGHLIGHT_ITEM;
+    }
+
     private void updateState() {
         // We can only be certain of bookmark state if an item is a bookmark item.
         // Otherwise, due to the underlying highlights query, we have to look up states when
         // menus are displayed.
         switch (source) {
             case BOOKMARKED:
                 isBookmarked = true;
                 isPinned = null;