Fix UrlOverridingTest#testOpenWindowFromUserGesture for touchless.

Fixes handling of intent URLs in touchless mode.
window.open('intent://...'); should now fire the BROWSABLE intent as
expected. We do this by special casing the 'new window opens in CCT'
logic to ignore intent:// schemes, and handle the resulting New Tab
View Intent with the ExternalNavigationHandler.

Bug: 983699
Change-Id: Ic28930db53686ae46c55496554e7388d12aa937c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1733458Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Commit-Queue: Michael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#683667}

chrome/android/java/src/org/chromium/chrome/browser/tab/InterceptNavigationDelegateImpl.java

@@ -127,7 +127,8 @@ public class InterceptNavigationDelegateImpl implements InterceptNavigationDeleg
                 .setTab(mTab)
                 .setOpenInNewTab(true)
                 .build();
-        return mExternalNavHandler.shouldOverrideUrlLoading(params)
+        mLastOverrideUrlLoadingResult = mExternalNavHandler.shouldOverrideUrlLoading(params);
+        return mLastOverrideUrlLoadingResult
                 != ExternalNavigationHandler.OverrideUrlLoadingResult.NO_OVERRIDE;
     }
 

chrome/android/javatests/src/org/chromium/chrome/browser/externalnav/UrlOverridingTest.java

@@ -36,6 +36,7 @@ import org.chromium.chrome.browser.tab.EmptyTabObserver;
 import org.chromium.chrome.browser.tab.InterceptNavigationDelegateImpl;
 import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.chrome.browser.tabmodel.EmptyTabModelSelectorObserver;
+import org.chromium.chrome.browser.tabmodel.SingleTabModel;
 import org.chromium.chrome.test.ChromeActivityTestRule;
 import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
 import org.chromium.content_public.browser.LoadUrlParams;
