Pin the Home Page Tile to the first row in the UI.

Calculates the number of columns tiles are layed out in
at initialization and reorders the home page tile so that
it is pinned to the first row.
Follow-up issue: https://crbug.com/746274

BUG=732913

Change-Id: Ieeddf88e8262827c7a22787230c386fe3b43cc17
Reviewed-on: https://chromium-review.googlesource.com/567925
Commit-Queue: Ondrej Škopek <oskopek@google.com>
Reviewed-by: Chris Pickel <sfiera@chromium.org>
Reviewed-by: Nicolas Dossou-Gbété <dgn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488203}

chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java

@@ -289,7 +289,7 @@ public class NewTabPageView extends FrameLayout implements TileGroup.Observer {
         setSearchProviderHasLogo(searchProviderHasLogo);
         mSearchProviderLogoView.showSearchProviderInitialView();
 
-        mTileGroup.startObserving(getMaxTileRows(searchProviderHasLogo) * getMaxTileColumns());
+        mTileGroup.startObserving(getMaxTileRows(searchProviderHasLogo), getMaxTileColumns());
 
         mRecyclerView.init(mUiConfig, mContextMenuManager);
 

chrome/android/java/src/org/chromium/chrome/browser/suggestions/TileGrid.java

@@ -44,7 +44,7 @@ public class TileGrid extends OptionalLeaf implements TileGroup.Observer {
         mTileGroup = new TileGroup(ContextUtils.getApplicationContext(), uiDelegate,
                 contextMenuManager, tileGroupDelegate,
                 /* observer = */ this, offlinePageBridge, getTileTitleLines());
-        mTileGroup.startObserving(getMaxTileRows() * MAX_TILE_COLUMNS);
+        mTileGroup.startObserving(getMaxTileRows(), MAX_TILE_COLUMNS);
     }
 
     @Override

chrome/android/java/src/org/chromium/chrome/browser/suggestions/TileGridLayout.java

@@ -8,10 +8,13 @@ import android.content.Context;
 import android.content.res.Resources;
 import android.text.TextUtils;
 import android.util.AttributeSet;
+import android.util.DisplayMetrics;
+import android.util.TypedValue;
 import android.view.View;
 import android.widget.FrameLayout;
 
 import org.chromium.base.ApiCompatibilityUtils;
+import org.chromium.base.ContextUtils;
 import org.chromium.base.VisibleForTesting;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.util.MathUtils;
@@ -20,6 +23,12 @@ import org.chromium.chrome.browser.util.MathUtils;
  * A layout that arranges tiles in a grid.
  */
 public class TileGridLayout extends FrameLayout {
+    public static final int PADDING_START_PX = 0;
+    public static final int PADDING_END_PX = 0;
+
+    public static final int MARGIN_START_DP = 12;
+    public static final int MARGIN_END_DP = 12;
+
     private final int mVerticalSpacing;
     private final int mMinHorizontalSpacing;
     private final int mMaxHorizontalSpacing;
@@ -29,6 +38,36 @@ public class TileGridLayout extends FrameLayout {
     private int mMaxColumns;
     private int mExtraVerticalSpacing;
 
+    /**
+     * Calculate the number of tile columns that will actually be rendered. Uses constants, does not
+     * rely on anything already being rendered. Used for calculating the position of the home page
+     * tile.
+     *
+     * @see TileGroup#renderTileViews(ViewGroup, boolean)
+     * @return The number of rendered columns (at least 1). Still needs to be capped from above
+     * by the maximum number of columns.
+     */
+    public static int calculateNumColumns() {
+        Resources res = ContextUtils.getApplicationContext().getResources();
+        int minHorizontalSpacingPx =
+                res.getDimensionPixelOffset(R.dimen.tile_grid_layout_min_horizontal_spacing);
+        int maxWidthPx = res.getDimensionPixelOffset(R.dimen.tile_grid_layout_max_width);
+
+        DisplayMetrics metrics = res.getDisplayMetrics();
+        int totalWidthPx =
+                Math.min(maxWidthPx, Math.min(metrics.widthPixels, metrics.heightPixels));
+
+        // Determine the number of columns that will fit.
+        int marginsPx = Math.round(TypedValue.applyDimension(
+                TypedValue.COMPLEX_UNIT_DIP, MARGIN_START_DP + MARGIN_END_DP, metrics));
+        int gridWidthPx = totalWidthPx - PADDING_START_PX - PADDING_END_PX - marginsPx;
+        int childWidthPx = res.getDimensionPixelOffset(R.dimen.tile_view_width);
+
+        return Math.max(
+                (gridWidthPx + minHorizontalSpacingPx) / (childWidthPx + minHorizontalSpacingPx),
+                1);
+    }
+
     /**
      * Constructor for inflating from XML.
      *
@@ -108,8 +147,7 @@ public class TileGridLayout extends FrameLayout {
         }
 
         // Determine the number of columns that will fit.
-        int gridWidth = totalWidth - ApiCompatibilityUtils.getPaddingStart(this)
-                - ApiCompatibilityUtils.getPaddingEnd(this);
+        int gridWidth = totalWidth - PADDING_START_PX - PADDING_END_PX;
         int childHeight = getChildAt(0).getMeasuredHeight();
         int childWidth = getChildAt(0).getMeasuredWidth();
         int numColumns = MathUtils.clamp(

chrome/android/java/src/org/chromium/chrome/browser/suggestions/TileGroup.java

@@ -200,6 +200,14 @@ public class TileGroup implements MostVisitedSites.Observer {
 
     private boolean mHasReceivedData;
 
+    /**
+     * The number of columns tiles get rendered in. Precalculated upon calling
+     * {@link #startObserving(int, int)} and constant from then on. Used for pinning the home page
+     * tile to the first tile row.
+     * @see #renderTileViews(ViewGroup, boolean)
+     */
+    private int mNumColumns;
+
     /**
      * @param context Used for initialisation and resolving resources.
      * @param uiDelegate Delegate used to interact with the rest of the system.
@@ -260,7 +268,22 @@ public class TileGroup implements MostVisitedSites.Observer {
             // send non dupes URLs. Remove once https://crbug.com/703628 is fixed.
             if (addedUrls.contains(urls[i])) continue;
 
-            mPendingTiles.add(new Tile(titles[i], urls[i], whitelistIconPaths[i], i, sources[i]));
+            // The home page tile is pinned to the first row of tiles. It will appear on
+            // the position corresponding to its ranking among all tiles (obtained from the
+            // ntp_tiles C++ component). If its position is larger than the number of tiles
+            // in the first row, it will appear on the last position of the first row.
+            // Do note, that the number of tiles in a row (column number) is determined upon
+            // initialization and not changed afterwards.
+            if (sources[i] == TileSource.HOMEPAGE) {
+                int homeTilePosition = Math.min(mPendingTiles.size(), mNumColumns - 1);
+                mPendingTiles.add(homeTilePosition,
+                        new Tile(titles[i], urls[i], whitelistIconPaths[i], homeTilePosition,
+                                sources[i]));
+            } else {
+                mPendingTiles.add(new Tile(titles[i], urls[i], whitelistIconPaths[i],
+                        mPendingTiles.size(), sources[i]));
+            }
+
             addedUrls.add(urls[i]);
 
             if (urls[i].equals(mPendingRemovalUrl)) removalCompleted = false;
@@ -293,11 +316,13 @@ public class TileGroup implements MostVisitedSites.Observer {
     /**
      * Instructs this instance to start listening for data. The {@link TileGroup.Observer} may be
      * called immediately if new data is received synchronously.
-     * @param maxResults The maximum number of sites to retrieve.
+     * @param maxRows The maximum number of rows to fetch.
+     * @param maxColumns The maximum number of columns to fetch.
      */
-    public void startObserving(int maxResults) {
+    public void startObserving(int maxRows, int maxColumns) {
         addTask(TileTask.FETCH_DATA);
-        mTileGroupDelegate.setMostVisitedSitesObserver(this, maxResults);
+        mNumColumns = Math.min(maxColumns, TileGridLayout.calculateNumColumns());
+        mTileGroupDelegate.setMostVisitedSitesObserver(this, maxRows * maxColumns);
     }
 
     /**

chrome/android/java_sources.gni

@@ -1629,6 +1629,7 @@ chrome_test_java_sources = [
   "javatests/src/org/chromium/chrome/browser/suggestions/SuggestionsBottomSheetTest.java",
   "javatests/src/org/chromium/chrome/browser/suggestions/SuggestionsBottomSheetUiCaptureTest.java",
   "javatests/src/org/chromium/chrome/browser/suggestions/TileGroupTest.java",
+  "javatests/src/org/chromium/chrome/browser/suggestions/TileGridLayoutTest.java",
   "javatests/src/org/chromium/chrome/browser/superviseduser/SupervisedUserContentProviderTest.java",
   "javatests/src/org/chromium/chrome/browser/sync/FakeProfileSyncService.java",
   "javatests/src/org/chromium/chrome/browser/sync/ui/PassphraseActivityTest.java",

chrome/android/javatests/src/org/chromium/chrome/browser/suggestions/TileGridLayoutTest.java

+// Copyright 2017 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.suggestions;
+
+import android.app.Activity;
+import android.content.pm.ActivityInfo;
+import android.support.test.InstrumentationRegistry;
+import android.support.test.filters.MediumTest;
+import android.support.test.filters.SmallTest;
+import android.util.DisplayMetrics;
+import android.util.TypedValue;
+import android.view.View;
+import android.view.View.MeasureSpec;
+import android.view.ViewGroup;
+import android.widget.LinearLayout;
+
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import org.chromium.base.ApiCompatibilityUtils;
+import org.chromium.base.ThreadUtils;
+import org.chromium.base.test.util.CommandLineFlags;
+import org.chromium.base.test.util.Feature;
+import org.chromium.base.test.util.RetryOnFailure;
+import org.chromium.chrome.R;
+import org.chromium.chrome.browser.ChromeSwitches;
+import org.chromium.chrome.browser.UrlConstants;
+import org.chromium.chrome.browser.ntp.NewTabPage;
+import org.chromium.chrome.browser.ntp.cards.NewTabPageRecyclerView;
+import org.chromium.chrome.browser.tab.Tab;
+import org.chromium.chrome.test.ChromeActivityTestRule;
+import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
+import org.chromium.chrome.test.ChromeTabbedActivityTestRule;
+import org.chromium.chrome.test.util.NewTabPageTestUtils;
+import org.chromium.chrome.test.util.browser.RecyclerViewTestUtils;
+import org.chromium.chrome.test.util.browser.suggestions.FakeSuggestionsSource;
+import org.chromium.chrome.test.util.browser.suggestions.SuggestionsDependenciesRule;
+import org.chromium.content.browser.test.util.Criteria;
+import org.chromium.content.browser.test.util.CriteriaHelper;
+import org.chromium.net.test.EmbeddedTestServerRule;
+
+import java.util.Arrays;
+
+/**
+ * Instrumentation tests for the {@link TileGridLayout} on the New Tab Page.
+ */
+@RunWith(ChromeJUnit4ClassRunner.class)
+@CommandLineFlags.Add({
+        ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE,
+        ChromeActivityTestRule.DISABLE_NETWORK_PREDICTION_FLAG,
+})
+@RetryOnFailure
+public class TileGridLayoutTest {
+    @Rule
+    public ChromeTabbedActivityTestRule mActivityTestRule = new ChromeTabbedActivityTestRule();
+
+    @Rule
+    public SuggestionsDependenciesRule mSuggestionsDeps = new SuggestionsDependenciesRule();
+
+    @Rule
+    public EmbeddedTestServerRule mTestServerRule = new EmbeddedTestServerRule();
+
+    private static final String HOME_PAGE_URL = "http://ho.me/";
+
+    private static final String[] FAKE_MOST_VISITED_URLS = new String[] {
+            "/chrome/test/data/android/navigate/one.html",
+            "/chrome/test/data/android/navigate/two.html",
+            "/chrome/test/data/android/navigate/three.html",
+            "/chrome/test/data/android/navigate/four.html",
+            "/chrome/test/data/android/navigate/five.html",
+            "/chrome/test/data/android/navigate/six.html",
+            "/chrome/test/data/android/navigate/seven.html",
+            "/chrome/test/data/android/navigate/eight.html",
+            "/chrome/test/data/android/navigate/nine.html",
+    };
+
+    private NewTabPage mNtp;
+    private String[] mSiteSuggestionUrls;
+    private FakeMostVisitedSites mMostVisitedSites;
+
+    @Before
+    public void setUp() throws Exception {
+        mSiteSuggestionUrls = mTestServerRule.getServer().getURLs(FAKE_MOST_VISITED_URLS);
+        String[] tmpSiteSuggestionUrls = new String[mSiteSuggestionUrls.length + 1];
+        System.arraycopy(
+                mSiteSuggestionUrls, 0, tmpSiteSuggestionUrls, 0, mSiteSuggestionUrls.length);
+        tmpSiteSuggestionUrls[tmpSiteSuggestionUrls.length - 1] = HOME_PAGE_URL;
+        mSiteSuggestionUrls = tmpSiteSuggestionUrls;
+
+        mMostVisitedSites = new FakeMostVisitedSites();
+        mSuggestionsDeps.getFactory().mostVisitedSites = mMostVisitedSites;
+
+        String[] whitelistIconPaths = new String[mSiteSuggestionUrls.length];
+        Arrays.fill(whitelistIconPaths, "");
+
+        int[] sources = new int[mSiteSuggestionUrls.length];
+        Arrays.fill(sources, TileSource.TOP_SITES);
+        sources[mSiteSuggestionUrls.length - 1] = TileSource.HOMEPAGE;
+
+        mMostVisitedSites.setTileSuggestions(
+                mSiteSuggestionUrls, mSiteSuggestionUrls.clone(), whitelistIconPaths, sources);
+
+        mSuggestionsDeps.getFactory().suggestionsSource = new FakeSuggestionsSource();
+
+        mActivityTestRule.startMainActivityWithURL(UrlConstants.NTP_URL);
+        Tab mTab = mActivityTestRule.getActivity().getActivityTab();
+        NewTabPageTestUtils.waitForNtpLoaded(mTab);
+
+        Assert.assertTrue(mTab.getNativePage() instanceof NewTabPage);
+        mNtp = (NewTabPage) mTab.getNativePage();
+
+        RecyclerViewTestUtils.waitForStableRecyclerView(getRecyclerView());
+    }
+
+    /**
+     * Verifies that the assumed constants in {@link TileGridLayout#calculateNumColumns()}
+     * are correct.
+     */
+    @Test
+    @SmallTest
+    @Feature({"NewTabPage"})
+    public void testNumColumnsPaddingAndMarginSizes() {
+        TileGridLayout tileGridLayout = getTileGridLayout();
+        Assert.assertEquals(TileGridLayout.PADDING_START_PX,
+                ApiCompatibilityUtils.getPaddingStart(tileGridLayout));
+        Assert.assertEquals(
+                TileGridLayout.PADDING_END_PX, ApiCompatibilityUtils.getPaddingEnd(tileGridLayout));
+
+        DisplayMetrics metrics = tileGridLayout.getResources().getDisplayMetrics();
+        LinearLayout.LayoutParams layoutParams =
+                (LinearLayout.LayoutParams) tileGridLayout.getLayoutParams();
+        Assert.assertEquals(convertToPx(TileGridLayout.MARGIN_START_DP, metrics),
+                ApiCompatibilityUtils.getMarginStart(layoutParams));
+        Assert.assertEquals(convertToPx(TileGridLayout.MARGIN_END_DP, metrics),
+                ApiCompatibilityUtils.getMarginEnd(layoutParams));
+    }
+
+    @Test
+    @MediumTest
+    @Feature({"NewTabPage"})
+    public void testNumColumnsCalculatedEqualToMeasurement() {
+        final int maxColumns = 6;
+        final TileGridLayout layout = getTileGridLayout();
+        layout.setMaxRows(1);
+        layout.setMaxColumns(maxColumns);
+
+        reMeasureLayout(layout);
+        int calculatedNumColumns = Math.min(maxColumns, TileGridLayout.calculateNumColumns());
+
+        final Activity activity = mActivityTestRule.getActivity();
+        activity.setRequestedOrientation(ActivityInfo.SCREEN_ORIENTATION_PORTRAIT);
+        InstrumentationRegistry.getInstrumentation().waitForIdleSync();
+        int portraitVisibleTiles = countVisibleTiles(layout, calculatedNumColumns);
+        activity.setRequestedOrientation(ActivityInfo.SCREEN_ORIENTATION_LANDSCAPE);
+        InstrumentationRegistry.getInstrumentation().waitForIdleSync();
+        int landscapeVisibleTiles = countVisibleTiles(layout, calculatedNumColumns);
+        Assert.assertEquals("The calculated number of columns should be equal to the minimum number"
+                        + " of columns across screen orientations.",
+                Math.min(portraitVisibleTiles, landscapeVisibleTiles), calculatedNumColumns);
+    }
+
+    private int convertToPx(int dp, DisplayMetrics metrics) {
+        return Math.round(TypedValue.applyDimension(TypedValue.COMPLEX_UNIT_DIP, dp, metrics));
+    }
+
+    private void reMeasureLayout(final ViewGroup group) {
+        ThreadUtils.runOnUiThreadBlocking(new Runnable() {
+            @Override
+            public void run() {
+                group.measure(MeasureSpec.UNSPECIFIED, MeasureSpec.UNSPECIFIED);
+            }
+        });
+
+        // Wait until the view is updated. TODO(oskopek): Is there a better criterion to check for?
+        CriteriaHelper.pollUiThread(new Criteria() {
+            @Override
+            public boolean isSatisfied() {
+                boolean hasHiddenTile = false;
+                for (int i = 0; i < group.getChildCount(); i++) {
+                    if (!hasHiddenTile) {
+                        hasHiddenTile = group.getChildAt(i).getVisibility() == View.GONE;
+                    }
+                }
+
+                return hasHiddenTile;
+            }
+        });
+    }
+
+    private int countVisibleTiles(ViewGroup group, int calculatedNumColumns) {
+        reMeasureLayout(group);
+        int visibleTileViews = 0;
+        boolean hasHomePage = false;
+        for (int i = 0; i < group.getChildCount(); i++) {
+            TileView tileView = (TileView) group.getChildAt(i);
+            if (tileView.getVisibility() == View.VISIBLE) {
+                visibleTileViews++;
+                if (HOME_PAGE_URL.equals(tileView.getUrl())) {
+                    hasHomePage = true;
+                }
+            }
+        }
+
+        Assert.assertTrue("The calculated number of columns should be less than or equal to"
+                        + "the measured one.",
+                calculatedNumColumns <= visibleTileViews);
+        Assert.assertTrue("Visible tiles should contain the home page tile.", hasHomePage);
+
+        return visibleTileViews;
+    }
+
+    private NewTabPageRecyclerView getRecyclerView() {
+        return mNtp.getNewTabPageView().getRecyclerView();
+    }
+
+    private TileGridLayout getTileGridLayout() {
+        TileGridLayout tileGridLayout =
+                (TileGridLayout) mNtp.getNewTabPageView().findViewById(R.id.tile_grid_layout);
+        Assert.assertNotNull("Unable to retrieve the TileGridLayout.", tileGridLayout);
+        return tileGridLayout;
+    }
+}

chrome/android/junit/src/org/chromium/chrome/browser/suggestions/TileGroupTest.java

@@ -60,7 +60,8 @@ import java.util.List;
 @Features({@Features.Register(ChromeFeatureList.NTP_OFFLINE_PAGES_FEATURE_NAME),
         @Features.Register(ChromeFeatureList.SUGGESTIONS_HOME_MODERN_LAYOUT)})
 public class TileGroupTest {
-    private static final int MAX_TILES_TO_FETCH = 4;
+    private static final int MAX_COLUMNS_TO_FETCH = 4;
+    private static final int MAX_ROWS_TO_FETCH = 1;
     private static final int TILE_TITLE_LINES = 1;
     private static final String[] URLS = {"https://www.google.com", "https://tellmedadjokes.com"};
 
@@ -93,7 +94,7 @@ public class TileGroupTest {
                 new TileGroup(RuntimeEnvironment.application, mock(SuggestionsUiDelegate.class),
                         mock(ContextMenuManager.class), mTileGroupDelegate, mTileGroupObserver,
                         mock(OfflinePageBridge.class), TILE_TITLE_LINES);
-        tileGroup.startObserving(MAX_TILES_TO_FETCH);
+        tileGroup.startObserving(MAX_ROWS_TO_FETCH, MAX_COLUMNS_TO_FETCH);
 
         notifyTileUrlsAvailable(URLS);
 
@@ -115,7 +116,7 @@ public class TileGroupTest {
                 new TileGroup(RuntimeEnvironment.application, mock(SuggestionsUiDelegate.class),
                         mock(ContextMenuManager.class), mTileGroupDelegate, mTileGroupObserver,
                         mock(OfflinePageBridge.class), TILE_TITLE_LINES);
-        tileGroup.startObserving(MAX_TILES_TO_FETCH);
+        tileGroup.startObserving(MAX_ROWS_TO_FETCH, MAX_COLUMNS_TO_FETCH);
 
         notifyTileUrlsAvailable(/* nothing! */);
 
@@ -205,7 +206,7 @@ public class TileGroupTest {
         TileGroup tileGroup = new TileGroup(RuntimeEnvironment.application, uiDelegate,
                 mock(ContextMenuManager.class), mTileGroupDelegate, mTileGroupObserver,
                 mock(OfflinePageBridge.class), TILE_TITLE_LINES);
-        tileGroup.startObserving(MAX_TILES_TO_FETCH);
+        tileGroup.startObserving(MAX_ROWS_TO_FETCH, MAX_COLUMNS_TO_FETCH);
 
         notifyTileUrlsAvailable(URLS);
 
@@ -220,7 +221,7 @@ public class TileGroupTest {
         TileGroup tileGroup = new TileGroup(RuntimeEnvironment.application, uiDelegate,
                 mock(ContextMenuManager.class), mTileGroupDelegate, mTileGroupObserver,
                 mock(OfflinePageBridge.class), TILE_TITLE_LINES);
-        tileGroup.startObserving(MAX_TILES_TO_FETCH);
+        tileGroup.startObserving(MAX_ROWS_TO_FETCH, MAX_COLUMNS_TO_FETCH);
 
         notifyTileUrlsAvailable(URLS);
         reset(mTileGroupObserver);
@@ -260,7 +261,7 @@ public class TileGroupTest {
         TileGroup tileGroup = new TileGroup(RuntimeEnvironment.application, uiDelegate,
                 mock(ContextMenuManager.class), mTileGroupDelegate, mTileGroupObserver,
                 mock(OfflinePageBridge.class), TILE_TITLE_LINES);
-        tileGroup.startObserving(MAX_TILES_TO_FETCH);
+        tileGroup.startObserving(MAX_ROWS_TO_FETCH, MAX_COLUMNS_TO_FETCH);
         ViewGroup layout = new FrameLayout(RuntimeEnvironment.application, null);
 
         // Initialise the internal list of tiles
@@ -281,7 +282,7 @@ public class TileGroupTest {
         TileGroup tileGroup = new TileGroup(RuntimeEnvironment.application, uiDelegate,
                 mock(ContextMenuManager.class), mTileGroupDelegate, mTileGroupObserver,
                 mock(OfflinePageBridge.class), TILE_TITLE_LINES);
-        tileGroup.startObserving(MAX_TILES_TO_FETCH);
+        tileGroup.startObserving(MAX_ROWS_TO_FETCH, MAX_COLUMNS_TO_FETCH);
         ViewGroup layout = new FrameLayout(RuntimeEnvironment.application, null);
 
         // Initialise the internal list of tiles
@@ -301,7 +302,7 @@ public class TileGroupTest {
         TileGroup tileGroup = new TileGroup(RuntimeEnvironment.application, uiDelegate,
                 mock(ContextMenuManager.class), mTileGroupDelegate, mTileGroupObserver,
                 mock(OfflinePageBridge.class), TILE_TITLE_LINES);
-        tileGroup.startObserving(MAX_TILES_TO_FETCH);
+        tileGroup.startObserving(MAX_ROWS_TO_FETCH, MAX_COLUMNS_TO_FETCH);
         notifyTileUrlsAvailable(URLS);
 
         // Initialise the layout with views whose URLs don't match the ones of the new tiles.
@@ -327,7 +328,7 @@ public class TileGroupTest {
                 new TileGroup(RuntimeEnvironment.application, mock(SuggestionsUiDelegate.class),
                         mock(ContextMenuManager.class), mTileGroupDelegate, mTileGroupObserver,
                         mock(OfflinePageBridge.class), TILE_TITLE_LINES);
-        tileGroup.startObserving(MAX_TILES_TO_FETCH);
+        tileGroup.startObserving(MAX_ROWS_TO_FETCH, MAX_COLUMNS_TO_FETCH);
         notifyTileUrlsAvailable(URLS);
 
         // Initialise the layout with views whose URLs match the ones of the new tiles.
@@ -438,7 +439,7 @@ public class TileGroupTest {
 
     /**
      * Notifies the tile group of new tiles created from the provided URLs. Requires
-     * {@link TileGroup#startObserving(int)} to have been called on the tile group under test.
+     * {@link TileGroup#startObserving(int, int)} to have been called on the tile group under test.
      * @see TileGroup#onMostVisitedURLsAvailable(String[], String[], String[], int[])
      */
     private void notifyTileUrlsAvailable(String... urls) {
@@ -471,7 +472,7 @@ public class TileGroupTest {
         TileGroup tileGroup = new TileGroup(RuntimeEnvironment.application, uiDelegate,
                 mock(ContextMenuManager.class), mTileGroupDelegate, mTileGroupObserver,
                 mock(OfflinePageBridge.class), TILE_TITLE_LINES);
-        tileGroup.startObserving(MAX_TILES_TO_FETCH);
+        tileGroup.startObserving(MAX_ROWS_TO_FETCH, MAX_COLUMNS_TO_FETCH);
         notifyTileUrlsAvailable(urls);
 
         ViewGroup layout = new FrameLayout(RuntimeEnvironment.application, null);

components/ntp_tiles/most_visited_sites.cc

@@ -33,9 +33,6 @@ namespace {
 const base::Feature kDisplaySuggestionsServiceTiles{
     "DisplaySuggestionsServiceTiles", base::FEATURE_ENABLED_BY_DEFAULT};
 
-// The maximum index of the home page tile.
-const size_t kMaxHomeTileIndex = 3;
-
 // Determine whether we need any tiles from PopularSites to fill up a grid of
 // |num_tiles| tiles.
 bool NeedPopularSites(const PrefService* prefs, int num_tiles) {
@@ -404,38 +401,34 @@ NTPTilesVector MostVisitedSites::InsertHomeTile(
 
   const GURL& home_page_url = home_page_client_->GetHomePageUrl();
   NTPTilesVector new_tiles;
-  // Add the home tile to the first four tiles.
-  NTPTile home_tile;
-  home_tile.url = home_page_url;
-  home_tile.title = title;
-  home_tile.source = TileSource::HOMEPAGE;
-
   bool home_tile_added = false;
-  size_t index = 0;
 
-  while (index < tiles.size() && new_tiles.size() < num_sites_) {
-    bool hosts_are_equal = tiles[index].url.host() == home_page_url.host();
+  for (auto& tile : tiles) {
+    if (new_tiles.size() >= num_sites_) {
+      break;
+    }
 
-    // Add the home tile to the first four tiles
-    // or at the position of a tile that has the same host
-    // and is ranked higher.
     // TODO(fhorschig): Introduce a more sophisticated deduplication.
-    if (!home_tile_added && (index >= kMaxHomeTileIndex || hosts_are_equal)) {
-      new_tiles.push_back(std::move(home_tile));
+    if (tile.url.host() == home_page_url.host()) {
+      tile.source = TileSource::HOMEPAGE;
       home_tile_added = true;
-      continue;  // Do not advance the current tile index.
     }
-
-    // Add non-home page tiles.
-    if (!hosts_are_equal) {
-      new_tiles.push_back(std::move(tiles[index]));
-    }
-    ++index;
+    new_tiles.push_back(std::move(tile));
   }
 
   // Add the home page tile if there are less than 4 tiles
   // and none of them is the home page (and there is space left).
-  if (!home_tile_added && new_tiles.size() < num_sites_) {
+  if (!home_tile_added) {
+    // Make room for the home page tile.
+    if (new_tiles.size() >= num_sites_) {
+      new_tiles.pop_back();
+    }
+
+    NTPTile home_tile;
+    home_tile.url = home_page_url;
+    home_tile.title = title;
+    home_tile.source = TileSource::HOMEPAGE;
+
     new_tiles.push_back(std::move(home_tile));
   }
   return new_tiles;

components/ntp_tiles/most_visited_sites_unittest.cc

@@ -549,7 +549,7 @@ TEST_P(MostVisitedSitesTest, ShouldNotIncludeHomePageIfNoTileRequested) {
   base::RunLoop().RunUntilIdle();
 }
 
-TEST_P(MostVisitedSitesTest, ShouldReturnMostPopularPageIfOneTileRequested) {
+TEST_P(MostVisitedSitesTest, ShouldReturnHomePageIfOneTileRequested) {
   FakeHomePageClient* home_page_client = RegisterNewHomePageClient();
   home_page_client->SetHomePageEnabled(true);
   DisableRemoteSuggestions();
@@ -561,14 +561,14 @@ TEST_P(MostVisitedSitesTest, ShouldReturnMostPopularPageIfOneTileRequested) {
       .Times(AnyNumber())
       .WillRepeatedly(Return(false));
   EXPECT_CALL(mock_observer_,
-              OnMostVisitedURLsAvailable(ElementsAre(MatchesTile(
-                  "Site 1", "http://site1/", TileSource::TOP_SITES))));
+              OnMostVisitedURLsAvailable(ElementsAre(
+                  MatchesTile("", kHomePageUrl, TileSource::HOMEPAGE))));
   most_visited_sites_->SetMostVisitedURLsObserver(&mock_observer_,
                                                   /*num_sites=*/1);
   base::RunLoop().RunUntilIdle();
 }
 
-TEST_P(MostVisitedSitesTest, ShouldContainHomePageInFirstFourTiles) {
+TEST_P(MostVisitedSitesTest, ShouldReplaceLastTileWithHomePageWhenFull) {
   FakeHomePageClient* home_page_client = RegisterNewHomePageClient();
   home_page_client->SetHomePageEnabled(true);
   DisableRemoteSuggestions();
@@ -588,12 +588,38 @@ TEST_P(MostVisitedSitesTest, ShouldContainHomePageInFirstFourTiles) {
   EXPECT_CALL(mock_observer_, OnMostVisitedURLsAvailable(_))
       .WillOnce(SaveArg<0>(&tiles));
   most_visited_sites_->SetMostVisitedURLsObserver(&mock_observer_,
-                                                  /*num_sites=*/8);
+                                                  /*num_sites=*/4);
   base::RunLoop().RunUntilIdle();
-  // Assert that the home page tile is in the first four tiles.
+  // Assert that the home page replaces the final tile.
   EXPECT_THAT(tiles[3], MatchesTile("", kHomePageUrl, TileSource::HOMEPAGE));
 }
 
+TEST_P(MostVisitedSitesTest, ShouldAppendHomePageWhenNotFull) {
+  FakeHomePageClient* home_page_client = RegisterNewHomePageClient();
+  home_page_client->SetHomePageEnabled(true);
+  DisableRemoteSuggestions();
+  EXPECT_CALL(*mock_top_sites_, GetMostVisitedURLs(_, false))
+      .WillRepeatedly(InvokeCallbackArgument<0>((MostVisitedURLList{
+          MakeMostVisitedURL("Site 1", "http://site1/"),
+          MakeMostVisitedURL("Site 2", "http://site2/"),
+          MakeMostVisitedURL("Site 3", "http://site3/"),
+          MakeMostVisitedURL("Site 4", "http://site4/"),
+          MakeMostVisitedURL("Site 5", "http://site5/"),
+      })));
+  EXPECT_CALL(*mock_top_sites_, SyncWithHistory());
+  EXPECT_CALL(*mock_top_sites_, IsBlacklisted(Eq(GURL(kHomePageUrl))))
+      .Times(AnyNumber())
+      .WillRepeatedly(Return(false));
+  std::vector<NTPTile> tiles;
+  EXPECT_CALL(mock_observer_, OnMostVisitedURLsAvailable(_))
+      .WillOnce(SaveArg<0>(&tiles));
+  most_visited_sites_->SetMostVisitedURLsObserver(&mock_observer_,
+                                                  /*num_sites=*/8);
+  base::RunLoop().RunUntilIdle();
+  // Assert that the home page is appended as the final tile.
+  EXPECT_THAT(tiles[5], MatchesTile("", kHomePageUrl, TileSource::HOMEPAGE));
+}
+
 TEST_P(MostVisitedSitesTest, ShouldDeduplicateHomePageWithTopSites) {
   FakeHomePageClient* home_page_client = RegisterNewHomePageClient();
   home_page_client->SetHomePageEnabled(true);