Dynamically set up top margins for start surface

http://crrev.com/c/2247142 sets top margins for some sections in start
surface. However, setting top margin for some sections with
app:layout_scrollFlags="scroll" makes the scroll behaves weird for
variation with omnibox_scroll_mode = quick. Without digging into the
root cause, this CL fixes issue by dynamically setting the margins in
code and skip setting top margins for OMNIBOX_ONLY variations.

Bug: 1091988
Change-Id: I88e8b9b0524c148879262e0250adf9b3c57777c7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2250934
Commit-Queue: Yue Zhang <yuezhanggg@chromium.org>
Reviewed-by: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#779650}

chrome/android/features/start_surface/internal/java/res/values/dimens.xml

@@ -5,4 +5,6 @@
 <resources xmlns:tools="http://schemas.android.com/tools">
     <dimen name="ss_bottom_bar_height">64dp</dimen>
     <dimen name="tasks_surface_body_top_margin">16dp</dimen>
+    <dimen name="mv_tiles_container_top_margin">12dp</dimen>
+    <dimen name="tab_switcher_title_top_margin">16dp</dimen>
 </resources>

chrome/android/features/start_surface/internal/java/src/org/chromium/chrome/features/start_surface/StartSurfaceMediator.java

@@ -11,7 +11,9 @@ import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.IS_INCOGN
 import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.IS_TAB_CAROUSEL_VISIBLE;
 import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.IS_VOICE_RECOGNITION_BUTTON_VISIBLE;
 import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.MORE_TABS_CLICK_LISTENER;
+import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.MV_TILES_CONTAINER_TOP_MARGIN;
 import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.MV_TILES_VISIBLE;
+import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.TAB_SWITCHER_TITLE_TOP_MARGIN;
 import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.TASKS_SURFACE_BODY_TOP_MARGIN;
 import static org.chromium.chrome.features.start_surface.StartSurfaceProperties.BOTTOM_BAR_CLICKLISTENER;
 import static org.chromium.chrome.features.start_surface.StartSurfaceProperties.BOTTOM_BAR_HEIGHT;
@@ -24,6 +26,7 @@ import static org.chromium.chrome.features.start_surface.StartSurfaceProperties.
 import static org.chromium.chrome.features.start_surface.StartSurfaceProperties.IS_SHOWING_STACK_TAB_SWITCHER;
 import static org.chromium.chrome.features.start_surface.StartSurfaceProperties.TOP_BAR_HEIGHT;
 
+import android.content.res.Resources;
 import android.view.View;
 
 import androidx.annotation.IntDef;
@@ -257,13 +260,15 @@ class StartSurfaceMediator
                 }
             };
 
-            // For OMNIBOX_ONLY variation, the grid tab switcher uses the same parent view as the
-            // feed section. Thus, we only set the top margin for above section when we are not in
-            // OMNIBOX_ONLY.
+            // Only tweak the margins between sections for non-OMNIBOX_ONLY variations.
             if (surfaceMode != SurfaceMode.OMNIBOX_ONLY) {
+                Resources resources = ContextUtils.getApplicationContext().getResources();
                 mPropertyModel.set(TASKS_SURFACE_BODY_TOP_MARGIN,
-                        ContextUtils.getApplicationContext().getResources().getDimensionPixelSize(
-                                R.dimen.tasks_surface_body_top_margin));
+                        resources.getDimensionPixelSize(R.dimen.tasks_surface_body_top_margin));
+                mPropertyModel.set(MV_TILES_CONTAINER_TOP_MARGIN,
+                        resources.getDimensionPixelSize(R.dimen.mv_tiles_container_top_margin));
+                mPropertyModel.set(TAB_SWITCHER_TITLE_TOP_MARGIN,
+                        resources.getDimensionPixelSize(R.dimen.tab_switcher_title_top_margin));
             }
         }
         mController.addOverviewModeObserver(this);

chrome/android/features/start_surface/internal/junit/src/org/chromium/chrome/features/start_surface/StartSurfaceMediatorUnitTest.java

@@ -23,7 +23,9 @@ import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.IS_INCOGN
 import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.IS_INCOGNITO_DESCRIPTION_VISIBLE;
 import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.IS_TAB_CAROUSEL_VISIBLE;
 import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.IS_VOICE_RECOGNITION_BUTTON_VISIBLE;
