VR: Disable back button when back would background Chrome.

Refactors some code from CTAs back button flow to be reusable by VR to
know in advance whether clicking the back button will backgroud chrome
(or try to show Chrome Home when that's enabled, which isn't supported
in VR anyways).

Bug: 732475
Change-Id: Ife41c456fac880f55ba3190f257fa7ed340d00e0
Reviewed-on: https://chromium-review.googlesource.com/571289
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Reviewed-by: Yash Malik <ymalik@chromium.org>
Reviewed-by: Maria Khomenko <mariakhomenko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488283}

chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java

@@ -1773,16 +1773,7 @@ public class ChromeTabbedActivity
             return true;
         }
 
-        // [true]: Reached the bottom of the back stack on a tab the user did not explicitly
-        // create (i.e. it was created by an external app or opening a link in background, etc).
-        // [false]: Reached the bottom of the back stack on a tab that the user explicitly
-        // created (e.g. selecting "new tab" from menu).
-        final int parentId = currentTab.getParentId();
-        final boolean shouldCloseTab = type == TabLaunchType.FROM_LINK
-                || type == TabLaunchType.FROM_EXTERNAL_APP
-                || type == TabLaunchType.FROM_LONGPRESS_FOREGROUND
-                || type == TabLaunchType.FROM_LONGPRESS_BACKGROUND
-                || (type == TabLaunchType.FROM_RESTORE && parentId != Tab.INVALID_TAB_ID);
+        final boolean shouldCloseTab = backShouldCloseTab(currentTab);
 
         // Minimize the app if either:
         // - we decided not to close the tab
@@ -1790,12 +1781,6 @@ public class ChromeTabbedActivity
         //   exit Chrome on top of closing the tab
         final boolean minimizeApp = !shouldCloseTab || currentTab.isCreatedForExternalApp();
         if (minimizeApp) {
-            // TODO(mthiesse): We never want to close Chrome through the in-vr back button (but we
-            // want to reuse CTA logic for how the back button should otherwise behave). We should
-            // refactor the behaviour in this function so that we can either re-use the parts of
-            // this behaviour we want, or be able to know in advance whether or not clicking the
-            // back button would close Chrome so that we can disable it.
-            if (VrShellDelegate.isInVr()) return true;
             if (shouldCloseTab) {
                 recordBackPressedUma("Minimized and closed tab", BACK_PRESSED_MINIMIZED_TAB_CLOSED);
                 mActivityStopMetrics.setStopReason(ActivityStopMetrics.STOP_REASON_BACK_BUTTON);
@@ -1818,6 +1803,23 @@ public class ChromeTabbedActivity
         return false;
     }
 
+    /**
+     * [true]: Reached the bottom of the back stack on a tab the user did not explicitly
+     * create (i.e. it was created by an external app or opening a link in background, etc).
+     * [false]: Reached the bottom of the back stack on a tab that the user explicitly
+     * created (e.g. selecting "new tab" from menu).
+     *
+     * @return Whether pressing the back button on the provided Tab should close the Tab.
+     */
+    public static boolean backShouldCloseTab(Tab tab) {
+        TabLaunchType type = tab.getLaunchType();
+
+        return type == TabLaunchType.FROM_LINK || type == TabLaunchType.FROM_EXTERNAL_APP
+                || type == TabLaunchType.FROM_LONGPRESS_FOREGROUND
+                || type == TabLaunchType.FROM_LONGPRESS_BACKGROUND
+                || (type == TabLaunchType.FROM_RESTORE && tab.getParentId() != Tab.INVALID_TAB_ID);
+    }
+
     /**
      * Sends this Activity to the background.
      *

chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShell.java

@@ -54,4 +54,9 @@ public interface VrShell {
      * Returns the GVRLayout as a FrameLayout.
      */
     FrameLayout getContainer();
+
+    /**
+     * Returns whether the back button is enabled.
+     */
+    Boolean isBackButtonEnabled();
 }

chrome/android/java/src/org/chromium/chrome/browser/vr_shell/VrShellImpl.java

@@ -33,7 +33,6 @@ import org.chromium.chrome.browser.ChromeActivity;
 import org.chromium.chrome.browser.ChromeTabbedActivity;
 import org.chromium.chrome.browser.NativePage;
 import org.chromium.chrome.browser.UrlConstants;
