Add an option to TabGridDialog menu to edit group name

This CL adds and option to the TabGridDialog toolbar menu to update
the group name. Also, this CL cleans up a bit the dependency
relationship between title text focus and keyboard visibility, so that
keyboard visibility is controlled by title text focus. When title gains
focus, we try to show the keyboard; when title text loses focus, we
hide the keyboard.

Apart from the above rule of thumb, we handle three cases specifically:

* When "Edit group name" item is selected in dialog menu, we request
  focus programmatically which also shows the keyboard.
* When keyboard hides but the title still has focus, we manually clear
  the focus.
* When dialog hides, we always clear focus on dialog title.

This CL is gated by Finch parameter "enable_launch_polish" under flag
"enable-tab-grid-layout", with gate function
TabUiFeatureUtilities#isLaunchPolishEnabled. This CL is verified by the
formal equivalence tool in http://crrev.com/c/1934235 to be completely
behind flag.

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

chrome/android/expectations/lint-suppressions.xml

@@ -281,7 +281,6 @@ Still reading?
     <ignore regexp="The resource `R.string.accessibility_close_tab_group_button` appears to be unused"/>
     <ignore regexp="The resource `R.string.accessibility_close_tab_group_button_with_group_name` appears to be unused"/>
     <ignore regexp="The resource `R.string.accessibility_expand_tab_group_with_group_name` appears to be unused"/>
-    <ignore regexp="The resource `R.string.tab_grid_dialog_toolbar_edit_group_name` appears to be unused"/>
     <!-- Old-style and new-style WebAPKs use same resources for simplicity. Old-style WebAPKs do
          not use R.style.SplashTheme but new-style WebAPKs do.
          TODO(crbug.com/971254): Remove suppression once old-style WebAPKs are deprecated. -->

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

@@ -227,8 +227,10 @@ public class TabGridDialogMediator implements SnackbarManager.SnackbarController
 
         // Setup ScrimView click Runnable.
         mScrimClickRunnable = () -> {
-            mModel.set(TabGridPanelProperties.IS_KEYBOARD_VISIBLE, false);
-            mModel.set(TabGridPanelProperties.IS_TITLE_TEXT_FOCUSED, false);
+            if (!TabUiFeatureUtilities.isLaunchPolishEnabled()) {
+                mModel.set(TabGridPanelProperties.IS_KEYBOARD_VISIBLE, false);
+                mModel.set(TabGridPanelProperties.IS_TITLE_TEXT_FOCUSED, false);
+            }
             hideDialog(true);
             RecordUserAction.record("TabGridDialog.Exit");
         };
@@ -247,7 +249,9 @@ public class TabGridDialogMediator implements SnackbarManager.SnackbarController
 
         mToolbarMenuCallback = result -> {
             if (result == R.id.ungroup_tab) {
-                mModel.set(TabGridPanelProperties.IS_KEYBOARD_VISIBLE, false);
+                if (!TabUiFeatureUtilities.isLaunchPolishEnabled()) {
+                    mModel.set(TabGridPanelProperties.IS_KEYBOARD_VISIBLE, false);
+                }
                 mModel.set(TabGridPanelProperties.IS_TITLE_TEXT_FOCUSED, false);
                 List<Tab> tabs = getRelatedTabs(mCurrentTabId);
                 if (mTabSelectionEditorController != null) {
@@ -279,6 +283,12 @@ public class TabGridDialogMediator implements SnackbarManager.SnackbarController
                                                               .build();
                 mShareDelegateSupplier.get().share(shareParams, chromeShareExtras);
             }
+
+            if (TabUiFeatureUtilities.isLaunchPolishEnabled()) {
+                if (result == R.id.edit_group_name) {
+                    mModel.set(TabGridPanelProperties.IS_TITLE_TEXT_FOCUSED, true);
+                }
+            }
         };
 
         // Setup toolbar button click listeners.
@@ -308,6 +318,9 @@ public class TabGridDialogMediator implements SnackbarManager.SnackbarController
             mTabSelectionEditorController.hide();
         }
         saveCurrentGroupModifiedTitle();
