Skip to content

GitLab

T
tangled
Commit 3474d136 authored by Tommy Nyquist's avatar Tommy Nyquist Committed by Commit Bot
Browse files
Options

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/+/2518083Reviewed-by: default avatarTommy Nyquist <nyquist@chromium.org>
Commit-Queue: Tommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823643}
parent cdc6f38e
Hide whitespace changes
Inline Side-by-side
Showing with 5 additions and 138 deletions
chrome/android/features/tab_ui/java/src/org/chromium/chrome/browser/tasks/tab_groups/TabGroupUtils.java
View file @ 3474d136
......@@ -25,9 +25,6 @@ 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;
......@@ -44,8 +41,7 @@ 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,
@Nullable BottomSheetController bottomSheetController) {
public static void maybeShowIPH(@FeatureConstants String featureName, View view) {
// 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()
......@@ -91,37 +87,7 @@ public class TabGroupUtils {
TextBubble textBubble = new TextBubble(view.getContext(), view, textId, accessibilityTextId,
true, rectProvider, ChromeAccessibilityUtil.get().isAccessibilityEnabled());
textBubble.setDismissOnTouchInteraction(true);
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.addOnDismissListener(() -> tracker.dismissed(featureName));
textBubble.show();
}
......@@ -147,7 +113,7 @@ public class TabGroupUtils {
&& (transition & PageTransition.CORE_MASK)
== PageTransition.GENERATED)) {
maybeShowIPH(FeatureConstants.TAB_GROUPS_QUICKLY_COMPARE_PAGES_FEATURE,
tab.getView(), null);
tab.getView());
sTabModelSelectorTabObserver.destroy();
}
}
......
chrome/android/features/tab_ui/java/src/org/chromium/chrome/browser/tasks/tab_management/TabGroupUiCoordinator.java
View file @ 3474d136
......@@ -172,8 +172,7 @@ 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(),
TabUiFeatureUtilities.isLaunchBugFixEnabled() ? bottomSheetController : null);
mTabStripCoordinator.getContainerView());
}
mTabStripCoordinator.resetWithListOfTabs(tabs);
}
......
chrome/android/features/tab_ui/java/src/org/chromium/chrome/browser/tasks/tab_management/TabListMediator.java
View file @ 3474d136
......@@ -233,7 +233,7 @@ class TabListMediator {
()
-> TabGroupUtils.maybeShowIPH(
FeatureConstants.TAB_GROUPS_YOUR_TABS_ARE_TOGETHER_FEATURE,
anchor, null),
anchor),
IPH_DELAY_MS);
}
};
......
chrome/android/features/tab_ui/javatests/src/org/chromium/chrome/browser/tasks/tab_management/TabGroupUiTest.java
View file @ 3474d136
......@@ -7,20 +7,15 @@ 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;
......@@ -35,13 +30,10 @@ 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.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 androidx.recyclerview.widget.RecyclerView;
import androidx.test.espresso.NoMatchingRootException;
import androidx.test.filters.MediumTest;
import org.junit.Before;
......@@ -58,17 +50,12 @@ 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.CriteriaHelper;
import org.chromium.content_public.browser.test.util.TestThreadUtils;
import org.chromium.ui.test.util.UiRestriction;
......@@ -210,89 +197,4 @@ 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
@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