0

[TabContextMenu] Fix divider appearance in incognito

Incognito tab group before: https://screenshot.googleplex.com/BprFKAZWD8og6UY
Incognito tab group after: https://screenshot.googleplex.com/AcU4BWr7iiHugTE
Incognito tab before: https://screenshot.googleplex.com/9qZtWjCprtJVomU
Incognito tab after: https://screenshot.googleplex.com/BqT5Jfh24zvBen7
Tab group before: https://screenshot.googleplex.com/6dEY7xMqX6ATvjo
Tab group after (verifying no visual change): https://screenshot.googleplex.com/9DPwgzqfYBaT4vJ

Bug: 382293975
Test: BrowserUiListMenuRenderTest, TabContextMenuCoordinatorUnitTest
Change-Id: Ibeeb43359061de26988ebe02a868ab3dbdfc64f1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6436475
Reviewed-by: Wenyu Fu <wenyufu@chromium.org>
Reviewed-by: Nate Fischer <ntfschr@chromium.org>
Commit-Queue: Jenna Himawan <jhimawan@google.com>
Cr-Commit-Position: refs/heads/main@{#1443864}
This commit is contained in:
Jenna Himawan
2025-04-07 17:54:04 -07:00
committed by Chromium LUCI CQ
parent b3206c872a
commit 805d3a7822
10 changed files with 210 additions and 44 deletions
android_webview/java/src/org/chromium/android_webview
chrome
android
java
src
org
chromium
junit
src
org
chromium
chrome
browser
browser
ui
android
toolbar
java
src
org
chromium
components/browser_ui/widget/android/java/src/org/chromium/components/browser_ui/widget
ui/android/java/src/org/chromium/ui/listmenu

@ -166,7 +166,7 @@ public class AwSelectionDropdownMenuDelegate implements SelectionDropdownMenuDel
@Override
public ListItem getDivider() {
return BasicListMenu.buildMenuDivider();
return BasicListMenu.buildMenuDivider(/* isIncognito= */ false);
}
@Override

@ -178,45 +178,50 @@ public class TabContextMenuCoordinator extends TabOverflowMenuCoordinator<Intege
protected void buildMenuActionItems(ModelList itemList, Integer id) {
var tab = mTabModelSupplier.get().getTabById(id);
if (tab == null) return;
boolean isIncognito = mTabModelSupplier.get().isIncognitoBranded();
itemList.add(
BrowserUiListMenuUtils.buildMenuListItem(
R.string.add_tab_to_group, R.id.add_to_tab_group, /* startIconId= */ 0));
BrowserUiListMenuUtils.buildMenuListItemWithIncognitoBranding(
R.string.add_tab_to_group,
R.id.add_to_tab_group,
isIncognito,
/* enabled= */ true));
if (tab.getTabGroupId() != null) {
// Show the option to remove the tab from its group iff the tab is already in a group.
itemList.add(
BrowserUiListMenuUtils.buildMenuListItem(
BrowserUiListMenuUtils.buildMenuListItemWithIncognitoBranding(
R.string.remove_tab_from_group,
R.id.remove_from_tab_group,
/* startIconId= */ 0));
isIncognito,
/* enabled= */ true));
}
if (tab.getTabGroupId() == null && MultiWindowUtils.isMultiInstanceApi31Enabled()) {
// Show the option to move the tab to another window iff the tab is not in a group.
Activity activity = mWindowAndroid.getActivity().get();
itemList.add(
BrowserUiListMenuUtils.buildMenuListItem(
BrowserUiListMenuUtils.buildMenuListItemWithIncognitoBranding(
activity.getResources()
.getQuantityString(
R.plurals.move_tab_to_another_window,
MultiWindowUtils.getInstanceCount()),
R.id.move_to_other_window_menu_id,
/* startIconId= */ 0,
isIncognito,
/* enabled= */ true));
}
itemList.add(buildMenuDivider());
itemList.add(buildMenuDivider(isIncognito));
if (ShareUtils.shouldEnableShare(tab)) {
itemList.add(
BrowserUiListMenuUtils.buildMenuListItem(
R.string.share, R.id.share_tab, /* startIconId= */ 0));
BrowserUiListMenuUtils.buildMenuListItemWithIncognitoBranding(
R.string.share, R.id.share_tab, isIncognito, /* enabled= */ true));
}
itemList.add(
BrowserUiListMenuUtils.buildMenuListItem(
R.string.close_tab, R.id.close_tab, /* startIconId= */ 0));
BrowserUiListMenuUtils.buildMenuListItemWithIncognitoBranding(
R.string.close_tab, R.id.close_tab, isIncognito, /* enabled= */ true));
}
@Override

