Skip to content

Commit

Permalink
[Tablet GTS]Compute and set tab thumbnail aspect ratio
Browse files Browse the repository at this point in the history 
Demo: http://shortn/_KJ391lp4JU

(cherry picked from commit a364347)

Bug: 1298973
Change-Id: Ib16bec7e18f85ba94a983dcc20fe6067b33fac82
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3526493
Reviewed-by: Neil Coronado <nemco@google.com>
Reviewed-by: Theresa Sullivan <twellington@chromium.org>
Commit-Queue: Sirisha Kavuluru <skavuluru@google.com>
Cr-Original-Commit-Position: refs/heads/main@{#982336}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3540703
Auto-Submit: Sirisha Kavuluru <skavuluru@google.com>
Commit-Queue: Theresa Sullivan <twellington@chromium.org>
Cr-Commit-Position: refs/branch-heads/4896@{#746}
Cr-Branched-From: 1f63ff4-refs/heads/main@{#972766}
  • Loading branch information
Sirisha Kavuluru authored and Chromium LUCI CQ committed Mar 21, 2022
1 parent d82b609 commit 87a7a72
Show file tree
Hide file tree
Showing 8 changed files with 114 additions and 24 deletions.
2 changes: 0 additions & 2 deletions chrome/android/features/tab_ui/java/res/layout/tab_grid_card_item.xml
View file
Expand Up @@ -46,8 +46,6 @@
android:layout_height="wrap_content"
android:layout_below="@id/tab_title"
android:gravity="center_horizontal"
android:adjustViewBounds="true"
android:scaleType="fitCenter"
android:importantForAccessibility="no"
android:src="@color/thumbnail_placeholder_on_primary_bg"
style="?attr/tabGridThumbnailStyle"/>
Expand Down
17 changes: 17 additions & 0 deletions ...s/tab_ui/java/src/org/chromium/chrome/browser/tasks/tab_management/TabGridViewBinder.java
View file
Expand Up @@ -15,6 +15,7 @@
import android.view.View;
import android.view.ViewGroup;
import android.widget.ImageView;
import android.widget.ImageView.ScaleType;
import android.widget.TextView;

import androidx.annotation.Nullable;
Expand All @@ -28,6 +29,7 @@
import org.chromium.base.Callback;
import org.chromium.chrome.tab_ui.R;
import org.chromium.components.browser_ui.widget.chips.ChipView;
import org.chromium.ui.base.DeviceFormFactor;
import org.chromium.ui.modelutil.PropertyKey;
import org.chromium.ui.modelutil.PropertyModel;
import org.chromium.ui.widget.ButtonCompat;
Expand Down Expand Up @@ -138,6 +140,14 @@ private static void bindCommonProperties(PropertyModel model, ViewLookupCachingF
updateThumbnail(view, model);
} else if (TabProperties.CONTENT_DESCRIPTION_STRING == propertyKey) {
view.setContentDescription(model.get(TabProperties.CONTENT_DESCRIPTION_STRING));
} else if (TabProperties.GRID_CARD_HEIGHT == propertyKey) {
view.setMinimumHeight(model.get(TabProperties.GRID_CARD_HEIGHT));
view.getLayoutParams().height = model.get(TabProperties.GRID_CARD_HEIGHT);
view.setLayoutParams(view.getLayoutParams());
} else if (TabProperties.GRID_CARD_WIDTH == propertyKey) {
view.setMinimumWidth(model.get(TabProperties.GRID_CARD_WIDTH));
view.getLayoutParams().width = model.get(TabProperties.GRID_CARD_WIDTH);
view.setLayoutParams(view.getLayoutParams());
}
}

Expand Down Expand Up @@ -326,6 +336,13 @@ private static void updateThumbnail(ViewLookupCachingFrameLayout view, PropertyM
TabGridThumbnailView thumbnail =
(TabGridThumbnailView) view.fastFindViewById(R.id.tab_thumbnail);
thumbnail.maybeAdjustThumbnailHeight();
if (DeviceFormFactor.isNonMultiDisplayContextOnTablet(view.getContext())
&& TabUiFeatureUtilities.isGridTabSwitcherEnabled(view.getContext())) {
thumbnail.setScaleType(ScaleType.CENTER_CROP);
} else {
thumbnail.setScaleType(ScaleType.FIT_CENTER);
thumbnail.setAdjustViewBounds(true);
}
if (fetcher == null) {
thumbnail.setImageDrawable(null);
return;
Expand Down
26 changes: 23 additions & 3 deletions .../tab_ui/java/src/org/chromium/chrome/browser/tasks/tab_management/TabListCoordinator.java
View file
Expand Up @@ -14,6 +14,7 @@
import android.view.ViewGroup;
import android.view.ViewTreeObserver;
import android.widget.ImageView;
import android.widget.ImageView.ScaleType;

import androidx.annotation.IntDef;
import androidx.annotation.NonNull;
Expand All @@ -33,6 +34,7 @@
import org.chromium.chrome.browser.tasks.tab_management.TabProperties.UiType;
import org.chromium.chrome.tab_ui.R;
import org.chromium.ui.base.DeviceFormFactor;
import org.chromium.ui.base.ViewUtils;
import org.chromium.ui.modelutil.MVCListAdapter;
import org.chromium.ui.modelutil.PropertyKey;
import org.chromium.ui.modelutil.PropertyModel;
Expand Down Expand Up @@ -159,6 +161,13 @@ public class TabListCoordinator
ViewLookupCachingFrameLayout root = (ViewLookupCachingFrameLayout) holder.itemView;
ImageView thumbnail = (ImageView) root.fastFindViewById(R.id.tab_thumbnail);
if (thumbnail == null) return;
if (DeviceFormFactor.isNonMultiDisplayContextOnTablet(context)
&& TabUiFeatureUtilities.isGridTabSwitcherEnabled(context)) {
thumbnail.setScaleType(ScaleType.CENTER_CROP);
} else {
thumbnail.setScaleType(ScaleType.FIT_CENTER);
thumbnail.setAdjustViewBounds(true);
}

if (TabUiFeatureUtilities.isLaunchPolishEnabled()) {
thumbnail.setImageDrawable(null);
Expand Down Expand Up @@ -310,13 +319,24 @@ boolean updateThumbnailLocation() {

private void updateThumbnailAndSpanCount() {
updateThumbnailLocation();
// Resetting span count for tablets.
if (mMode == TabListMode.GRID && DeviceFormFactor.isNonMultiDisplayContextOnTablet(mContext)
&& TabUiFeatureUtilities.isGridTabSwitcherEnabled(mContext)) {
mMediator.updateSpanCount(
(GridLayoutManager) mRecyclerView.getLayoutManager(),
// Determine and set span count
final GridLayoutManager layoutManager =
(GridLayoutManager) mRecyclerView.getLayoutManager();
mMediator.updateSpanCount(layoutManager,
mContext.getResources().getConfiguration().orientation,
mContext.getResources().getConfiguration().screenWidthDp);

final int screenWidthPx = ViewUtils.dpToPx(
mContext, mContext.getResources().getConfiguration().screenWidthDp);
int itemWidthPx = (screenWidthPx / layoutManager.getSpanCount());
int itemHeightPx =
((int) ((itemWidthPx * 1f) / TabUtils.getTabThumbnailAspectRatio(mContext)));
for (int i = 0; i < mModel.size(); i++) {
mModel.get(i).model.set(TabProperties.GRID_CARD_WIDTH, itemWidthPx);
mModel.get(i).model.set(TabProperties.GRID_CARD_HEIGHT, itemHeightPx);
}
}
}

Expand Down
4 changes: 3 additions & 1 deletion ...res/tab_ui/java/src/org/chromium/chrome/browser/tasks/tab_management/TabListMediator.java
View file
Expand Up @@ -1316,11 +1316,13 @@ public int getSpanSize(int position) {
private int getSpanCount(int orientation, int screenWidthDp) {
if (DeviceFormFactor.isNonMultiDisplayContextOnTablet(mContext)
&& TabUiFeatureUtilities.isGridTabSwitcherEnabled(mContext)) {
return screenWidthDp < TabListCoordinator.MAX_SCREEN_WIDTH_COMPACT_DP
int spanCount = screenWidthDp < TabListCoordinator.MAX_SCREEN_WIDTH_COMPACT_DP
? TabListCoordinator.GRID_LAYOUT_SPAN_COUNT_COMPACT
: screenWidthDp < TabListCoordinator.MAX_SCREEN_WIDTH_MEDIUM_DP
? TabListCoordinator.GRID_LAYOUT_SPAN_COUNT_MEDIUM
: TabListCoordinator.GRID_LAYOUT_SPAN_COUNT_LARGE;
if (orientation == Configuration.ORIENTATION_PORTRAIT || mModel.size() == 0) return spanCount;
return mModel.size() == 1 ? 2 : Math.min(spanCount, mModel.size());
}
return orientation == Configuration.ORIENTATION_PORTRAIT
|| MultiWindowUtils.getInstance().isInMultiWindowMode((Activity) mContext)
Expand Down
7 changes: 6 additions & 1 deletion ...tures/tab_ui/java/src/org/chromium/chrome/browser/tasks/tab_management/TabProperties.java
View file
Expand Up @@ -16,6 +16,7 @@
import org.chromium.ui.modelutil.PropertyKey;
import org.chromium.ui.modelutil.PropertyModel;
import org.chromium.ui.modelutil.PropertyModel.WritableBooleanPropertyKey;
import org.chromium.ui.modelutil.PropertyModel.WritableIntPropertyKey;
import org.chromium.ui.modelutil.PropertyModel.WritableObjectPropertyKey;

import java.lang.annotation.Retention;
Expand Down Expand Up @@ -54,6 +55,10 @@ public class TabProperties {
public static final WritableObjectPropertyKey<TabListMediator.ThumbnailFetcher>
THUMBNAIL_FETCHER = new WritableObjectPropertyKey<>(true);

public static final WritableIntPropertyKey GRID_CARD_WIDTH = new WritableIntPropertyKey();

public static final WritableIntPropertyKey GRID_CARD_HEIGHT = new WritableIntPropertyKey();

public static final WritableObjectPropertyKey<TabListMediator.IphProvider> IPH_PROVIDER =
new WritableObjectPropertyKey<>();

Expand Down Expand Up @@ -133,7 +138,7 @@ public class TabProperties {
SEARCH_QUERY, PAGE_INFO_LISTENER, PAGE_INFO_ICON_DRAWABLE_ID, CARD_TYPE,
CONTENT_DESCRIPTION_STRING, CLOSE_BUTTON_DESCRIPTION_STRING,
SHOPPING_PERSISTED_TAB_DATA_FETCHER, STORE_PERSISTED_TAB_DATA_FETCHER,
SHOULD_SHOW_PRICE_DROP_TOOLTIP};
SHOULD_SHOW_PRICE_DROP_TOOLTIP, GRID_CARD_WIDTH, GRID_CARD_HEIGHT};

public static final PropertyKey[] ALL_KEYS_TAB_STRIP =
new PropertyKey[] {TAB_ID, TAB_SELECTED_LISTENER, TAB_CLOSED_LISTENER, FAVICON,
Expand Down
11 changes: 9 additions & 2 deletions ...e/android/features/tab_ui/junit/src/org/chromium/chrome/browser/tab/TabUtilsUnitTest.java
View file
Expand Up @@ -5,6 +5,9 @@

import static com.google.common.truth.Truth.assertThat;

import android.content.Context;
import android.content.res.Configuration;

import org.junit.Test;
import org.junit.runner.RunWith;
import org.robolectric.annotation.Config;
Expand Down Expand Up @@ -40,7 +43,11 @@ public void testGetTabThumbnailAspectRatio_withTabletPortraitContext() {
@Test
@Config(qualifiers = "sw800dp-land")
public void testGetTabThumbnailAspectRatio_withTabletLandscapeContext() {
assertThat(TabUtils.getTabThumbnailAspectRatio(ContextUtils.getApplicationContext()))
.isEqualTo(TabUtils.TABLET_LANDSCAPE_TAB_THUMBNAIL_ASPECT_RATIO);
final Context applicationContext = ContextUtils.getApplicationContext();
final Configuration configuration = applicationContext.getResources().getConfiguration();
float expectedAspectRatio =
(configuration.screenWidthDp * 1f) / (configuration.screenHeightDp * 1f);
assertThat(TabUtils.getTabThumbnailAspectRatio(applicationContext))
.isEqualTo(expectedAspectRatio);
}
}
66 changes: 54 additions & 12 deletions ...i/junit/src/org/chromium/chrome/browser/tasks/tab_management/TabListMediatorUnitTest.java
View file
Expand Up @@ -775,9 +775,7 @@ public void tabAddition_GTS() {
doReturn(mTab2).when(mTabModelFilter).getTabAt(1);
doReturn(newTab).when(mTabModelFilter).getTabAt(2);
doReturn(3).when(mTabModelFilter).getCount();
doReturn(Arrays.asList(newTab))
.when(mTabModelFilter)
.getRelatedTabList(eq(TAB3_ID));
doReturn(Arrays.asList(newTab)).when(mTabModelFilter).getRelatedTabList(eq(TAB3_ID));
assertThat(mModel.size(), equalTo(2));

mTabModelObserverCaptor.getValue().didAddTab(
Expand All @@ -799,9 +797,7 @@ public void tabAddition_GTS_Skip() {
doReturn(mTab1).when(mTabModelFilter).getTabAt(0);
doReturn(mTab2).when(mTabModelFilter).getTabAt(1);
doReturn(2).when(mTabModelFilter).getCount();
doReturn(Arrays.asList(mTab2, newTab))
.when(mTabModelFilter)
.getRelatedTabList(eq(TAB3_ID));
doReturn(Arrays.asList(mTab2, newTab)).when(mTabModelFilter).getRelatedTabList(eq(TAB3_ID));
assertThat(mModel.size(), equalTo(2));

mTabModelObserverCaptor.getValue().didAddTab(
Expand All @@ -821,9 +817,7 @@ public void tabAddition_GTS_Middle() {
doReturn(newTab).when(mTabModelFilter).getTabAt(1);
doReturn(mTab2).when(mTabModelFilter).getTabAt(2);
doReturn(3).when(mTabModelFilter).getCount();
doReturn(Arrays.asList(newTab))
.when(mTabModelFilter)
.getRelatedTabList(eq(TAB3_ID));
doReturn(Arrays.asList(newTab)).when(mTabModelFilter).getRelatedTabList(eq(TAB3_ID));
assertThat(mModel.size(), equalTo(2));

mTabModelObserverCaptor.getValue().didAddTab(
Expand Down Expand Up @@ -1534,12 +1528,12 @@ public void updateSpanCount_MultiWindow() {
@Test
@Features.EnableFeatures(GRID_TAB_SWITCHER_FOR_TABLETS)
public void updateSpanCount_onTablet_multipleScreenWidths() {
initAndAssertAllProperties();
initAndAssertAllProperties(3);
// Mock tablet
when(mResources.getInteger(R.integer.min_screen_width_bucket))
.thenReturn(TabListCoordinator.MAX_SCREEN_WIDTH_MEDIUM_DP + 1);
Configuration portraitConfiguration = new Configuration();
portraitConfiguration.orientation = Configuration.ORIENTATION_PORTRAIT;
portraitConfiguration.orientation = Configuration.ORIENTATION_LANDSCAPE;

// Mock that we are in single window mode.
MultiWindowUtils.getInstance().setIsInMultiWindowModeForTesting(false);
Expand All @@ -1560,6 +1554,40 @@ public void updateSpanCount_onTablet_multipleScreenWidths() {
verify(mGridLayoutManager).setSpanCount(TabListCoordinator.GRID_LAYOUT_SPAN_COUNT_LARGE);
}

@Test
@Features.EnableFeatures(GRID_TAB_SWITCHER_FOR_TABLETS)
public void updateSpanCount_onLargeTabletWithThreeTabs_landscape() {
// Init 3 tabs
initAndAssertAllProperties(1);
// Mock large tablet
when(mResources.getInteger(R.integer.min_screen_width_bucket))
.thenReturn(TabListCoordinator.MAX_SCREEN_WIDTH_MEDIUM_DP + 1);

Configuration portraitConfiguration = new Configuration();
portraitConfiguration.orientation = Configuration.ORIENTATION_LANDSCAPE;
portraitConfiguration.screenWidthDp = TabListCoordinator.MAX_SCREEN_WIDTH_MEDIUM_DP + 1;
mComponentCallbacksCaptor.getValue().onConfigurationChanged(portraitConfiguration);

verify(mGridLayoutManager).setSpanCount(mTabModel.getCount());
}

@Test
@Features.EnableFeatures(GRID_TAB_SWITCHER_FOR_TABLETS)
public void updateSpanCount_onLargeTabletWithThreeTabs_portrait() {
// Init 3 tabs
initAndAssertAllProperties(1);
// Mock large tablet
when(mResources.getInteger(R.integer.min_screen_width_bucket))
.thenReturn(TabListCoordinator.MAX_SCREEN_WIDTH_MEDIUM_DP + 1);

Configuration portraitConfiguration = new Configuration();
portraitConfiguration.orientation = Configuration.ORIENTATION_PORTRAIT;
portraitConfiguration.screenWidthDp = TabListCoordinator.MAX_SCREEN_WIDTH_MEDIUM_DP + 1;
mComponentCallbacksCaptor.getValue().onConfigurationChanged(portraitConfiguration);

verify(mGridLayoutManager).setSpanCount(TabListCoordinator.GRID_LAYOUT_SPAN_COUNT_LARGE);
}

@Test
public void resetWithListOfTabs_MruOrder() {
List<Tab> tabs = new ArrayList<>();
Expand Down Expand Up @@ -3097,6 +3125,20 @@ private void initAndAssertAllProperties() {
initAndAssertAllProperties(mMediator);
}

// initAndAssertAllProperties called with regular mMediator
private void initAndAssertAllProperties(int extraTabCount) {
int index = mTabModel.getCount();
int totalCount = mTabModel.getCount() + extraTabCount;
while (index < totalCount) {
Tab tab = prepareTab(index, TAB1_TITLE, TAB1_URL);
doReturn(tab).when(mTabModel).getTabAt(index);
doReturn(index).when(mTabModel).indexOf(tab);
index++;
}
doReturn(totalCount).when(mTabModel).getCount();
initAndAssertAllProperties(mMediator);
}

// initAndAssertAllProperties called with custom mMediator (e.g. if spy needs to be used)
private void initAndAssertAllProperties(TabListMediator mediator) {
List<Tab> tabs = new ArrayList<>();
Expand All @@ -3108,7 +3150,7 @@ private void initAndAssertAllProperties(TabListMediator mediator) {
callback.onResult(mFavicon);
}

assertThat(mModel.size(), equalTo(2));
assertThat(mModel.size(), equalTo(mTabModel.getCount()));

assertThat(mModel.get(0).model.get(TabProperties.TAB_ID), equalTo(TAB1_ID));
assertThat(mModel.get(1).model.get(TabProperties.TAB_ID), equalTo(TAB2_ID));
Expand Down
5 changes: 2 additions & 3 deletions chrome/android/java/src/org/chromium/chrome/browser/tab/TabUtils.java
View file
Expand Up @@ -36,8 +36,6 @@
public class TabUtils {
private static final String REQUEST_DESKTOP_SCREEN_WIDTH_PARAM = "screen_width_dp";
@VisibleForTesting
static final float TABLET_LANDSCAPE_TAB_THUMBNAIL_ASPECT_RATIO = 1.33f;
@VisibleForTesting
static final float TAB_THUMBNAIL_ASPECT_RATIO = 0.85f;
// Do not instantiate this class.
private TabUtils() {}
Expand Down Expand Up @@ -172,7 +170,8 @@ public static float getTabThumbnailAspectRatio(Context context) {
if (context != null && DeviceFormFactor.isNonMultiDisplayContextOnTablet(context)
&& context.getResources().getConfiguration().orientation
== Configuration.ORIENTATION_LANDSCAPE) {
return TABLET_LANDSCAPE_TAB_THUMBNAIL_ASPECT_RATIO;
return (context.getResources().getConfiguration().screenWidthDp * 1f)
/ (context.getResources().getConfiguration().screenHeightDp * 1f);
}

return TAB_THUMBNAIL_ASPECT_RATIO;
Expand Down

0 comments on commit 87a7a72

Please sign in to comment.