[NTP Client] Split TreeNode#init into SetParent and SetChildren

Unties Node construction with the initialisation of their parent and
children. This allows controlling when the notifications about child
modifications are propagated, and simplifies the initialisation order.

The InnerNode#setParent() call is now what enables the notifications
and is done as the last step of the NewTabPageAdapter constructor.

ChildNode and InnerNode share almost all of the logic related to
managing the tree and their implementations make use of that without
specific modifications.

This also fixes some bugs related to resetting the section list.

BUG=616090,674023

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

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

@@ -41,7 +41,6 @@ class ActionItem extends OptionalLeaf {
     private boolean mImpressionTracked;
 
     public ActionItem(SuggestionsSection section) {
-        super(section);
         mCategoryInfo = section.getCategoryInfo();
         mParentSection = section;
     }

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

@@ -22,12 +22,6 @@ import java.util.Calendar;
  * to restore the dismissed sections and load new suggestions from the server.
  */
 public class AllDismissedItem extends OptionalLeaf {
-    /**
-     * @param parent The item's parent node.
-     */
-    public AllDismissedItem(NodeParent parent) {
-        super(parent);
-    }
 
     @Override
     @ItemViewType

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

@@ -4,31 +4,34 @@
 
 package org.chromium.chrome.browser.ntp.cards;
 
+import android.support.annotation.CallSuper;
+
 /**
  * A node in the tree that has a parent and can notify it about changes.
  *
  * This class mostly serves as a convenience base class for implementations of {@link TreeNode}.
  */
 public abstract class ChildNode implements TreeNode {
-    private final NodeParent mParent;
+    private NodeParent mParent;
 
-    protected ChildNode(NodeParent parent) {
+    @Override
+    @CallSuper
+    public void setParent(NodeParent parent) {
+        assert mParent == null;
+        assert parent != null;
         mParent = parent;
     }
 
-    @Override
-    public void init() {}
-
     protected void notifyItemRangeChanged(int index, int count) {
-        mParent.onItemRangeChanged(this, index, count);
+        if (mParent != null) mParent.onItemRangeChanged(this, index, count);
     }
 
     protected void notifyItemRangeInserted(int index, int count) {
-        mParent.onItemRangeInserted(this, index, count);
+        if (mParent != null) mParent.onItemRangeInserted(this, index, count);
     }
 
     protected void notifyItemRangeRemoved(int index, int count) {
-        mParent.onItemRangeRemoved(this, index, count);
+        if (mParent != null) mParent.onItemRangeRemoved(this, index, count);
     }
 
     protected void notifyItemChanged(int index) {

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

@@ -19,12 +19,6 @@ import org.chromium.ui.text.SpanApplier;
  * A footer to show some text and a link to learn more.
  */
 public class Footer extends OptionalLeaf {
-    /**
-     * @param parent The footer's parent node.
-     */
-    public Footer(NodeParent parent) {
-        super(parent);
-    }
 
     @Override
     @ItemViewType

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

@@ -4,62 +4,58 @@
 
 package org.chromium.chrome.browser.ntp.cards;
 
+import org.chromium.base.VisibleForTesting;
 import org.chromium.chrome.browser.ntp.snippets.SnippetArticle;
 
+import java.util.ArrayList;
 import java.util.List;
 
 /**
  * An inner node in the tree: the root of a subtree, with a list of child nodes.
  */
-public abstract class InnerNode extends ChildNode implements NodeParent {
-    public InnerNode(NodeParent parent) {
-        super(parent);
-    }
-
-    protected abstract List<TreeNode> getChildren();
+public class InnerNode extends ChildNode implements NodeParent {
+    private final List<TreeNode> mChildren = new ArrayList<>();
 
     private int getChildIndexForPosition(int position) {
         if (position < 0) {
             throw new IndexOutOfBoundsException(Integer.toString(position));
         }
-        List<TreeNode> children = getChildren();
         int numItems = 0;
-        int numChildren = children.size();
+        int numChildren = mChildren.size();
         for (int i = 0; i < numChildren; i++) {
-            numItems += children.get(i).getItemCount();
+            numItems += mChildren.get(i).getItemCount();
             if (position < numItems) return i;
         }
         throw new IndexOutOfBoundsException(position + "/" + numItems);
     }
 
     private int getStartingOffsetForChildIndex(int childIndex) {
-        List<TreeNode> children = getChildren();
-        if (childIndex < 0 || childIndex >= children.size()) {
-            throw new IndexOutOfBoundsException(childIndex + "/" + children.size());
+        if (childIndex < 0 || childIndex >= mChildren.size()) {
+            throw new IndexOutOfBoundsException(childIndex + "/" + mChildren.size());
         }
 
         int offset = 0;
         for (int i = 0; i < childIndex; i++) {
-            offset += children.get(i).getItemCount();
+            offset += mChildren.get(i).getItemCount();
         }
         return offset;
     }
 
     int getStartingOffsetForChild(TreeNode child) {
-        return getStartingOffsetForChildIndex(getChildren().indexOf(child));
+        return getStartingOffsetForChildIndex(mChildren.indexOf(child));
     }
 
     /**
      * Returns the child whose subtree contains the item at the given position.
      */
     TreeNode getChildForPosition(int position) {
-        return getChildren().get(getChildIndexForPosition(position));
+        return mChildren.get(getChildIndexForPosition(position));
     }
 
     @Override
     public int getItemCount() {
         int numItems = 0;
-        for (TreeNode child : getChildren()) {
+        for (TreeNode child : mChildren) {
             numItems += child.getItemCount();
         }
         return numItems;
@@ -69,28 +65,28 @@ public abstract class InnerNode extends ChildNode implements NodeParent {
     @ItemViewType
     public int getItemViewType(int position) {
         int index = getChildIndexForPosition(position);
-        return getChildren().get(index).getItemViewType(
+        return mChildren.get(index).getItemViewType(
                 position - getStartingOffsetForChildIndex(index));
     }
 
     @Override
     public void onBindViewHolder(NewTabPageViewHolder holder, int position) {
         int index = getChildIndexForPosition(position);
-        getChildren().get(index).onBindViewHolder(
+        mChildren.get(index).onBindViewHolder(
                 holder, position - getStartingOffsetForChildIndex(index));
     }
 
     @Override
     public SnippetArticle getSuggestionAt(int position) {
         int index = getChildIndexForPosition(position);
-        return getChildren().get(index).getSuggestionAt(
+        return mChildren.get(index).getSuggestionAt(
                 position - getStartingOffsetForChildIndex(index));
     }
 
     @Override
     public int getDismissSiblingPosDelta(int position) {
         int index = getChildIndexForPosition(position);
-        return getChildren().get(index).getDismissSiblingPosDelta(
+        return mChildren.get(index).getDismissSiblingPosDelta(
                 position - getStartingOffsetForChildIndex(index));
     }
 
@@ -109,33 +105,63 @@ public abstract class InnerNode extends ChildNode implements NodeParent {
         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.
+     * Helper method that adds a new child node and notifies about its insertion.
      *
      * @param child The child node to be added.
      */
-    protected void didAddChild(TreeNode child) {
+    protected void addChild(TreeNode child) {
+        int insertedIndex = getItemCount();
+        mChildren.add(child);
+        child.setParent(this);
+
         int count = child.getItemCount();
-        if (count > 0) onItemRangeInserted(child, 0, count);
-        child.init();
+        if (count > 0) notifyItemRangeInserted(insertedIndex, count);
+    }
+
+    /**
+     * Helper method that adds all the children and notifies about the inserted items.
+     */
+    protected void addChildren(TreeNode... children) {
+        int initialCount = getItemCount();
+        for (TreeNode child : children) {
+            mChildren.add(child);
+            child.setParent(this);
+        }
+
+        int addedCount = getItemCount() - initialCount;
+        if (addedCount > 0) notifyItemRangeInserted(initialCount, addedCount);
     }
 
     /**
-     * Helper method for removing a child node. Notifies about the removed items.
+     * Helper method that removes a child node and notifies about the removed items.
      *
      * @param child The child node to be removed.
      */
-    protected void willRemoveChild(TreeNode child) {
+    protected void removeChild(TreeNode child) {
+        int removedIndex = mChildren.indexOf(child);
+        if (removedIndex == -1) throw new IndexOutOfBoundsException();
+
         int count = child.getItemCount();
-        if (count > 0) onItemRangeRemoved(child, 0, count);
+        int childStartingOffset = getStartingOffsetForChildIndex(removedIndex);
+
+        mChildren.remove(removedIndex);
+        if (count > 0) notifyItemRangeRemoved(childStartingOffset, count);
+    }
+
+    /**
+     * Helper method that removes all the children and notifies about the removed items.
+     */
+    protected void removeChildren() {
+        int itemCount = getItemCount();
+        if (itemCount == 0) return;
+
+        mChildren.clear();
+        notifyItemRangeRemoved(0, itemCount);
+    }
+
+    @VisibleForTesting
+    final List<TreeNode> getChildren() {
+        return mChildren;
     }
 }

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

@@ -11,7 +11,7 @@ import org.chromium.chrome.browser.ntp.snippets.SnippetArticle;
  * If the leaf is not to be a permanent member of the tree, see {@link OptionalLeaf} for an
  * implementation that will take care of hiding or showing the item.
  */
-public abstract class Leaf implements TreeNode {
+public abstract class Leaf extends ChildNode {
     @Override
     public int getItemCount() {
         return 1;
@@ -42,9 +42,6 @@ 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/NewTabPageAdapter.java

@@ -25,9 +25,6 @@ import org.chromium.chrome.browser.ntp.snippets.SnippetArticleViewHolder;
 import org.chromium.chrome.browser.ntp.snippets.SuggestionsSource;
 import org.chromium.chrome.browser.offlinepages.OfflinePageBridge;
 
-import java.util.Arrays;
-import java.util.List;
-
 /**
  * A class that handles merging above the fold elements and below the fold cards into an adapter
  * that will be used to back the NTP RecyclerView. The first element in the adapter should always be
@@ -43,10 +40,6 @@ public class NewTabPageAdapter extends Adapter<NewTabPageViewHolder> implements
     private final ItemTouchCallbacks mItemTouchCallbacks = new ItemTouchCallbacks();
     private NewTabPageRecyclerView mRecyclerView;
 
-    /**
-     * List of all child nodes (which can themselves contain multiple child nodes).
-     */
-    private final List<TreeNode> mChildren;
     private final InnerNode mRoot;
 
     private final AboveTheFoldItem mAboveTheFold = new AboveTheFoldItem();
@@ -132,23 +125,18 @@ public class NewTabPageAdapter extends Adapter<NewTabPageViewHolder> implements
         mNewTabPageManager = manager;
         mAboveTheFoldView = aboveTheFoldView;
         mUiConfig = uiConfig;
-        mRoot = new InnerNode(this) {
-            @Override
-            protected List<TreeNode> getChildren() {
-                return mChildren;
-            }
-        };
+        mRoot = new InnerNode();
 
-        mSections = new SectionList(mRoot, mNewTabPageManager, offlinePageBridge);
-        mSigninPromo = new SignInPromo(mRoot, mNewTabPageManager);
-        mAllDismissed = new AllDismissedItem(mRoot);
-        mFooter = new Footer(mRoot);
+        mSections = new SectionList(mNewTabPageManager, offlinePageBridge);
+        mSigninPromo = new SignInPromo(mNewTabPageManager);
+        mAllDismissed = new AllDismissedItem();
+        mFooter = new Footer();
 
-        mChildren = Arrays.asList(
+        mRoot.addChildren(
                 mAboveTheFold, mSections, mSigninPromo, mAllDismissed, mFooter, mBottomSpacer);
-        mRoot.init();
 
         updateAllDismissedVisibility();
+        mRoot.setParent(this);
     }
 
     /** Returns callbacks to configure the interactions with the RecyclerView's items. */
@@ -251,7 +239,7 @@ public class NewTabPageAdapter extends Adapter<NewTabPageViewHolder> implements
     public void onItemRangeInserted(TreeNode child, int itemPosition, int itemCount) {
         assert child == mRoot;
         notifyItemRangeInserted(itemPosition, itemCount);
-        notifyItemChanged(getItemCount() - 1); // Refresh the spacer too.
+        mBottomSpacer.refresh();
 
         updateAllDismissedVisibility();
     }
@@ -260,7 +248,7 @@ public class NewTabPageAdapter extends Adapter<NewTabPageViewHolder> implements
     public void onItemRangeRemoved(TreeNode child, int itemPosition, int itemCount) {
         assert child == mRoot;
         notifyItemRangeRemoved(itemPosition, itemCount);
-        notifyItemChanged(getItemCount() - 1); // Refresh the spacer too.
+        mBottomSpacer.refresh();
 
         updateAllDismissedVisibility();
     }

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

@@ -19,14 +19,6 @@ import org.chromium.chrome.browser.ntp.snippets.SnippetArticle;
 public abstract class OptionalLeaf extends ChildNode {
     private boolean mVisible;
 
-    /**
-     * Constructor for {@link OptionalLeaf}.
-     * By default it is not visible. See {@link #setVisible(boolean)} to update the visibility.
-     */
-    public OptionalLeaf(NodeParent parent) {
-        super(parent);
-    }
-
     @Override
     public int getItemCount() {
         return isVisible() ? 1 : 0;

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

@@ -11,9 +11,6 @@ package org.chromium.chrome.browser.ntp.cards;
  * @see ProgressViewHolder
  */
 class ProgressItem extends OptionalLeaf {
-    protected ProgressItem(NodeParent parent) {
-        super(parent);
-    }
 
     @Override
     @ItemViewType

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

@@ -5,6 +5,7 @@
 package org.chromium.chrome.browser.ntp.cards;
 
 import org.chromium.base.Log;
+import org.chromium.base.VisibleForTesting;
 import org.chromium.chrome.browser.ntp.NewTabPageView.NewTabPageManager;
 import org.chromium.chrome.browser.ntp.snippets.CategoryInt;
 import org.chromium.chrome.browser.ntp.snippets.CategoryStatus;
@@ -15,7 +16,6 @@ import org.chromium.chrome.browser.ntp.snippets.SnippetsConfig;
 import org.chromium.chrome.browser.ntp.snippets.SuggestionsSource;
 import org.chromium.chrome.browser.offlinepages.OfflinePageBridge;
 
-import java.util.ArrayList;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
@@ -29,37 +29,23 @@ public class SectionList extends InnerNode implements SuggestionsSource.Observer
 
     /** Maps suggestion categories to sections, with stable iteration ordering. */
     private final Map<Integer, SuggestionsSection> mSections = new LinkedHashMap<>();
-    private final List<TreeNode> mChildren = new ArrayList<>();
     private final NewTabPageManager mNewTabPageManager;
     private final OfflinePageBridge mOfflinePageBridge;
 
-    public SectionList(NodeParent parent, NewTabPageManager newTabPageManager,
-            OfflinePageBridge offlinePageBridge) {
-        super(parent);
+    public SectionList(NewTabPageManager newTabPageManager, OfflinePageBridge offlinePageBridge) {
         mNewTabPageManager = newTabPageManager;
         mNewTabPageManager.getSuggestionsSource().setObserver(this);
         mOfflinePageBridge = offlinePageBridge;
-    }
-
-    @Override
-    public void init() {
-        super.init();
         resetSections(/* alwaysAllowEmptySections = */ false);
     }
 
-    @Override
-    protected List<TreeNode> getChildren() {
-        return mChildren;
-    }
-
     /**
      * Resets the sections, reloading the whole new tab page content.
      * @param alwaysAllowEmptySections Whether sections are always allowed to be displayed when
      *     they are empty, even when they are normally not.
      */
     public void resetSections(boolean alwaysAllowEmptySections) {
-        mSections.clear();
-        mChildren.clear();
+        removeAllSections();
 
         SuggestionsSource suggestionsSource = mNewTabPageManager.getSuggestionsSource();
         int[] categories = suggestionsSource.getCategories();
@@ -105,10 +91,9 @@ public class SectionList extends InnerNode implements SuggestionsSource.Observer
 
         // Create the section if needed.
         if (section == null) {
-            section = new SuggestionsSection(this, mNewTabPageManager, mOfflinePageBridge, info);
+            section = new SuggestionsSection(mNewTabPageManager, mOfflinePageBridge, info);
             mSections.put(category, section);
-            mChildren.add(section);
-            didAddChild(section);
+            addChild(section);
         }
 
         // Add the new suggestions.
@@ -229,10 +214,15 @@ public class SectionList extends InnerNode implements SuggestionsSource.Observer
         removeSection(section);
     }
 
-    private void removeSection(SuggestionsSection section) {
+    @VisibleForTesting
+    void removeSection(SuggestionsSection section) {
         mSections.remove(section.getCategory());
-        willRemoveChild(section);
-        mChildren.remove(section);
+        removeChild(section);
+    }
+
+    private void removeAllSections() {
+        mSections.clear();
+        removeChildren();
     }
 
     /**

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

@@ -42,8 +42,7 @@ public class SignInPromo extends OptionalLeaf
     @Nullable
     private final SigninObserver mObserver;
 
-    public SignInPromo(NodeParent parent, NewTabPageManager newTabPageManager) {
-        super(parent);
+    public SignInPromo(NewTabPageManager newTabPageManager) {
         mDismissed = ChromePreferenceManager.getInstance(ContextUtils.getApplicationContext())
                              .getNewTabPageSigninPromoDismissed();
 
@@ -54,12 +53,7 @@ public class SignInPromo extends OptionalLeaf
             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/SpacingItem.java

@@ -41,4 +41,9 @@ public class SpacingItem extends Leaf {
     protected void onBindViewHolder(NewTabPageViewHolder holder) {
         // Nothing to do.
     }
+
+    /** Schedules a recalculation of the space occupied by the item. */
+    public void refresh() {
+        notifyItemChanged(0);
+    }
 }
\ No newline at end of file

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

@@ -14,20 +14,14 @@ import org.chromium.chrome.R;
  * configuration that affects the NTP suggestions.
  */
 public abstract class StatusItem extends OptionalLeaf implements StatusCardViewHolder.DataSource {
-
-    protected StatusItem(NodeParent parent) {
-        super(parent);
-    }
-
-    public static StatusItem createNoSuggestionsItem(SuggestionsSection parentSection) {
-        return new NoSuggestionsItem(parentSection);
+    public static StatusItem createNoSuggestionsItem(SuggestionsCategoryInfo categoryInfo) {
+        return new NoSuggestionsItem(categoryInfo);
     }
 
     private static class NoSuggestionsItem extends StatusItem {
         private final String mDescription;
-        public NoSuggestionsItem(SuggestionsSection parentSection) {
-            super(parentSection);
-            mDescription = parentSection.getCategoryInfo().getNoSuggestionsMessage();
+        public NoSuggestionsItem(SuggestionsCategoryInfo categoryInfo) {
+            mDescription = categoryInfo.getNoSuggestionsMessage();
         }
 
         @Override

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

@@ -20,7 +20,6 @@ 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;
 
@@ -41,35 +40,21 @@ public class SuggestionsSection extends InnerNode {
     private final ActionItem mMoreButton;
     private final ProgressItem mProgressIndicator;
 
-    private final List<TreeNode> mChildren;
-
     private boolean mIsNtpDestroyed;
 
-    public SuggestionsSection(NodeParent parent, NewTabPageManager manager,
-            OfflinePageBridge offlinePageBridge, SuggestionsCategoryInfo info) {
-        super(parent);
+    public SuggestionsSection(NewTabPageManager manager, OfflinePageBridge offlinePageBridge,
+            SuggestionsCategoryInfo info) {
         mCategoryInfo = info;
         mOfflinePageBridge = offlinePageBridge;
 
         mHeader = new SectionHeader(info.getTitle());
-        mSuggestionsList = new SuggestionsList(this, info);
-        mStatus = StatusItem.createNoSuggestionsItem(this);
+        mSuggestionsList = new SuggestionsList(info);
+        mStatus = StatusItem.createNoSuggestionsItem(info);
         mMoreButton = new ActionItem(this);
-        mProgressIndicator = new ProgressItem(this);
-
-        mChildren = Arrays.asList(
-                mHeader,
-                mSuggestionsList,
-                mStatus,
-                mMoreButton,
-                mProgressIndicator);
+        mProgressIndicator = new ProgressItem();
+        addChildren(mHeader, mSuggestionsList, mStatus, mMoreButton, mProgressIndicator);
 
         setupOfflinePageBridgeObserver(manager);
-    }
-
-    @Override
-    public void init() {
-        super.init();
         refreshChildrenVisibility();
     }
 
@@ -77,8 +62,7 @@ public class SuggestionsSection extends InnerNode {
         private final List<SnippetArticle> mSuggestions = new ArrayList<>();
         private final SuggestionsCategoryInfo mCategoryInfo;
 
-        public SuggestionsList(NodeParent parent, SuggestionsCategoryInfo categoryInfo) {
-            super(parent);
+        public SuggestionsList(SuggestionsCategoryInfo categoryInfo) {
             mCategoryInfo = categoryInfo;
         }
 
@@ -140,11 +124,6 @@ public class SuggestionsSection extends InnerNode {
         }
     }
 
-    @Override
-    protected List<TreeNode> getChildren() {
-        return mChildren;
-    }
-
     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,8 +4,6 @@
 
 package org.chromium.chrome.browser.ntp.cards;
 
-import android.support.annotation.CallSuper;
-
 import org.chromium.chrome.browser.ntp.snippets.SnippetArticle;
 
 /**
@@ -17,8 +15,7 @@ interface TreeNode {
      * 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();
+    void setParent(NodeParent parent);
 
     /**
      * Returns the number of items under this subtree. This method may be called

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

@@ -6,11 +6,19 @@ package org.chromium.chrome.browser.ntp.cards;
 
 import static org.hamcrest.CoreMatchers.is;
 import static org.junit.Assert.assertThat;
+import static org.junit.Assert.fail;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.reset;
+import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
+import static org.mockito.Mockito.verifyZeroInteractions;
 import static org.mockito.Mockito.when;
 
+import android.support.annotation.Nullable;
+
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -39,26 +47,15 @@ public class InnerNodeTest {
     @Before
     public void setUp() {
         MockitoAnnotations.initMocks(this);
+        mInnerNode = spy(new InnerNode());
 
-        for (int i = 0; i < ITEM_COUNTS.length; i++) {
-            TreeNode child = mock(TreeNode.class);
-            when(child.getItemCount()).thenReturn(ITEM_COUNTS[i]);
+        for (int childItemCount : ITEM_COUNTS) {
+            TreeNode child = makeDummyNode(childItemCount);
             mChildren.add(child);
+            mInnerNode.addChild(child);
         }
-        mInnerNode = new InnerNode(mParent) {
-            @Override
-            protected List<TreeNode> getChildren() {
-                return mChildren;
-            }
-        };
-    }
 
-    @Test
-    public void testInit() {
-        mInnerNode.init();
-        for (TreeNode child : mChildren) {
-            verify(child).init();
-        }
+        mInnerNode.setParent(mParent);
     }
 
     @Test
@@ -112,37 +109,36 @@ public class InnerNodeTest {
     }
 
     @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();
+    public void testAddChild() {
+        final int itemCountBefore = mInnerNode.getItemCount();
+
+        TreeNode child = makeDummyNode(23);
+        mInnerNode.addChild(child);
+
+        // The child should have been initialized and the parent hierarchy notified about it.
+        verify(child).setParent(eq(mInnerNode));
+        verify(mParent).onItemRangeInserted(mInnerNode, itemCountBefore, 23);
+
+        TreeNode child2 = makeDummyNode(0);
+        mInnerNode.addChild(child2);
 
         // The empty child should have been initialized, but there should be no change
         // notifications.
+        verify(child2).setParent(eq(mInnerNode));
         verifyNoMoreInteractions(mParent);
     }
 
     @Test
-    public void testWillRemoveChild() {
-        mInnerNode.willRemoveChild(mChildren.get(4));
-        mChildren.remove(4);
+    public void testRemoveChild() {
+        TreeNode child = mChildren.get(4);
+        mInnerNode.removeChild(child);
 
         // The parent should have been notified about the removed items.
         verify(mParent).onItemRangeRemoved(mInnerNode, 6, 3);
 
-        mInnerNode.willRemoveChild(mChildren.get(3));
-        mChildren.remove(3);
+        reset(mParent); // Prepare for the #verifyNoMoreInteractions() call below.
+        TreeNode child2 = mChildren.get(3);
+        mInnerNode.removeChild(child2);
 
         // There should be no change notifications about the empty child.
         verifyNoMoreInteractions(mParent);
@@ -160,5 +156,78 @@ public class InnerNodeTest {
         verify(mParent).onItemRangeChanged(mInnerNode, 6, 6502);
         verify(mParent).onItemRangeRemoved(mInnerNode, 11, 8086);
     }
+
+    /**
+     * Tests that {@link ChildNode} sends the change notifications AFTER its child list is modified.
+     */
+    @Test
+    public void testChangeNotificationsTiming() {
+        // The MockModeParent will enforce a given number of items in the child when notified.
+        MockNodeParent parent = spy(new MockNodeParent());
+        InnerNode rootNode = new InnerNode();
+
+        TreeNode[] children = {makeDummyNode(3), makeDummyNode(5)};
+        rootNode.addChildren(children);
+        rootNode.setParent(parent);
+
+        assertThat(rootNode.getItemCount(), is(8));
+        verifyZeroInteractions(parent); // Before the parent is set, no notifications.
+
+        parent.expectItemCount(24);
+        rootNode.addChildren(makeDummyNode(7), makeDummyNode(9)); // Should bundle the insertions.
+        verify(parent).onItemRangeInserted(eq(rootNode), eq(8), eq(16));
+
+        parent.expectItemCount(28);
+        rootNode.addChild(makeDummyNode(4));
+        verify(parent).onItemRangeInserted(eq(rootNode), eq(24), eq(4));
+
+        parent.expectItemCount(23);
+        rootNode.removeChild(children[1]);
+        verify(parent).onItemRangeRemoved(eq(rootNode), eq(3), eq(5));
+
+        parent.expectItemCount(0);
+        rootNode.removeChildren(); // Bundles the removals in a single change notification
+        verify(parent).onItemRangeRemoved(eq(rootNode), eq(0), eq(23));
+    }
+
+    /**
+     * Implementation of {@link NodeParent} that checks the item count from the node that
+     * sends notifications against defined expectations. Fails on unexpected calls.
+     */
+    private static class MockNodeParent implements NodeParent {
+        @Nullable
+        private Integer mNextExpectedItemCount;
+
+        public void expectItemCount(int count) {
+            mNextExpectedItemCount = count;
+        }
+
+        @Override
+        public void onItemRangeChanged(TreeNode child, int index, int count) {
+            checkCount(child);
+        }
+
+        @Override
+        public void onItemRangeInserted(TreeNode child, int index, int count) {
+            checkCount(child);
+        }
+
+        @Override
+        public void onItemRangeRemoved(TreeNode child, int index, int count) {
+            checkCount(child);
+        }
+
+        private void checkCount(TreeNode child) {
+            if (mNextExpectedItemCount == null) fail("Unexpected call");
+            assertThat(child.getItemCount(), is(mNextExpectedItemCount));
+            mNextExpectedItemCount = null;
+        }
+    }
+
+    private static TreeNode makeDummyNode(int itemCount) {
+        TreeNode node = mock(TreeNode.class);
+        doReturn(itemCount).when(node).getItemCount();
+        return node;
+    }
 }
 

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

@@ -347,8 +347,8 @@ public class SuggestionsSectionTest {
     }
 
     private SuggestionsSection createSection(SuggestionsCategoryInfo info) {
-        SuggestionsSection section = new SuggestionsSection(mParent, mManager, mBridge, info);
-        section.init();
+        SuggestionsSection section = new SuggestionsSection(mManager, mBridge, info);
+        section.setParent(mParent);
         return section;
     }