@@ -92,10 +93,13 @@ public class UrlOverridingTest {
     private static class TestTabObserver extends EmptyTabObserver {
         private final CallbackHelper mFinishCallback;
         private final CallbackHelper mFailCallback;
+        private final CallbackHelper mDestroyedCallback;
 
-        TestTabObserver(final CallbackHelper finishCallback, final CallbackHelper failCallback) {
+        TestTabObserver(final CallbackHelper finishCallback, final CallbackHelper failCallback,
+                final CallbackHelper destroyedCallback) {
             mFinishCallback = finishCallback;
             mFailCallback = failCallback;
+            mDestroyedCallback = destroyedCallback;
         }
 
         @Override
@@ -112,7 +116,7 @@ public class UrlOverridingTest {
         @Override
         public void onDestroyed(Tab tab) {
             // A new tab is destroyed when loading is overridden while opening it.
-            mFailCallback.notifyCalled();
+            mDestroyedCallback.notifyCalled();
         }
     }
 
@@ -137,14 +141,16 @@ public class UrlOverridingTest {
 
     private void loadUrlAndWaitForIntentUrl(final String url, boolean needClick,
             boolean shouldLaunchExternalIntent) throws InterruptedException {
-        loadUrlAndWaitForIntentUrl(url, needClick, 0, shouldLaunchExternalIntent, url);
+        loadUrlAndWaitForIntentUrl(url, needClick, false, shouldLaunchExternalIntent, url, true);
     }
 
     private void loadUrlAndWaitForIntentUrl(final String url, boolean needClick,
-            int expectedNewTabCount, final boolean shouldLaunchExternalIntent,
-            final String expectedFinalUrl) throws InterruptedException {
+            boolean createsNewTab, final boolean shouldLaunchExternalIntent,
+            final String expectedFinalUrl, final boolean shouldFailNavigation)
+            throws InterruptedException {
         final CallbackHelper finishCallback = new CallbackHelper();
         final CallbackHelper failCallback = new CallbackHelper();
+        final CallbackHelper destroyedCallback = new CallbackHelper();
         final CallbackHelper newTabCallback = new CallbackHelper();
 
         final Tab tab = mActivityTestRule.getActivity().getActivityTab();
@@ -153,14 +159,15 @@ public class UrlOverridingTest {
                 new InterceptNavigationDelegateImpl[1];
         latestTabHolder[0] = tab;
         latestDelegateHolder[0] = getInterceptNavigationDelegate(tab);
-        tab.addObserver(new TestTabObserver(finishCallback, failCallback));
-        if (expectedNewTabCount > 0) {
+        tab.addObserver(new TestTabObserver(finishCallback, failCallback, destroyedCallback));
+        if (createsNewTab) {
             mActivityTestRule.getActivity().getTabModelSelector().addObserver(
                     new EmptyTabModelSelectorObserver() {
                         @Override
                         public void onNewTabCreated(Tab newTab) {
                             newTabCallback.notifyCalled();
-                            newTab.addObserver(new TestTabObserver(finishCallback, failCallback));
+                            newTab.addObserver(new TestTabObserver(
+                                    finishCallback, failCallback, destroyedCallback));
                             latestTabHolder[0] = newTab;
                             latestDelegateHolder[0] = getInterceptNavigationDelegate(newTab);
                         }
@@ -190,13 +197,20 @@ public class UrlOverridingTest {
             TouchCommon.singleClickView(tab.getView());
         }
 
-        if (failCallback.getCallCount() == 0) {
+        if (shouldFailNavigation) {
             try {
                 failCallback.waitForCallback(0, 1, 20, TimeUnit.SECONDS);
             } catch (TimeoutException ex) {
                 Assert.fail("Haven't received navigation failure of intents.");
                 return;
             }
+        } else if (createsNewTab) {
+            try {
+                destroyedCallback.waitForCallback(0, 1, 20, TimeUnit.SECONDS);
+            } catch (TimeoutException ex) {
+                Assert.fail("Intercepted new tab wasn't destroyed.");
+                return;
+            }
         }
 
         boolean hasFallbackUrl =
@@ -213,7 +227,7 @@ public class UrlOverridingTest {
             }
         }
 
-        Assert.assertEquals(expectedNewTabCount, newTabCallback.getCallCount());
+        Assert.assertEquals(createsNewTab ? 1 : 0, newTabCallback.getCallCount());
         // For sub frames, the |loadFailCallback| run through different threads
         // from the ExternalNavigationHandler. As a result, there is no guarantee
         // when url override result would come.
@@ -247,8 +261,8 @@ public class UrlOverridingTest {
                     }
                 }));
         Assert.assertEquals(1 + (hasFallbackUrl ? 1 : 0), finishCallback.getCallCount());
-        // failCallback can be called second time when the current tab is destroyed.
-        Assert.assertTrue(failCallback.getCallCount() >= 1);
+
+        Assert.assertEquals(failCallback.getCallCount(), shouldFailNavigation ? 1 : 0);
     }
 
     private static InterceptNavigationDelegateImpl getInterceptNavigationDelegate(Tab tab) {
@@ -332,7 +346,7 @@ public class UrlOverridingTest {
                 + ":"
                 + Base64.encodeToString(
                           ApiCompatibilityUtils.getBytesUtf8(fallbackUrl), Base64.URL_SAFE));
-        loadUrlAndWaitForIntentUrl(originalUrl, true, 0, false, fallbackUrl);
+        loadUrlAndWaitForIntentUrl(originalUrl, true, false, false, fallbackUrl, true);
     }
 
     @Test
@@ -368,8 +382,10 @@ public class UrlOverridingTest {
     @SmallTest
     @RetryOnFailure
     public void testOpenWindowFromUserGesture() throws InterruptedException {
-        loadUrlAndWaitForIntentUrl(
-                mTestServer.getURL(OPEN_WINDOW_FROM_USER_GESTURE_PAGE), true, 1, true, null);
+        boolean opensNewTab =
+                !(mActivityTestRule.getActivity().getCurrentTabModel() instanceof SingleTabModel);
+        loadUrlAndWaitForIntentUrl(mTestServer.getURL(OPEN_WINDOW_FROM_USER_GESTURE_PAGE), true,
+                opensNewTab, true, null, false);
     }
 
     @Test

chrome/android/touchless/java/src/org/chromium/chrome/browser/touchless/NoTouchActivity.java

@@ -25,6 +25,7 @@ import org.chromium.chrome.browser.SingleTabActivity;
 import org.chromium.chrome.browser.compositor.layouts.LayoutManager;
 import org.chromium.chrome.browser.fullscreen.ChromeFullscreenManager;
 import org.chromium.chrome.browser.preferences.ChromePreferenceManager;
+import org.chromium.chrome.browser.tab.InterceptNavigationDelegateImpl;
 import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.chrome.browser.tab.TabDelegateFactory;
 import org.chromium.chrome.browser.tab.TabRedirectHandler;
@@ -48,8 +49,6 @@ public class NoTouchActivity extends SingleTabActivity {
     // Time at which an intent was received and handled.
     private long mIntentHandlingTimeMs;
 
-    private TouchlessUiCoordinator mUiCoordinator;
-
     /** The class that finishes this activity after a timeout. */
     private ChromeInactivityTracker mInactivityTracker;
 
@@ -77,9 +76,13 @@ public class NoTouchActivity extends SingleTabActivity {
                 case TabOpenType.REUSE_URL_MATCHING_TAB_ELSE_NEW_TAB:
                     if (getActivityTab().getUrl().contentEquals(url)) break;
                     // fall through
-                case TabOpenType.BRING_TAB_TO_FRONT: // fall through
                 case TabOpenType.REUSE_APP_ID_MATCHING_TAB_ELSE_NEW_TAB: // fall through
                 case TabOpenType.OPEN_NEW_TAB: // fall through
+                    InterceptNavigationDelegateImpl delegate =
+                            InterceptNavigationDelegateImpl.get(getActivityTab());
+                    if (delegate != null && delegate.shouldIgnoreNewTab(url, false)) return;
+                    // fall through
+                case TabOpenType.BRING_TAB_TO_FRONT: // fall through
                 case TabOpenType.CLOBBER_CURRENT_TAB:
                     // TODO(mthiesse): For now, let's just clobber current tab always. Are the other
                     // tab open types meaningful when we only have a single tab?

chrome/android/touchless/java/src/org/chromium/chrome/browser/touchless/TouchlessTabDelegate.java

@@ -18,6 +18,7 @@ import org.chromium.chrome.browser.tabmodel.AsyncTabParamsManager;
 import org.chromium.chrome.browser.tabmodel.TabLaunchType;
 import org.chromium.chrome.browser.tabmodel.document.AsyncTabCreationParams;
 import org.chromium.chrome.browser.tabmodel.document.TabDelegate;
+import org.chromium.chrome.browser.util.UrlUtilities;
 
 /**
  * Asynchronously creates Tabs for navigation originating from {@link NoTouchActivity}.
@@ -39,6 +40,12 @@ public class TouchlessTabDelegate extends TabDelegate {
             super.createNewTab(asyncParams, type, parentId);
             return;
         }
+        if (UrlUtilities.validateIntentUrl(asyncParams.getLoadUrlParams().getUrl())) {
+            // Handle intent URLs as normal.
+            super.createNewTab(asyncParams, type, parentId);
+            return;
+        }
+
         String url = asyncParams.getLoadUrlParams().getUrl();
 
         int assignedTabId = TabIdManager.getInstance().generateValidId(Tab.INVALID_TAB_ID);