Commit 27749786 authored by Mei Liang's avatar Mei Liang Committed by Commit Bot

Avoid exposing StartSurface.OverviewModeObserver in StartSurfaceLayout

This is a follow up for crrev.com/c/2483031. StartSurfaceLayout
implements StartSurface.OverviewModeObserver#finishedShowing and
overrides Layout#doneShowing. These two methods have similar names, and
doneShowing() depends on finishedShowing(). It is hard to reason about
without a dive in investigation.

This CL avoids the confusion between these two methods by modifying
StartSurfaceLayout to stop implementing
StartSurface.OverviewModeObserver. Instead, StartSurfaceLayout
instantiates a StartSurface.OverviewModeObserver member and attaches
that member instance to StartSurfaceController directly.

Change-Id: I5904513b5fcd72fef3b571ecb513c334bd4a2d06
Bug: 1108496
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2503392
Commit-Queue: Mei Liang <meiliang@chromium.org>
Auto-Submit: Mei Liang <meiliang@chromium.org>
Reviewed-by: default avatarWei-Yin Chen (陳威尹) <wychen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823852}
parent edd0bc3e
...@@ -52,7 +52,7 @@ import java.util.Locale; ...@@ -52,7 +52,7 @@ import java.util.Locale;
/** /**
* A {@link Layout} that shows all tabs in one grid or carousel view. * A {@link Layout} that shows all tabs in one grid or carousel view.
*/ */
public class StartSurfaceLayout extends Layout implements StartSurface.OverviewModeObserver { public class StartSurfaceLayout extends Layout {
private static final String TAG = "SSLayout"; private static final String TAG = "SSLayout";
// Duration of the transition animation // Duration of the transition animation
...@@ -66,6 +66,7 @@ public class StartSurfaceLayout extends Layout implements StartSurface.OverviewM ...@@ -66,6 +66,7 @@ public class StartSurfaceLayout extends Layout implements StartSurface.OverviewM
private TabListSceneLayer mSceneLayer; private TabListSceneLayer mSceneLayer;
private final StartSurface mStartSurface; private final StartSurface mStartSurface;
private final StartSurface.Controller mController; private final StartSurface.Controller mController;
private final StartSurface.OverviewModeObserver mStartSurfaceObserver;
private final TabSwitcher.TabListDelegate mTabListDelegate; private final TabSwitcher.TabListDelegate mTabListDelegate;
// To force Toolbar finishes its animation when this Layout finished hiding. // To force Toolbar finishes its animation when this Layout finished hiding.
private final LayoutTab mDummyLayoutTab; private final LayoutTab mDummyLayoutTab;
...@@ -97,25 +98,8 @@ public class StartSurfaceLayout extends Layout implements StartSurface.OverviewM ...@@ -97,25 +98,8 @@ public class StartSurfaceLayout extends Layout implements StartSurface.OverviewM
mStartSurface = startSurface; mStartSurface = startSurface;
mStartSurface.setOnTabSelectingListener(this::onTabSelecting); mStartSurface.setOnTabSelectingListener(this::onTabSelecting);
mController = mStartSurface.getController(); mController = mStartSurface.getController();
mController.addOverviewModeObserver(this);
mTabListDelegate = mStartSurface.getTabListDelegate();
if (TabUiFeatureUtilities.isTabThumbnailAspectRatioNotOne()) {
mThumbnailAspectRatio = (float) TabUiFeatureUtilities.THUMBNAIL_ASPECT_RATIO.getValue();
mThumbnailAspectRatio = MathUtils.clamp(mThumbnailAspectRatio, 0.5f, 2.0f);
}
}
@Override mStartSurfaceObserver = new StartSurface.OverviewModeObserver() {
public void onFinishNativeInitialization() {
if (mIsInitialized) return;
mIsInitialized = true;
mStartSurface.initWithNative();
ensureSceneLayerCreated();
mSceneLayer.setTabModelSelector(mTabModelSelector);
}
// StartSurface.OverviewModeObserver implementation.
@Override @Override
public void startedShowing() { public void startedShowing() {
mAndroidViewFinishedShowing = false; mAndroidViewFinishedShowing = false;
...@@ -125,12 +109,13 @@ public class StartSurfaceLayout extends Layout implements StartSurface.OverviewM ...@@ -125,12 +109,13 @@ public class StartSurfaceLayout extends Layout implements StartSurface.OverviewM
public void finishedShowing() { public void finishedShowing() {
mAndroidViewFinishedShowing = true; mAndroidViewFinishedShowing = true;
doneShowing(); doneShowing();
// The Tab-to-GTS animation is done, and it's time to renew the thumbnail without causing // The Tab-to-GTS animation is done, and it's time to renew the thumbnail without
// janky frames. // causing janky frames. When animation is off, the thumbnail is already updated
// When animation is off, the thumbnail is already updated when showing the GTS. // when showing the GTS.
if (TabUiFeatureUtilities.isTabToGtsAnimationEnabled()) { if (TabUiFeatureUtilities.isTabToGtsAnimationEnabled()) {
// Delay thumbnail taking a bit more to make it less likely to happen before the // Delay thumbnail taking a bit more to make it less likely to happen before the
// thumbnail taking triggered by ThumbnailFetcher. See crbug.com/996385 for details. // thumbnail taking triggered by ThumbnailFetcher. See crbug.com/996385 for
// details.
new Handler().postDelayed(() -> { new Handler().postDelayed(() -> {
Tab currentTab = mTabModelSelector.getCurrentTab(); Tab currentTab = mTabModelSelector.getCurrentTab();
if (currentTab != null) mTabContentManager.cacheTabThumbnail(currentTab); if (currentTab != null) mTabContentManager.cacheTabThumbnail(currentTab);
...@@ -144,16 +129,36 @@ public class StartSurfaceLayout extends Layout implements StartSurface.OverviewM ...@@ -144,16 +129,36 @@ public class StartSurfaceLayout extends Layout implements StartSurface.OverviewM
@Override @Override
public void finishedHiding() { public void finishedHiding() {
// The Android View version of GTS overview is hidden. // The Android View version of GTS overview is hidden.
// If not doing GTS-to-Tab transition animation, we show the fade-out instead, which was // If not doing GTS-to-Tab transition animation, we show the fade-out instead, which
// already done. // was already done.
if (!TabUiFeatureUtilities.isTabToGtsAnimationEnabled()) { if (!TabUiFeatureUtilities.isTabToGtsAnimationEnabled()) {
postHiding(); postHiding();
return; return;
} }
// If we are doing GTS-to-Tab transition animation, we start showing the Bitmap version of // If we are doing GTS-to-Tab transition animation, we start showing the Bitmap
// the GTS overview in the background while expanding the thumbnail to the viewport. // version of the GTS overview in the background while expanding the thumbnail to
// the viewport.
expandTab(mTabListDelegate.getThumbnailLocationOfCurrentTab(true)); expandTab(mTabListDelegate.getThumbnailLocationOfCurrentTab(true));
} }
};
mController.addOverviewModeObserver(mStartSurfaceObserver);
mTabListDelegate = mStartSurface.getTabListDelegate();
if (TabUiFeatureUtilities.isTabThumbnailAspectRatioNotOne()) {
mThumbnailAspectRatio = (float) TabUiFeatureUtilities.THUMBNAIL_ASPECT_RATIO.getValue();
mThumbnailAspectRatio = MathUtils.clamp(mThumbnailAspectRatio, 0.5f, 2.0f);
}
}
@Override
public void onFinishNativeInitialization() {
if (mIsInitialized) return;
mIsInitialized = true;
mStartSurface.initWithNative();
ensureSceneLayerCreated();
mSceneLayer.setTabModelSelector(mTabModelSelector);
}
// Layout implementation. // Layout implementation.
@Override @Override
...@@ -172,7 +177,7 @@ public class StartSurfaceLayout extends Layout implements StartSurface.OverviewM ...@@ -172,7 +177,7 @@ public class StartSurfaceLayout extends Layout implements StartSurface.OverviewM
@Override @Override
public void destroy() { public void destroy() {
if (mController != null) { if (mController != null) {
mController.removeOverviewModeObserver(this); mController.removeOverviewModeObserver(mStartSurfaceObserver);
} }
} }
......
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