[NTP Snippets] Fill space below the last snippet if necessary

chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapter.java

Index: chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapter.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapter.java b/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapter.java
index 4048be5cddb9a19c98e00fda7e0cfad87b7b5782..c6266c82322223fdb77dd7052da3c27fc1710168 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapter.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapter.java
@@ -39,12 +39,14 @@
     private final AboveTheFoldListItem mAboveTheFoldListItem;
     private final List<NewTabPageListItem> mNewTabPageListItems;
     private final ItemTouchCallbacks mItemTouchCallbacks;
+    private NewTabPageRecyclerView mRecyclerView;
 

[gone, 2016/05/12 18:04:38]

nit: private static finals above private finals?

[dgn, 2016/05/13 13:49:58]

Done.

     private static final Interpolator FADE_INTERPOLATOR = new FastOutLinearInInterpolator();
 
     private class ItemTouchCallbacks extends ItemTouchHelper.Callback {
         @Override
         public void onSwiped(ViewHolder viewHolder, int direction) {
+            mRecyclerView.refreshBottomSpacing();

[Bernhard Bauer, 2016/05/12 11:21:00]

You could move this to dismissItem()?

[dgn, 2016/05/13 13:49:58]

I added a clearView callback, in which I have a call paired with calls here. I
think having them here is clearer with the recent changes.

             NewTabPageAdapter.this.dismissItem(viewHolder.getAdapterPosition());
         }
 
@@ -123,6 +125,7 @@ public void onSnippetsReceived(List<SnippetArticle> listSnippets) {
         mNewTabPageListItems.add(mAboveTheFoldListItem);
         mNewTabPageListItems.add(new SnippetHeaderListItem());
         mNewTabPageListItems.addAll(listSnippets);
+        mNewTabPageListItems.add(new SpacingListItem());
 
         notifyDataSetChanged();
 
@@ -151,6 +154,10 @@ public NewTabPageViewHolder onCreateViewHolder(ViewGroup parent, int viewType) {
                     SnippetArticleViewHolder.createView(parent), mNewTabPageManager);
         }
 
+        if (viewType == NewTabPageListItem.VIEW_TYPE_SPACING) {
+            return new NewTabPageViewHolder(SpacingListItem.createView(parent));
+        }
+
         return null;
     }
 
@@ -164,15 +171,27 @@ public int getItemCount() {
         return mNewTabPageListItems.size();
     }
 
+    @Override
+    public void onAttachedToRecyclerView(RecyclerView recyclerView) {
+        // We are assuming for now that the adapter is used with a single RecyclerView.
+        // getting the reference as we are doing here is going to be broken if that changes.

[Bernhard Bauer, 2016/05/12 11:20:59]

What would change? Could the same adapter in principle actually be used in
multiple views at the same time? Or could the adapter be attached to different
views sequentially? The latter we could deal with, I think.

[gone, 2016/05/12 18:04:38]

I'd be worried about cases where the same adapter were being used to display
data for one RecyclerView, then fail to update the other one.

Out of scope for the current CL, but can Zine handle two different
ChromeTabbedActivities showing NTPs side-by-side in Android N?

[dgn, 2016/05/13 13:49:58]

It supports being watched by multiple views at the same time. I don't think
we'll ever go this route, but I preferred writing that down here

[dgn, 2016/05/13 13:49:58]

That should work, as the objects here are scoped to a specific NewTabPage, and
they are different instances for each new tab.

+        assert mRecyclerView == null;
+        super.onAttachedToRecyclerView(recyclerView);
+        mRecyclerView = (NewTabPageRecyclerView) recyclerView;
+    }
+
     private void dismissItem(int position) {
         assert getItemViewType(position) == NewTabPageListItem.VIEW_TYPE_SNIPPET;
         mNewTabPageManager.onSnippetDismissed((SnippetArticle) mNewTabPageListItems.get(position));
         mNewTabPageListItems.remove(position);
 
         int numRemovedItems = 1;
-        if (mNewTabPageListItems.size() == 2) {
-            // There's only the above-the-fold item and the header left, so we remove the header.
-            position = 1; // When present, the header is always at that position.
+
+        // Check the position before the dismissed item, and the new one that replaced it. If both
+        // are not snippets, they all have been dismissed and we can remove the header.
+        if (getItemViewType(position - 1) == NewTabPageListItem.VIEW_TYPE_HEADER
+                && getItemViewType(position) != NewTabPageListItem.VIEW_TYPE_SNIPPET) {

[gone, 2016/05/12 18:04:39]

Should you explicitly be checking for a SPACING here?  If you add another type
later, this code might break.

[dgn, 2016/05/13 13:49:58]

Removed that code.

+            position -= 1; // Position of the header

[Bernhard Bauer, 2016/05/12 11:21:00]

position--?

[dgn, 2016/05/13 13:49:58]

Removed that code.

             mNewTabPageListItems.remove(position);
             ++numRemovedItems;

[Bernhard Bauer, 2016/05/12 11:21:00]

Also, use postincrement here. The compiler should be able to know the value of
the expression isn't used and optimize correspondingly :)

[dgn, 2016/05/13 13:49:58]

Removed that code.

         }