@ -566,6 +566,10 @@ public class TabGroupContextMenuCoordinator extends TabGroupOverflowMenuCoordina
.with(
ListSectionDividerProperties.RIGHT_PADDING_DIMEN_ID,
R.dimen.list_menu_item_horizontal_padding);
if (mTabModelSupplier.get().isIncognitoBranded()) {
builder.with(
ListSectionDividerProperties.COLOR_ID, R.color.divider_line_bg_color_light);
}
return new ListItem(ListMenuItemType.DIVIDER, builder.build());
}

@ -12,6 +12,7 @@ import static org.mockito.Mockito.when;
import static org.chromium.chrome.browser.contextmenu.ContextMenuCoordinator.ListItemType.DIVIDER;
import static org.chromium.chrome.browser.share.ShareDelegate.ShareOrigin.TAB_STRIP_CONTEXT_MENU;
import static org.chromium.ui.listmenu.ListSectionDividerProperties.COLOR_ID;
import android.app.Activity;
@ -114,7 +115,6 @@ public class TabContextMenuCoordinatorUnitTest {
when(mWindowAndroid.getActivity()).thenReturn(mWeakReferenceActivity);
when(mWeakReferenceActivity.get()).thenReturn(activity);
mTabModel = spy(new MockTabModel(mProfile, null));
when(mTabModel.isIncognito()).thenReturn(false);
when(mTabModel.getTabById(TAB_ID)).thenReturn(mTab1);
when(mTabModel.getTabById(TAB_OUTSIDE_OF_GROUP_ID)).thenReturn(mTabOutsideOfGroup);
when(mTabModel.getTabById(NON_URL_TAB_ID)).thenReturn(mNonUrlTab);
@ -128,8 +128,18 @@ public class TabContextMenuCoordinatorUnitTest {
when(mNonUrlTab.getUrl()).thenReturn(CHROME_SCHEME_URL);
when(mTabGroupModelFilter.getTabModel()).thenReturn(mTabModel);
when(mTabGroupModelFilter.getTabUngrouper()).thenReturn(mTabUngrouper);
when(mProfile.isOffTheRecord()).thenReturn(true);
mSavedTabGroup.collaborationId = COLLABORATION_ID;
setupWithIncognito(/* incognito= */ false); // Most tests will run not in incognito mode
initializeCoordinator();
}
private void setupWithIncognito(boolean incognito) {
when(mTabModel.isIncognito()).thenReturn(incognito);
when(mTabModel.isIncognitoBranded()).thenReturn(incognito);
when(mProfile.isOffTheRecord()).thenReturn(incognito);
}
private void initializeCoordinator() {
mOnItemClickedCallback =
TabContextMenuCoordinator.getMenuItemClickedCallback(
() -> mTabModel,
@ -173,6 +183,10 @@ public class TabContextMenuCoordinatorUnitTest {
// List item 3
assertEquals(DIVIDER, modelList.get(2).type);
assertEquals(
"Expected divider to have have COLOR_ID unset when not in incognito mode",
0,
modelList.get(2).model.get(COLOR_ID));
// List item 4
assertEquals(R.string.share, modelList.get(3).model.get(ListMenuItemProperties.TITLE_ID));
@ -343,6 +357,71 @@ public class TabContextMenuCoordinatorUnitTest {
R.id.close_tab, modelList.get(3).model.get(ListMenuItemProperties.MENU_ITEM_ID));
}
@Test
public void testListMenuItems_incognito() {
setupWithIncognito(/* incognito= */ true);
initializeCoordinator();
var modelList = new ModelList();
mTabContextMenuCoordinator.buildMenuActionItems(modelList, TAB_ID);
assertEquals("Number of items in the list menu is incorrect", 5, modelList.size());
// List item 1
assertEquals(
R.string.add_tab_to_group,
modelList.get(0).model.get(ListMenuItemProperties.TITLE_ID));
assertEquals(
R.id.add_to_tab_group,
modelList.get(0).model.get(ListMenuItemProperties.MENU_ITEM_ID));
assertEquals(
"Expected text appearance ID to be set to"
+ " R.style.TextAppearance_TextLarge_Primary_Baseline_Light in incognito",
R.style.TextAppearance_TextLarge_Primary_Baseline_Light,
modelList.get(0).model.get(ListMenuItemProperties.TEXT_APPEARANCE_ID));
// List item 2
assertEquals(
R.string.remove_tab_from_group,
modelList.get(1).model.get(ListMenuItemProperties.TITLE_ID));
assertEquals(
R.id.remove_from_tab_group,
modelList.get(1).model.get(ListMenuItemProperties.MENU_ITEM_ID));
assertEquals(
"Expected text appearance ID to be set to"
+ " R.style.TextAppearance_TextLarge_Primary_Baseline_Light in incognito",
R.style.TextAppearance_TextLarge_Primary_Baseline_Light,
modelList.get(1).model.get(ListMenuItemProperties.TEXT_APPEARANCE_ID));
// List item 3
assertEquals(DIVIDER, modelList.get(2).type);
assertEquals(
"Expected divider to have COLOR_ID set to R.color.divider_line_bg_color_light in"
+ " incognito mode",
R.color.divider_line_bg_color_light,
modelList.get(2).model.get(COLOR_ID));
// List item 4
assertEquals(R.string.share, modelList.get(3).model.get(ListMenuItemProperties.TITLE_ID));
assertEquals(
R.id.share_tab, modelList.get(3).model.get(ListMenuItemProperties.MENU_ITEM_ID));
assertEquals(
"Expected text appearance ID to be set to"
+ " R.style.TextAppearance_TextLarge_Primary_Baseline_Light in incognito",
R.style.TextAppearance_TextLarge_Primary_Baseline_Light,
modelList.get(3).model.get(ListMenuItemProperties.TEXT_APPEARANCE_ID));
// List item 5
assertEquals(
R.string.close_tab, modelList.get(4).model.get(ListMenuItemProperties.TITLE_ID));
assertEquals(
R.id.close_tab, modelList.get(4).model.get(ListMenuItemProperties.MENU_ITEM_ID));
assertEquals(
"Expected text appearance ID to be set to"
+ " R.style.TextAppearance_TextLarge_Primary_Baseline_Light in incognito",
R.style.TextAppearance_TextLarge_Primary_Baseline_Light,
modelList.get(4).model.get(ListMenuItemProperties.TEXT_APPEARANCE_ID));
}
@Test
@Feature("Tab Strip Context Menu")
public void testRemoveFromGroup() {

@ -269,7 +269,7 @@ public class TabSwitcherActionMenuCoordinator {
R.drawable.ic_widgets);
case MenuItemType.DIVIDER:
default:
return buildMenuDivider();
return buildMenuDivider(mProfile.isIncognitoBranded());
}
}

@ -184,6 +184,54 @@ public class BrowserUiListMenuUtils {
return new MVCListAdapter.ListItem(ListMenuItemType.MENU_ITEM, builder.build());
}
/**
* Helper function to build a list menu item. Pass 0 for attributes that aren't applicable to
* the menu item (e.g. if there is no icon or text).
*
* @param title The text on the menu item.
* @param menuId Id of the menu item.
* @param isIncognito Whether the current menu item will be displayed in incognito mode.
* @param enabled Whether or not this menu item should be enabled.
* @return ListItem Representing an item with text and maybe an icon.
*/
public static ListItem buildMenuListItemWithIncognitoBranding(
String title, @IdRes int menuId, boolean isIncognito, boolean enabled) {
PropertyModel.Builder builder =
getListItemPropertyBuilder()
.with(ListMenuItemProperties.TITLE, title)
.with(ListMenuItemProperties.MENU_ITEM_ID, menuId)
.with(ListMenuItemProperties.ENABLED, enabled);
if (isIncognito) {
builder.with(
ListMenuItemProperties.TEXT_APPEARANCE_ID,
R.style.TextAppearance_TextLarge_Primary_Baseline_Light);
}
return new MVCListAdapter.ListItem(ListMenuItemType.MENU_ITEM, builder.build());
}
/**
* Helper function to build a list menu item. Pass 0 for attributes that aren't applicable to
* the menu item (e.g. if there is no icon or text).
*
* @param titleId The text on the menu item.
* @param menuId Id of the menu item.
* @param isIncognito Whether the current menu item will be displayed in incognito mode.
* @param enabled Whether or not this menu item should be enabled.
* @return ListItem Representing an item with text and maybe an icon.
*/
public static ListItem buildMenuListItemWithIncognitoBranding(
@StringRes int titleId, @IdRes int menuId, boolean isIncognito, boolean enabled) {
return buildMenuListItemWithIncognitoBranding(
titleId,
menuId,
/* startIconId= */ 0,
/* iconTintColorStateList= */ 0,
R.style.TextAppearance_TextLarge_Primary_Baseline_Light,
isIncognito,
enabled);
}
/**
* Helper function to build a list menu item. Pass 0 for attributes that aren't applicable to
* the menu item (e.g. if there is no icon or text).

@ -14,18 +14,18 @@ import androidx.appcompat.content.res.AppCompatResources;
import androidx.test.filters.MediumTest;
import org.junit.After;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.ClassRule;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.MethodRule;
import org.junit.runner.RunWith;
import org.chromium.base.ThreadUtils;
import org.chromium.base.test.BaseActivityTestRule;
import org.chromium.base.test.params.BaseJUnit4RunnerDelegate;
import org.chromium.base.test.params.ParameterAnnotations;
import org.chromium.base.test.params.MethodParamAnnotationRule;
import org.chromium.base.test.params.ParameterAnnotations.UseMethodParameter;
import org.chromium.base.test.params.ParameterAnnotations.UseRunnerDelegate;
import org.chromium.base.test.params.ParameterProvider;
import org.chromium.base.test.params.ParameterSet;
import org.chromium.base.test.params.ParameterizedRunner;
import org.chromium.base.test.util.Batch;
@ -37,9 +37,11 @@ import org.chromium.ui.listmenu.ListMenu;
import org.chromium.ui.modelutil.MVCListAdapter.ModelList;
import org.chromium.ui.test.util.BlankUiTestActivity;
import org.chromium.ui.test.util.NightModeTestUtils;
import org.chromium.ui.test.util.NightModeTestUtils.NightModeParams;
import org.chromium.ui.test.util.RenderTestRule;
import java.io.IOException;
import java.util.Collections;
import java.util.List;
/** Render tests for {@link BasicListMenu}. */
@ -47,39 +49,38 @@ import java.util.List;
@UseRunnerDelegate(BaseJUnit4RunnerDelegate.class)
@Batch(Batch.UNIT_TESTS)
public class BrowserUiListMenuRenderTest {
@ParameterAnnotations.ClassParameter
private static List<ParameterSet> sClassParams =
new NightModeTestUtils.NightModeParams().getParameters();
/** Used to run a test only with night mode. */
public static class NightModeOnlyParameterProvider implements ParameterProvider {
@ClassRule
public static final BaseActivityTestRule<BlankUiTestActivity> sActivityTestRule =
private static List<ParameterSet> sNightModeOnly =
Collections.singletonList(new ParameterSet().value(true).name("NightModeEnabled"));
@Override
public Iterable<ParameterSet> getParameters() {
return sNightModeOnly;
}
}
@Rule
public final BaseActivityTestRule<BlankUiTestActivity> mActivityTestRule =
new BaseActivityTestRule<>(BlankUiTestActivity.class);
private static Activity sActivity;
@Rule
public RenderTestRule mRenderTestRule =
RenderTestRule.Builder.withPublicCorpus()
.setBugComponent(RenderTestRule.Component.UI_BROWSER_MOBILE)
.build();
@Rule public MethodRule mMethodParamAnnotationProcessor = new MethodParamAnnotationRule();
private View mView;
public BrowserUiListMenuRenderTest(boolean nightModeEnabled) {
NightModeTestUtils.setUpNightModeForBlankUiTestActivity(nightModeEnabled);
mRenderTestRule.setNightModeEnabled(nightModeEnabled);
}
@BeforeClass
public static void setupSuite() {
sActivity = sActivityTestRule.launchActivity(null);
}
@Before
public void setUp() throws Exception {
private void setup(boolean nightMode, boolean incognito) {
Activity activity = mActivityTestRule.launchActivity(null);
NightModeTestUtils.setUpNightModeForBlankUiTestActivity(nightMode);
mRenderTestRule.setNightModeEnabled(nightMode);
ThreadUtils.runOnUiThreadBlocking(
() -> {
Activity activity = sActivity;
ModelList data = new ModelList();
data.add(
BrowserUiListMenuUtils.buildMenuListItem(
@ -108,7 +109,7 @@ public class BrowserUiListMenuRenderTest {
data.add(
BrowserUiListMenuUtils.buildMenuListItem(
R.string.test_primary_1, 0, 0, false));
data.add(BasicListMenu.buildMenuDivider());
data.add(BasicListMenu.buildMenuDivider(incognito));
ListMenu.Delegate delegate = item -> {};
BasicListMenu listMenu =
@ -130,7 +131,18 @@ public class BrowserUiListMenuRenderTest {
@Test
@MediumTest
@Feature({"RenderTest"})
public void testRender_BasicListMenu() throws IOException {
@UseMethodParameter(NightModeParams.class)
public void testRender_BasicListMenu(boolean nightMode) throws IOException {
setup(nightMode, /* incognito= */ false);
mRenderTestRule.render(mView, "basic_list_menu");
}
@Test
@MediumTest
@UseMethodParameter(NightModeOnlyParameterProvider.class)
@Feature({"RenderTest"})
public void testRender_BasicListMenu_Incognito(boolean nightMode) throws IOException {
setup(nightMode, /* incognito= */ true);
mRenderTestRule.render(mView, "basic_list_menu_incognito");
}
}

@ -49,10 +49,17 @@ public class BasicListMenu implements ListMenu, OnItemClickListener {
/**
* Helper function to build a ListItem of a divider.
*
* @param isIncognito Whether we're creating an incognito-themed menu.
* @return ListItem Representing a divider.
*/
public static ListItem buildMenuDivider() {
return new ListItem(ListMenuItemType.DIVIDER, new PropertyModel());
public static ListItem buildMenuDivider(boolean isIncognito) {
PropertyModel.Builder builder =
new PropertyModel.Builder(ListSectionDividerProperties.ALL_KEYS);
if (isIncognito) {
builder.with(
ListSectionDividerProperties.COLOR_ID, R.color.divider_line_bg_color_light);
}
return new ListItem(ListMenuItemType.DIVIDER, builder.build());
}
/**

@ -14,6 +14,9 @@ public class ListSectionDividerProperties {
public static final WritableIntPropertyKey LEFT_PADDING_DIMEN_ID = new WritableIntPropertyKey();
public static final WritableIntPropertyKey RIGHT_PADDING_DIMEN_ID =
new WritableIntPropertyKey();
public static final WritableIntPropertyKey COLOR_ID = new WritableIntPropertyKey();
public static final PropertyKey[] ALL_KEYS = {LEFT_PADDING_DIMEN_ID, RIGHT_PADDING_DIMEN_ID};
public static final PropertyKey[] ALL_KEYS = {
LEFT_PADDING_DIMEN_ID, RIGHT_PADDING_DIMEN_ID, COLOR_ID
};
}

@ -10,9 +10,11 @@ import android.view.View;
import androidx.annotation.DimenRes;
import androidx.annotation.Px;
import androidx.core.content.ContextCompat;
import org.chromium.build.annotations.NullMarked;
import org.chromium.build.annotations.Nullable;
import org.chromium.ui.R;
import org.chromium.ui.modelutil.PropertyKey;
import org.chromium.ui.modelutil.PropertyModel;
@ -49,6 +51,12 @@ public class ListSectionDividerViewBinder {
rightPaddingPx,
view.getPaddingBottom());
}
} else if (propertyKey == ListSectionDividerProperties.COLOR_ID) {
view.findViewById(R.id.divider_view)
.setBackgroundColor(
ContextCompat.getColor(
view.getContext(),
model.get(ListSectionDividerProperties.COLOR_ID)));
}
}
}