+        if (TabUiFeatureUtilities.isLaunchPolishEnabled()) {
+            mModel.set(TabGridPanelProperties.IS_TITLE_TEXT_FOCUSED, false);
+        }
         mDialogController.resetWithListOfTabs(null);
     }
 
@@ -431,8 +444,12 @@ public class TabGridDialogMediator implements SnackbarManager.SnackbarController
     private void setupToolbarEditText() {
         mKeyboardVisibilityListener = isShowing -> {
             mModel.set(TabGridPanelProperties.TITLE_CURSOR_VISIBILITY, isShowing);
-            mModel.set(TabGridPanelProperties.IS_TITLE_TEXT_FOCUSED, isShowing);
-            mModel.set(TabGridPanelProperties.IS_KEYBOARD_VISIBLE, isShowing);
+            if (!TabUiFeatureUtilities.isLaunchPolishEnabled()) {
+                mModel.set(TabGridPanelProperties.IS_TITLE_TEXT_FOCUSED, isShowing);
+                mModel.set(TabGridPanelProperties.IS_KEYBOARD_VISIBLE, isShowing);
+            } else if (TabUiFeatureUtilities.isLaunchPolishEnabled() && !isShowing) {
+                mModel.set(TabGridPanelProperties.IS_TITLE_TEXT_FOCUSED, false);
+            }
             if (!isShowing) {
                 saveCurrentGroupModifiedTitle();
             }
@@ -455,14 +472,20 @@ public class TabGridDialogMediator implements SnackbarManager.SnackbarController
         };
         mModel.set(TabGridPanelProperties.TITLE_TEXT_WATCHER, textWatcher);
 
-        View.OnFocusChangeListener onFocusChangeListener =
-                (v, hasFocus) -> mIsUpdatingTitle = hasFocus;
+        View.OnFocusChangeListener onFocusChangeListener = (v, hasFocus) -> {
+            mIsUpdatingTitle = hasFocus;
+            if (!TabUiFeatureUtilities.isLaunchPolishEnabled()) return;
+            mModel.set(TabGridPanelProperties.IS_KEYBOARD_VISIBLE, hasFocus);
+            mModel.set(TabGridPanelProperties.IS_TITLE_TEXT_FOCUSED, hasFocus);
+        };
         mModel.set(TabGridPanelProperties.TITLE_TEXT_ON_FOCUS_LISTENER, onFocusChangeListener);
     }
 
     private View.OnClickListener getCollapseButtonClickListener() {
         return view -> {
-            mModel.set(TabGridPanelProperties.IS_KEYBOARD_VISIBLE, false);
+            if (!TabUiFeatureUtilities.isLaunchPolishEnabled()) {
+                mModel.set(TabGridPanelProperties.IS_KEYBOARD_VISIBLE, false);
+            }
             hideDialog(true);
             RecordUserAction.record("TabGridDialog.Exit");
         };

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

@@ -149,6 +149,11 @@ public class TabGridDialogMenuCoordinator {
         itemList.add(new ListItem(ListItemType.MENU_ITEM,
                 buildPropertyModel(context, R.string.tab_grid_dialog_toolbar_share_group,
                         R.id.share_tab_group)));
+        if (TabUiFeatureUtilities.isLaunchPolishEnabled()) {
+            itemList.add(new ListItem(ListItemType.MENU_ITEM,
+                    buildPropertyModel(context, R.string.tab_grid_dialog_toolbar_edit_group_name,
+                            R.id.edit_group_name)));
+        }
         return itemList;
     }
 

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

@@ -131,11 +131,19 @@ class TabGridPanelViewBinder {
         } else if (TITLE_CURSOR_VISIBILITY == propertyKey) {
             viewHolder.toolbarView.setTitleCursorVisibility(model.get(TITLE_CURSOR_VISIBILITY));
         } else if (IS_TITLE_TEXT_FOCUSED == propertyKey) {
+            if (TabUiFeatureUtilities.isLaunchPolishEnabled()) {
+                viewHolder.toolbarView.updateTitleTextFocus(model.get(IS_TITLE_TEXT_FOCUSED));
+                return;
+            }
             // Don't explicitly request focus since it should happen automatically.
             if (!model.get(IS_TITLE_TEXT_FOCUSED)) {
                 viewHolder.toolbarView.clearTitleTextFocus();
             }
         } else if (IS_KEYBOARD_VISIBLE == propertyKey) {
+            if (TabUiFeatureUtilities.isLaunchPolishEnabled()) {
+                viewHolder.toolbarView.updateKeyboardVisibility(model.get(IS_KEYBOARD_VISIBLE));
+                return;
+            }
             // Don't explicitly show keyboard since it should happen automatically.
             if (!model.get(IS_KEYBOARD_VISIBLE)) {
                 viewHolder.toolbarView.hideKeyboard();

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

@@ -80,6 +80,40 @@ public class TabGroupUiToolbarView extends FrameLayout {
         mTitleTextView.setCursorVisible(isVisible);
     }
 
+    void updateTitleTextFocus(boolean shouldFocus) {
+        if (!TabUiFeatureUtilities.isLaunchPolishEnabled()) return;
+        if (mTitleTextView.isFocused() == shouldFocus) return;
+        if (shouldFocus) {
+            mTitleTextView.requestFocus();
+        } else {
+            clearTitleTextFocus();
+        }
+    }
+
+    void updateKeyboardVisibility(boolean shouldShow) {
+        if (!TabUiFeatureUtilities.isLaunchPolishEnabled()) return;
+        // This is equal to the animation duration of toolbar menu hiding.
+        int showKeyboardDelay = 150;
+        if (shouldShow) {
+            // TODO(crbug.com/1116644) Figure out why a call to show keyboard without delay still
+            // won't work when the window gets focus in onWindowFocusChanged call.
+            // Wait until the current window has focus to show the keyboard. This is to deal with
+            // the case where the keyboard showing is caused by toolbar menu. In this case, we need
+            // to wait for the menu window to hide and current window to gain focus so that we can
+            // show the keyboard.
+            KeyboardVisibilityDelegate delegate = KeyboardVisibilityDelegate.getInstance();
+            postDelayed(new Runnable() {
+                @Override
+                public void run() {
+                    assert hasWindowFocus();
+                    delegate.showKeyboard(mTitleTextView);
+                }
+            }, showKeyboardDelay);
+        } else {
+            hideKeyboard();
+        }
+    }
+
     void clearTitleTextFocus() {
         mTitleTextView.clearFocus();
     }

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

@@ -623,6 +623,37 @@ public class TabGridDialogTest {
         verifyFirstCardTitle(CUSTOMIZED_TITLE2);
     }
 
+    @Test
+    @MediumTest
+    // clang-format off
+    @Features.EnableFeatures({ChromeFeatureList.TAB_GROUPS_CONTINUATION_ANDROID,
+            ChromeFeatureList.TAB_GRID_LAYOUT_ANDROID + "<Study"})
+    @CommandLineFlags.Add({"force-fieldtrials=Study/Group", TAB_GROUP_LAUNCH_POLISH_PARAMS})
+    public void testTabGroupNaming_KeyboardVisibility() throws ExecutionException {
+        // clang-format on
+        final ChromeTabbedActivity cta = mActivityTestRule.getActivity();
+        createTabs(cta, false, 2);
+        enterTabSwitcher(cta);
+        verifyTabSwitcherCardCount(cta, 2);
+
+        // Create a tab group.
+        mergeAllNormalTabsToAGroup(cta);
+        verifyTabSwitcherCardCount(cta, 1);
+        openDialogFromTabSwitcherAndVerify(cta, 2,
+                cta.getResources().getQuantityString(
+                        R.plurals.bottom_tab_grid_title_placeholder, 2, 2));
+
+        // Test title text focus in dialog in tab switcher.
+        testTitleTextFocus(cta);
+
+        // Test title text focus in dialog from tab strip.
+        openDialogFromTabSwitcherAndVerify(cta, 2, null);
+        clickFirstTabInDialog(cta);
+        waitForDialogHidingAnimation(cta);
+        openDialogFromStripAndVerify(cta, 2, null);
+        testTitleTextFocus(cta);
+    }
+
     @Test
     @MediumTest
     @DisableIf.Build(supported_abis_includes = "x86", message = "https://crbug.com/1124336")
@@ -1035,6 +1066,11 @@ public class TabGridDialogTest {
                     // Verify if we can grab focus on the editText or not.
                     assertEquals(isEnabled, v.isFocused());
                 });
+        // Verify if the keyboard shows or not.
+        CriteriaHelper.pollUiThread(()
+                                            -> isEnabled
+                        == KeyboardVisibilityDelegate.getInstance().isKeyboardShowing(
+                                cta, cta.getCompositorViewHolder()));
     }
 
     private void openDialogToolbarMenuAndVerify(ChromeTabbedActivity cta) {
@@ -1046,11 +1082,17 @@ public class TabGridDialogTest {
                     if (noMatchException != null) throw noMatchException;
                     Assert.assertTrue(v instanceof ListView);
                     ListView listView = (ListView) v;
-                    assertEquals(2, listView.getCount());
                     verifyTabGridDialogToolbarMenuItem(listView, 0,
                             cta.getString(R.string.tab_grid_dialog_toolbar_remove_from_group));
                     verifyTabGridDialogToolbarMenuItem(listView, 1,
                             cta.getString(R.string.tab_grid_dialog_toolbar_share_group));
+                    if (TabUiFeatureUtilities.isLaunchPolishEnabled()) {
+                        assertEquals(3, listView.getCount());
+                        verifyTabGridDialogToolbarMenuItem(listView, 2,
+                                cta.getString(R.string.tab_grid_dialog_toolbar_edit_group_name));
+                    } else {
+                        assertEquals(2, listView.getCount());
+                    }
                 });
     }
 