-import org.chromium.chrome.browser.ntp.NewTabPage;
 import org.chromium.chrome.browser.tab.EmptyTabObserver;
 import org.chromium.chrome.browser.tab.InterceptNavigationDelegateImpl;
 import org.chromium.chrome.browser.tab.Tab;
@@ -673,12 +672,14 @@ public class VrShellImpl
 
     @CalledByNative
     public void navigateForward() {
+        if (!mCanGoForward) return;
         mActivity.getToolbarManager().forward();
         updateHistoryButtonsVisibility();
     }
 
     @CalledByNative
     public void navigateBack() {
+        if (!mCanGoBack) return;
         if (mActivity instanceof ChromeTabbedActivity) {
             // TODO(mthiesse): We should do this for custom tabs as well, as back for custom tabs
             // is also expected to close tabs.
@@ -694,11 +695,14 @@ public class VrShellImpl
             nativeSetHistoryButtonsEnabled(mNativeVrShell, false, false);
             return;
         }
-        // Hitting back when on the NTP usually closes Chrome, which we don't allow in VR, so we
-        // just disable the back button.
-        boolean shouldAlwaysGoBack = mActivity instanceof ChromeTabbedActivity
-                && (mNativePage == null || !(mNativePage instanceof NewTabPage));
-        boolean canGoBack = mTab.canGoBack() || shouldAlwaysGoBack;
+        boolean willCloseTab = false;
+        if (mActivity instanceof ChromeTabbedActivity) {
+            // If hitting back would minimize Chrome, disable the back button.
+            // See ChromeTabbedActivity#handleBackPressed().
+            willCloseTab = ChromeTabbedActivity.backShouldCloseTab(mTab)
+                    && !mTab.isCreatedForExternalApp();
+        }
+        boolean canGoBack = mTab.canGoBack() || willCloseTab;
         boolean canGoForward = mTab.canGoForward();
         if ((mCanGoBack != null && canGoBack == mCanGoBack)
                 && (mCanGoForward != null && canGoForward == mCanGoForward)) {
@@ -741,6 +745,12 @@ public class VrShellImpl
         mOnDispatchTouchEventForTesting = callback;
     }
 
+    @VisibleForTesting
+    @Override
+    public Boolean isBackButtonEnabled() {
+        return mCanGoBack;
+    }
+
     private native long nativeInit(VrShellDelegate delegate, long nativeWindowAndroid,
             boolean forWebVR, boolean webVrAutopresentationExpected, boolean inCct, long gvrApi,
             boolean reprojectedRendering);

chrome/android/javatests/src/org/chromium/chrome/browser/vr_shell/VrShellNavigationTest.java

@@ -18,11 +18,14 @@ import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
+import org.chromium.base.ThreadUtils;
 import org.chromium.base.test.util.CommandLineFlags;
 import org.chromium.base.test.util.Restriction;
 import org.chromium.chrome.browser.ChromeSwitches;
 import org.chromium.chrome.browser.history.HistoryItemView;
 import org.chromium.chrome.browser.history.HistoryPage;
+import org.chromium.chrome.browser.tab.Tab;
+import org.chromium.chrome.browser.tabmodel.TabModel.TabLaunchType;
 import org.chromium.chrome.browser.vr_shell.util.VrInfoBarUtils;
 import org.chromium.chrome.browser.vr_shell.util.VrTransitionUtils;
 import org.chromium.chrome.test.ChromeActivityTestCaseBase;
@@ -273,4 +276,27 @@ public class VrShellNavigationTest {
         assertState(mVrTestRule.getFirstTabWebContents(), Page.PAGE_WEBVR,
                 PresentationMode.NON_PRESENTING, FullscreenMode.NON_FULLSCREENED);
     }
+
+    /**
+     * Tests navigation from a fullscreened WebVR to a WebVR page.
+     */
+    @Test
+    @MediumTest
+    @Restriction(RESTRICTION_TYPE_VIEWER_DAYDREAM)
+    public void testBackDoesntBackgroundChrome()
+            throws IllegalArgumentException, InterruptedException, TimeoutException {
+        Assert.assertFalse("Back button isn't disabled.", VrTransitionUtils.isBackButtonEnabled());
+        mVrTestRule.loadUrlInNewTab(getUrl(Page.PAGE_2D), false, TabLaunchType.FROM_CHROME_UI);
+        Assert.assertFalse("Back button isn't disabled.", VrTransitionUtils.isBackButtonEnabled());
+        final Tab tab =
+                mVrTestRule.loadUrlInNewTab(getUrl(Page.PAGE_2D), false, TabLaunchType.FROM_LINK);
+        Assert.assertTrue("Back button isn't enabled.", VrTransitionUtils.isBackButtonEnabled());
+        ThreadUtils.runOnUiThreadBlocking(new Runnable() {
+            @Override
+            public void run() {
+                mVrTestRule.getActivity().getTabModelSelector().closeTab(tab);
+            }
+        });
+        Assert.assertFalse("Back button isn't disabled.", VrTransitionUtils.isBackButtonEnabled());
+    }
 }

