Commit e27a554c authored by Yue Zhang's avatar Yue Zhang Committed by Commit Bot

Remove usage of cached parent view measures in TabGridDialogParent

http://crrev.com/c/2023250 assumes that we can get updated parent view
height and width by swapping the value when orientation changes, which
doesn't hold true for all devices. This CL fixes this issue by
dynamically getting the parent view measures instead of using cached
values. Also, this CL adds a regression test to make sure that
TabGridDialog and selection editor that shows within TabGridDialog
always show in the same position with same size.

Bug: 1060472
Change-Id: I65dc93ebbbdababfc699671a2339a53a47d6a3e5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2099239Reviewed-by: default avatarWei-Yin Chen (陳威尹) <wychen@chromium.org>
Commit-Queue: Yue Zhang <yuezhanggg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#750586}
parent 46d87bda
...@@ -66,8 +66,6 @@ public class TabGridDialogParent ...@@ -66,8 +66,6 @@ public class TabGridDialogParent
private final FrameLayout.LayoutParams mContainerParams; private final FrameLayout.LayoutParams mContainerParams;
private final ViewGroup mParent; private final ViewGroup mParent;
private final int mToolbarHeight; private final int mToolbarHeight;
private final int mParentViewHeight;
private final int mParentViewWidth;
private final int mUngroupBarHeight; private final int mUngroupBarHeight;
private final float mTabGridCardPadding; private final float mTabGridCardPadding;
private PopupWindow mPopupWindow; private PopupWindow mPopupWindow;
...@@ -93,8 +91,6 @@ public class TabGridDialogParent ...@@ -93,8 +91,6 @@ public class TabGridDialogParent
private AnimatorListenerAdapter mHideDialogAnimationListener; private AnimatorListenerAdapter mHideDialogAnimationListener;
private int mSideMargin; private int mSideMargin;
private int mTopMargin; private int mTopMargin;
private int mParentViewCurrentHeight;
private int mParentViewCurrentWidth;
private int mOrientation; private int mOrientation;
private int mUngroupBarStatus = UngroupBarStatus.HIDE; private int mUngroupBarStatus = UngroupBarStatus.HIDE;
private int mUngroupBarBackgroundColorResourceId = R.color.tab_grid_dialog_background_color; private int mUngroupBarBackgroundColorResourceId = R.color.tab_grid_dialog_background_color;
...@@ -113,9 +109,6 @@ public class TabGridDialogParent ...@@ -113,9 +109,6 @@ public class TabGridDialogParent
(int) context.getResources().getDimension(R.dimen.bottom_sheet_peek_height); (int) context.getResources().getDimension(R.dimen.bottom_sheet_peek_height);
mContainerParams = new FrameLayout.LayoutParams( mContainerParams = new FrameLayout.LayoutParams(
ViewGroup.LayoutParams.MATCH_PARENT, ViewGroup.LayoutParams.MATCH_PARENT); ViewGroup.LayoutParams.MATCH_PARENT, ViewGroup.LayoutParams.MATCH_PARENT);
// Parent view height and width when in portrait mode.
mParentViewHeight = Math.max(mParent.getHeight(), mParent.getWidth());
mParentViewWidth = Math.min(mParent.getHeight(), mParent.getWidth());
mComponentCallbacks = new ComponentCallbacks() { mComponentCallbacks = new ComponentCallbacks() {
@Override @Override
...@@ -526,15 +519,11 @@ public class TabGridDialogParent ...@@ -526,15 +519,11 @@ public class TabGridDialogParent
(int) context.getResources().getDimension(R.dimen.tab_grid_dialog_side_margin); (int) context.getResources().getDimension(R.dimen.tab_grid_dialog_side_margin);
mTopMargin = mTopMargin =
(int) context.getResources().getDimension(R.dimen.tab_grid_dialog_top_margin); (int) context.getResources().getDimension(R.dimen.tab_grid_dialog_top_margin);
mParentViewCurrentHeight = mParentViewHeight;
mParentViewCurrentWidth = mParentViewWidth;
} else { } else {
mSideMargin = mSideMargin =
(int) context.getResources().getDimension(R.dimen.tab_grid_dialog_top_margin); (int) context.getResources().getDimension(R.dimen.tab_grid_dialog_top_margin);
mTopMargin = mTopMargin =
(int) context.getResources().getDimension(R.dimen.tab_grid_dialog_side_margin); (int) context.getResources().getDimension(R.dimen.tab_grid_dialog_side_margin);
mParentViewCurrentWidth = mParentViewHeight;
mParentViewCurrentHeight = mParentViewWidth;
} }
mContainerParams.setMargins(mSideMargin, mTopMargin, mSideMargin, mTopMargin); mContainerParams.setMargins(mSideMargin, mTopMargin, mSideMargin, mTopMargin);
mOrientation = orientation; mOrientation = orientation;
...@@ -643,8 +632,8 @@ public class TabGridDialogParent ...@@ -643,8 +632,8 @@ public class TabGridDialogParent
// Get the status bar height as offset. // Get the status bar height as offset.
Rect parentRect = new Rect(); Rect parentRect = new Rect();
mParent.getGlobalVisibleRect(parentRect); mParent.getGlobalVisibleRect(parentRect);
return new Rect(mSideMargin, mTopMargin + parentRect.top, return new Rect(mSideMargin, mTopMargin + parentRect.top, mParent.getWidth() - mSideMargin,
mParentViewCurrentWidth - mSideMargin, mParentViewCurrentHeight - mTopMargin); mParent.getHeight() - mTopMargin + parentRect.top);
} }
/** /**
......
...@@ -35,11 +35,13 @@ import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.e ...@@ -35,11 +35,13 @@ import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.e
import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.getSwipeToDismissAction; import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.getSwipeToDismissAction;
import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.isShowingPopupTabList; import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.isShowingPopupTabList;
import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.mergeAllNormalTabsToAGroup; import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.mergeAllNormalTabsToAGroup;
import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.rotateDeviceToOrientation;
import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.verifyShowingPopupTabList; import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.verifyShowingPopupTabList;
import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.verifyTabStripFaviconCount; import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.verifyTabStripFaviconCount;
import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.verifyTabSwitcherCardCount; import static org.chromium.chrome.browser.tasks.tab_management.TabUiTestHelper.verifyTabSwitcherCardCount;
import android.content.Intent; import android.content.Intent;
import android.content.res.Configuration;
import android.graphics.Rect; import android.graphics.Rect;
import android.support.test.espresso.Espresso; import android.support.test.espresso.Espresso;
import android.support.test.espresso.contrib.RecyclerViewActions; import android.support.test.espresso.contrib.RecyclerViewActions;
...@@ -452,6 +454,43 @@ public class TabGridDialogTest { ...@@ -452,6 +454,43 @@ public class TabGridDialogTest {
verifyTabSwitcherCardCount(cta, 0); verifyTabSwitcherCardCount(cta, 0);
} }
@Test
@MediumTest
@Features.EnableFeatures(ChromeFeatureList.TAB_GROUPS_CONTINUATION_ANDROID)
public void testSelectionEditorPosition() throws InterruptedException {
final ChromeTabbedActivity cta = mActivityTestRule.getActivity();
View parentView = cta.getCompositorViewHolder();
createTabs(cta, false, 3);
enterTabSwitcher(cta);
verifyTabSwitcherCardCount(cta, 3);
// Create a tab group.
mergeAllNormalTabsToAGroup(cta);
verifyTabSwitcherCardCount(cta, 1);
// Verify the size and position of TabGridDialog in portrait mode.
openDialogFromTabSwitcherAndVerify(cta, 3);
checkPopupPosition(cta, true, true);
// Verify the size and position of TabSelectionEditor in portrait mode.
openSelectionEditorAndVerify(cta, 3);
checkPopupPosition(cta, false, true);
// Verify the size and position of TabSelectionEditor in landscape mode.
rotateDeviceToOrientation(cta, Configuration.ORIENTATION_LANDSCAPE);
CriteriaHelper.pollUiThread(() -> parentView.getHeight() < parentView.getWidth());
checkPopupPosition(cta, false, false);
// Verify the size and position of TabGridDialog in landscape mode.
mSelectionEditorRobot.actionRobot.clickToolbarNavigationButton();
mSelectionEditorRobot.resultRobot.verifyTabSelectionEditorIsHidden();
assertTrue(isDialogShowing(cta));
checkPopupPosition(cta, true, false);
// Reset orientation.
rotateDeviceToOrientation(cta, Configuration.ORIENTATION_PORTRAIT);
}
private void openDialogFromTabSwitcherAndVerify(ChromeTabbedActivity cta, int tabCount) { private void openDialogFromTabSwitcherAndVerify(ChromeTabbedActivity cta, int tabCount) {
clickFirstCardFromTabSwitcher(cta); clickFirstCardFromTabSwitcher(cta);
CriteriaHelper.pollInstrumentationThread(() -> isDialogShowing(cta)); CriteriaHelper.pollInstrumentationThread(() -> isDialogShowing(cta));
...@@ -600,4 +639,32 @@ public class TabGridDialogTest { ...@@ -600,4 +639,32 @@ public class TabGridDialogTest {
R.string.tab_selection_editor_toolbar_select_tabs) R.string.tab_selection_editor_toolbar_select_tabs)
.verifyAdapterHasItemCount(count); .verifyAdapterHasItemCount(count);
} }
private void checkPopupPosition(
ChromeTabbedActivity cta, boolean isDialog, boolean isPortrait) {
// If isDialog is true, we are checking the position of TabGridDialog; otherwise we are
// checking the position of TabSelectionEditor.
int contentViewId = isDialog ? R.id.dialog_container_view : R.id.selectable_list;
int smallMargin =
(int) cta.getResources().getDimension(R.dimen.tab_grid_dialog_side_margin);
int largeMargin = (int) cta.getResources().getDimension(R.dimen.tab_grid_dialog_top_margin);
int topMargin = isPortrait ? largeMargin : smallMargin;
int sideMargin = isPortrait ? smallMargin : largeMargin;
View parentView = cta.getCompositorViewHolder();
Rect parentRect = new Rect();
parentView.getGlobalVisibleRect(parentRect);
onView(withId(contentViewId))
.inRoot(withDecorView(not(cta.getWindow().getDecorView())))
.check((v, e) -> {
int[] location = new int[2];
v.getLocationOnScreen(location);
// Check the position.
assertEquals(sideMargin, location[0]);
assertEquals(topMargin + parentRect.top, location[1]);
// Check the size.
assertEquals(parentView.getHeight() - 2 * topMargin, v.getHeight());
assertEquals(parentView.getWidth() - 2 * sideMargin, v.getWidth());
});
}
} }
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