Skip to content

Commit

Permalink
[Journeys] Remove bottom sheet path
Browse files Browse the repository at this point in the history
The mocks no longer call for the bottom sheet to be shown as an
intermediate step when triggering journeys from the omnibox. This CL
removes the bottom sheet code path and associated strings, layouts etc.

Bug: 1303171
Change-Id: Ibc37cd9cf4231dc1315b6138fec50cfb3520b554
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3630630
Reviewed-by: Theresa Sullivan <twellington@chromium.org>
Reviewed-by: Tommy Li <tommycli@chromium.org>
Reviewed-by: Gang Wu <gangwu@chromium.org>
Reviewed-by: Tomasz Wiszkowski <ender@google.com>
Commit-Queue: Patrick Noland <pnoland@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1001891}
  • Loading branch information
Patrick Noland authored and Chromium LUCI CQ committed May 11, 2022
1 parent bb19d97 commit 5347863
Show file tree
Hide file tree
Showing 15 changed files with 37 additions and 381 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ private void executeNonPedalAction(OmniboxPedal omniboxPedal) {
assert omniboxPedal instanceof HistoryClustersAction;
String query = ((HistoryClustersAction) omniboxPedal).getQuery();
assert !TextUtils.isEmpty(query);
mHistoryClustersCoordinator.showBottomSheet(query);
mHistoryClustersCoordinator.openHistoryClustersUi(query);
}
break;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ public HistoryManager(@NonNull Activity activity, boolean isSeparateActivity,
ChromeFeatureList.isEnabled(ChromeFeatureList.HISTORY_JOURNEYS);
if (historyClustersEnabled) {
mHistoryClustersCoordinator = new HistoryClustersCoordinator(
Profile.getLastUsedRegularProfile(), activity, null, null, tabSupplier);
Profile.getLastUsedRegularProfile(), activity, null, tabSupplier);
if (!TextUtils.isEmpty(historyClustersQuery)) {
mHistoryClustersCoordinator.setQuery(historyClustersQuery);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -717,11 +717,10 @@ public void onFinishNativeInitialization() {

private void initHistoryClustersCoordinator(Profile profile) {
if (ChromeFeatureList.isEnabled(ChromeFeatureList.HISTORY_JOURNEYS)) {
mHistoryClustersCoordinator =
new HistoryClustersCoordinator(profile, mActivity, mBottomSheetController,
()
-> new Intent().setClass(mActivity, HistoryActivity.class),
mActivityTabProvider);
mHistoryClustersCoordinator = new HistoryClustersCoordinator(profile, mActivity,
()
-> new Intent().setClass(mActivity, HistoryActivity.class),
mActivityTabProvider);
mHistoryClustersCoordinatorSupplier.set(mHistoryClustersCoordinator);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.flags.ChromeSwitches;
import org.chromium.chrome.browser.history.HistoryActivity;
import org.chromium.chrome.browser.history_clusters.HistoryClustersBottomSheetContent;
import org.chromium.chrome.browser.omnibox.LocationBarLayout;
import org.chromium.chrome.browser.omnibox.OmniboxSuggestionType;
import org.chromium.chrome.browser.omnibox.action.OmniboxActionType;
Expand All @@ -56,8 +55,8 @@
import org.chromium.chrome.test.util.ActivityTestUtils;
import org.chromium.chrome.test.util.OmniboxTestUtils;
import org.chromium.chrome.test.util.OmniboxTestUtils.SuggestionInfo;
import org.chromium.chrome.test.util.browser.Features.EnableFeatures;
import org.chromium.components.browser_ui.accessibility.AccessibilitySettings;
import org.chromium.components.browser_ui.bottomsheet.BottomSheetController;
import org.chromium.components.browser_ui.site_settings.SiteSettings;
import org.chromium.components.embedder_support.util.UrlConstants;
import org.chromium.components.omnibox.AutocompleteMatch;
Expand Down Expand Up @@ -556,6 +555,7 @@ public void testShownPedalSuggestionInTop3() {

@Test
@MediumTest
@EnableFeatures({ChromeFeatureList.HISTORY_JOURNEYS})
public void testHistoryClustersAction() {
mOmniboxUtils.requestFocus();
List<AutocompleteMatch> suggestionsList = buildDummySuggestionsList(2, "Suggestion");
Expand All @@ -571,12 +571,16 @@ public void testHistoryClustersAction() {

clickOnPedal();

CriteriaHelper.pollUiThread(() -> {
BottomSheetController bottomSheetController = sActivityTestRule.getActivity()
.getRootUiCoordinatorForTesting()
.getBottomSheetController();
Criteria.checkThat(bottomSheetController.getCurrentSheetContent(),
Matchers.instanceOf(HistoryClustersBottomSheetContent.class));
});
if (DeviceFormFactor.isNonMultiDisplayContextOnTablet(sActivityTestRule.getActivity())) {
CriteriaHelper.pollUiThread(() -> {
Tab tab = sActivityTestRule.getActivity().getActivityTab();
Criteria.checkThat(tab, Matchers.notNullValue());
Criteria.checkThat(
tab.getUrl().getSpec(), Matchers.startsWith("chrome://history/journeys"));
});
} else {
ActivityTestUtils.waitForActivity(
InstrumentationRegistry.getInstrumentation(), HistoryActivity.class);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,8 @@ public void setUp() {

mActivityScenario = ActivityScenario.launch(ChromeTabbedActivity.class);
mActivityScenario.onActivity(activity -> {
mHistoryClustersCoordinator = new HistoryClustersCoordinator(mProfile, activity,
activity.getRootUiCoordinatorForTesting().getBottomSheetController(),
() -> mIntent, () -> mTab);
mHistoryClustersCoordinator =
new HistoryClustersCoordinator(mProfile, activity, () -> mIntent, () -> mTab);
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
import org.chromium.base.test.BaseRobolectricTestRunner;
import org.chromium.chrome.browser.history_clusters.HistoryClustersItemProperties.ItemType;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.components.browser_ui.bottomsheet.BottomSheetController;
import org.chromium.components.favicon.LargeIconBridge;
import org.chromium.content_public.browser.LoadUrlParams;
import org.chromium.ui.modelutil.MVCListAdapter.ListItem;
Expand Down Expand Up @@ -64,8 +63,6 @@ public class HistoryClustersMediatorTest {
@Mock
private GURL mGurl3;
@Mock
private BottomSheetController mBottomSheetController;
@Mock
private Tab mTab;

private ClusterVisit mVisit1;
Expand All @@ -76,8 +73,6 @@ public class HistoryClustersMediatorTest {
private HistoryClustersResult mHistoryClustersResult;
private ModelList mModelList;
private PropertyModel mToolbarModel;
private HistoryClustersBottomSheetContent mBottomSheetContent =
new HistoryClustersBottomSheetContent();
private Intent mIntent = new Intent();
private Supplier<Intent> mHistoryActivityIntentFactory = () -> mIntent;
private Supplier<Tab> mTabSupplier = () -> mTab;
Expand All @@ -88,11 +83,8 @@ public void setUp() {
doReturn(mResources).when(mContext).getResources();
mModelList = new ModelList();
mToolbarModel = new PropertyModel(HistoryClustersToolbarProperties.ALL_KEYS);
mBottomSheetContent = new HistoryClustersBottomSheetContent();
mMediator = new HistoryClustersMediator(mBridge, mLargeIconBridge, mContext, mResources,
mModelList, new PropertyModel(HistoryClustersBottomSheetToolbarProperties.ALL_KEYS),
mToolbarModel, mBottomSheetController, mBottomSheetContent,
mHistoryActivityIntentFactory, mTabSupplier);
mModelList, mToolbarModel, mHistoryActivityIntentFactory, mTabSupplier);
mVisit1 = new ClusterVisit(1.0F, mGurl1, "Title 1");
mVisit2 = new ClusterVisit(1.0F, mGurl2, "Title 1");
mVisit3 = new ClusterVisit(1.0F, mGurl3, "Title 1");
Expand Down Expand Up @@ -127,41 +119,17 @@ public void testQuery() {
HistoryClustersItemProperties.TITLE, HistoryClustersItemProperties.URL)));
}

@Test
public void testShowBottomSheet() {
Promise<HistoryClustersResult> promise = new Promise<>();
doReturn(promise).when(mBridge).queryClusters("foo");

mMediator.showBottomSheet("foo");
fulfillPromise(promise, mHistoryClustersResult);
verify(mBottomSheetController).requestShowContent(mBottomSheetContent, true);

assertEquals(mModelList.size(), 3);
}

@Test
public void testShowBottomSheet_emptyQuery() {
Promise<HistoryClustersResult> promise = new Promise<>();
doReturn(promise).when(mBridge).queryClusters("");

mMediator.showBottomSheet("");
fulfillPromise(promise, mHistoryClustersResult);

verify(mBottomSheetController).requestShowContent(mBottomSheetContent, true);
assertEquals(mModelList.size(), 3);
}

@Test
public void testOpenInFullPageTablet() {
doReturn(2).when(mResources).getInteger(R.integer.min_screen_width_bucket);
mMediator.openHistoryClustersInFullPage("pandas");
mMediator.openHistoryClustersUi("pandas");
verify(mTab).loadUrl(argThat(hasSameUrl("chrome://history/journeys?q=pandas")));
}

@Test
public void testOpenInFullPagePhone() {
doReturn(1).when(mResources).getInteger(R.integer.min_screen_width_bucket);
mMediator.openHistoryClustersInFullPage("pandas");
mMediator.openHistoryClustersUi("pandas");

verify(mContext).startActivity(mIntent);
assertTrue(IntentUtils.safeGetBooleanExtra(
Expand Down
5 changes: 0 additions & 5 deletions chrome/browser/history_clusters/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ android_resources("java_resources") {
"java/res/drawable/ic_journeys.xml",
"java/res/layout/history_cluster_visit.xml",
"java/res/layout/history_clusters_activity_content.xml",
"java/res/layout/history_clusters_bottom_sheet_content.xml",
"java/res/layout/history_clusters_bottom_sheet_toolbar.xml",
"java/res/layout/history_clusters_toolbar.xml",
"java/res/menu/history_clusters_menu.xml",
]
Expand All @@ -23,8 +21,6 @@ android_library("java") {
sources = [
"java/src/org/chromium/chrome/browser/history_clusters/ClusterVisit.java",
"java/src/org/chromium/chrome/browser/history_clusters/HistoryCluster.java",
"java/src/org/chromium/chrome/browser/history_clusters/HistoryClustersBottomSheetContent.java",
"java/src/org/chromium/chrome/browser/history_clusters/HistoryClustersBottomSheetToolbarProperties.java",
"java/src/org/chromium/chrome/browser/history_clusters/HistoryClustersBridge.java",
"java/src/org/chromium/chrome/browser/history_clusters/HistoryClustersConstants.java",
"java/src/org/chromium/chrome/browser/history_clusters/HistoryClustersCoordinator.java",
Expand All @@ -47,7 +43,6 @@ android_library("java") {
"//chrome/browser/tab:java",
"//chrome/browser/ui/android/favicon:java",
"//chrome/browser/ui/android/strings:ui_strings_grd",
"//components/browser_ui/bottomsheet/android:java",
"//components/browser_ui/widget/android:java",
"//components/embedder_support/android:util_java",
"//components/favicon/android:java",
Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

0 comments on commit 5347863

Please sign in to comment.