chrome/android/javatests/src/org/chromium/chrome/browser/vr_shell/util/VrTransitionUtils.java

@@ -20,6 +20,7 @@ import org.chromium.content_public.browser.WebContents;
 
 import java.util.concurrent.Callable;
 import java.util.concurrent.TimeoutException;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 /**
  * Class containing utility functions for transitioning between different
@@ -114,4 +115,19 @@ public class VrTransitionUtils {
                 "vrDisplay.isPresenting", POLL_TIMEOUT_LONG_MS, cvc.getWebContents());
         Assert.assertTrue(VrShellDelegate.getVrShellForTesting().getWebVrModeEnabled());
     }
+
+    /**
+     * @return Whether the VR back button is enabled.
+     */
+    public static Boolean isBackButtonEnabled() {
+        final AtomicBoolean isBackButtonEnabled = new AtomicBoolean();
+        ThreadUtils.runOnUiThreadBlocking(new Runnable() {
+            @Override
+            public void run() {
+                isBackButtonEnabled.set(
+                        VrShellDelegate.getVrShellForTesting().isBackButtonEnabled());
+            }
+        });
+        return isBackButtonEnabled.get();
+    }
 }

chrome/test/android/javatests/src/org/chromium/chrome/test/ChromeActivityTestCommon.java

