Skip to content

Commit

Permalink
[StartSurface] Add a trendy-terms variation
Browse files Browse the repository at this point in the history 
Trendy-terms is like omnibox-only, with the only difference that there
is a horizontal RecyclerView below the omnibox that contains clickable
trendy terms. Note that since this component is not yet hooked with
data source, the trendy-terms variation looks the same as omnibox-only
right now. This CL only introduces the components to contain the
trendy terms that come later.

Bug: 1099037
Change-Id: I579589123efc275d62d14f0c202f575d2609ce6f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2265236
Reviewed-by: Theresa  <twellington@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Commit-Queue: Yue Zhang <yuezhanggg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#782662}
  • Loading branch information
Yue Zhang authored and Commit Bot committed Jun 25, 2020
1 parent fba12cb commit a7dd8f3
Show file tree
Hide file tree
Showing 25 changed files with 531 additions and 30 deletions.
21 changes: 14 additions & 7 deletions ...internal/java/src/org/chromium/chrome/features/start_surface/StartSurfaceCoordinator.java
View file
Expand Up @@ -95,13 +95,15 @@ public StartSurfaceCoordinator(ChromeActivity activity, ScrimCoordinator scrimCo

boolean excludeMVTiles = StartSurfaceConfiguration.START_SURFACE_EXCLUDE_MV_TILES.getValue()
|| mSurfaceMode == SurfaceMode.OMNIBOX_ONLY
|| mSurfaceMode == SurfaceMode.TRENDY_TERMS
|| mSurfaceMode == SurfaceMode.NO_START_SURFACE;
boolean hasTrendyTerms = mSurfaceMode == SurfaceMode.TRENDY_TERMS;
if (mSurfaceMode == SurfaceMode.NO_START_SURFACE) {
// Create Tab switcher directly to save one layer in the view hierarchy.
mTabSwitcher = TabManagementModuleProvider.getDelegate().createGridTabSwitcher(
mActivity, mActivity.getCompositorViewHolder(), scrimCoordinator);
} else {
createAndSetStartSurface(excludeMVTiles);
createAndSetStartSurface(excludeMVTiles, hasTrendyTerms);
}

TabSwitcher.Controller controller =
Expand Down Expand Up @@ -278,14 +280,16 @@ public boolean isSecondaryTaskInitPendingForTesting() {

if (feature.equals("omniboxonly")) return SurfaceMode.OMNIBOX_ONLY;

if (feature.equals("trendyterms")) return SurfaceMode.TRENDY_TERMS;

// Default to SurfaceMode.TASKS_ONLY. This could happen when the start surface has been
// changed from enabled to disabled in native side, but the cached flag has not been updated
// yet, so StartSurfaceConfiguration.isStartSurfaceEnabled() above returns true.
// TODO(crbug.com/1016548): Remember the last surface mode so as to default to it.
return SurfaceMode.TASKS_ONLY;
}

private void createAndSetStartSurface(boolean excludeMVTiles) {
private void createAndSetStartSurface(boolean excludeMVTiles, boolean hasTrendyTerms) {
ArrayList<PropertyKey> allProperties =
new ArrayList<>(Arrays.asList(TasksSurfaceProperties.ALL_KEYS));
allProperties.addAll(Arrays.asList(StartSurfaceProperties.ALL_KEYS));
Expand All @@ -296,8 +300,9 @@ private void createAndSetStartSurface(boolean excludeMVTiles) {
if (StartSurfaceConfiguration.START_SURFACE_LAST_ACTIVE_TAB_ONLY.getValue()) {
tabSwitcherType = TabSwitcherType.SINGLE;
}
mTasksSurface = TabManagementModuleProvider.getDelegate().createTasksSurface(
mActivity, mScrimCoordinator, mPropertyModel, tabSwitcherType, !excludeMVTiles);
mTasksSurface = TabManagementModuleProvider.getDelegate().createTasksSurface(mActivity,
mScrimCoordinator, mPropertyModel, tabSwitcherType, !excludeMVTiles,
hasTrendyTerms);
mTasksSurface.getView().setId(R.id.primary_tasks_surface_view);

mTasksSurfacePropertyModelChangeProcessor =
Expand All @@ -306,8 +311,10 @@ private void createAndSetStartSurface(boolean excludeMVTiles) {
mActivity.getCompositorViewHolder(), mTasksSurface.getView()),
TasksSurfaceViewBinder::bind);

// There is nothing else to do for SurfaceMode.TASKS_ONLY and SurfaceMode.OMNIBOX_ONLY.
if (mSurfaceMode == SurfaceMode.TASKS_ONLY || mSurfaceMode == SurfaceMode.OMNIBOX_ONLY) {
// There is nothing else to do for SurfaceMode.TASKS_ONLY, SurfaceMode.OMNIBOX_ONLY and
// SurfaceMode.TRENDY_TERMS.
if (mSurfaceMode == SurfaceMode.TASKS_ONLY || mSurfaceMode == SurfaceMode.OMNIBOX_ONLY
|| mSurfaceMode == SurfaceMode.TRENDY_TERMS) {
return;
}

Expand All @@ -328,7 +335,7 @@ private TabSwitcher.Controller initializeSecondaryTasksSurface() {
StartSurfaceConfiguration.isStartSurfaceStackTabSwitcherEnabled()
? TabSwitcherType.NONE
: TabSwitcherType.GRID,
false);
false, false);
if (mIsInitializedWithNative) {
mSecondaryTasksSurface.onFinishNativeInitialization(
mActivity, mActivity.getToolbarManager().getFakeboxDelegate());
Expand Down
32 changes: 27 additions & 5 deletions ...ce/internal/java/src/org/chromium/chrome/features/start_surface/StartSurfaceMediator.java
View file
Expand Up @@ -16,6 +16,7 @@
import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.MV_TILES_VISIBLE;
import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.TAB_SWITCHER_TITLE_TOP_MARGIN;
import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.TASKS_SURFACE_BODY_TOP_MARGIN;
import static org.chromium.chrome.browser.tasks.TasksSurfaceProperties.TRENDY_TERMS_VISIBLE;
import static org.chromium.chrome.features.start_surface.StartSurfaceProperties.BOTTOM_BAR_CLICKLISTENER;
import static org.chromium.chrome.features.start_surface.StartSurfaceProperties.BOTTOM_BAR_HEIGHT;
import static org.chromium.chrome.features.start_surface.StartSurfaceProperties.BOTTOM_BAR_SELECTED_TAB_POSITION;
Expand Down Expand Up @@ -66,14 +67,15 @@
class StartSurfaceMediator
implements StartSurface.Controller, TabSwitcher.OverviewModeObserver, View.OnClickListener {
@IntDef({SurfaceMode.NO_START_SURFACE, SurfaceMode.TASKS_ONLY, SurfaceMode.TWO_PANES,
SurfaceMode.SINGLE_PANE, SurfaceMode.OMNIBOX_ONLY})
SurfaceMode.SINGLE_PANE, SurfaceMode.OMNIBOX_ONLY, SurfaceMode.TRENDY_TERMS})
@Retention(RetentionPolicy.SOURCE)
@interface SurfaceMode {
int NO_START_SURFACE = 0;
int TASKS_ONLY = 1;
int TWO_PANES = 2;
int SINGLE_PANE = 3;
int OMNIBOX_ONLY = 4;
int TRENDY_TERMS = 5;
}

/** Interface to initialize a secondary tasks surface for more tabs. */
Expand Down Expand Up @@ -157,7 +159,8 @@ interface ActivityStateChecker {
if (mPropertyModel != null) {
assert mSurfaceMode == SurfaceMode.SINGLE_PANE || mSurfaceMode == SurfaceMode.TWO_PANES
|| mSurfaceMode == SurfaceMode.TASKS_ONLY
|| mSurfaceMode == SurfaceMode.OMNIBOX_ONLY;
|| mSurfaceMode == SurfaceMode.OMNIBOX_ONLY
|| mSurfaceMode == SurfaceMode.TRENDY_TERMS;

mIsIncognito = mTabModelSelector.isIncognitoSelected();

Expand Down Expand Up @@ -261,8 +264,10 @@ public void onUrlFocusChange(boolean hasFocus) {
}
};

// Only tweak the margins between sections for non-OMNIBOX_ONLY variations.
if (surfaceMode != SurfaceMode.OMNIBOX_ONLY) {
// Only tweak the margins between sections for non-OMNIBOX_ONLY and non-TRENDY_TERMS
// variations.
if (surfaceMode != SurfaceMode.OMNIBOX_ONLY
&& surfaceMode != SurfaceMode.TRENDY_TERMS) {
Resources resources = ContextUtils.getApplicationContext().getResources();
mPropertyModel.set(TASKS_SURFACE_BODY_TOP_MARGIN,
resources.getDimensionPixelSize(R.dimen.tasks_surface_body_top_margin));
Expand Down Expand Up @@ -372,6 +377,8 @@ public void setOverviewState(@OverviewModeState int state) {
RecordUserAction.record("StartSurface.TasksOnly");
} else if (mOverviewModeState == OverviewModeState.SHOWN_TABSWITCHER_OMNIBOX_ONLY) {
RecordUserAction.record("StartSurface.OmniboxOnly");
} else if (mOverviewModeState == OverviewModeState.SHOWN_TABSWITCHER_TRENDY_TERMS) {
RecordUserAction.record("StartSurface.TrendyTerms");
}
}

Expand Down Expand Up @@ -431,6 +438,11 @@ private void setOverviewStateInternal() {
setMVTilesVisibility(false);
setExploreSurfaceVisibility(false);
setFakeBoxVisibility(true);
} else if (mOverviewModeState == OverviewModeState.SHOWN_TABSWITCHER_TRENDY_TERMS) {
setMVTilesVisibility(false);
setExploreSurfaceVisibility(false);
setFakeBoxVisibility(true);
setTrendyTermsVisibility(true);
} else if (mOverviewModeState == OverviewModeState.NOT_SHOWN) {
if (mSecondaryTasksSurfacePropertyModel != null) {
setSecondaryTasksSurfaceVisibility(false);
Expand Down Expand Up @@ -696,6 +708,7 @@ private boolean hasFakeSearchBox() {
if (mOverviewModeState == OverviewModeState.SHOWN_HOMEPAGE
|| mOverviewModeState == OverviewModeState.SHOWN_TABSWITCHER_TASKS_ONLY
|| mOverviewModeState == OverviewModeState.SHOWN_TABSWITCHER_OMNIBOX_ONLY
|| mOverviewModeState == OverviewModeState.SHOWN_TABSWITCHER_TRENDY_TERMS
|| (mOverviewModeState == OverviewModeState.SHOWN_TABSWITCHER_TWO_PANES
&& !mPropertyModel.get(IS_EXPLORE_SURFACE_VISIBLE))) {
return true;
Expand Down Expand Up @@ -737,6 +750,11 @@ private void setMVTilesVisibility(boolean isVisible) {
mPropertyModel.set(MV_TILES_VISIBLE, isVisible);
}

private void setTrendyTermsVisibility(boolean isVisible) {
if (isVisible == mPropertyModel.get(TRENDY_TERMS_VISIBLE)) return;
mPropertyModel.set(TRENDY_TERMS_VISIBLE, isVisible);
}

private void setFakeBoxVisibility(boolean isVisible) {
if (mPropertyModel == null) return;
mPropertyModel.set(IS_FAKE_SEARCH_BOX_VISIBLE, isVisible);
Expand Down Expand Up @@ -806,6 +824,9 @@ private int computeOverviewStateShown() {
if (mSurfaceMode == SurfaceMode.OMNIBOX_ONLY) {
return OverviewModeState.SHOWN_TABSWITCHER_OMNIBOX_ONLY;
}
if (mSurfaceMode == SurfaceMode.TRENDY_TERMS) {
return OverviewModeState.SHOWN_TABSWITCHER_TRENDY_TERMS;
}
return OverviewModeState.DISABLED;
}

Expand All @@ -814,6 +835,7 @@ private boolean isShownState(@OverviewModeState int state) {
|| state == OverviewModeState.SHOWN_TABSWITCHER
|| state == OverviewModeState.SHOWN_TABSWITCHER_TWO_PANES
|| state == OverviewModeState.SHOWN_TABSWITCHER_TASKS_ONLY
|| state == OverviewModeState.SHOWN_TABSWITCHER_OMNIBOX_ONLY;
|| state == OverviewModeState.SHOWN_TABSWITCHER_OMNIBOX_ONLY
|| state == OverviewModeState.SHOWN_TABSWITCHER_TRENDY_TERMS;
}
}
78 changes: 74 additions & 4 deletions ...e/internal/javatests/src/org/chromium/chrome/features/start_surface/StartSurfaceTest.java
View file
Expand Up @@ -13,6 +13,7 @@
import static androidx.test.espresso.action.ViewActions.replaceText;
import static androidx.test.espresso.assertion.ViewAssertions.matches;
import static androidx.test.espresso.matcher.ViewMatchers.Visibility.GONE;
import static androidx.test.espresso.matcher.ViewMatchers.Visibility.VISIBLE;
import static androidx.test.espresso.matcher.ViewMatchers.isDisplayed;
import static androidx.test.espresso.matcher.ViewMatchers.withEffectiveVisibility;
import static androidx.test.espresso.matcher.ViewMatchers.withId;
Expand Down Expand Up @@ -200,18 +201,20 @@ public void setUp() throws IOException {
@DisabledTest(message = "crbug.com/1084176")
@CommandLineFlags.Add({BASE_PARAMS + "/tasksonly"})
public void testShow_TasksOnly() {
final ChromeTabbedActivity cta = mActivityTestRule.getActivity();
if (!mImmediateReturn) {
TabUiTestHelper.enterTabSwitcher(mActivityTestRule.getActivity());
TabUiTestHelper.enterTabSwitcher(cta);
}
onViewWaiting(allOf(withId(R.id.primary_tasks_surface_view), isDisplayed()));
onView(withId(org.chromium.chrome.tab_ui.R.id.mv_tiles_container))
.check(matches(isDisplayed()));
onView(withId(org.chromium.chrome.tab_ui.R.id.tasks_surface_body))
.check(matches(isDisplayed()));
onView(withId(org.chromium.chrome.tab_ui.R.id.trendy_terms_recycler_view))
.check(matches(withEffectiveVisibility(GONE)));

TestThreadUtils.runOnUiThreadBlocking(
() -> mActivityTestRule.getActivity().getLayoutManager().hideOverview(false));
assertFalse(mActivityTestRule.getActivity().getLayoutManager().overviewVisible());
TestThreadUtils.runOnUiThreadBlocking(() -> cta.getLayoutManager().hideOverview(false));
assertFalse(cta.getLayoutManager().overviewVisible());
}

@Test
Expand All @@ -232,6 +235,63 @@ public void testShow_OmniboxOnly() {
.check(matches(withEffectiveVisibility(GONE)));
onView(withId(org.chromium.chrome.tab_ui.R.id.tasks_surface_body))
.check(matches(isDisplayed()));
onView(withId(org.chromium.chrome.tab_ui.R.id.trendy_terms_recycler_view))
.check(matches(withEffectiveVisibility(GONE)));

if (!isInstantReturn()) {
// TODO(crbug.com/1076274): fix toolbar to make incognito switch part of the view.
onView(withId(org.chromium.chrome.tab_ui.R.id.incognito_switch))
.check(matches(withEffectiveVisibility(GONE)));
}
mActivityTestRule.waitForActivityNativeInitializationComplete();

TabUiTestHelper.createTabs(cta, true, 1);
TabUiTestHelper.verifyTabModelTabCount(cta, 1, 1);
if (isInstantReturn()) {
// TODO(crbug.com/1076274): fix toolbar to avoid wrongly focusing on the toolbar
// omnibox.
return;
}
TabUiTestHelper.enterTabSwitcher(cta);
if (!isInstantReturn()) {
// TODO(crbug.com/1076274): fix toolbar to make incognito switch part of the view.
onView(withId(org.chromium.chrome.tab_ui.R.id.incognito_switch))
.check(matches(isDisplayed()));
}
TestThreadUtils.runOnUiThreadBlocking(
() -> cta.getTabModelSelector().getModel(true).closeAllTabs());
TabUiTestHelper.verifyTabModelTabCount(cta, 1, 0);
assertTrue(mActivityTestRule.getActivity().getLayoutManager().overviewVisible());
if (!isInstantReturn()) {
// TODO(crbug.com/1076274): fix toolbar to make incognito switch part of the view.
onView(withId(org.chromium.chrome.tab_ui.R.id.incognito_switch))
.check(matches(withEffectiveVisibility(GONE)));
}

TestThreadUtils.runOnUiThreadBlocking(
() -> mActivityTestRule.getActivity().getLayoutManager().hideOverview(false));
assertFalse(mActivityTestRule.getActivity().getLayoutManager().overviewVisible());
}

@Test
@MediumTest
@Feature({"StartSurface"})
// clang-format off
@CommandLineFlags.Add({BASE_PARAMS + "/trendyterms" +
"/hide_switch_when_no_incognito_tabs/true"})
public void testShow_TrendyTerms() {
// clang-format on
final ChromeTabbedActivity cta = mActivityTestRule.getActivity();
if (!mImmediateReturn) {
TabUiTestHelper.enterTabSwitcher(cta);
}
onViewWaiting(allOf(withId(R.id.primary_tasks_surface_view), isDisplayed()));
onView(withId(org.chromium.chrome.tab_ui.R.id.mv_tiles_container))
.check(matches(withEffectiveVisibility(GONE)));
onView(withId(org.chromium.chrome.tab_ui.R.id.tasks_surface_body))
.check(matches(isDisplayed()));
onView(withId(org.chromium.chrome.tab_ui.R.id.trendy_terms_recycler_view))
.check(matches(withEffectiveVisibility(VISIBLE)));

if (!isInstantReturn()) {
// TODO(crbug.com/1076274): fix toolbar to make incognito switch part of the view.
Expand Down Expand Up @@ -284,6 +344,8 @@ public void testShow_TwoPanes() {
.check(matches(isDisplayed()));
onView(withId(org.chromium.chrome.tab_ui.R.id.tasks_surface_body))
.check(matches(isDisplayed()));
onView(withId(org.chromium.chrome.tab_ui.R.id.trendy_terms_recycler_view))
.check(matches(withEffectiveVisibility(GONE)));

onView(withId(R.id.ss_explore_tab)).perform(click());
onViewWaiting(withId(R.id.start_surface_explore_view));
Expand Down Expand Up @@ -319,6 +381,8 @@ public void testShow_SingleAsHomepage() {
.check(matches(isDisplayed()));
onView(withId(org.chromium.chrome.tab_ui.R.id.tasks_surface_body))
.check(matches(isDisplayed()));
onView(withId(org.chromium.chrome.tab_ui.R.id.trendy_terms_recycler_view))
.check(matches(withEffectiveVisibility(GONE)));

// Note that onView(R.id.more_tabs).perform(click()) can not be used since it requires 90
// percent of the view's area is displayed to the users. However, this view has negative
Expand Down Expand Up @@ -378,6 +442,8 @@ public void testShow_SingleAsHomepage_NoMVTiles() {
.check(matches(isDisplayed()));
onView(withId(org.chromium.chrome.tab_ui.R.id.tasks_surface_body))
.check(matches(isDisplayed()));
onView(withId(org.chromium.chrome.tab_ui.R.id.trendy_terms_recycler_view))
.check(matches(withEffectiveVisibility(GONE)));

if (!isInstantReturn()) {
// TODO(crbug.com/1076274): fix toolbar to make incognito switch part of the view.
Expand Down Expand Up @@ -445,6 +511,8 @@ public void testShow_SingleAsHomepage_SingleTabNoMVTiles() {
.check(matches(isDisplayed()));
onView(withId(org.chromium.chrome.tab_ui.R.id.tasks_surface_body))
.check(matches(isDisplayed()));
onView(withId(org.chromium.chrome.tab_ui.R.id.trendy_terms_recycler_view))
.check(matches(withEffectiveVisibility(GONE)));

if (!isInstantReturn()) {
// TODO(crbug.com/1076274): fix toolbar to make incognito switch part of the view.
Expand Down Expand Up @@ -511,6 +579,8 @@ public void testShow_SingleAsHomepage_V2() {
.check(matches(isDisplayed()));
onView(withId(org.chromium.chrome.tab_ui.R.id.tasks_surface_body))
.check(matches(isDisplayed()));
onView(withId(org.chromium.chrome.tab_ui.R.id.trendy_terms_recycler_view))
.check(matches(withEffectiveVisibility(GONE)));

onView(withId(org.chromium.chrome.tab_ui.R.id.incognito_switch))
.check(matches(withEffectiveVisibility(GONE)));
Expand Down

0 comments on commit a7dd8f3

Please sign in to comment.