Commit 81d97164 authored by Robbie McElrath's avatar Robbie McElrath Committed by Commit Bot

[WebLayer] Fix initial top controls positioning

This is a followup to crrev.com/c/2412810, which fixed the controls
positioning during page loads in every case except the one we were
supposed to fix in the original bug. The issue with that CL was that it
always set the controls height to be the same as the previous height, or
0 if there weren't controls before. However, the renderer defaults the
controls to expanded initially, so collapsing them on first load wasn't
the right behavior. To fix this I make onLayout position new controls so
that the same amount of them is showing as *the last time a view was
set*, and defaults to an expanded view. This is the behavior the
renderer follows.

Bug: 1128391
Change-Id: I6a2c65629a8d24476e7c2e07158295574cac0c37
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2423007Reviewed-by: default avatarBo <boliu@chromium.org>
Commit-Queue: Robbie McElrath <rmcelrath@chromium.org>
Cr-Commit-Position: refs/heads/master@{#809496}
parent 3d27db20
......@@ -61,6 +61,8 @@ class BrowserControlsContainerView extends FrameLayout {
private static final long SYSTEM_UI_VIEWPORT_UPDATE_DELAY_MS = 500;
private static final int DEFAULT_LAST_SHOWN_AMOUNT = -1;
/** Stores the state needed to reconstruct offsets after recreating this class. */
/* package */ static class State {
private final int mControlsOffset;
......@@ -107,6 +109,12 @@ class BrowserControlsContainerView extends FrameLayout {
// bottom-controls, the value ranges from 0 (completely shown) to height (completely hidden).
private int mControlsOffset;
// Stores how much of the controls in pixels is visible when a view is set. This does NOT get
// set to 0 when you remove a view; it always stores the most recent control visibility amount
// as of the last time a view was actually set, or a default value. This is used to help mimic
// the positioning behavior of the renderer.
private int mLastShownAmountWithView = DEFAULT_LAST_SHOWN_AMOUNT;
// The minimum height that the controls should collapse to. Only used for top controls.
private int mMinHeight;
......@@ -323,9 +331,9 @@ class BrowserControlsContainerView extends FrameLayout {
if (mIsFullscreen) return;
setControlsOffset(controlsOffsetY, contentOffsetY);
if (controlsOffsetY == 0
if (mControlsOffset == 0
|| (mIsTop && getMinHeight() > 0
&& controlsOffsetY == -mLastHeight + getMinHeight())) {
&& mControlsOffset == -mLastHeight + getMinHeight())) {
finishScroll();
} else if (!mInScroll) {
prepareForScroll();
......@@ -359,7 +367,7 @@ class BrowserControlsContainerView extends FrameLayout {
mLastHeight = height;
if (mLastWidth > 0 && mLastHeight > 0 && mViewResourceAdapter == null) {
createAdapterAndLayer();
if (prevHeight == 0 && mSavedState != null) {
if (mLastShownAmountWithView == DEFAULT_LAST_SHOWN_AMOUNT && mSavedState != null) {
// If there wasn't a View before and we have non-empty saved state from a previous
// BrowserControlsContainerView instance, apply those saved offsets now. We can't
// rely on BrowserControlsOffsetManager to notify us of the correct location as we
......@@ -368,18 +376,30 @@ class BrowserControlsContainerView extends FrameLayout {
// because it thinks we already have the correct offsets.
onOffsetsChanged(mSavedState.mControlsOffset, mSavedState.mContentOffset);
} else {
// Position the new controls so the same amount of them is visible as with the
// previous controls (which will be 0 if there weren't controls or they were
// hidden). Ideally we'd hide the controls and wait for BrowserControlsOffsetManager
// Ideally we'd leave the controls hidden and wait for BrowserControlsOffsetManager
// to tell us where it wants them, but it communicates with us via frame metadata
// that is generated when the renderer's compositor submits a frame to viz, which is
// paused during page loads. Because of this, we might not get positioning
// information from the renderer for several seconds during page loads, so we need
// to attempt to position the controls ourselves here. This could in theory cause a
// flicker if we decide on a different position than the renderer does, but this
// hasn't been an issue in practice.
int delta = mIsTop ? height - prevHeight : prevHeight - height;
onOffsetsChanged(mControlsOffset - delta, mContentOffset);
// to attempt to position the controls ourselves here.
int targetShownAmount;
if (mShouldAnimate) {
// If animations are enabled, leave the amount of visible controls unchanged
// (e.g. hide them if there weren't previously controls or if they were hidden
// before).
targetShownAmount = mIsTop ? (prevHeight + mControlsOffset)
: (prevHeight - mControlsOffset);
} else {
// If animations are disabled, restore the positioning the controls had the last
// time they were non-null. This mimics the behavior of
// BrowserControlsOffsetManager in the renderer.
targetShownAmount = (mLastShownAmountWithView == DEFAULT_LAST_SHOWN_AMOUNT)
? height
: mLastShownAmountWithView;
}
onOffsetsChanged(
mIsTop ? (targetShownAmount - height) : (height - targetShownAmount),
mIsTop ? targetShownAmount : 0);
}
mSavedState = null;
} else if (mViewResourceAdapter != null) {
......@@ -460,6 +480,10 @@ class BrowserControlsContainerView extends FrameLayout {
}
mContentOffset = MathUtils.clamp(contentOffsetY, 0, mLastHeight);
if (mView != null) {
mLastShownAmountWithView =
mIsTop ? (mLastHeight + mControlsOffset) : (mLastHeight - mControlsOffset);
}
if (isCompletelyExpandedOrCollapsed()) {
mDelegate.refreshPageHeight();
}
......
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