Fix a bug in TabGridDialog initial visibility setup

http://crrev.com/c/2242222 sets the dialog visibility into false in
TabGridDialog constructor as a default value for this field. Setting
this in PropertyModel will emit a signal to hide the dialog, but the
signal should be no-op as the dialog is not showing. Otherwise this will
break a assumption in ScrimView and crash. http://crrev.com/c/2242222
enforces this is no-op by ignoring the signal when mScrimPropertyModel
is not setup. The assumption here is that the dummy signal will always
arrive earlier than the mScrimPropertyModel setup, which happens when
dialog is first shown.

However, this assumption no longer holds for StartSurface, because
for StartSurface, the initialization of TabGridDialog (which is
controlled by the initialization of TabSwitcher) can happen later than
the first show of TabGridDialog. Such a flow can be: Step1) Open dialog
from strip and dismiss. (This is when mScrimPropertyModel sets up.)
Step2) Click the tab switcher button to enter tab switcher. (This is
when the dummy hide dialog signal is sent out.)

This CL fixes this issue by switching to a more stable way to filter
out dummy signal: whether the dialog is showing. This way we make
sure that the signals that should be no-op on dialog side are in
fact no-op.

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

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

@@ -8,7 +8,8 @@
     xmlns:tools="http://schemas.android.com/tools"
     android:layout_width="match_parent"
     android:layout_height="match_parent"
-    android:id="@+id/dialog_parent_view">
+    android:id="@+id/dialog_parent_view"
+    android:visibility="gone">
     <RelativeLayout
         android:layout_width="match_parent"
         android:layout_height="match_parent"

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

@@ -698,11 +698,13 @@ public class TabGridDialogView extends FrameLayout
      * Hide {@link PopupWindow} for dialog with animation.
      */
     void hideDialog() {
+        // Skip the hideDialog call caused by initializing the dialog visibility as false.
+        if (getVisibility() != VISIBLE) return;
+        assert mScrimCoordinator != null && mScrimPropertyModel != null;
         if (mCurrentDialogAnimator != null && mCurrentDialogAnimator != mHideDialogAnimation) {
             mCurrentDialogAnimator.end();
         }
         mCurrentDialogAnimator = mHideDialogAnimation;
-        if (mScrimCoordinator == null || mScrimPropertyModel == null) return;
         mScrimCoordinator.hideScrim(true);
         mHideDialogAnimation.start();
     }

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

@@ -18,6 +18,7 @@ import static androidx.test.espresso.intent.matcher.IntentMatchers.hasAction;
 import static androidx.test.espresso.intent.matcher.IntentMatchers.hasExtras;
 import static androidx.test.espresso.intent.matcher.IntentMatchers.hasType;
 import static androidx.test.espresso.matcher.RootMatchers.withDecorView;
+import static androidx.test.espresso.matcher.ViewMatchers.isCompletelyDisplayed;
 import static androidx.test.espresso.matcher.ViewMatchers.isDescendantOfA;
 import static androidx.test.espresso.matcher.ViewMatchers.isDisplayed;
 import static androidx.test.espresso.matcher.ViewMatchers.withId;
@@ -41,6 +42,7 @@ import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.c
 import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.closeNthTabInDialog;
 import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.createTabs;
 import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.enterTabSwitcher;
+import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.finishActivity;
 import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.getSwipeToDismissAction;
 import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.mergeAllNormalTabsToAGroup;
 import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.prepareTabsWithThumbnail;
@@ -48,6 +50,9 @@ import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.r
 import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.verifyAllTabsHaveThumbnail;
 import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.verifyTabStripFaviconCount;
 import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.verifyTabSwitcherCardCount;
+import static org.chromium.chrome.features.start_surface.InstantStartTest.createTabStateFile;
+import static org.chromium.chrome.features.start_surface.InstantStartTest.createThumbnailBitmapAndWriteToFile;
+import static org.chromium.chrome.test.util.ViewUtils.waitForView;
 
 import android.content.Intent;
 import android.content.pm.ActivityInfo;
