📰 Make the MORE button more configurable

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

Index: chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/OptionalLeaf.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/OptionalLeaf.java b/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/OptionalLeaf.java
new file mode 100644
index 0000000000000000000000000000000000000000..daffe84f978c557a9c66ae1b9982b8ad7bbac923
--- /dev/null
+++ b/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/OptionalLeaf.java
@@ -0,0 +1,91 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.chrome.browser.ntp.cards;
+
+import org.chromium.chrome.browser.ntp.cards.ImpressionTracker.Listener;
+import org.chromium.chrome.browser.ntp.snippets.SnippetArticle;
+
+/**
+ * An optional leaf (i.e. single item) in the tree. Depending on its internal state (see
+ * {@link #isShown()}), the item will be present or absent from the tree, by manipulating the values
+ * returned from {@link ChildNode} methods. This allows the parent node to not have to add or remove
+ * the optional leaf from its children manually.
+ *
+ * For a non optional leaf, see {@link Leaf}. They have similar interfaces.
+ *
+ * Additionally, {@link OptionalLeaf} can take care of setting up a one-shot
+ * {@link ImpressionTracker}, for convenience. If this is enabled (via the constructor),
+ * {@link #onImpression()} will have to be overridden.
+ */
+public abstract class OptionalLeaf extends ChildNode implements Listener {

[PEConn, 2016/11/01 11:05:44]

Write this as "implements ImpressionTracker.Listener"?

[dgn, 2016/11/01 14:48:11]

Left the impression tracker as it is.

+    private final ImpressionTracker mImpressionTracker;
+
+    protected OptionalLeaf(NodeParent parent, boolean enableImpressionTracking) {
+        super(parent);
+        mImpressionTracker = enableImpressionTracking
+                ? new ImpressionTracker(null, this, /*isOneShot=*/true)
+                : null;
+    }
+
+    @Override
+    public int getItemCount() {
+        return isShown() ? 1 : 0;
+    }
+
+    @Override
+    public int getItemViewType(int position) {
+        checkIndex(position);
+        return getItemViewType();
+    }
+
+    @Override
+    public void onBindViewHolder(NewTabPageViewHolder holder, int position) {
+        checkIndex(position);
+        onBindViewHolder(holder);
+        if (mImpressionTracker == null) return;
+        mImpressionTracker.reset(mImpressionTracker.wasTriggered() ? null : holder.itemView);
+    }
+
+    @Override
+    public SnippetArticle getSuggestionAt(int position) {
+        checkIndex(position);
+        return null;
+    }
+
+    @Override
+    public int getDismissSiblingPosDelta(int position) {
+        checkIndex(position);
+        return 0;
+    }
+
+    @Override
+    public void onImpression() {
+        // No-op so that only implementations that want to enable impression tracking have to
+        // override it.
+    }
+
+    /**
+     * Display the data for this item.
+     * @param holder The view holder that should be updated.
+     * @see #onBindViewHolder(NewTabPageViewHolder, int)
+     * @see android.support.v7.widget.RecyclerView.Adapter#onBindViewHolder
+     */
+    protected abstract void onBindViewHolder(NewTabPageViewHolder holder);
+
+    /**
+     * @return The view type of this item.
+     * @see android.support.v7.widget.RecyclerView.Adapter#getItemViewType
+     */
+    @ItemViewType
+    protected abstract int getItemViewType();
+
+    public abstract boolean isShown();
+
+    protected void checkIndex(int position) {
+        if (position < 0 || position >= getItemCount()) {

[PEConn, 2016/11/01 11:05:44]

Shouldn't this just be
if (position != 0)
?

[dgn, 2016/11/01 14:48:11]

getItemCount can return 0 or 1 depending on #isShown(). So we want position == 0
to be allowed only when isShown() is true. When there are no Items we should not
do any item manipulation and we would throw the exception.

+            throw new IndexOutOfBoundsException(position + "/" + getItemCount());
+        }
+    }
+}