+import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.MV_TILES_CONTAINER_TOP_MARGIN;
 import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.MV_TILES_VISIBLE;
+import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.TAB_SWITCHER_TITLE_TOP_MARGIN;
 import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.TASKS_SURFACE_BODY_TOP_MARGIN;
 import static org.chromium.chrome.features.start_surface.StartSurfaceProperties.BOTTOM_BAR_CLICKLISTENER;
 import static org.chromium.chrome.features.start_surface.StartSurfaceProperties.BOTTOM_BAR_HEIGHT;
@@ -36,6 +38,7 @@ import static org.chromium.chrome.features.start_surface.StartSurfaceProperties.
 import static org.chromium.chrome.features.start_surface.StartSurfaceProperties.IS_SHOWING_STACK_TAB_SWITCHER;
 import static org.chromium.chrome.features.start_surface.StartSurfaceProperties.TOP_BAR_HEIGHT;
 
+import android.content.res.Resources;
 import android.view.View;
 
 import org.junit.After;
@@ -1473,15 +1476,27 @@ public class StartSurfaceMediatorUnitTest {
     }
 
     @Test
-    public void initializeTasksSurfaceBodyTopMargin() {
-        int topMargin = ContextUtils.getApplicationContext().getResources().getDimensionPixelSize(
-                R.dimen.tasks_surface_body_top_margin);
+    public void initializeStartSurfaceTopMargins() {
+        Resources resources = ContextUtils.getApplicationContext().getResources();
+        int tasksSurfaceBodyTopMargin =
+                resources.getDimensionPixelSize(R.dimen.tasks_surface_body_top_margin);
+        int mvTilesContainerTopMargin =
+                resources.getDimensionPixelSize(R.dimen.mv_tiles_container_top_margin);
+        int tabSwitcherTitleTopMargin =
+                resources.getDimensionPixelSize(R.dimen.tab_switcher_title_top_margin);
 
         createStartSurfaceMediatorWithoutInit(SurfaceMode.OMNIBOX_ONLY, false, false);
         assertThat(mPropertyModel.get(TASKS_SURFACE_BODY_TOP_MARGIN), equalTo(0));
+        assertThat(mPropertyModel.get(MV_TILES_CONTAINER_TOP_MARGIN), equalTo(0));
+        assertThat(mPropertyModel.get(TAB_SWITCHER_TITLE_TOP_MARGIN), equalTo(0));
 
         createStartSurfaceMediatorWithoutInit(SurfaceMode.SINGLE_PANE, false, false);
-        assertThat(mPropertyModel.get(TASKS_SURFACE_BODY_TOP_MARGIN), equalTo(topMargin));
+        assertThat(mPropertyModel.get(TASKS_SURFACE_BODY_TOP_MARGIN),
+                equalTo(tasksSurfaceBodyTopMargin));
+        assertThat(mPropertyModel.get(MV_TILES_CONTAINER_TOP_MARGIN),
+                equalTo(mvTilesContainerTopMargin));
+        assertThat(mPropertyModel.get(TAB_SWITCHER_TITLE_TOP_MARGIN),
+                equalTo(tabSwitcherTitleTopMargin));
     }
 
     private StartSurfaceMediator createStartSurfaceMediator(

chrome/android/features/tab_ui/java/res/layout/tasks_view_layout.xml

@@ -25,7 +25,6 @@
         <HorizontalScrollView android:id="@+id/mv_tiles_container"
             android:layout_width="match_parent"
             android:layout_height="wrap_content"
-            android:layout_marginTop="12dp"
             android:background="@color/default_bg_color"
             android:visibility="gone"
             android:scrollbars="none"
@@ -44,7 +43,6 @@
             android:paddingEnd="4dp"
             android:paddingTop="8dp"
             android:paddingBottom="8dp"
-            android:layout_marginTop="16dp"
             android:visibility="gone"
             android:background="@color/default_bg_color"
             android:orientation="horizontal"

chrome/android/features/tab_ui/java/src/org/chromium/chrome/browser/tasks/TasksSurfaceProperties.java

@@ -68,6 +68,10 @@ public class TasksSurfaceProperties {
             new PropertyModel.WritableObjectPropertyKey<View.OnClickListener>();
     public static final PropertyModel.WritableIntPropertyKey TASKS_SURFACE_BODY_TOP_MARGIN =
             new PropertyModel.WritableIntPropertyKey();
+    public static final PropertyModel.WritableIntPropertyKey MV_TILES_CONTAINER_TOP_MARGIN =
+            new PropertyModel.WritableIntPropertyKey();
+    public static final PropertyModel.WritableIntPropertyKey TAB_SWITCHER_TITLE_TOP_MARGIN =
+            new PropertyModel.WritableIntPropertyKey();
     public static final PropertyKey[] ALL_KEYS = new PropertyKey[] {IS_FAKE_SEARCH_BOX_VISIBLE,
             IS_INCOGNITO, IS_INCOGNITO_DESCRIPTION_INITIALIZED, IS_INCOGNITO_DESCRIPTION_VISIBLE,
             IS_TAB_CAROUSEL_VISIBLE, IS_VOICE_RECOGNITION_BUTTON_VISIBLE,
@@ -77,5 +81,6 @@ public class TasksSurfaceProperties {
             INCOGNITO_COOKIE_CONTROLS_TOGGLE_ENFORCEMENT, INCOGNITO_COOKIE_CONTROLS_MANAGER,
             INCOGNITO_LEARN_MORE_CLICK_LISTENER, FAKE_SEARCH_BOX_CLICK_LISTENER,
             FAKE_SEARCH_BOX_TEXT_WATCHER, MORE_TABS_CLICK_LISTENER, MV_TILES_VISIBLE,
-            VOICE_SEARCH_BUTTON_CLICK_LISTENER, TASKS_SURFACE_BODY_TOP_MARGIN};
+            VOICE_SEARCH_BUTTON_CLICK_LISTENER, TASKS_SURFACE_BODY_TOP_MARGIN,
+            MV_TILES_CONTAINER_TOP_MARGIN, TAB_SWITCHER_TITLE_TOP_MARGIN};
 }

chrome/android/features/tab_ui/java/src/org/chromium/chrome/browser/tasks/TasksView.java

@@ -292,4 +292,26 @@ class TasksView extends CoordinatorLayoutForPointer {
         MarginLayoutParams params = (MarginLayoutParams) getBodyViewContainer().getLayoutParams();
         params.topMargin = topMargin;
     }
+
+    /**
+     * Set the top margin for the mv tiles container.
+     * @param topMargin The top margin to set.
+     */
+    void setMVTilesContainerTopMargin(int topMargin) {
+        MarginLayoutParams params =
+                (MarginLayoutParams) mHeaderView.findViewById(R.id.mv_tiles_container)
+                        .getLayoutParams();
+        params.topMargin = topMargin;
+    }
+
+    /**
+     * Set the top margin for the tab switcher title.
+     * @param topMargin The top margin to set.
+     */
+    void setTabSwitcherTitleTopMargin(int topMargin) {
+        MarginLayoutParams params =
+                (MarginLayoutParams) mHeaderView.findViewById(R.id.tab_switcher_title)
+                        .getLayoutParams();
+        params.topMargin = topMargin;
+    }
 }

chrome/android/features/tab_ui/java/src/org/chromium/chrome/browser/tasks/TasksViewBinder.java

@@ -20,7 +20,9 @@ import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.IS_INCOGN
 import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.IS_TAB_CAROUSEL_VISIBLE;
 import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.IS_VOICE_RECOGNITION_BUTTON_VISIBLE;
 import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.MORE_TABS_CLICK_LISTENER;
+import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.MV_TILES_CONTAINER_TOP_MARGIN;
 import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.MV_TILES_VISIBLE;
+import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.TAB_SWITCHER_TITLE_TOP_MARGIN;
 import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.TASKS_SURFACE_BODY_TOP_MARGIN;
 import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.VOICE_SEARCH_BUTTON_CLICK_LISTENER;
 
@@ -86,6 +88,10 @@ class TasksViewBinder {
                     model.get(VOICE_SEARCH_BUTTON_CLICK_LISTENER));
         } else if (propertyKey == TASKS_SURFACE_BODY_TOP_MARGIN) {
             view.setTasksSurfaceBodyTopMargin(model.get(TASKS_SURFACE_BODY_TOP_MARGIN));
+        } else if (propertyKey == MV_TILES_CONTAINER_TOP_MARGIN) {
+            view.setMVTilesContainerTopMargin(model.get(MV_TILES_CONTAINER_TOP_MARGIN));
+        } else if (propertyKey == TAB_SWITCHER_TITLE_TOP_MARGIN) {
+            view.setTabSwitcherTitleTopMargin(model.get(TAB_SWITCHER_TITLE_TOP_MARGIN));
         }
     }
 }