@@ -87,6 +92,7 @@ import org.chromium.chrome.browser.compositor.layouts.Layout;
 import org.chromium.chrome.browser.flags.ChromeFeatureList;
 import org.chromium.chrome.browser.flags.ChromeSwitches;
 import org.chromium.chrome.browser.night_mode.ChromeNightModeTestUtils;
+import org.chromium.chrome.browser.tasks.pseudotab.TabAttributeCache;
 import org.chromium.chrome.browser.tasks.tab_groups.TabGroupModelFilter;
 import org.chromium.chrome.features.start_surface.StartSurfaceLayout;
 import org.chromium.chrome.tab_ui.R;
@@ -113,6 +119,8 @@ public class TabGridDialogTest {
     // clang-format on
     private static final String CUSTOMIZED_TITLE1 = "wfh tips";
     private static final String CUSTOMIZED_TITLE2 = "wfh funs";
+    private static final String START_SURFACE_BASE_PARAMS =
+            "force-fieldtrial-params=Study.Group:start_surface_variation";
 
     private boolean mHasReceivedSourceRect;
     private TabSelectionEditorTestingRobot mSelectionEditorRobot =
@@ -653,7 +661,7 @@ public class TabGridDialogTest {
         verifyTabSwitcherCardCount(cta, 1);
 
         // Restart the activity and open the dialog from strip to check the initial setup of dialog.
-        TabUiTestHelper.finishActivity(cta);
+        finishActivity(cta);
         mActivityTestRule.startMainActivityFromLauncher();
         CriteriaHelper.pollUiThread(
                 mActivityTestRule.getActivity().getTabModelSelector()::isTabStateInitialized);
@@ -821,6 +829,42 @@ public class TabGridDialogTest {
         assertEquals(null, firstItem.getContentDescription());
     }
 
+    @Test
+    @MediumTest
+    @Features.EnableFeatures({ChromeFeatureList.START_SURFACE_ANDROID + "<Study"})
+    @CommandLineFlags.Add({"force-fieldtrials=Study/Group", START_SURFACE_BASE_PARAMS + "/single"})
+    public void testDialogSetup_WithStartSurface() throws Exception {
+        // Create a tab group with 2 tabs.
+        finishActivity(mActivityTestRule.getActivity());
+        createThumbnailBitmapAndWriteToFile(0);
+        createThumbnailBitmapAndWriteToFile(1);
+        TabAttributeCache.setRootIdForTesting(0, 0);
+        TabAttributeCache.setRootIdForTesting(1, 0);
+        createTabStateFile(new int[] {0, 1});
+
+        // Restart Chrome and make sure tab strip is showing.
+        mActivityTestRule.startMainActivityFromLauncher();
+        ChromeTabbedActivity cta = mActivityTestRule.getActivity();
+        CriteriaHelper.pollUiThread(cta.getTabModelSelector()::isTabStateInitialized);
+        CriteriaHelper.pollUiThread(()
+                                            -> mActivityTestRule.getActivity()
+                                                       .getBrowserControlsManager()
+                                                       .getBottomControlOffset()
+                        == 0);
+        waitForView(allOf(withId(R.id.toolbar_left_button), isCompletelyDisplayed()));
+
+        // Test opening dialog from strip and from tab switcher.
+        openDialogFromStripAndVerify(cta, 2, null);
+        Espresso.pressBack();
+        // Tab switcher is created, and the dummy signal to hide dialog is sent. This line would
+        // crash if the dummy signal is not properly handled. See crbug.com/1096358.
+        enterTabSwitcher(cta);
+        onView(allOf(withParent(withId(R.id.tasks_surface_body)), withId(R.id.tab_list_view)))
+                .perform(RecyclerViewActions.actionOnItemAtPosition(0, click()));
+        CriteriaHelper.pollUiThread(() -> isDialogShowing(mActivityTestRule.getActivity()));
+        verifyShowingDialog(cta, 2, null);
+    }
+
     private void openDialogFromTabSwitcherAndVerify(
             ChromeTabbedActivity cta, int tabCount, String customizedTitle) {
         clickFirstCardFromTabSwitcher(cta);