@@ -224,7 +224,7 @@ final class ChromeActivityTestCommon<T extends ChromeActivity> {
     /**
      * Navigates to a URL directly without going through the UrlBar. This bypasses the page
      * preloading mechanism of the UrlBar.
-     * @param url            The url to load in the current tab.
+     * @param url            The URL to load in the current tab.
      * @param secondsToWait  The number of seconds to wait for the page to be loaded.
      * @return FULL_PRERENDERED_PAGE_LOAD or PARTIAL_PRERENDERED_PAGE_LOAD if the page has been
      *         prerendered. DEFAULT_PAGE_LOAD if it had not.
@@ -238,7 +238,7 @@ final class ChromeActivityTestCommon<T extends ChromeActivity> {
     /**
      * Navigates to a URL directly without going through the UrlBar. This bypasses the page
      * preloading mechanism of the UrlBar.
-     * @param url The url to load in the current tab.
+     * @param url The URL to load in the current tab.
      * @return FULL_PRERENDERED_PAGE_LOAD or PARTIAL_PRERENDERED_PAGE_LOAD if the page has been
      *         prerendered. DEFAULT_PAGE_LOAD if it had not.
      */
@@ -248,18 +248,18 @@ final class ChromeActivityTestCommon<T extends ChromeActivity> {
     }
 
     /**
-     * @param url            The url of the page to load.
+     * @param url            The URL of the page to load.
      * @param pageTransition The type of transition. see
      *                       {@link org.chromium.ui.base.PageTransition}
      *                       for valid values.
-     * @param tab            The tab to load the url into.
+     * @param tab            The tab to load the URL into.
      * @param secondsToWait  The number of seconds to wait for the page to be loaded.
      * @return               FULL_PRERENDERED_PAGE_LOAD or PARTIAL_PRERENDERED_PAGE_LOAD if the
      *                       page has been prerendered. DEFAULT_PAGE_LOAD if it had not.
      */
     int loadUrlInTab(final String url, final int pageTransition, final Tab tab, long secondsToWait)
             throws InterruptedException {
-        Assert.assertNotNull("Cannot load the url in a null tab", tab);
+        Assert.assertNotNull("Cannot load the URL in a null tab", tab);
         final AtomicInteger result = new AtomicInteger();
 
         ChromeTabUtils.waitForTabPageLoaded(tab, new Runnable() {
@@ -278,11 +278,11 @@ final class ChromeActivityTestCommon<T extends ChromeActivity> {
     }
 
     /**
-     * @param url            The url of the page to load.
+     * @param url            The URL of the page to load.
      * @param pageTransition The type of transition. see
      *                       {@link org.chromium.ui.base.PageTransition}
      *                       for valid values.
-     * @param tab            The tab to load the url into.
+     * @param tab            The tab to load the URL into.
      * @return               FULL_PRERENDERED_PAGE_LOAD or PARTIAL_PRERENDERED_PAGE_LOAD if the
      *                       page has been prerendered. DEFAULT_PAGE_LOAD if it had not.
      */
@@ -292,26 +292,38 @@ final class ChromeActivityTestCommon<T extends ChromeActivity> {
     }
 
     /**
-     * Load a url in a new tab. The {@link Tab} will pretend to be created from a link.
-     * @param url The url of the page to load.
+     * Load a URL in a new tab. The {@link Tab} will pretend to be created from a link.
+     * @param url The URL of the page to load.
      */
     Tab loadUrlInNewTab(final String url) throws InterruptedException {
         return loadUrlInNewTab(url, false);
     }
 
     /**
-     * Load a url in a new tab. The {@link Tab} will pretend to be created from a link.
-     * @param url The url of the page to load.
+     * Load a URL in a new tab. The {@link Tab} will pretend to be created from a link.
+     * @param url The URL of the page to load.
      * @param incognito Whether the new tab should be incognito.
      */
     Tab loadUrlInNewTab(final String url, final boolean incognito) throws InterruptedException {
+        return loadUrlInNewTab(url, incognito, TabLaunchType.FROM_LINK);
+    }
+
+    /**
+     * Load a URL in a new tab, with the given transition type.
+     *
+     * @param url The URL of the page to load.
+     * @param incognito Whether the new tab should be incognito.
+     * @param launchType The type of Tab Launch.
+     */
+    Tab loadUrlInNewTab(final String url, final boolean incognito, final TabLaunchType launchType)
+            throws InterruptedException {
         Tab tab = null;
         try {
             tab = ThreadUtils.runOnUiThreadBlocking(new Callable<Tab>() {
                 @Override
                 public Tab call() throws Exception {
                     return mCallback.getActivity().getTabCreator(incognito).launchUrl(
-                            url, TabLaunchType.FROM_LINK);
+                            url, launchType);
                 }
             });
         } catch (ExecutionException e) {

chrome/test/android/javatests/src/org/chromium/chrome/test/ChromeActivityTestRule.java

@@ -19,6 +19,7 @@ import org.chromium.chrome.browser.infobar.InfoBar;
 import org.chromium.chrome.browser.omnibox.OmniboxSuggestion;
 import org.chromium.chrome.browser.preferences.Preferences;
 import org.chromium.chrome.browser.tab.Tab;
+import org.chromium.chrome.browser.tabmodel.TabModel.TabLaunchType;
 import org.chromium.chrome.test.ChromeActivityTestCommon.ChromeTestCommonCallback;
 import org.chromium.chrome.test.util.ApplicationTestUtils;
 
@@ -26,6 +27,11 @@ import java.util.List;
 import java.util.concurrent.TimeoutException;
 
 //TODO(yolandyan): break this test rule down to smaller rules once the junit4 migration is over
+/**
+ * Custom  {@link ActivityTestRule} for test using  {@link ChromeActivity}.
+ *
+ * @param <T> The {@link Activity} class under test.
+ */
 public class ChromeActivityTestRule<T extends ChromeActivity>
         extends ActivityTestRule<T> implements ChromeTestCommonCallback<T> {
     private final ChromeActivityTestCommon<T> mTestCommon;
@@ -140,7 +146,7 @@ public class ChromeActivityTestRule<T extends ChromeActivity>
     /**
      * Navigates to a URL directly without going through the UrlBar. This bypasses the page
      * preloading mechanism of the UrlBar.
-     * @param url            The url to load in the current tab.
+     * @param url            The URL to load in the current tab.
      * @param secondsToWait  The number of seconds to wait for the page to be loaded.
      * @return FULL_PRERENDERED_PAGE_LOAD or PARTIAL_PRERENDERED_PAGE_LOAD if the page has been
      *         prerendered. DEFAULT_PAGE_LOAD if it had not.
@@ -153,7 +159,7 @@ public class ChromeActivityTestRule<T extends ChromeActivity>
     /**
      * Navigates to a URL directly without going through the UrlBar. This bypasses the page
      * preloading mechanism of the UrlBar.
-     * @param url The url to load in the current tab.
+     * @param url The URL to load in the current tab.
      * @return FULL_PRERENDERED_PAGE_LOAD or PARTIAL_PRERENDERED_PAGE_LOAD if the page has been
      *         prerendered. DEFAULT_PAGE_LOAD if it had not.
      */
@@ -162,11 +168,11 @@ public class ChromeActivityTestRule<T extends ChromeActivity>
     }
 
     /**
-     * @param url            The url of the page to load.
+     * @param url            The URL of the page to load.
      * @param pageTransition The type of transition. see
      *                       {@link org.chromium.ui.base.PageTransition}
      *                       for valid values.
-     * @param tab            The tab to load the url into.
+     * @param tab            The tab to load the URL into.
      * @param secondsToWait  The number of seconds to wait for the page to be loaded.
      * @return               FULL_PRERENDERED_PAGE_LOAD or PARTIAL_PRERENDERED_PAGE_LOAD if the
      *                       page has been prerendered. DEFAULT_PAGE_LOAD if it had not.
@@ -177,11 +183,11 @@ public class ChromeActivityTestRule<T extends ChromeActivity>
     }
 
     /**
-     * @param url            The url of the page to load.
+     * @param url            The URL of the page to load.
      * @param pageTransition The type of transition. see
      *                       {@link org.chromium.ui.base.PageTransition}
      *                       for valid values.
-     * @param tab            The tab to load the url into.
+     * @param tab            The tab to load the URL into.
      * @return               FULL_PRERENDERED_PAGE_LOAD or PARTIAL_PRERENDERED_PAGE_LOAD if the
      *                       page has been prerendered. DEFAULT_PAGE_LOAD if it had not.
      */
@@ -190,16 +196,16 @@ public class ChromeActivityTestRule<T extends ChromeActivity>
     }
 
     /**
-     * Load a url in a new tab. The {@link Tab} will pretend to be created from a link.
-     * @param url The url of the page to load.
+     * Load a URL in a new tab. The {@link Tab} will pretend to be created from a link.
+     * @param url The URL of the page to load.
      */
     public Tab loadUrlInNewTab(String url) throws InterruptedException {
         return mTestCommon.loadUrlInNewTab(url);
     }
 
     /**
-     * Load a url in a new tab. The {@link Tab} will pretend to be created from a link.
-     * @param url The url of the page to load.
+     * Load a URL in a new tab. The {@link Tab} will pretend to be created from a link.
+     * @param url The URL of the page to load.
      * @param incognito Whether the new tab should be incognito.
      */
     public Tab loadUrlInNewTab(final String url, final boolean incognito)
@@ -207,6 +213,17 @@ public class ChromeActivityTestRule<T extends ChromeActivity>
         return mTestCommon.loadUrlInNewTab(url, incognito);
     }
 
+    /**
+     * Load a URL in a new tab, with the given transition type.
+     * @param url The URL of the page to load.
+     * @param incognito Whether the new tab should be incognito.
+     * @param launchType The type of Tab Launch.
+     */
+    public Tab loadUrlInNewTab(final String url, final boolean incognito,
+            final TabLaunchType launchType) throws InterruptedException {
+        return mTestCommon.loadUrlInNewTab(url, incognito, launchType);
+    }
+
     /**
      * Simulates starting Main Activity from launcher, blocks until it is started.
      */