Commit 6eb1f2c1 authored by Yue Zhang's avatar Yue Zhang Committed by Commit Bot

Reland "Suppress TabStrip IPH when bottom sheet shows"

This reverts commit 3474d136.

Reason for revert: Try to fix the failed test.

Original change's description:
> Revert "Suppress TabStrip IPH when bottom sheet shows"
>
> This reverts commit fffc75e9.
>
> Reason for revert: New test consistently fails Marshmallow 64-bit Tester bot.
> See: https://ci.chromium.org/p/chromium/builders/ci/Marshmallow%2064%20bit%20Tester
>
> Original change's description:
> > 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/+/2506618
> > Reviewed-by: Matthew Jones <mdjones@chromium.org>
> > Reviewed-by: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
> > Commit-Queue: Yue Zhang <yuezhanggg@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#823416}
>
> TBR=mdjones@chromium.org,wychen@chromium.org,yuezhanggg@chromium.org
>
> Change-Id: I942224058529eb3e2eda72ca531f30d61d984724
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: 1135926
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2518083
> Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
> Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#823643}

TBR=nyquist@chromium.org,mdjones@chromium.org,wychen@chromium.org,yuezhanggg@chromium.org

# Not skipping CQ checks because this is a reland.

Bug: 1135926
Change-Id: I9dba86c6464f53ef76a8613af759258eee6f3931
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2518262Reviewed-by: default avatarYue Zhang <yuezhanggg@chromium.org>
Reviewed-by: default avatarMatthew Jones <mdjones@chromium.org>
Reviewed-by: default avatarTommy Nyquist <nyquist@chromium.org>
Reviewed-by: default avatarWei-Yin Chen (陳威尹) <wychen@chromium.org>
Commit-Queue: Yue Zhang <yuezhanggg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#824142}
parent 2643ba60
......@@ -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.util.ChromeAccessibilityUtil;
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.feature_engagement.FeatureConstants;
import org.chromium.components.feature_engagement.Tracker;
......@@ -41,7 +44,8 @@ public class TabGroupUtils {
private static TabModelSelectorTabObserver sTabModelSelectorTabObserver;
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
// below is TAB_GROUPS_TAP_TO_SEE_ANOTHER_TAB_FEATURE.
if (!TabUiFeatureUtilities.isTabGroupsAndroidEnabled()
......@@ -87,7 +91,37 @@ public class TabGroupUtils {
TextBubble textBubble = new TextBubble(view.getContext(), view, textId, accessibilityTextId,
true, rectProvider, ChromeAccessibilityUtil.get().isAccessibilityEnabled());
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();
}
......@@ -113,7 +147,7 @@ public class TabGroupUtils {
&& (transition & PageTransition.CORE_MASK)
== PageTransition.GENERATED)) {
maybeShowIPH(FeatureConstants.TAB_GROUPS_QUICKLY_COMPARE_PAGES_FEATURE,
tab.getView());
tab.getView(), null);
sTabModelSelectorTabObserver.destroy();
}
}
......
......@@ -172,7 +172,8 @@ public class TabGroupUiCoordinator implements TabGroupUiMediator.ResetHandler, T
&& bottomSheetController.getSheetState()
== BottomSheetController.SheetState.HIDDEN) {
TabGroupUtils.maybeShowIPH(FeatureConstants.TAB_GROUPS_TAP_TO_SEE_ANOTHER_TAB_FEATURE,
mTabStripCoordinator.getContainerView());
mTabStripCoordinator.getContainerView(),
TabUiFeatureUtilities.isLaunchBugFixEnabled() ? bottomSheetController : null);
}
mTabStripCoordinator.resetWithListOfTabs(tabs);
}
......
......@@ -233,7 +233,7 @@ class TabListMediator {
()
-> TabGroupUtils.maybeShowIPH(
FeatureConstants.TAB_GROUPS_YOUR_TABS_ARE_TOGETHER_FEATURE,
anchor),
anchor, null),
IPH_DELAY_MS);
}
};
......
......@@ -7,15 +7,20 @@ package org.chromium.chrome.browser.tasks.tab_management;
import static androidx.test.espresso.Espresso.onView;
import static androidx.test.espresso.action.ViewActions.click;
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.VISIBLE;
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.withEffectiveVisibility;
import static androidx.test.espresso.matcher.ViewMatchers.withId;
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.not;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.chromium.chrome.browser.flags.ChromeFeatureList.TAB_GRID_LAYOUT_ANDROID;
......@@ -30,10 +35,12 @@ 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.createThumbnailBitmapAndWriteToFile;
import static org.chromium.chrome.test.util.ViewUtils.waitForView;
import static org.chromium.content_public.browser.test.util.TestThreadUtils.runOnUiThreadBlocking;
import android.view.ViewGroup;
import androidx.recyclerview.widget.RecyclerView;
import androidx.test.espresso.NoMatchingRootException;
import androidx.test.filters.MediumTest;
import org.junit.Before;
......@@ -51,12 +58,17 @@ 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.tasks.pseudotab.TabAttributeCache;
import org.chromium.chrome.browser.widget.bottomsheet.TestBottomSheetContent;
import org.chromium.chrome.features.start_surface.StartSurfaceLayout;
import org.chromium.chrome.tab_ui.R;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.chrome.test.ChromeTabbedActivityTestRule;
import org.chromium.chrome.test.util.ChromeRenderTestRule;
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.TestThreadUtils;
import org.chromium.ui.test.util.UiRestriction;
......@@ -197,4 +209,89 @@ public class TabGroupUiTest {
onView(allOf(withId(R.id.tab_list_view), isDescendantOfA(withId(R.id.bottom_controls))))
.check(matches(withEffectiveVisibility((INVISIBLE))));
}
@Test
@MediumTest
// clang-format off
@CommandLineFlags.Add({
"enable-features=IPH_TabGroupsTapToSeeAnotherTab<TabGroupsTapToSeeAnotherTab",
"force-fieldtrials=TabGroupsTapToSeeAnotherTab/Enabled/",
"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"})
@DisabledTest(message = "https://crbug.com/1135926")
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