Commit 18a3e0c7 authored by Robbie McElrath's avatar Robbie McElrath Committed by Commit Bot

[WebLayer] Always position new browser controls in onLayout

This CL partially fixes an issue where the browser controls wouldn't be
visible during page loads. This was caused by setView hiding the
controls initially until it got positioning information from the
renderer about where it wanted the controls to be. During page loads,
this positioning information doesn't come because compositor commits are
paused, which means the controls can be hidden for a couple seconds.

Bug: 1128391
Change-Id: Iaa6b3dd1d7ef65523d19dddd94587aec9cbc6ffd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2412810Reviewed-by: default avatarBo <boliu@chromium.org>
Commit-Queue: Robbie McElrath <rmcelrath@chromium.org>
Cr-Commit-Position: refs/heads/master@{#808050}
parent 2ae9296c
...@@ -130,13 +130,19 @@ class BrowserControlsContainerView extends FrameLayout { ...@@ -130,13 +130,19 @@ class BrowserControlsContainerView extends FrameLayout {
// Used to delay processing fullscreen requests. // Used to delay processing fullscreen requests.
private Runnable mSystemUiFullscreenResizeRunnable; private Runnable mSystemUiFullscreenResizeRunnable;
// Used to delay updating the image for the layer. // Used to delay updating the image for the layer.
private final Runnable mRefreshResourceIdRunnable = () -> { private final Runnable mRefreshResourceIdRunnable = () -> {
if (mView == null || mViewResourceAdapter == null) return; if (mView == null || mViewResourceAdapter == null) return;
BrowserControlsContainerViewJni.get().updateControlsResource( BrowserControlsContainerViewJni.get().updateControlsResource(
mNativeBrowserControlsContainerView); mNativeBrowserControlsContainerView);
}; };
// Used to delay hiding the controls.
private final Runnable mHideControlsRunnable = this::hideControlsNow;
// Used to delay showing the controls.
private final Runnable mShowControlsRunnable = this::showControlsNow;
public interface Delegate { public interface Delegate {
/** /**
* Requests that the page height be recalculated due to browser controls height changes. * Requests that the page height be recalculated due to browser controls height changes.
...@@ -268,10 +274,13 @@ class BrowserControlsContainerView extends FrameLayout { ...@@ -268,10 +274,13 @@ class BrowserControlsContainerView extends FrameLayout {
addView(view, addView(view,
new FrameLayout.LayoutParams(LayoutParams.MATCH_PARENT, LayoutParams.WRAP_CONTENT, new FrameLayout.LayoutParams(LayoutParams.MATCH_PARENT, LayoutParams.WRAP_CONTENT,
FrameLayout.LayoutParams.UNSPECIFIED_GRAVITY)); FrameLayout.LayoutParams.UNSPECIFIED_GRAVITY));
// We always want to hide the real controls so they don't flash for a frame before we've // The controls will be positioned in onLayout, which will result in showControls or
// figured out where to position them. // hideControls being called. hideControls may delay the hide by a frame, resulting in the
removeCallbacks(this::showControls); // View flashing during the current frame. To work around this, we hide the controls here.
hideControls(); // showControls will also delay the showing by a frame, but that doesn't cause a flash
// because the bitmap will be visible until the setVisibility call completes.
hideControlsNow();
mContentViewRenderView.removeCallbacks(mShowControlsRunnable);
mDelegate.setAnimationConstraint(BrowserControlsState.BOTH); mDelegate.setAnimationConstraint(BrowserControlsState.BOTH);
} }
...@@ -350,24 +359,29 @@ class BrowserControlsContainerView extends FrameLayout { ...@@ -350,24 +359,29 @@ class BrowserControlsContainerView extends FrameLayout {
mLastHeight = height; mLastHeight = height;
if (mLastWidth > 0 && mLastHeight > 0 && mViewResourceAdapter == null) { if (mLastWidth > 0 && mLastHeight > 0 && mViewResourceAdapter == null) {
createAdapterAndLayer(); createAdapterAndLayer();
if (prevHeight == 0) { if (prevHeight == 0 && mSavedState != null) {
assert heightChanged;
// If there wasn't a View before and we have non-empty saved state from a previous // 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 // BrowserControlsContainerView instance, apply those saved offsets now. We can't
// rely on BrowserControlsOffsetManager to notify us of the correct location as we // rely on BrowserControlsOffsetManager to notify us of the correct location as we
// usually do because it only notifies us when offsets change, but it likely didn't // usually do because it only notifies us when offsets change, but it likely didn't
// get destroyed when the BrowserFragment got recreated, so it won't notify us // get destroyed when the BrowserFragment got recreated, so it won't notify us
// because it thinks we already have the correct offsets. // because it thinks we already have the correct offsets.
if (mSavedState != null) { onOffsetsChanged(mSavedState.mControlsOffset, mSavedState.mContentOffset);
onOffsetsChanged(mSavedState.mControlsOffset, mSavedState.mContentOffset); } else {
} else { // Position the new controls so the same amount of them is visible as with the
// If there wasn't a View before (or it had 0 height) and there's no state from // previous controls (which will be 0 if there weren't controls or they were
// a previous instance of this class, move the new View off the screen until // hidden). Ideally we'd hide the controls and wait for BrowserControlsOffsetManager
// BrowserControlsOffsetManager tells us where to position it. // to tell us where it wants them, but it communicates with us via frame metadata
moveControlsOffScreen(); // 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
mSavedState = null; // 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);
} }
mSavedState = null;
} else if (mViewResourceAdapter != null) { } else if (mViewResourceAdapter != null) {
BrowserControlsContainerViewJni.get().setControlsSize( BrowserControlsContainerViewJni.get().setControlsSize(
mNativeBrowserControlsContainerView, mLastWidth, mLastHeight); mNativeBrowserControlsContainerView, mLastWidth, mLastHeight);
...@@ -466,29 +480,41 @@ class BrowserControlsContainerView extends FrameLayout { ...@@ -466,29 +480,41 @@ class BrowserControlsContainerView extends FrameLayout {
private void prepareForScroll() { private void prepareForScroll() {
mInScroll = true; mInScroll = true;
if (BrowserControlsContainerViewJni.get().shouldDelayVisibilityChange()) { hideControls();
mContentViewRenderView.postOnAnimation(this::hideControls);
} else {
hideControls();
}
} }
private void finishScroll() { private void finishScroll() {
mInScroll = false; mInScroll = false;
showControls();
}
private void hideControls() {
if (BrowserControlsContainerViewJni.get().shouldDelayVisibilityChange()) { if (BrowserControlsContainerViewJni.get().shouldDelayVisibilityChange()) {
mContentViewRenderView.postOnAnimation(this::showControls); mContentViewRenderView.postOnAnimation(mHideControlsRunnable);
} else { } else {
showControls(); hideControlsNow();
} }
} }
private void hideControls() { private void hideControlsNow() {
if (mView != null) mView.setVisibility(View.INVISIBLE); if (mView != null) {
mView.setVisibility(View.INVISIBLE);
}
} }
private void showControls() { private void showControls() {
if (BrowserControlsContainerViewJni.get().shouldDelayVisibilityChange()) {
mContentViewRenderView.postOnAnimation(mShowControlsRunnable);
} else {
showControlsNow();
}
}
private void showControlsNow() {
if (mView != null) { if (mView != null) {
if (mIsTop) mView.setTranslationY(mControlsOffset); if (mIsTop) {
mView.setTranslationY(mControlsOffset);
}
mView.setVisibility(View.VISIBLE); mView.setVisibility(View.VISIBLE);
} }
} }
......
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