chrome/android/features/tab_ui/javatests/src/org/chromium/chrome/browser/tasks/TasksViewBinderTest.java

@@ -24,7 +24,9 @@ import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.IS_INCOGN
 import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.IS_TAB_CAROUSEL_VISIBLE;
 import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.IS_VOICE_RECOGNITION_BUTTON_VISIBLE;
 import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.MORE_TABS_CLICK_LISTENER;
+import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.MV_TILES_CONTAINER_TOP_MARGIN;
 import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.MV_TILES_VISIBLE;
+import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.TAB_SWITCHER_TITLE_TOP_MARGIN;
 import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.TASKS_SURFACE_BODY_TOP_MARGIN;
 import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.VOICE_SEARCH_BUTTON_CLICK_LISTENER;
 
@@ -251,4 +253,32 @@ public class TasksViewBinderTest extends DummyUiActivityTestCase {
 
         assertEquals(16, params.topMargin);
     }
+
+    @Test
+    @UiThreadTest
+    @SmallTest
+    public void testSetMVTilesContainerTopMargin() {
+        ViewGroup.MarginLayoutParams params =
+                (ViewGroup.MarginLayoutParams) mTasksView.findViewById(R.id.mv_tiles_container)
+                        .getLayoutParams();
+        assertEquals(0, params.topMargin);
+
+        mTasksViewPropertyModel.set(MV_TILES_CONTAINER_TOP_MARGIN, 16);
+
+        assertEquals(16, params.topMargin);
+    }
+
+    @Test
+    @UiThreadTest
+    @SmallTest
+    public void testSetTabSwitcherTitleTopMargin() {
+        ViewGroup.MarginLayoutParams params =
+                (ViewGroup.MarginLayoutParams) mTasksView.findViewById(R.id.tab_switcher_title)
+                        .getLayoutParams();
+        assertEquals(0, params.topMargin);
+
+        mTasksViewPropertyModel.set(TAB_SWITCHER_TITLE_TOP_MARGIN, 16);
+
+        assertEquals(16, params.topMargin);
+    }
 }

