[Android NTP] Move suggestion sections into a separate node.

Also, define how initialization of nodes works, by adding an init()
method that is called after creating a node.

Initialization now happens after the full tree structure has been
created, and recursively processes all nodes in the tree. Subtrees that
are added later use the same pattern; InnerNode now has helper methods
for this.

BUG=616090

Review-Url: https://codereview.chromium.org/2513453004
Cr-Commit-Position: refs/heads/master@{#438135}

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

@@ -14,7 +14,6 @@ import android.widget.TextView;
 
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.ntp.NewTabPageUma;
-import org.chromium.chrome.browser.ntp.NewTabPageView.NewTabPageManager;
 
 import java.util.Calendar;
 
@@ -48,8 +47,7 @@ public class AllDismissedItem extends OptionalLeaf {
     public static class ViewHolder extends NewTabPageViewHolder {
         private final TextView mBodyTextView;
 
-        public ViewHolder(
-                ViewGroup root, final NewTabPageManager manager, final NewTabPageAdapter adapter) {
+        public ViewHolder(ViewGroup root, final SectionList sections) {
             super(LayoutInflater.from(root.getContext())
                             .inflate(R.layout.new_tab_page_all_dismissed, root, false));
             mBodyTextView = (TextView) itemView.findViewById(R.id.body_text);
@@ -60,9 +58,7 @@ public class AllDismissedItem extends OptionalLeaf {
                         public void onClick(View v) {
                             NewTabPageUma.recordAction(
                                     NewTabPageUma.ACTION_CLICKED_ALL_DISMISSED_REFRESH);
-                            manager.getSuggestionsSource().restoreDismissedCategories();
-                            adapter.resetSections(/*allowEmptySections=*/true);
-                            manager.getSuggestionsSource().fetchRemoteSuggestions();
+                            sections.restoreDismissedSections();
                         }
                     });
         }

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

@@ -16,6 +16,9 @@ public abstract class ChildNode implements TreeNode {
         mParent = parent;
     }
 
+    @Override
+    public void init() {}
+
     protected void notifyItemRangeChanged(int index, int count) {
         mParent.onItemRangeChanged(this, index, count);
     }

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

@@ -108,4 +108,34 @@ public abstract class InnerNode extends ChildNode implements NodeParent {
     public void onItemRangeRemoved(TreeNode child, int index, int count) {
         notifyItemRangeRemoved(getStartingOffsetForChild(child) + index, count);
     }
+
+    @Override
+    public void init() {
+        super.init();
+        for (TreeNode child : getChildren()) {
+            child.init();
+        }
+    }
+
+    /**
+     * Helper method for adding a new child node. Notifies about the inserted items and initializes
+     * the child.
+     *
+     * @param child The child node to be added.
+     */
+    protected void didAddChild(TreeNode child) {
+        int count = child.getItemCount();
+        if (count > 0) onItemRangeInserted(child, 0, count);
+        child.init();
+    }
+
+    /**
+     * Helper method for removing a child node. Notifies about the removed items.
+     *
+     * @param child The child node to be removed.
+     */
+    protected void willRemoveChild(TreeNode child) {
+        int count = child.getItemCount();
+        if (count > 0) onItemRangeRemoved(child, 0, count);
+    }
 }

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

@@ -42,6 +42,9 @@ public abstract class Leaf implements TreeNode {
         return 0;
     }
 
+    @Override
+    public void init() {}
+
     /**
      * Display the data for this item.
      * @param holder The view holder that should be updated.

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

@@ -42,13 +42,24 @@ public class SignInPromo extends OptionalLeaf
     @Nullable
     private final SigninObserver mObserver;
 
-    public SignInPromo(NodeParent parent) {
+    public SignInPromo(NodeParent parent, NewTabPageManager newTabPageManager) {
         super(parent);
         mDismissed = ChromePreferenceManager.getInstance(ContextUtils.getApplicationContext())
                              .getNewTabPageSigninPromoDismissed();
 
-        final SigninManager signinManager = SigninManager.get(ContextUtils.getApplicationContext());
-        mObserver = mDismissed ? null : new SigninObserver(signinManager);
+        SigninManager signinManager = SigninManager.get(ContextUtils.getApplicationContext());
+        if (mDismissed) {
+            mObserver = null;
+        } else {
+            mObserver = new SigninObserver(signinManager);
+            newTabPageManager.addDestructionObserver(mObserver);
+        }
+    }
+
+    @Override
+    public void init() {
+        super.init();
+        SigninManager signinManager = SigninManager.get(ContextUtils.getApplicationContext());
         setVisible(signinManager.isSignInAllowed() && !signinManager.isSignedInOnNative());
     }
 

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

@@ -20,6 +20,7 @@ import org.chromium.chrome.browser.offlinepages.OfflinePageBridge;
 import org.chromium.chrome.browser.offlinepages.OfflinePageItem;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Iterator;
 import java.util.List;
 
@@ -32,19 +33,20 @@ public class SuggestionsSection extends InnerNode {
 
     private final SuggestionsCategoryInfo mCategoryInfo;
     private final OfflinePageBridge mOfflinePageBridge;
-    private final List<TreeNode> mChildren = new ArrayList<>();
 
     // Children
     private final SectionHeader mHeader;
     private final SuggestionsList mSuggestionsList;
     private final StatusItem mStatus;
-    private final ProgressItem mProgressIndicator;
     private final ActionItem mMoreButton;
+    private final ProgressItem mProgressIndicator;
+
+    private final List<TreeNode> mChildren;
 
     private boolean mIsNtpDestroyed = false;
 
-    public SuggestionsSection(NodeParent parent, SuggestionsCategoryInfo info,
-            NewTabPageManager manager, OfflinePageBridge offlinePageBridge) {
+    public SuggestionsSection(NodeParent parent, NewTabPageManager manager,
+            OfflinePageBridge offlinePageBridge, SuggestionsCategoryInfo info) {
         super(parent);
         mCategoryInfo = info;
         mOfflinePageBridge = offlinePageBridge;
@@ -54,11 +56,23 @@ public class SuggestionsSection extends InnerNode {
         mStatus = StatusItem.createNoSuggestionsItem(this);
         mMoreButton = new ActionItem(this);
         mProgressIndicator = new ProgressItem(this);
-        initializeChildren();
+
+        mChildren = Arrays.asList(
+                mHeader,
+                mSuggestionsList,
+                mStatus,
+                mMoreButton,
+                mProgressIndicator);
 
         setupOfflinePageBridgeObserver(manager);
     }
 
+    @Override
+    public void init() {
+        super.init();
+        refreshChildrenVisibility();
+    }
+
     private static class SuggestionsList extends ChildNode implements Iterable<SnippetArticle> {
         private final List<SnippetArticle> mSuggestions = new ArrayList<>();
         private final SuggestionsCategoryInfo mCategoryInfo;
@@ -131,18 +145,6 @@ public class SuggestionsSection extends InnerNode {
         return mChildren;
     }
 
-    private void initializeChildren() {
-        mChildren.add(mHeader);
-        mChildren.add(mSuggestionsList);
-
-        // Optional leaves.
-        mChildren.add(mStatus); // Needs to be refreshed when the status changes.
-
-        mChildren.add(mMoreButton); // Needs to be refreshed when the suggestions change.
-        mChildren.add(mProgressIndicator); // Needs to be refreshed when the suggestions change.
-        refreshChildrenVisibility();
-    }
-
     private void setupOfflinePageBridgeObserver(NewTabPageManager manager) {
         final OfflinePageBridge.OfflinePageModelObserver observer =
                 new OfflinePageBridge.OfflinePageModelObserver() {

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

@@ -4,6 +4,8 @@
 
 package org.chromium.chrome.browser.ntp.cards;
 
+import android.support.annotation.CallSuper;
+
 import org.chromium.chrome.browser.ntp.snippets.SnippetArticle;
 
 /**
@@ -11,6 +13,17 @@ import org.chromium.chrome.browser.ntp.snippets.SnippetArticle;
  */
 interface TreeNode {
     /**
+     * Initialize the node (and any children underneath it). This method should be called after the
+     * node has been added to the tree, i.e. when it is in the list of its parent's children.
+     * The node may notify its parent about changes that happen during initialization.
+     */
+    @CallSuper
+    void init();
+
+    /**
+     * Returns the number of items under this subtree. This method may be called
+     * before initialization.
+     *
      * @return The number of items under this subtree.
      * @see android.support.v7.widget.RecyclerView.Adapter#getItemCount()
      */

chrome/android/java_sources.gni

@@ -600,6 +600,7 @@ chrome_java_sources = [
   "java/src/org/chromium/chrome/browser/ntp/cards/ProgressIndicatorView.java",
   "java/src/org/chromium/chrome/browser/ntp/cards/ProgressItem.java",
   "java/src/org/chromium/chrome/browser/ntp/cards/ProgressViewHolder.java",
+  "java/src/org/chromium/chrome/browser/ntp/cards/SectionList.java",
   "java/src/org/chromium/chrome/browser/ntp/cards/SignInPromo.java",
   "java/src/org/chromium/chrome/browser/ntp/cards/SpacingItem.java",
   "java/src/org/chromium/chrome/browser/ntp/cards/StatusCardViewHolder.java",

chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/InnerNodeTest.java

@@ -8,6 +8,7 @@ import static org.hamcrest.CoreMatchers.is;
 import static org.junit.Assert.assertThat;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
 import static org.mockito.Mockito.when;
 
 import org.junit.Before;
@@ -20,7 +21,7 @@ import org.robolectric.annotation.Config;
 import org.chromium.chrome.browser.ntp.snippets.SnippetArticle;
 import org.chromium.testing.local.LocalRobolectricTestRunner;
 
-import java.util.Arrays;
+import java.util.ArrayList;
 import java.util.List;
 
 /**
@@ -31,7 +32,7 @@ import java.util.List;
 public class InnerNodeTest {
 
     private static final int[] ITEM_COUNTS = {1, 2, 3, 0, 3, 2, 1};
-    private final List<TreeNode> mChildren = Arrays.asList(new TreeNode[ITEM_COUNTS.length]);
+    private final List<TreeNode> mChildren = new ArrayList<>();
     @Mock private NodeParent mParent;
     private InnerNode mInnerNode;
 
@@ -42,7 +43,7 @@ public class InnerNodeTest {
         for (int i = 0; i < ITEM_COUNTS.length; i++) {
             TreeNode child = mock(TreeNode.class);
             when(child.getItemCount()).thenReturn(ITEM_COUNTS[i]);
-            mChildren.set(i, child);
+            mChildren.add(child);
         }
         mInnerNode = new InnerNode(mParent) {
             @Override
@@ -52,6 +53,14 @@ public class InnerNodeTest {
         };
     }
 
+    @Test
+    public void testInit() {
+        mInnerNode.init();
+        for (TreeNode child : mChildren) {
+            verify(child).init();
+        }
+    }
+
     @Test
     public void testItemCount() {
         assertThat(mInnerNode.getItemCount(), is(12));
@@ -102,6 +111,43 @@ public class InnerNodeTest {
         assertThat(mInnerNode.getSuggestionAt(11), is(article4));
     }
 
+    @Test
+    public void testDidAddChild() {
+        TreeNode child = mock(TreeNode.class);
+        when(child.getItemCount()).thenReturn(23);
+        mChildren.add(3, child);
+        mInnerNode.didAddChild(child);
+
+        // The child should have been initialized and the parent notified about the added items.
+        verify(child).init();
+        verify(mParent).onItemRangeInserted(mInnerNode, 6, 23);
+
+        TreeNode child2 = mock(TreeNode.class);
+        when(child2.getItemCount()).thenReturn(0);
+        mChildren.add(4, child2);
+        mInnerNode.didAddChild(child2);
+        verify(child2).init();
+
+        // The empty child should have been initialized, but there should be no change
+        // notifications.
+        verifyNoMoreInteractions(mParent);
+    }
+
+    @Test
+    public void testWillRemoveChild() {
+        mInnerNode.willRemoveChild(mChildren.get(4));
+        mChildren.remove(4);
+
+        // The parent should have been notified about the removed items.
+        verify(mParent).onItemRangeRemoved(mInnerNode, 6, 3);
+
+        mInnerNode.willRemoveChild(mChildren.get(3));
+        mChildren.remove(3);
+
+        // There should be no change notifications about the empty child.
+        verifyNoMoreInteractions(mParent);
+    }
+
     @Test
     public void testNotifications() {
         mInnerNode.onItemRangeInserted(mChildren.get(0), 0, 23);

chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java

@@ -308,7 +308,8 @@ public class NewTabPageAdapterTest {
     @Test
     @Feature({"Ntp"})
     public void testProgressIndicatorDisplay() {
-        SuggestionsSection section = mAdapter.getSectionForTesting(KnownCategories.ARTICLES);
+        SuggestionsSection section =
+                mAdapter.getSectionListForTesting().getSectionForTesting(KnownCategories.ARTICLES);
         ProgressItem progress = section.getProgressItemForTesting();
 
         mSource.setStatusForCategory(KnownCategories.ARTICLES, CategoryStatus.INITIALIZING);
@@ -402,7 +403,8 @@ public class NewTabPageAdapterTest {
         assertItemsFor(section(3));
 
         // 1.3 - When all suggestions are dismissed
-        SuggestionsSection section42 = mAdapter.getSectionForTesting(category);
+        SuggestionsSection section42 =
+                mAdapter.getSectionListForTesting().getSectionForTesting(category);
         assertSectionMatches(section(3), section42);
         section42.removeSuggestion(articles.get(0));
         section42.removeSuggestion(articles.get(1));
@@ -456,7 +458,8 @@ public class NewTabPageAdapterTest {
         assertItemsFor(section(3).withActionButton());
 
         // 1.3 - When all suggestions are dismissed.
-        SuggestionsSection section42 = mAdapter.getSectionForTesting(category);
+        SuggestionsSection section42 =
+                mAdapter.getSectionListForTesting().getSectionForTesting(category);
         assertSectionMatches(section(3).withActionButton(), section42);
         section42.removeSuggestion(articles.get(0));
         section42.removeSuggestion(articles.get(1));
@@ -480,7 +483,7 @@ public class NewTabPageAdapterTest {
         assertItemsFor(section(3));
 
         // 2.3 - When all suggestions are dismissed.
-        section42 = mAdapter.getSectionForTesting(category);
+        section42 = mAdapter.getSectionListForTesting().getSectionForTesting(category);
         assertSectionMatches(section(3), section42);
         section42.removeSuggestion(articles.get(0));
         section42.removeSuggestion(articles.get(1));
@@ -543,8 +546,6 @@ public class NewTabPageAdapterTest {
     @Test
     @Feature({"Ntp"})
     public void testCategoryOrder() {
-        // Above-the-fold, sign in promo, all-dismissed, footer, spacer.
-        final int basicChildCount = 5;
         FakeSuggestionsSource suggestionsSource = new FakeSuggestionsSource();
         when(mNewTabPageManager.getSuggestionsSource()).thenReturn(suggestionsSource);
         registerCategory(suggestionsSource, KnownCategories.ARTICLES, 0);
@@ -553,17 +554,16 @@ public class NewTabPageAdapterTest {
         registerCategory(suggestionsSource, KnownCategories.DOWNLOADS, 0);
         reloadNtp();
 
-        List<TreeNode> children = mAdapter.getRootForTesting().getChildren();
-        assertEquals(basicChildCount + 4, children.size());
-        assertEquals(AboveTheFoldItem.class, children.get(0).getClass());
+        List<TreeNode> children = mAdapter.getSectionListForTesting().getChildren();
+        assertEquals(4, children.size());
+        assertEquals(SuggestionsSection.class, children.get(0).getClass());
+        assertEquals(KnownCategories.ARTICLES, getCategory(children.get(0)));
         assertEquals(SuggestionsSection.class, children.get(1).getClass());
-        assertEquals(KnownCategories.ARTICLES, getCategory(children.get(1)));
+        assertEquals(KnownCategories.BOOKMARKS, getCategory(children.get(1)));
         assertEquals(SuggestionsSection.class, children.get(2).getClass());
-        assertEquals(KnownCategories.BOOKMARKS, getCategory(children.get(2)));
+        assertEquals(KnownCategories.PHYSICAL_WEB_PAGES, getCategory(children.get(2)));
         assertEquals(SuggestionsSection.class, children.get(3).getClass());
-        assertEquals(KnownCategories.PHYSICAL_WEB_PAGES, getCategory(children.get(3)));
-        assertEquals(SuggestionsSection.class, children.get(4).getClass());
-        assertEquals(KnownCategories.DOWNLOADS, getCategory(children.get(4)));
+        assertEquals(KnownCategories.DOWNLOADS, getCategory(children.get(3)));
 
         // With a different order.
         suggestionsSource = new FakeSuggestionsSource();
@@ -574,17 +574,16 @@ public class NewTabPageAdapterTest {
         registerCategory(suggestionsSource, KnownCategories.BOOKMARKS, 0);
         reloadNtp();
 
-        children = mAdapter.getRootForTesting().getChildren();
-        assertEquals(basicChildCount + 4, children.size());
-        assertEquals(AboveTheFoldItem.class, children.get(0).getClass());
+        children = mAdapter.getSectionListForTesting().getChildren();
+        assertEquals(4, children.size());
+        assertEquals(SuggestionsSection.class, children.get(0).getClass());
+        assertEquals(KnownCategories.ARTICLES, getCategory(children.get(0)));
         assertEquals(SuggestionsSection.class, children.get(1).getClass());
-        assertEquals(KnownCategories.ARTICLES, getCategory(children.get(1)));
+        assertEquals(KnownCategories.PHYSICAL_WEB_PAGES, getCategory(children.get(1)));
         assertEquals(SuggestionsSection.class, children.get(2).getClass());
-        assertEquals(KnownCategories.PHYSICAL_WEB_PAGES, getCategory(children.get(2)));
+        assertEquals(KnownCategories.DOWNLOADS, getCategory(children.get(2)));
         assertEquals(SuggestionsSection.class, children.get(3).getClass());
-        assertEquals(KnownCategories.DOWNLOADS, getCategory(children.get(3)));
-        assertEquals(SuggestionsSection.class, children.get(4).getClass());
-        assertEquals(KnownCategories.BOOKMARKS, getCategory(children.get(4)));
+        assertEquals(KnownCategories.BOOKMARKS, getCategory(children.get(3)));
 
         // With unknown categories.
         suggestionsSource = new FakeSuggestionsSource();
@@ -598,15 +597,14 @@ public class NewTabPageAdapterTest {
         registerCategory(suggestionsSource, 42, 1);
         registerCategory(suggestionsSource, KnownCategories.BOOKMARKS, 1);
 
-        children = mAdapter.getRootForTesting().getChildren();
-        assertEquals(basicChildCount + 3, children.size());
-        assertEquals(AboveTheFoldItem.class, children.get(0).getClass());
+        children = mAdapter.getSectionListForTesting().getChildren();
+        assertEquals(3, children.size());
+        assertEquals(SuggestionsSection.class, children.get(0).getClass());
+        assertEquals(KnownCategories.ARTICLES, getCategory(children.get(0)));
         assertEquals(SuggestionsSection.class, children.get(1).getClass());
-        assertEquals(KnownCategories.ARTICLES, getCategory(children.get(1)));
+        assertEquals(KnownCategories.PHYSICAL_WEB_PAGES, getCategory(children.get(1)));
         assertEquals(SuggestionsSection.class, children.get(2).getClass());
-        assertEquals(KnownCategories.PHYSICAL_WEB_PAGES, getCategory(children.get(2)));
-        assertEquals(SuggestionsSection.class, children.get(3).getClass());
-        assertEquals(KnownCategories.DOWNLOADS, getCategory(children.get(3)));
+        assertEquals(KnownCategories.DOWNLOADS, getCategory(children.get(2)));
     }
 
     @Test
@@ -665,7 +663,7 @@ public class NewTabPageAdapterTest {
         reset(dataObserver);
         suggestionsSource.setSuggestionsForCategory(
                 KnownCategories.ARTICLES, createDummySuggestions(newSuggestionCount));
-        mAdapter.onNewSuggestions(KnownCategories.ARTICLES);
+        mAdapter.getSectionListForTesting().onNewSuggestions(KnownCategories.ARTICLES);
         verify(dataObserver).onItemRangeInserted(2, newSuggestionCount);
         verify(dataObserver).onItemRangeChanged(5 + newSuggestionCount, 1, null); // Spacer refresh
         verify(dataObserver, times(2)).onItemRangeRemoved(2 + newSuggestionCount, 1);
@@ -685,7 +683,8 @@ public class NewTabPageAdapterTest {
         reset(dataObserver);
         suggestionsSource.setSuggestionsForCategory(
                 KnownCategories.ARTICLES, createDummySuggestions(0));
-        mAdapter.onCategoryStatusChanged(KnownCategories.ARTICLES, CategoryStatus.SIGNED_OUT);
+        mAdapter.getSectionListForTesting().onCategoryStatusChanged(
+                KnownCategories.ARTICLES, CategoryStatus.SIGNED_OUT);
         verify(dataObserver).onItemRangeRemoved(2, newSuggestionCount);
         verify(dataObserver).onItemRangeChanged(3, 1, null); // Spacer refresh
         verify(dataObserver).onItemRangeInserted(2, 1); // Status card added
@@ -850,7 +849,7 @@ public class NewTabPageAdapterTest {
 
         // On Sign in, we should reset the sections, bring back suggestions instead of the All
         // Dismissed item.
-        mAdapter.onFullRefreshRequired();
+        mAdapter.getSectionListForTesting().onFullRefreshRequired();
         when(mMockSigninManager.isSignInAllowed()).thenReturn(true);
         signinObserver.onSignedIn();
         // Adapter content:

chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSectionTest.java

@@ -347,7 +347,9 @@ public class SuggestionsSectionTest {
     }
 
     private SuggestionsSection createSection(SuggestionsCategoryInfo info) {
-        return new SuggestionsSection(mParent, info, mManager, mBridge);
+        SuggestionsSection section = new SuggestionsSection(mParent, mManager, mBridge, info);
+        section.init();
+        return section;
     }
 
     private OfflinePageItem createOfflinePageItem(String url, long offlineId) {