Commit fffc75e9 authored by Yue Zhang's avatar Yue Zhang Committed by Commit Bot

Suppress TabStrip IPH when bottom sheet shows

This CL hides TabStrip IPH when bottom sheet shows and reshows IPH
right after bottom sheet hides. This CL is gated by Finch parameter
"enable_launch_bug_fix" under flag "enable-tab-grid-layout", with gate
function TabUiFeatureUtilities#isLaunchBugFixEnabled.

Bug: 1135926
Change-Id: I97c80177b3fabf82aab751c6bf95a1c9b1e27432
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2506618Reviewed-by: default avatarMatthew Jones <mdjones@chromium.org>
Reviewed-by: default avatarWei-Yin Chen (陳威尹) <wychen@chromium.org>
Commit-Queue: Yue Zhang <yuezhanggg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823416}
parent d7772802
...@@ -25,6 +25,9 @@ import org.chromium.chrome.browser.tabmodel.TabModelSelectorTabObserver; ...@@ -25,6 +25,9 @@ import org.chromium.chrome.browser.tabmodel.TabModelSelectorTabObserver;
import org.chromium.chrome.browser.tasks.tab_management.TabUiFeatureUtilities; import org.chromium.chrome.browser.tasks.tab_management.TabUiFeatureUtilities;
import org.chromium.chrome.browser.util.ChromeAccessibilityUtil; import org.chromium.chrome.browser.util.ChromeAccessibilityUtil;
import org.chromium.chrome.tab_ui.R; import org.chromium.chrome.tab_ui.R;
import org.chromium.components.browser_ui.bottomsheet.BottomSheetController;
import org.chromium.components.browser_ui.bottomsheet.BottomSheetObserver;
import org.chromium.components.browser_ui.bottomsheet.EmptyBottomSheetObserver;
import org.chromium.components.browser_ui.widget.textbubble.TextBubble; import org.chromium.components.browser_ui.widget.textbubble.TextBubble;
import org.chromium.components.feature_engagement.FeatureConstants; import org.chromium.components.feature_engagement.FeatureConstants;
import org.chromium.components.feature_engagement.Tracker; import org.chromium.components.feature_engagement.Tracker;
...@@ -41,7 +44,8 @@ public class TabGroupUtils { ...@@ -41,7 +44,8 @@ public class TabGroupUtils {
private static TabModelSelectorTabObserver sTabModelSelectorTabObserver; private static TabModelSelectorTabObserver sTabModelSelectorTabObserver;
private static final String TAB_GROUP_TITLES_FILE_NAME = "tab_group_titles"; private static final String TAB_GROUP_TITLES_FILE_NAME = "tab_group_titles";
public static void maybeShowIPH(@FeatureConstants String featureName, View view) { public static void maybeShowIPH(@FeatureConstants String featureName, View view,
@Nullable BottomSheetController bottomSheetController) {
// For tab group, all three IPHs are valid. For conditional tab strip, the only valid IPH // For tab group, all three IPHs are valid. For conditional tab strip, the only valid IPH
// below is TAB_GROUPS_TAP_TO_SEE_ANOTHER_TAB_FEATURE. // below is TAB_GROUPS_TAP_TO_SEE_ANOTHER_TAB_FEATURE.
if (!TabUiFeatureUtilities.isTabGroupsAndroidEnabled() if (!TabUiFeatureUtilities.isTabGroupsAndroidEnabled()
...@@ -87,7 +91,37 @@ public class TabGroupUtils { ...@@ -87,7 +91,37 @@ public class TabGroupUtils {
TextBubble textBubble = new TextBubble(view.getContext(), view, textId, accessibilityTextId, TextBubble textBubble = new TextBubble(view.getContext(), view, textId, accessibilityTextId,
true, rectProvider, ChromeAccessibilityUtil.get().isAccessibilityEnabled()); true, rectProvider, ChromeAccessibilityUtil.get().isAccessibilityEnabled());
textBubble.setDismissOnTouchInteraction(true); textBubble.setDismissOnTouchInteraction(true);
textBubble.addOnDismissListener(() -> tracker.dismissed(featureName)); if (!TabUiFeatureUtilities.isLaunchBugFixEnabled()) {
textBubble.addOnDismissListener(() -> tracker.dismissed(featureName));
textBubble.show();
return;
}
if (bottomSheetController == null) return;
assert featureName.equals(FeatureConstants.TAB_GROUPS_TAP_TO_SEE_ANOTHER_TAB_FEATURE);
// This observer is added when IPH shows and is removed when IPH is dismissed via user
// explicitly closing the text bubble.
BottomSheetObserver bottomSheetObserver = new EmptyBottomSheetObserver() {
@Override
public void onSheetStateChanged(int newState) {
if (newState == BottomSheetController.SheetState.HIDDEN) {
textBubble.show();
} else {
textBubble.dismiss();
}
}
};
textBubble.addOnDismissListener(() -> {
// Don't dismiss the feature when the hide is caused by bottom sheet showing.
if (bottomSheetController.getSheetState() != BottomSheetController.SheetState.HIDDEN) {
return;
}
tracker.dismissed(featureName);
bottomSheetController.removeObserver(bottomSheetObserver);
});
bottomSheetController.addObserver(bottomSheetObserver);
textBubble.show(); textBubble.show();
} }
...@@ -113,7 +147,7 @@ public class TabGroupUtils { ...@@ -113,7 +147,7 @@ public class TabGroupUtils {
&& (transition & PageTransition.CORE_MASK) && (transition & PageTransition.CORE_MASK)
== PageTransition.GENERATED)) { == PageTransition.GENERATED)) {
maybeShowIPH(FeatureConstants.TAB_GROUPS_QUICKLY_COMPARE_PAGES_FEATURE, maybeShowIPH(FeatureConstants.TAB_GROUPS_QUICKLY_COMPARE_PAGES_FEATURE,
tab.getView()); tab.getView(), null);
sTabModelSelectorTabObserver.destroy(); sTabModelSelectorTabObserver.destroy();
} }
} }
......
...@@ -172,7 +172,8 @@ public class TabGroupUiCoordinator implements TabGroupUiMediator.ResetHandler, T ...@@ -172,7 +172,8 @@ public class TabGroupUiCoordinator implements TabGroupUiMediator.ResetHandler, T
&& bottomSheetController.getSheetState() && bottomSheetController.getSheetState()
== BottomSheetController.SheetState.HIDDEN) { == BottomSheetController.SheetState.HIDDEN) {
TabGroupUtils.maybeShowIPH(FeatureConstants.TAB_GROUPS_TAP_TO_SEE_ANOTHER_TAB_FEATURE, TabGroupUtils.maybeShowIPH(FeatureConstants.TAB_GROUPS_TAP_TO_SEE_ANOTHER_TAB_FEATURE,
mTabStripCoordinator.getContainerView()); mTabStripCoordinator.getContainerView(),
TabUiFeatureUtilities.isLaunchBugFixEnabled() ? bottomSheetController : null);
} }
mTabStripCoordinator.resetWithListOfTabs(tabs); mTabStripCoordinator.resetWithListOfTabs(tabs);
} }
......
...@@ -233,7 +233,7 @@ class TabListMediator { ...@@ -233,7 +233,7 @@ class TabListMediator {
() ()
-> TabGroupUtils.maybeShowIPH( -> TabGroupUtils.maybeShowIPH(
FeatureConstants.TAB_GROUPS_YOUR_TABS_ARE_TOGETHER_FEATURE, FeatureConstants.TAB_GROUPS_YOUR_TABS_ARE_TOGETHER_FEATURE,
anchor), anchor, null),
IPH_DELAY_MS); IPH_DELAY_MS);
} }
}; };
......
...@@ -7,15 +7,20 @@ package org.chromium.chrome.browser.tasks.tab_management; ...@@ -7,15 +7,20 @@ package org.chromium.chrome.browser.tasks.tab_management;
import static androidx.test.espresso.Espresso.onView; import static androidx.test.espresso.Espresso.onView;
import static androidx.test.espresso.action.ViewActions.click; import static androidx.test.espresso.action.ViewActions.click;
import static androidx.test.espresso.assertion.ViewAssertions.matches; import static androidx.test.espresso.assertion.ViewAssertions.matches;
import static androidx.test.espresso.matcher.RootMatchers.withDecorView;
import static androidx.test.espresso.matcher.ViewMatchers.Visibility.INVISIBLE; import static androidx.test.espresso.matcher.ViewMatchers.Visibility.INVISIBLE;
import static androidx.test.espresso.matcher.ViewMatchers.Visibility.VISIBLE; import static androidx.test.espresso.matcher.ViewMatchers.Visibility.VISIBLE;
import static androidx.test.espresso.matcher.ViewMatchers.isCompletelyDisplayed; import static androidx.test.espresso.matcher.ViewMatchers.isCompletelyDisplayed;
import static androidx.test.espresso.matcher.ViewMatchers.isDescendantOfA; import static androidx.test.espresso.matcher.ViewMatchers.isDescendantOfA;
import static androidx.test.espresso.matcher.ViewMatchers.isDisplayed;
import static androidx.test.espresso.matcher.ViewMatchers.withEffectiveVisibility; import static androidx.test.espresso.matcher.ViewMatchers.withEffectiveVisibility;
import static androidx.test.espresso.matcher.ViewMatchers.withId; import static androidx.test.espresso.matcher.ViewMatchers.withId;
import static androidx.test.espresso.matcher.ViewMatchers.withParent; import static androidx.test.espresso.matcher.ViewMatchers.withParent;
import static androidx.test.espresso.matcher.ViewMatchers.withText;
import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.allOf;
import static org.hamcrest.Matchers.not;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertTrue;
import static org.chromium.chrome.browser.flags.ChromeFeatureList.TAB_GRID_LAYOUT_ANDROID; import static org.chromium.chrome.browser.flags.ChromeFeatureList.TAB_GRID_LAYOUT_ANDROID;
...@@ -30,10 +35,13 @@ import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.v ...@@ -30,10 +35,13 @@ import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.v
import static org.chromium.chrome.features.start_surface.InstantStartTest.createTabStateFile; 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.features.start_surface.InstantStartTest.createThumbnailBitmapAndWriteToFile;
import static org.chromium.chrome.test.util.ViewUtils.waitForView; import static org.chromium.chrome.test.util.ViewUtils.waitForView;
import static org.chromium.components.feature_engagement.FeatureConstants.TAB_GROUPS_TAP_TO_SEE_ANOTHER_TAB_FEATURE;
import static org.chromium.content_public.browser.test.util.TestThreadUtils.runOnUiThreadBlocking;
import android.view.ViewGroup; import android.view.ViewGroup;
import androidx.recyclerview.widget.RecyclerView; import androidx.recyclerview.widget.RecyclerView;
import androidx.test.espresso.NoMatchingRootException;
import androidx.test.filters.MediumTest; import androidx.test.filters.MediumTest;
import org.junit.Before; import org.junit.Before;
...@@ -50,12 +58,17 @@ import org.chromium.chrome.browser.compositor.layouts.Layout; ...@@ -50,12 +58,17 @@ import org.chromium.chrome.browser.compositor.layouts.Layout;
import org.chromium.chrome.browser.flags.ChromeFeatureList; import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.flags.ChromeSwitches; import org.chromium.chrome.browser.flags.ChromeSwitches;
import org.chromium.chrome.browser.tasks.pseudotab.TabAttributeCache; import org.chromium.chrome.browser.tasks.pseudotab.TabAttributeCache;
import org.chromium.chrome.browser.widget.bottomsheet.TestBottomSheetContent;
import org.chromium.chrome.features.start_surface.StartSurfaceLayout; import org.chromium.chrome.features.start_surface.StartSurfaceLayout;
import org.chromium.chrome.tab_ui.R; import org.chromium.chrome.tab_ui.R;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner; import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.chrome.test.ChromeTabbedActivityTestRule; import org.chromium.chrome.test.ChromeTabbedActivityTestRule;
import org.chromium.chrome.test.util.ChromeRenderTestRule; import org.chromium.chrome.test.util.ChromeRenderTestRule;
import org.chromium.chrome.test.util.browser.Features; import org.chromium.chrome.test.util.browser.Features;
import org.chromium.components.browser_ui.bottomsheet.BottomSheetContent;
import org.chromium.components.browser_ui.bottomsheet.BottomSheetController;
import org.chromium.components.browser_ui.bottomsheet.BottomSheetController.SheetState;
import org.chromium.components.browser_ui.bottomsheet.BottomSheetTestSupport;
import org.chromium.content_public.browser.test.util.CriteriaHelper; import org.chromium.content_public.browser.test.util.CriteriaHelper;
import org.chromium.content_public.browser.test.util.TestThreadUtils; import org.chromium.content_public.browser.test.util.TestThreadUtils;
import org.chromium.ui.test.util.UiRestriction; import org.chromium.ui.test.util.UiRestriction;
...@@ -197,4 +210,89 @@ public class TabGroupUiTest { ...@@ -197,4 +210,89 @@ public class TabGroupUiTest {
onView(allOf(withId(R.id.tab_list_view), isDescendantOfA(withId(R.id.bottom_controls)))) onView(allOf(withId(R.id.tab_list_view), isDescendantOfA(withId(R.id.bottom_controls))))
.check(matches(withEffectiveVisibility((INVISIBLE)))); .check(matches(withEffectiveVisibility((INVISIBLE))));
} }
@Test
@MediumTest
// clang-format off
@Features.EnableFeatures({TAB_GROUPS_TAP_TO_SEE_ANOTHER_TAB_FEATURE
+ "<TabGroupsTapToSeeAnotherTab"})
@CommandLineFlags.Add({
"force-fieldtrials=TabGroupsTapToSeeAnotherTab/Enabled/, Study/Group",
"force-fieldtrial-params=TabGroupsTapToSeeAnotherTab.Enabled:availability/any/" +
"event_trigger/" +
"name%3Aiph_tabgroups_strip;comparator%3A==0;window%3A30;storage%3A365/" +
"event_trigger2/" +
"name%3Aiph_tabgroups_strip;comparator%3A<2;window%3A90;storage%3A365/" +
"event_used/" +
"name%3Aiph_tabgroups_strip;comparator%3A==0;window%3A365;storage%3A365/" +
"session_rate/<1"})
public void testIphSuppressedByBottomSheet() throws Exception {
// clang-format on
// Create a tab group with 2 tabs, and turn on enable_launch_bug_fix variation.
finishActivity(mActivityTestRule.getActivity());
createThumbnailBitmapAndWriteToFile(0);
createThumbnailBitmapAndWriteToFile(1);
TabAttributeCache.setRootIdForTesting(0, 0);
TabAttributeCache.setRootIdForTesting(1, 0);
createTabStateFile(new int[] {0, 1});
TabUiFeatureUtilities.ENABLE_LAUNCH_BUG_FIX.setForTesting(true);
// Restart Chrome and make sure both tab strip and IPH text bubble are showing.
mActivityTestRule.startMainActivityFromLauncher();
ChromeTabbedActivity cta = mActivityTestRule.getActivity();
CriteriaHelper.pollUiThread(cta.getTabModelSelector()::isTabStateInitialized);
waitForView(allOf(withId(R.id.tab_list_view), isDescendantOfA(withId(R.id.bottom_controls)),
isCompletelyDisplayed()));
CriteriaHelper.pollInstrumentationThread(() -> isTabStripIphShowing(cta));
// Show a dummy bottom sheet, and the IPH should be hidden.
final BottomSheetController bottomSheetController =
cta.getRootUiCoordinatorForTesting().getBottomSheetController();
final BottomSheetTestSupport bottomSheetTestSupport =
new BottomSheetTestSupport(bottomSheetController);
runOnUiThreadBlocking(() -> {
TestBottomSheetContent bottomSheetContent =
new TestBottomSheetContent(cta, BottomSheetContent.ContentPriority.HIGH, false);
bottomSheetController.requestShowContent(bottomSheetContent, false);
});
CriteriaHelper.pollUiThread(
() -> bottomSheetController.getSheetState() != SheetState.HIDDEN);
assertFalse(isTabStripIphShowing(cta));
// Hide the dummy bottom sheet, and the IPH should reshow.
runOnUiThreadBlocking(() -> bottomSheetTestSupport.setSheetState(SheetState.HIDDEN, false));
CriteriaHelper.pollUiThread(
() -> bottomSheetController.getSheetState() == SheetState.HIDDEN);
CriteriaHelper.pollInstrumentationThread(() -> isTabStripIphShowing(cta));
// When the IPH is clicked and dismissed, opening bottom sheet should never reshow it.
onView(withText(cta.getString(R.string.iph_tab_groups_tap_to_see_another_tab_text)))
.inRoot(withDecorView(not(cta.getWindow().getDecorView())))
.perform(click());
CriteriaHelper.pollInstrumentationThread(() -> !isTabStripIphShowing(cta));
runOnUiThreadBlocking(() -> {
TestBottomSheetContent bottomSheetContent =
new TestBottomSheetContent(cta, BottomSheetContent.ContentPriority.HIGH, false);
bottomSheetController.requestShowContent(bottomSheetContent, false);
});
CriteriaHelper.pollUiThread(
() -> bottomSheetController.getSheetState() != SheetState.HIDDEN);
assertFalse(isTabStripIphShowing(cta));
}
private boolean isTabStripIphShowing(ChromeTabbedActivity cta) {
String iphText = cta.getString(R.string.iph_tab_groups_tap_to_see_another_tab_text);
boolean isShowing = true;
try {
onView(withText(iphText))
.inRoot(withDecorView(not(cta.getWindow().getDecorView())))
.check(matches(isDisplayed()));
} catch (NoMatchingRootException e) {
isShowing = false;
} catch (Exception e) {
assert false : "error when inspecting IPH text bubble.";
}
return isShowing;
}
} }
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment