Skip to content

Commit

Permalink
[StartTablet] Fix transitions between Start surface and NTP homepage …
Browse files Browse the repository at this point in the history
…on foldable is broken.

The bug exists when the "Start surface refactor" is enabled. In this
CL, we update the check of whether Start surface | Tab switcher is
showing and it works with refactor flag is enabled and disabled.

(cherry picked from commit 38adf0e)

Bug: 1451844
Change-Id: I0b741fc70851e7484840e8d8af603e2e82c09bb1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4595923
Commit-Queue: Xi Han <hanxi@chromium.org>
Reviewed-by: Aishwarya Rajesh <aishwaryarj@google.com>
Code-Coverage: Findit <findit-for-me@appspot.gserviceaccount.com>
Cr-Original-Commit-Position: refs/heads/main@{#1154477}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4604830
Cr-Commit-Position: refs/branch-heads/5790@{#539}
Cr-Branched-From: 1d71a33-refs/heads/main@{#1148114}
  • Loading branch information
Xi Han authored and Chromium LUCI CQ committed Jun 9, 2023
1 parent 35d4c59 commit 4874ebb
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 17 deletions.
Expand Up @@ -89,11 +89,23 @@ public void saveUiState(
savedInstanceState.putBoolean(KEYBOARD_VISIBILITY_STATE, true);
}

if (mLayoutManagerSupplier.hasValue()
&& mLayoutManagerSupplier.get().isLayoutVisible(LayoutType.TAB_SWITCHER)) {
savedInstanceState.putBoolean(TAB_SWITCHER_VISIBILITY_STATE, true);

saveHomeSurfaceState(savedInstanceState, mStartSurfaceSupplier.get(), isIncognito);
if (mLayoutManagerSupplier.hasValue()) {
// Start surface used a TAB_SWITCHER LayoutType before the START_SURFACE_REFACTOR
// feature was supported. Therefore, it is safe to assume that the Tab switcher layout
// is actually visible only when the Start surface home page is not showing, and the Tab
// switcher visibility state will be saved to the instance state only in this case.
// TODO(https://crbug.com/1315676): Clean up the logic once the START_SURFACE_REFACTOR
// flag is enabled by default.
if (mLayoutManagerSupplier.get().isLayoutVisible(LayoutType.TAB_SWITCHER)
|| mLayoutManagerSupplier.get().isLayoutVisible(LayoutType.START_SURFACE)) {
if (mStartSurfaceSupplier.hasValue()
&& mStartSurfaceSupplier.get().isHomepageShown()) {
saveHomeSurfaceState(
savedInstanceState, mStartSurfaceSupplier.get(), isIncognito);
} else {
savedInstanceState.putBoolean(TAB_SWITCHER_VISIBILITY_STATE, true);
}
}
}
}

Expand Down Expand Up @@ -130,9 +142,7 @@ static void saveHomeSurfaceState(
return;
}

if (startSurface.isHomepageShown()) {
savedInstanceState.putBoolean(RESUME_HOME_SURFACE_ON_MODE_CHANGE, true);
}
savedInstanceState.putBoolean(RESUME_HOME_SURFACE_ON_MODE_CHANGE, true);
}

@VisibleForTesting
Expand Down
Expand Up @@ -82,6 +82,8 @@ public class FoldTransitionControllerUnitTest {
private Bundle mSavedInstanceState;
@Mock
private StartSurface mStartSurface;
@Mock
private OneshotSupplierImpl<StartSurface> mStartSurfaceSupplier;

private FoldTransitionController mFoldTransitionController;

Expand Down Expand Up @@ -245,25 +247,54 @@ public void testSaveUiState_tabSwitcherNotVisible() {

@Test
public void testSaveHomeSurfaceState() {
doReturn(true).when(mSavedInstanceState).getBoolean(DID_CHANGE_TABLET_MODE);

saveHomeSurfaceState(null, mStartSurface, false);
saveHomeSurfaceState(mSavedInstanceState, null, false);
saveHomeSurfaceState(mSavedInstanceState, mStartSurface, false);
saveHomeSurfaceState(mSavedInstanceState, mStartSurface, true);
verify(mSavedInstanceState, never()).putBoolean(RESUME_HOME_SURFACE_ON_MODE_CHANGE, true);
verify(mStartSurface, never()).isHomepageShown();

// Verifies that saved instance state should not contain RESUME_HOME_SURFACE_ON_MODE_CHANGE
// if Start surface isn't shown on phone.
// Verifies that saved instance state will have RESUME_HOME_SURFACE_ON_MODE_CHANGE == true
// if Start surface is shown on phone.
saveHomeSurfaceState(mSavedInstanceState, mStartSurface, false);
verify(mSavedInstanceState).putBoolean(RESUME_HOME_SURFACE_ON_MODE_CHANGE, true);
}

@Test
public void testSaveUiStateWithTabSwitcherAndStartSurface() {
doReturn(true).when(mSavedInstanceState).getBoolean(DID_CHANGE_TABLET_MODE);
doReturn(true).when(mLayoutManager).isLayoutVisible(LayoutType.TAB_SWITCHER);
// Sets Start surface is disabled.
doReturn(false).when(mStartSurfaceSupplier).hasValue();
// Tests the case when Tab switcher is showing with Start surface disabled.
mFoldTransitionController.saveUiState(mSavedInstanceState, true, false);
verify(mSavedInstanceState, never()).putBoolean(RESUME_HOME_SURFACE_ON_MODE_CHANGE, true);
verify(mSavedInstanceState).putBoolean(TAB_SWITCHER_VISIBILITY_STATE, true);

// Sets Start surface is enabled.
doReturn(true).when(mStartSurfaceSupplier).hasValue();
doReturn(mStartSurface).when(mStartSurfaceSupplier).get();
// Tests the case when Tab switcher is showing with Start surface enabled.
doReturn(false).when(mStartSurface).isHomepageShown();
saveHomeSurfaceState(mSavedInstanceState, mStartSurface, false);
mFoldTransitionController.saveUiState(mSavedInstanceState, true, false);
verify(mSavedInstanceState, never()).putBoolean(RESUME_HOME_SURFACE_ON_MODE_CHANGE, true);
verify(mSavedInstanceState, times(2)).putBoolean(TAB_SWITCHER_VISIBILITY_STATE, true);

// Verifies that saved instance state will have RESUME_HOME_SURFACE_ON_MODE_CHANGE == true
// if Start surface is shown on phone.
// Tests the case when Start surface is showing with "Start surface refactor" disabled.
doReturn(true).when(mStartSurface).isHomepageShown();
saveHomeSurfaceState(mSavedInstanceState, mStartSurface, false);
doReturn(true).when(mLayoutManager).isLayoutVisible(LayoutType.TAB_SWITCHER);
mFoldTransitionController.saveUiState(mSavedInstanceState, true, false);
verify(mSavedInstanceState).putBoolean(RESUME_HOME_SURFACE_ON_MODE_CHANGE, true);
verify(mSavedInstanceState, times(2)).putBoolean(TAB_SWITCHER_VISIBILITY_STATE, true);

// Sets the layout when "Start surface refactor" is enabled.
doReturn(false).when(mLayoutManager).isLayoutVisible(LayoutType.TAB_SWITCHER);
doReturn(true).when(mLayoutManager).isLayoutVisible(LayoutType.START_SURFACE);
// Tests the case when Start surface is showing with "Start surface refactor" enabled:
doReturn(true).when(mStartSurface).isHomepageShown();
mFoldTransitionController.saveUiState(mSavedInstanceState, true, false);
verify(mSavedInstanceState, times(2)).putBoolean(RESUME_HOME_SURFACE_ON_MODE_CHANGE, true);
verify(mSavedInstanceState, times(2)).putBoolean(TAB_SWITCHER_VISIBILITY_STATE, true);
}

@Test
Expand Down Expand Up @@ -370,6 +401,6 @@ private void initializeController() {
var layoutManagerSupplier = new ObservableSupplierImpl<LayoutManager>();
layoutManagerSupplier.set(mLayoutManager);
mFoldTransitionController = new FoldTransitionController(toolbarManagerSupplier,
layoutManagerSupplier, mActivityTabProvider, new OneshotSupplierImpl<>(), mHandler);
layoutManagerSupplier, mActivityTabProvider, mStartSurfaceSupplier, mHandler);
}
}

0 comments on commit 4874ebb

Please sign in to comment.