From 7b5ccaad4c31701313513c5aac3b723bd20b208c Mon Sep 17 00:00:00 2001 From: Robbie McElrath Date: Mon, 24 Aug 2020 21:46:06 +0000 Subject: [PATCH] [WebLayer] Save browser controls UI state between BrowserFragment restarts This CL makes BrowserImpl save the UI state of BrowserViewController when destroying/creating it so we can save the controls location. It doesn't use onStoreInstanceState directly because the BrowserControlsContainerView lifecycle doesn't exactly match what Android expects (onStoreInstanceState gets called after BCCV is already destroyed). Bug: 1120209 Change-Id: Ibedf4f8ad1e01f29518b404d94961b0e97689b9c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2368479 Reviewed-by: Bo Commit-Queue: Robbie McElrath Cr-Commit-Position: refs/heads/master@{#801158} --- .../BrowserControlsContainerView.java | 43 +++++++++++++++++-- .../weblayer_private/BrowserImpl.java | 7 ++- .../BrowserViewController.java | 29 +++++++++++-- 3 files changed, 70 insertions(+), 9 deletions(-) diff --git a/weblayer/browser/java/org/chromium/weblayer_private/BrowserControlsContainerView.java b/weblayer/browser/java/org/chromium/weblayer_private/BrowserControlsContainerView.java index 42a80b67cdc3e1..50b260fff2a38b 100644 --- a/weblayer/browser/java/org/chromium/weblayer_private/BrowserControlsContainerView.java +++ b/weblayer/browser/java/org/chromium/weblayer_private/BrowserControlsContainerView.java @@ -11,6 +11,8 @@ import android.view.ViewParent; import android.widget.FrameLayout; +import androidx.annotation.Nullable; + import org.chromium.base.MathUtils; import org.chromium.base.annotations.CalledByNative; import org.chromium.base.annotations.JNINamespace; @@ -59,9 +61,25 @@ class BrowserControlsContainerView extends FrameLayout { private static final long SYSTEM_UI_VIEWPORT_UPDATE_DELAY_MS = 500; + /** Stores the state needed to reconstruct offsets after recreating this class. */ + /* package */ static class State { + private final int mControlsOffset; + private final int mContentOffset; + + private State(int controlsOffset, int contentOffset) { + mControlsOffset = controlsOffset; + mContentOffset = contentOffset; + } + } + private final Delegate mDelegate; private final boolean mIsTop; + // The state returned by a previous BrowserControlsContainerView instance's getState() method. + // This is saved rather than directly applied because layout needs to occur before we can apply + // the offsets. + private State mSavedState; + private long mNativeBrowserControlsContainerView; private ViewResourceAdapter mViewResourceAdapter; @@ -132,10 +150,11 @@ public interface Delegate { } BrowserControlsContainerView(Context context, ContentViewRenderView contentViewRenderView, - Delegate delegate, boolean isTop) { + Delegate delegate, boolean isTop, @Nullable State savedState) { super(context); mDelegate = delegate; mIsTop = isTop; + mSavedState = savedState; mContentViewRenderView = contentViewRenderView; mNativeBrowserControlsContainerView = BrowserControlsContainerViewJni.get().createBrowserControlsContainerView( @@ -333,9 +352,21 @@ protected void onLayout(boolean changed, int left, int top, int right, int botto createAdapterAndLayer(); if (prevHeight == 0) { assert heightChanged; - // If there wasn't a View before (or it had 0 height), move the new View off the - // screen until we know where to position it. - moveControlsOffScreen(); + // 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 + // 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 + // because it thinks we already have the correct offsets. + if (mSavedState != null) { + onOffsetsChanged(mSavedState.mControlsOffset, mSavedState.mContentOffset); + } else { + // If there wasn't a View before (or it had 0 height) and there's no state from + // a previous instance of this class, move the new View off the screen until + // BrowserControlsOffsetManager tells us where to position it. + moveControlsOffScreen(); + } + mSavedState = null; } } else if (mViewResourceAdapter != null) { BrowserControlsContainerViewJni.get().setControlsSize( @@ -352,6 +383,10 @@ public void onDetachedFromWindow() { cancelDelayedFullscreenRunnable(); } + /* package */ State getState() { + return new State(mControlsOffset, mContentOffset); + } + private void cancelDelayedFullscreenRunnable() { if (mSystemUiFullscreenResizeRunnable == null) return; removeCallbacks(mSystemUiFullscreenResizeRunnable); diff --git a/weblayer/browser/java/org/chromium/weblayer_private/BrowserImpl.java b/weblayer/browser/java/org/chromium/weblayer_private/BrowserImpl.java index a87a285f033eb8..dfd34f0ec4c7a8 100644 --- a/weblayer/browser/java/org/chromium/weblayer_private/BrowserImpl.java +++ b/weblayer/browser/java/org/chromium/weblayer_private/BrowserImpl.java @@ -56,6 +56,10 @@ public class BrowserImpl extends IBrowser.Stub implements View.OnAttachStateChan private final ProfileImpl mProfile; private Context mEmbedderActivityContext; private BrowserViewController mViewController; + // Used to save UI state between destroyAttachmentState() and createAttachmentState() calls so + // it can be preserved during device rotations or other events that cause the Fragment to be + // recreated. + private BrowserViewController.State mViewControllerState; private FragmentWindowAndroid mWindowAndroid; private IBrowserClient mClient; private LocaleChangedBroadcastReceiver mLocaleReceiver; @@ -136,7 +140,7 @@ private void createAttachmentState( assert mEmbedderActivityContext == null; mWindowAndroid = windowAndroid; mEmbedderActivityContext = embedderAppContext; - mViewController = new BrowserViewController(windowAndroid, this); + mViewController = new BrowserViewController(windowAndroid, this, mViewControllerState); mLocaleReceiver = new LocaleChangedBroadcastReceiver(windowAndroid.getContext().get()); mPasswordEchoEnabled = null; } @@ -533,6 +537,7 @@ private void destroyAttachmentState() { mLocaleReceiver = null; } if (mViewController != null) { + mViewControllerState = mViewController.getState(); mViewController.destroy(); mViewController = null; mViewAttachedToWindow = false; diff --git a/weblayer/browser/java/org/chromium/weblayer_private/BrowserViewController.java b/weblayer/browser/java/org/chromium/weblayer_private/BrowserViewController.java index 5389e26aba3634..3b63dff91d98f6 100644 --- a/weblayer/browser/java/org/chromium/weblayer_private/BrowserViewController.java +++ b/weblayer/browser/java/org/chromium/weblayer_private/BrowserViewController.java @@ -15,6 +15,8 @@ import android.widget.FrameLayout; import android.widget.RelativeLayout; +import androidx.annotation.Nullable; + import org.chromium.base.annotations.JNINamespace; import org.chromium.components.browser_ui.modaldialog.AppModalPresenter; import org.chromium.components.browser_ui.widget.InsetObserverView; @@ -36,6 +38,18 @@ public final class BrowserViewController implements BrowserControlsContainerView.Delegate, WebContentsGestureStateTracker.OnGestureStateChangedListener, ModalDialogManager.ModalDialogManagerObserver { + /** Information needed to restore the UI state after recreating the BrowserViewController. */ + /* package */ static class State { + private BrowserControlsContainerView.State mTopControlsState; + private BrowserControlsContainerView.State mBottomControlsState; + + private State(BrowserControlsContainerView.State topControlsState, + BrowserControlsContainerView.State bottomControlsState) { + mTopControlsState = topControlsState; + mBottomControlsState = bottomControlsState; + } + } + private final ContentViewRenderView mContentViewRenderView; // Child of mContentViewRenderView. Be very careful adding Views to this, as any Views are not // accessible (ContentView provides it's own accessible implementation that interacts with @@ -72,8 +86,8 @@ public final class BrowserViewController */ private boolean mCachedDoBrowserControlsShrinkRendererSize; - public BrowserViewController( - FragmentWindowAndroid windowAndroid, View.OnAttachStateChangeListener listener) { + public BrowserViewController(FragmentWindowAndroid windowAndroid, + View.OnAttachStateChangeListener listener, @Nullable State savedState) { mWindowAndroid = windowAndroid; mOnAttachedStateChangeListener = listener; Context context = mWindowAndroid.getContext().get(); @@ -83,10 +97,12 @@ public BrowserViewController( mContentViewRenderView.onNativeLibraryLoaded( mWindowAndroid, ContentViewRenderView.MODE_SURFACE_VIEW); mTopControlsContainerView = - new BrowserControlsContainerView(context, mContentViewRenderView, this, true); + new BrowserControlsContainerView(context, mContentViewRenderView, this, true, + (savedState == null) ? null : savedState.mTopControlsState); mTopControlsContainerView.setId(View.generateViewId()); mBottomControlsContainerView = - new BrowserControlsContainerView(context, mContentViewRenderView, this, false); + new BrowserControlsContainerView(context, mContentViewRenderView, this, false, + (savedState == null) ? null : savedState.mBottomControlsState); mBottomControlsContainerView.setId(View.generateViewId()); mContentView = ContentView.createContentView( context, mTopControlsContainerView.getEventOffsetHandler(), null /* webContents */); @@ -275,6 +291,11 @@ public void onLastDialogDismissed() { onDialogVisibilityChanged(false); } + /* package */ State getState() { + return new State( + mTopControlsContainerView.getState(), mBottomControlsContainerView.getState()); + } + private void onDialogVisibilityChanged(boolean showing) { if (WebLayerFactoryImpl.getClientMajorVersion() < 82) return;