[StartTablet] Fix the interval padding of MVT on tablets.

This is a follow up of CL: https://crrev.com/c/4479149.
This change fixes the issue by incorporating the default calculation method when the interval padding of MVT, determined by the specifications, is either excessively large or too small. Otherwise Chrome would crash as the calculated interval padding is smaller than the min value.
The edge margin of MVT element and the top and bottom margin of the single tab card are also changed in this CL according to the new spec.
Additionally, a range for the asserting values in the MVT-related tests has been included.
See the updated screenshots: https://docs.google.com/document/d/1u1DKlofkFlEZyXzZkpRVR1XZurxp-ZRkvBQB9t5GoKw/edit?usp=sharing&resourcekey=0-bX_lfc8wUp4iPK-iIYN1vQ

(cherry picked from commit c184bda)

Bug: 1446043, 1416536
Change-Id: I54554edaff0b3437a676133fce7a9aa3e9e61d0f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4542600
Commit-Queue: Xinyi Ji <xinyiji@chromium.org>
Reviewed-by: Theresa Sullivan <twellington@chromium.org>
Reviewed-by: Xi Han <hanxi@chromium.org>
Code-Coverage: Findit <findit-for-me@appspot.gserviceaccount.com>
Cr-Original-Commit-Position: refs/heads/main@{#1148154}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4581807
Cr-Commit-Position: refs/branch-heads/5790@{#234}
Cr-Branched-From: 1d71a33-refs/heads/main@{#1148114}

chrome/android/features/start_surface/javatests/src/org/chromium/chrome/features/start_surface/StartSurfaceOnTabletTest.java

@@ -19,10 +19,12 @@
 
 import org.hamcrest.Matchers;
 import org.junit.Assert;
+import org.junit.Assume;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
+import org.chromium.base.MathUtils;
 import org.chromium.base.test.util.CommandLineFlags;
 import org.chromium.base.test.util.Criteria;
 import org.chromium.base.test.util.CriteriaHelper;
@@ -38,6 +40,7 @@
 import org.chromium.chrome.browser.ntp.NewTabPage;
 import org.chromium.chrome.browser.ntp.NewTabPageLayout;
 import org.chromium.chrome.browser.suggestions.tile.MostVisitedTilesCarouselLayout;
+import org.chromium.chrome.browser.suggestions.tile.MostVisitedTilesCoordinator;
 import org.chromium.chrome.browser.suggestions.tile.MostVisitedTilesGridLayout;
 import org.chromium.chrome.browser.suggestions.tile.MostVisitedTilesLayout;
 import org.chromium.chrome.browser.tab.Tab;
@@ -457,9 +460,11 @@ public void test1RowMvtMarginWithMultiColumnFeedsOnNtpHomePage() throws IOExcept
                 + res.getDimensionPixelSize(
                         org.chromium.chrome.R.dimen
                                 .mvt_container_to_ntp_right_extra_margin_two_feed_tablet)
-                + res.getDimensionPixelSize(org.chromium.chrome.R.dimen.tile_grid_layout_bleed);
+                + res.getDimensionPixelSize(org.chromium.chrome.R.dimen.tile_grid_layout_bleed) / 2
+                        * 2;
         int expectedContainerTwoSideMarginPortrait =
-                res.getDimensionPixelSize(org.chromium.chrome.R.dimen.tile_grid_layout_bleed)
+                res.getDimensionPixelSize(org.chromium.chrome.R.dimen.tile_grid_layout_bleed) / 2
+                        * 2
                 + res.getDimensionPixelSize(
                         org.chromium.chrome.R.dimen
                                 .mvt_container_to_ntp_right_extra_margin_two_feed_tablet);
@@ -469,13 +474,17 @@ public void test1RowMvtMarginWithMultiColumnFeedsOnNtpHomePage() throws IOExcept
 
         int expectedMvtBottomMargin = res.getDimensionPixelSize(
                 org.chromium.chrome.R.dimen.mvt_container_bottom_margin_tablet);
-        int expectedSingleTabCardTopAndBottomMargin = res.getDimensionPixelSize(
-                org.chromium.chrome.R.dimen
-                        .single_tab_card_top_and_bottom_margin_carousel_mvt_tablet);
+        int expectedSingleTabCardTopMargin = -res.getDimensionPixelSize(
+                org.chromium.chrome.R.dimen.single_tab_card_top_margin_tablet);
+        int expectedSingleTabCardBottomMargin =
+                res.getDimensionPixelOffset(
+                        org.chromium.chrome.R.dimen.single_tab_card_bottom_margin_tablet)
+                - res.getDimensionPixelOffset(
+                        org.chromium.chrome.R.dimen.feed_header_tab_list_view_top_bottom_margin);
         // Verifies the vertical margins of the module most visited tiles and single tab card are
         // correct.
         verifyMvtAndSingleTabCardVerticalMargins(expectedMvtBottomMargin,
-                -expectedSingleTabCardTopAndBottomMargin, expectedSingleTabCardTopAndBottomMargin,
+                expectedSingleTabCardTopMargin, expectedSingleTabCardBottomMargin,
                 /*isNtpHomepage=*/true, ntp);
     }
 
@@ -537,12 +546,17 @@ public void test2RowMvtMarginWithMultiColumnFeedsOnNtpHomePage() throws IOExcept
 
         int expectedMvtBottomMargin = res.getDimensionPixelSize(
                 org.chromium.chrome.R.dimen.mvt_container_bottom_margin_tablet);
-        int expectedSingleTabCardTopAndBottomMargin = res.getDimensionPixelSize(
-                org.chromium.chrome.R.dimen.single_tab_card_top_and_bottom_margin_grid_mvt_tablet);
+        int expectedSingleTabCardTopMargin = -res.getDimensionPixelSize(
+                org.chromium.chrome.R.dimen.single_tab_card_top_margin_tablet);
+        int expectedSingleTabCardBottomMargin =
+                res.getDimensionPixelOffset(
+                        org.chromium.chrome.R.dimen.single_tab_card_bottom_margin_tablet)
+                - res.getDimensionPixelOffset(
+                        org.chromium.chrome.R.dimen.feed_header_tab_list_view_top_bottom_margin);
         // Verifies the vertical margins of the module most visited tiles and single tab card are
         // correct.
         verifyMvtAndSingleTabCardVerticalMargins(expectedMvtBottomMargin,
-                -expectedSingleTabCardTopAndBottomMargin, expectedSingleTabCardTopAndBottomMargin,
+                expectedSingleTabCardTopMargin, expectedSingleTabCardBottomMargin,
                 /*isNtpHomepage=*/true, ntp);
     }
 
@@ -593,6 +607,7 @@ private void verifyMostVisitedTileMargin(int expectedContainerTwoSideMarginLandS
             int expectedContainerTwoSideMarginPortrait, int expectedEdgeMarginLandScape,
             int expectedEdgeMarginPortrait, boolean isScrollable, NewTabPage ntp) {
         NewTabPageLayout ntpLayout = ntp.getNewTabPageLayout();
+        Assume.assumeTrue(ntpLayout.getChildCount() >= 4);
         View mvTilesContainer =
                 ntpLayout.findViewById(org.chromium.chrome.test.R.id.mv_tiles_container);
         View mvTilesLayout = ntpLayout.findViewById(org.chromium.chrome.test.R.id.mv_tiles_layout);
@@ -645,7 +660,7 @@ private void verifyMostVisitedTileMarginImpl(View ntpLayout, View mvTilesContain
 
         Assert.assertEquals("The container's margin with respect to the layout of the new tab "
                         + "page is incorrect.",
-                expectedContainerTwoSideMargin, ntpLayout.getWidth() - mvtContainerWidth);
+                expectedContainerTwoSideMargin, ntpLayout.getWidth() - mvtContainerWidth, 3);
 
         if (isScrollable) {
             Assert.assertTrue("The width of the most visited tiles layout is wrong.",
@@ -660,15 +675,26 @@ private void verifyMostVisitedTileMarginImpl(View ntpLayout, View mvTilesContain
         } else {
             Assert.assertTrue("The width of the most visited tiles layout is wrong.",
                     mvtContainerWidth == mvTilesLayoutWidth);
-            Assert.assertEquals("The edge margin of the most visited tiles element to "
-                            + "the MV tiles layout is wrong.",
-                    mvt1LeftMargin, expectedEdgeMargin);
+            int minHorizontalSpacing = ((MostVisitedTilesGridLayout) mvTilesLayout)
+                                               .getMinHorizontalSpacingForTesting();
+            int maxHorizontalSpacing = ((MostVisitedTilesGridLayout) mvTilesLayout)
+                                               .getMaxHorizontalSpacingForTesting();
+            int numColumns = MathUtils.clamp((mvTilesLayoutWidth + minHorizontalSpacing)
+                            / (mvTilesItemWidth + minHorizontalSpacing),
+                    1, MostVisitedTilesCoordinator.MAX_TILE_COLUMNS_FOR_GRID);
             int expectedIntervalPadding =
-                    (mvTilesLayoutWidth - mvTilesItemWidth * 4 - expectedEdgeMargin * 2) / 3;
-            Assert.assertEquals(
-                    "The padding between each element of the most visited tiles is incorrect.",
-                    expectedIntervalPadding,
-                    mvt2LeftMargin - mvTilesItemWidth - expectedEdgeMargin);
+                    (mvTilesLayoutWidth - mvTilesItemWidth * numColumns - expectedEdgeMargin * 2)
+                    / (numColumns - 1);
+            if (expectedIntervalPadding >= minHorizontalSpacing
+                    && expectedIntervalPadding <= maxHorizontalSpacing) {
+                Assert.assertEquals("The edge margin of the most visited tiles element to "
+                                + "the MV tiles layout is wrong.",
+                        mvt1LeftMargin, expectedEdgeMargin);
+                Assert.assertEquals(
+                        "The padding between each element of the most visited tiles is incorrect.",
+                        expectedIntervalPadding,
+                        mvt2LeftMargin - mvTilesItemWidth - expectedEdgeMargin);
+            }
         }
     }
 

chrome/android/java/res/values/dimens.xml

@@ -163,11 +163,11 @@ found in the LICENSE file.
     <dimen name="tile_carousel_layout_bottom_margin">24dp</dimen>
     <dimen name="mvt_container_to_ntp_right_extra_margin_two_feed_tablet">16dp</dimen>
     <dimen name="mvt_container_bottom_margin_tablet">48dp</dimen>
-    <dimen name="tile_grid_layout_landscape_edge_margin_tablet">48dp</dimen>
-    <dimen name="tile_grid_layout_portrait_edge_margin_tablet">32dp</dimen>
+    <dimen name="tile_grid_layout_landscape_edge_margin_tablet">100dp</dimen>
+    <dimen name="tile_grid_layout_portrait_edge_margin_tablet">84dp</dimen>
     <dimen name="tile_carousel_layout_max_interval_margin_tablet">64dp</dimen>
-    <dimen name="single_tab_card_top_and_bottom_margin_carousel_mvt_tablet">48dp</dimen>
-    <dimen name="single_tab_card_top_and_bottom_margin_grid_mvt_tablet">36dp</dimen>
+    <dimen name="single_tab_card_top_margin_tablet">48dp</dimen>
+    <dimen name="single_tab_card_bottom_margin_tablet">40dp</dimen>
     <dimen name="ntp_iph_searchbox_y_inset">6dp</dimen>
     <dimen name="ntp_logo_height">100dp</dimen>
     <dimen name="ntp_logo_margin_top">26dp</dimen>
@@ -194,6 +194,7 @@ found in the LICENSE file.
     <dimen name="feed_header_icon_size">20dp</dimen>
     <dimen name="feed_header_tab_layout_height">40dp</dimen>
     <dimen name="feed_v2_header_menu_width">20dp</dimen>
+    <dimen name="feed_header_tab_list_view_top_bottom_margin">8dp</dimen>
 
     <!-- Incognito NTP -->
     <dimen name="incognito_ntp_total_space_between_views">14sp</dimen>

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

@@ -1116,18 +1116,13 @@ private void updateSingleTabCardContainerMargins() {
 
         MarginLayoutParams marginLayoutParams =
                 (MarginLayoutParams) mSingleTabCardContainer.getLayoutParams();
-        int SingleTabCardContainerTopAndBottomMargin;
-        if (isScrollableMvtEnabled(mContext)) {
-            SingleTabCardContainerTopAndBottomMargin =
-                    mNewTabPageLayout.getResources().getDimensionPixelOffset(
-                            R.dimen.single_tab_card_top_and_bottom_margin_carousel_mvt_tablet);
-        } else {
-            SingleTabCardContainerTopAndBottomMargin =
-                    mNewTabPageLayout.getResources().getDimensionPixelOffset(
-                            R.dimen.single_tab_card_top_and_bottom_margin_grid_mvt_tablet);
-        }
-        marginLayoutParams.topMargin = -SingleTabCardContainerTopAndBottomMargin;
-        marginLayoutParams.bottomMargin = SingleTabCardContainerTopAndBottomMargin;
+
+        marginLayoutParams.topMargin = -mNewTabPageLayout.getResources().getDimensionPixelOffset(
+                R.dimen.single_tab_card_top_margin_tablet);
+        marginLayoutParams.bottomMargin = mNewTabPageLayout.getResources().getDimensionPixelOffset(
+                                                  R.dimen.single_tab_card_bottom_margin_tablet)
+                - mNewTabPageLayout.getResources().getDimensionPixelOffset(
+                        R.dimen.feed_header_tab_list_view_top_bottom_margin);
     }
 
     /* Set the visibility of the single tab card.

chrome/android/java/src/org/chromium/chrome/browser/suggestions/tile/MostVisitedTilesCoordinator.java

@@ -41,7 +41,8 @@ public class MostVisitedTilesCoordinator implements ConfigurationChangedObserver
      * The maximum number of tiles to try and fit in a row. On smaller screens, there may not be
      * enough space to fit all of them.
      */
-    private static final int MAX_TILE_COLUMNS_FOR_GRID = 4;
+    @VisibleForTesting
+    public static final int MAX_TILE_COLUMNS_FOR_GRID = 4;
 
     private final Activity mActivity;
     private final ActivityLifecycleDispatcher mActivityLifecycleDispatcher;

chrome/android/java/src/org/chromium/chrome/browser/suggestions/tile/MostVisitedTilesGridLayout.java

@@ -113,7 +113,7 @@ protected void onMeasure(int widthMeasureSpec, int heightMeasureSpec) {
         // Determine how much padding to use between and around the tiles.
         int gridWidthMinusColumns = Math.max(0, totalWidth - numColumns * childWidth);
         Pair<Integer, Integer> gridProperties =
-                computeHorizontalDimensions(true, gridWidthMinusColumns, numColumns);
+                computeHorizontalDimensions(gridWidthMinusColumns, numColumns);
         int gridStart = gridProperties.first;
         int horizontalSpacing = gridProperties.second;
 
@@ -150,14 +150,12 @@ protected void onMeasure(int widthMeasureSpec, int heightMeasureSpec) {
     }
 
     /**
-     * @param spreadTiles Whether to spread the tiles with the same space between and around them.
      * @param availableWidth The space available to spread between and around the tiles.
      * @param numColumns The number of columns to be organised.
      * @return The [gridStart, horizontalSpacing] pair of dimensions.
      */
     @VisibleForTesting
-    Pair<Integer, Integer> computeHorizontalDimensions(
-            boolean spreadTiles, int availableWidth, int numColumns) {
+    Pair<Integer, Integer> computeHorizontalDimensions(int availableWidth, int numColumns) {
         int gridStart;
         float horizontalSpacing;
         if (mIsMultiColumnFeedOnTabletEnabled) {
@@ -167,25 +165,22 @@ Pair<Integer, Integer> computeHorizontalDimensions(
                     : mTileViewPortraitEdgePaddingTablet;
             horizontalSpacing = (availableWidth - gridStart * 2) / (numColumns - 1);
         } else {
-            if (spreadTiles) {
-                // Identically sized spacers are added both between and around the tiles.
-                int spacerCount = numColumns + 1;
-                horizontalSpacing = (float) availableWidth / spacerCount;
-                gridStart = Math.round(horizontalSpacing);
-                if (horizontalSpacing < mMinHorizontalSpacing) {
-                    return computeHorizontalDimensions(false, availableWidth, numColumns);
-                }
+            // Identically sized spacers are added both between and around the tiles.
+            int spacerCount = numColumns + 1;
+            horizontalSpacing = (float) availableWidth / spacerCount;
+            gridStart = Math.round(horizontalSpacing);
+        }
+
+        if (horizontalSpacing < mMinHorizontalSpacing
+                || horizontalSpacing > mMaxHorizontalSpacing) {
+            // Ensure column spacing isn't greater than mMaxHorizontalSpacing.
+            long gridSidePadding = availableWidth - (long) mMaxHorizontalSpacing * (numColumns - 1);
+            if (gridSidePadding > 0) {
+                horizontalSpacing = mMaxHorizontalSpacing;
+                gridStart = (int) (gridSidePadding / 2);
             } else {
-                // Ensure column spacing isn't greater than mMaxHorizontalSpacing.
-                long gridSidePadding =
-                        availableWidth - (long) mMaxHorizontalSpacing * (numColumns - 1);
-                if (gridSidePadding > 0) {
-                    horizontalSpacing = mMaxHorizontalSpacing;
-                    gridStart = (int) (gridSidePadding / 2);
-                } else {
-                    horizontalSpacing = (float) availableWidth / Math.max(1, numColumns - 1);
-                    gridStart = 0;
-                }
+                horizontalSpacing = (float) availableWidth / Math.max(1, numColumns - 1);
+                gridStart = 0;
             }
         }
 
@@ -219,6 +214,16 @@ public void setIsMultiColumnFeedOnTabletEnabled(boolean isMultiColumnFeedOnTable
         mIsMultiColumnFeedOnTabletEnabled = isMultiColumnFeedOnTabletEnabled;
     }
 
+    @VisibleForTesting
+    public int getMinHorizontalSpacingForTesting() {
+        return mMinHorizontalSpacing;
+    }
+
+    @VisibleForTesting
+    public int getMaxHorizontalSpacingForTesting() {
+        return mMaxHorizontalSpacing;
+    }
+
     // TODO(crbug.com/1329288): Remove this method when the Feed position experiment is cleaned up.
     private int getGridMVTVerticalSpacingResourcesId() {
         if (!LibraryLoader.getInstance().isInitialized() || !mSearchProviderHasLogo) {

chrome/browser/feed/android/java/res/layout/new_tab_page_multi_feed_header.xml

@@ -45,8 +45,8 @@ found in the LICENSE file.
           android:layout_width="wrap_content"
           android:layout_height="@dimen/feed_header_tab_layout_height"
           android:layout_gravity="center"
-          android:layout_marginTop="8dp"
-          android:layout_marginBottom="8dp"
+          android:layout_marginTop="@dimen/feed_header_tab_list_view_top_bottom_margin"
+          android:layout_marginBottom="@dimen/feed_header_tab_list_view_top_bottom_margin"
           style="@style/NtpHeaderTabLayoutStyle" />
 
       <TextView

chrome/browser/feed/android/java/res/layout/new_tab_page_multi_sticky_feed_header.xml

@@ -46,8 +46,8 @@ found in the LICENSE file.
         android:layout_height="@dimen/feed_header_tab_layout_height"
         android:layout_centerHorizontal="true"
         android:layout_centerVertical="true"
-        android:layout_marginTop="8dp"
-        android:layout_marginBottom="8dp"
+        android:layout_marginTop="@dimen/feed_header_tab_list_view_top_bottom_margin"
+        android:layout_marginBottom="@dimen/feed_header_tab_list_view_top_bottom_margin"
         style="@style/NtpHeaderTabLayoutStyle" />
 
     <TextView