chrome/test/data/android/render_tests/InstantStartTest.tabSwitcher_3tabs.Nexus_5X-23.png.sha1

-c364422d2c2b938077e52b9d198a54d81aca596a
+447470455aa2afcb3c2f0abe8a86a69c4840b84d
\ No newline at end of file

chrome/test/data/android/render_tests/InstantStartTest.tabSwitcher_3tabs_postNative.Nexus_5X-23.png.sha1

-8c49c3a07c8633e19b41780c8d4d857f3e6bb8a4
+684f77e63aef056e0d5568ab8ff78f9b17f6c5e9
\ No newline at end of file

chrome/test/data/android/render_tests/InstantStartTest.tabSwitcher_empty.Nexus_5X-23.png.sha1

-c368ed5722d25409c565950a6057433b7f71b8be
+6b84e01daf77a9204a8225528f9eead72c2d6b84
\ No newline at end of file

chrome/test/data/android/render_tests/InstantStartTest.tabSwitcher_tabGroups.Nexus_5X-23.png.sha1

-2613bfb1e6e6af9a73851b86db770ebbabcfe8ba
+54a386119693dbee82caed78871a661927b535a5
\ No newline at end of file

chrome/test/data/android/render_tests/ReturnToChromeTest.10_web_tabs-select_last.Nexus_5X-23.png.sha1

-a538a51c6635ad8738fe43c3e1cbd3f304fd8887
+22635e645468d82ffeaf0297a4d132b37821b4cc
\ No newline at end of file

chrome/test/data/android/render_tests/ReturnToChromeTest.10_web_tabs-select_last.Pixel_2-28.png.sha1

-2a4db2c3ceb84c1da5a8e140112e8b79e06f6771
+dab25947a177f0e9de3dac2b12fdc2c6638d6422
\ No newline at end of file