@@ -1218,4 +1260,39 @@ public class TabGridDialogTest {
                        isDescendantOfA(withId(R.id.dialog_container_view))))
                 .check((v, e) -> assertEquals(s, v.getContentDescription()));
     }
+
+    private void testTitleTextFocus(ChromeTabbedActivity cta) throws ExecutionException {
+        // Click the text field to grab focus and click back button to lose focus.
+        onView(allOf(withParent(withId(R.id.main_content)), withId(R.id.title))).perform(click());
+        verifyTitleTextFocus(cta, true);
+        Espresso.pressBack();
+        verifyTitleTextFocus(cta, false);
+        verifyShowingDialog(cta, 2, null);
+
+        // Use toolbar menu to grab focus and click back button to lose focus.
+        openDialogToolbarMenuAndVerify(cta);
+        selectTabGridDialogToolbarMenuItem(cta, "Edit group name");
+        verifyTitleTextFocus(cta, true);
+        Espresso.pressBack();
+        verifyTitleTextFocus(cta, false);
+        verifyShowingDialog(cta, 2, null);
+
+        // Click the text field to grab focus and click scrim to lose focus.
+        onView(allOf(withParent(withId(R.id.main_content)), withId(R.id.title))).perform(click());
+        verifyTitleTextFocus(cta, true);
+        clickScrimToExitDialog(cta);
+        waitForDialogHidingAnimation(cta);
+        verifyTitleTextFocus(cta, false);
+    }
+
+    private void verifyTitleTextFocus(ChromeTabbedActivity cta, boolean shouldFocus) {
+        CriteriaHelper.pollUiThread(() -> {
+            View titleTextView = cta.findViewById(R.id.tab_group_toolbar).findViewById(R.id.title);
+            KeyboardVisibilityDelegate delegate = KeyboardVisibilityDelegate.getInstance();
+            boolean keyboardVisible =
+                    delegate.isKeyboardShowing(cta, cta.getCompositorViewHolder());
+            boolean isFocused = titleTextView.isFocused();
+            return (!shouldFocus ^ isFocused) && (!shouldFocus ^ keyboardVisible);
+        });
+    }
 }