🤝 Finish Custom Tabs early when last tab is closed.

Bug: 1087108
Change-Id: Ibf1775d96ea5600a1c2d73ee31ef4db8fbe307e8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2228617Reviewed-by: Peter Kotwicz <pkotwicz@chromium.org>
Commit-Queue: Peter Conn <peconn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#776873}

chrome/android/java/src/org/chromium/chrome/browser/customtabs/content/CustomTabActivityNavigationController.java

@@ -94,9 +94,6 @@ public class CustomTabActivityNavigationController implements StartStopWithNativ
     @Nullable
     private ToolbarManager mToolbarManager;
 
-    @Nullable
-    private BackHandler mBackHandler;
-
     @Nullable
     private FinishHandler mFinishHandler;
 
@@ -204,26 +201,19 @@ public class CustomTabActivityNavigationController implements StartStopWithNativ
             return true;
         }
 
-        if (mBackHandler != null
-                && mBackHandler.handleBackPressed(this::executeDefaultBackHandling)) {
-            return true;
-        }
+        if (mToolbarManager != null && mToolbarManager.back()) return true;
 
-        executeDefaultBackHandling();
-        return true;
-    }
-
-    private void executeDefaultBackHandling() {
-        if (mToolbarManager != null && mToolbarManager.back()) return;
-
-        // mTabController.closeTab may result in either closing the only tab (through the back
-        // button or the close button), or swapping to the previous tab. In the first case we need
-        // finish to be called with USER_NAVIGATION reason.
-        mIsHandlingUserNavigation = true;
+        if (mTabController.onlyOneTabRemaining()) {
+            // If we're closing the last tab, just finish the Activity manually. If we had called
+            // mTabController.closeTab() and waited for the Activity to close as a result we would
+            // have a visual glitch: https://crbug.com/1087108.
+            finish(USER_NAVIGATION);
+        } else {
             mTabController.closeTab();
-        mIsHandlingUserNavigation = false;
         }
 
+        return true;
+    }
     /**
      * Handles close button navigation.
      */
@@ -310,14 +300,6 @@ public class CustomTabActivityNavigationController implements StartStopWithNativ
         }
     }
 
-    /**
-     * See {@link BackHandler}. Only one BackHandler at a time should be set.
-     */
-    public void setBackHandler(BackHandler handler) {
-        assert mBackHandler == null : "Multiple BackHandlers not supported";
-        mBackHandler = handler;
-    }
-
     /**
      * Sets a {@link FinishHandler} to be notified when the custom tab is being closed.
      */

chrome/android/java/src/org/chromium/chrome/browser/customtabs/content/CustomTabActivityTabController.java

@@ -181,8 +181,13 @@ public class CustomTabActivityTabController implements InflationObserver {
      * {@link CustomTabActivityTabProvider.Observer#onAllTabsClosed}.
      */
     public void closeTab() {
-        mTabFactory.getTabModelSelector().getCurrentModel().closeTab(mTabProvider.getTab(),
-                false, false, false);
+        TabModel model = mTabFactory.getTabModelSelector().getCurrentModel();
+        model.closeTab(mTabProvider.getTab(), false, false, false);
+    }
+
+    public boolean onlyOneTabRemaining() {
+        TabModel model = mTabFactory.getTabModelSelector().getCurrentModel();
+        return model.getCount() == 1;
     }
 
     public void closeAndForgetTab() {

chrome/android/junit/src/org/chromium/chrome/browser/customtabs/content/CustomTabActivityNavigationControllerTest.java

@@ -9,8 +9,6 @@ import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.doNothing;
-import static org.mockito.Mockito.doReturn;
-import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -28,7 +26,6 @@ import org.robolectric.annotation.Config;
 
 import org.chromium.base.task.TaskTraits;
 import org.chromium.base.task.test.ShadowPostTask;
-import org.chromium.chrome.browser.customtabs.content.CustomTabActivityNavigationController.BackHandler;
 import org.chromium.chrome.browser.customtabs.content.CustomTabActivityNavigationController.FinishHandler;
 import org.chromium.chrome.browser.customtabs.content.CustomTabActivityNavigationController.FinishReason;
 import org.chromium.chrome.browser.customtabs.shadows.ShadowExternalNavigationDelegateImpl;
@@ -69,37 +66,9 @@ public class CustomTabActivityNavigationControllerTest {
         env.tabProvider.setInitialTab(tab, TabCreationMode.DEFAULT);
     }
 
-    @Test
-    public void handlesBackNavigation_IfExternalBackHandlerRejectsSynchronously() {
-        mNavigationController.setBackHandler(notHandledRunnable -> false);
-        mNavigationController.navigateOnBack();
-        verify(mTabController).closeTab();
-    }
-
-    @Test
-    public void handlesBackNavigation_IfExternalBackHandlerRejectsAsynchronously() {
-        ArgumentCaptor<Runnable> notHandledRunnableCaptor = ArgumentCaptor.forClass(Runnable.class);
-        BackHandler backHandler = mock(BackHandler.class);
-        doReturn(true).when(backHandler).handleBackPressed(notHandledRunnableCaptor.capture());
-        mNavigationController.setBackHandler(backHandler);
-        mNavigationController.navigateOnBack();
-        notHandledRunnableCaptor.getValue().run();
-        verify(mTabController).closeTab();
-    }
-
-    @Test
-    public void doesntHandlesBackNavigation_IfExternalBackHandlerAccepts() {
-        mNavigationController.setBackHandler(notHandledRunnable -> true);
-        mNavigationController.navigateOnBack();
-        verify(mTabController, never()).closeTab();
-    }
-
     @Test
     public void finishes_IfBackNavigationClosesTheOnlyTab() {
-        doAnswer((Answer<Void>) invocation -> {
-            env.tabProvider.swapTab(null);
-            return null;
-        }).when(mTabController).closeTab();
+        when(mTabController.onlyOneTabRemaining()).thenReturn(true);
 
         mNavigationController.navigateOnBack();
         verify(mFinishHandler).onFinish(eq(FinishReason.USER_NAVIGATION));