Commit 4dbcecfb authored by piotrs's avatar piotrs Committed by Commit Bot

Fixes the redirect to external apps when navigating from PWA.

This is done with avoiding explicit chrome package name (using
component name instead) and using
EXTRA_SEND_TO_EXTERNAL_DEFAULT_HANDLER on the intent.

With this patch the redirect works, however it takes a long time for CCT to
consult ExternalNavigationHandler. This results in suboptimal UX. Same redirect
in ChromeTabbedActivity is almost instantaneous. Result is that
for _blank links CCT shows up briefly before redirecting to an external app.
In a follow up it should be investigated why it is the case and if it cannot
be improved, we might need to bypass CCT in such case, which would diverge
from existing navigation paths for _blank links in Clank.

BUG=740402

Review-Url: https://codereview.chromium.org/2969143002
Cr-Original-Commit-Position: refs/heads/master@{#485831}
Committed: https://chromium.googlesource.com/chromium/src/+/727468b41b2fe48bf73983ab0c3aa2632b47d89e
Review-Url: https://codereview.chromium.org/2969143002
Cr-Commit-Position: refs/heads/master@{#486275}
parent 184a75bc
......@@ -133,13 +133,6 @@ public class TabDelegate extends TabCreator {
Intent intent = new Intent(
Intent.ACTION_VIEW, Uri.parse(asyncParams.getLoadUrlParams().getUrl()));
ComponentName componentName = asyncParams.getComponentName();
if (componentName == null) {
intent.setClass(ContextUtils.getApplicationContext(), ChromeLauncherActivity.class);
} else {
intent.setComponent(componentName);
}
addAsyncTabExtras(asyncParams, parentId, isChromeUI, assignedTabId, intent);
return intent;
......@@ -147,6 +140,13 @@ public class TabDelegate extends TabCreator {
protected final void addAsyncTabExtras(AsyncTabCreationParams asyncParams, int parentId,
boolean isChromeUI, int assignedTabId, Intent intent) {
ComponentName componentName = asyncParams.getComponentName();
if (componentName == null) {
intent.setClass(ContextUtils.getApplicationContext(), ChromeLauncherActivity.class);
} else {
intent.setComponent(componentName);
}
Map<String, String> extraHeaders = asyncParams.getLoadUrlParams().getExtraHeaders();
if (extraHeaders != null && !extraHeaders.isEmpty()) {
Bundle bundle = new Bundle();
......
......@@ -657,7 +657,7 @@ public class WebappActivity extends SingleTabActivity {
@Override
protected TabDelegate createTabDelegate(boolean incognito) {
return new WebappTabDelegate(this, incognito);
return new WebappTabDelegate(incognito);
}
// We're temporarily disable CS on webapp since there are some issues. (http://crbug.com/471950)
......
......@@ -3,9 +3,12 @@
// found in the LICENSE file.
package org.chromium.chrome.browser.webapps;
import android.content.Intent;
import android.net.Uri;
import android.support.customtabs.CustomTabsIntent;
import org.chromium.chrome.browser.IntentHandler;
import org.chromium.chrome.browser.customtabs.CustomTabIntentDataProvider;
import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tab.TabIdManager;
import org.chromium.chrome.browser.tabmodel.AsyncTabParamsManager;
......@@ -20,11 +23,8 @@ import org.chromium.chrome.browser.tabmodel.document.TabDelegate;
* {@code _blank} links and {@code window.open(url)} calls instead of creating a new tab in Chrome.
*/
public class WebappTabDelegate extends TabDelegate {
private final WebappActivity mActivity;
public WebappTabDelegate(WebappActivity activity, boolean incognito) {
public WebappTabDelegate(boolean incognito) {
super(incognito);
this.mActivity = activity;
}
@Override
......@@ -32,11 +32,12 @@ public class WebappTabDelegate extends TabDelegate {
int assignedTabId = TabIdManager.getInstance().generateValidId(Tab.INVALID_TAB_ID);
AsyncTabParamsManager.add(assignedTabId, asyncParams);
CustomTabsIntent customTabIntent =
new CustomTabsIntent.Builder().setShowTitle(true).build();
Intent intent = new CustomTabsIntent.Builder().setShowTitle(true).build().intent;
intent.setData(Uri.parse(asyncParams.getLoadUrlParams().getUrl()));
intent.putExtra(CustomTabIntentDataProvider.EXTRA_SEND_TO_EXTERNAL_DEFAULT_HANDLER, true);
intent.putExtra(CustomTabIntentDataProvider.EXTRA_IS_OPENED_BY_CHROME, true);
addAsyncTabExtras(asyncParams, parentId, false /* isChromeUI */, assignedTabId, intent);
customTabIntent.intent.setPackage(mActivity.getPackageName());
addAsyncTabExtras(asyncParams, parentId, true, assignedTabId, customTabIntent.intent);
customTabIntent.launchUrl(mActivity, Uri.parse(asyncParams.getLoadUrlParams().getUrl()));
IntentHandler.startActivityForTrustedIntent(intent);
}
}
......@@ -1686,7 +1686,6 @@ chrome_test_java_sources = [
"javatests/src/org/chromium/chrome/browser/webapps/AddToHomescreenDialogTest.java",
"javatests/src/org/chromium/chrome/browser/webapps/AddToHomescreenManagerTest.java",
"javatests/src/org/chromium/chrome/browser/webapps/TestFetchStorageCallback.java",
"javatests/src/org/chromium/chrome/browser/webapps/TopActivityListener.java",
"javatests/src/org/chromium/chrome/browser/webapps/WebApkUpdateDataFetcherTest.java",
"javatests/src/org/chromium/chrome/browser/webapps/WebApkUpdateManagerTest.java",
"javatests/src/org/chromium/chrome/browser/webapps/WebappActivityTestRule.java",
......
// Copyright 2017 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
package org.chromium.chrome.browser.webapps;
import android.app.Activity;
import org.junit.rules.TestRule;
import org.junit.runner.Description;
import org.junit.runners.model.Statement;
import org.chromium.base.ActivityState;
import org.chromium.base.ApplicationStatus;
import org.chromium.base.ApplicationStatus.ActivityStateListener;
import org.chromium.content.browser.test.util.Criteria;
import org.chromium.content.browser.test.util.CriteriaHelper;
import javax.annotation.Nullable;
/**
* Test rule tracking which Chrome activity is currently at the top of the task.
*/
public class TopActivityListener implements TestRule {
private final ActivityStateListener mListener = new ActivityStateListener() {
@Override
public void onActivityStateChange(Activity activity, int newState) {
if (newState == ActivityState.RESUMED) {
mMostRecentActivity = activity;
}
}
};
private Activity mMostRecentActivity;
@Override
public Statement apply(final Statement base, Description description) {
return new Statement() {
@Override
public void evaluate() throws Throwable {
ApplicationStatus.registerStateListenerForAllActivities(mListener);
base.evaluate();
ApplicationStatus.unregisterActivityStateListener(mListener);
}
};
}
@Nullable
public Activity getMostRecentActivity() {
return mMostRecentActivity;
}
@SuppressWarnings("unchecked")
public <T extends Activity> T waitFor(final Class<T> expectedActivityClass) {
CriteriaHelper.pollUiThread(new Criteria() {
@Override
public boolean isSatisfied() {
return mMostRecentActivity != null
&& expectedActivityClass.isAssignableFrom(mMostRecentActivity.getClass());
}
});
return (T) mMostRecentActivity;
}
}
......@@ -4,6 +4,7 @@
package org.chromium.chrome.browser.webapps;
import android.app.Activity;
import android.content.Intent;
import android.support.test.InstrumentationRegistry;
import android.support.test.filters.LargeTest;
......@@ -15,11 +16,11 @@ import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.chromium.base.ApplicationStatus;
import org.chromium.base.metrics.RecordHistogram;
import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.base.test.util.Feature;
import org.chromium.base.test.util.ScalableTimeout;
import org.chromium.chrome.browser.ChromeActivity;
import org.chromium.chrome.browser.ChromeSwitches;
import org.chromium.chrome.browser.DeferredStartupHandler;
import org.chromium.chrome.browser.ShortcutHelper;
......@@ -45,9 +46,6 @@ public class WebApkIntegrationTest {
@Rule
public final NativeLibraryTestRule mNativeLibraryTestRule = new NativeLibraryTestRule();
@Rule
public final TopActivityListener activityListener = new TopActivityListener();
private static final long STARTUP_TIMEOUT = ScalableTimeout.scaleTimeout(10000);
private EmbeddedTestServer mTestServer;
......@@ -126,11 +124,14 @@ public class WebApkIntegrationTest {
CriteriaHelper.pollUiThread(new Criteria() {
@Override
public boolean isSatisfied() {
ChromeActivity activity = (ChromeActivity) activityListener.getMostRecentActivity();
return activity instanceof CustomTabActivity
&& activity.getActivityTab() != null
Activity activity = ApplicationStatus.getLastTrackedFocusedActivity();
if (!(activity instanceof CustomTabActivity)) {
return false;
}
CustomTabActivity customTab = (CustomTabActivity) activity;
return customTab.getActivityTab() != null
// Dropping the TLD as Google can redirect to a local site.
&& activity.getActivityTab().getUrl().startsWith("https://www.google.");
&& customTab.getActivityTab().getUrl().startsWith("https://www.google.");
}
});
}
......
......@@ -4,6 +4,11 @@
package org.chromium.chrome.browser.webapps;
import static org.chromium.base.ApplicationState.HAS_DESTROYED_ACTIVITIES;
import static org.chromium.base.ApplicationState.HAS_PAUSED_ACTIVITIES;
import static org.chromium.base.ApplicationState.HAS_STOPPED_ACTIVITIES;
import android.app.Activity;
import android.content.Intent;
import android.graphics.Color;
import android.support.test.InstrumentationRegistry;
......@@ -17,26 +22,36 @@ import org.junit.Test;
import org.junit.runner.RunWith;
import org.chromium.base.ApiCompatibilityUtils;
import org.chromium.base.ApplicationStatus;
import org.chromium.base.ThreadUtils;
import org.chromium.base.test.util.CommandLineFlags;
import org.chromium.base.test.util.Feature;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.ChromeActivity;
import org.chromium.chrome.browser.ChromeSwitches;
import org.chromium.chrome.browser.ChromeTabbedActivity;
import org.chromium.chrome.browser.ShortcutHelper;
import org.chromium.chrome.browser.customtabs.CustomTabActivity;
import org.chromium.chrome.browser.externalnav.ExternalNavigationHandler.OverrideUrlLoadingResult;
import org.chromium.chrome.browser.firstrun.FirstRunStatus;
import org.chromium.chrome.browser.tab.InterceptNavigationDelegateImpl;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.chrome.test.util.browser.contextmenu.ContextMenuUtils;
import org.chromium.content.browser.test.util.Criteria;
import org.chromium.content.browser.test.util.CriteriaHelper;
import org.chromium.content.browser.test.util.DOMUtils;
import org.chromium.net.test.EmbeddedTestServer;
import org.chromium.ui.base.PageTransition;
import java.util.concurrent.Callable;
/**
* Tests web navigations originating from a WebappActivity.
*/
@RunWith(ChromeJUnit4ClassRunner.class)
@CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE})
public class WebappNavigationTest {
private static final String YOUTUBE_URL = "https://www.youtube.com/watch?v=EYmjoW4vIX8";
private static final String OFF_ORIGIN_URL = "https://www.google.com/";
private static final String WEB_APP_PATH = "/chrome/test/data/banners/manifest_test_page.html";
private static final String IN_SCOPE_PAGE_PATH =
......@@ -45,9 +60,6 @@ public class WebappNavigationTest {
@Rule
public final WebappActivityTestRule mActivityTestRule = new WebappActivityTestRule();
@Rule
public final TopActivityListener activityListener = new TopActivityListener();
private EmbeddedTestServer mTestServer;
@Before
......@@ -120,7 +132,7 @@ public class WebappNavigationTest {
addAnchor("testId", mTestServer.getURL(IN_SCOPE_PAGE_PATH), "_blank");
DOMUtils.clickNode(
mActivityTestRule.getActivity().getActivityTab().getContentViewCore(), "testId");
CustomTabActivity customTab = activityListener.waitFor(CustomTabActivity.class);
CustomTabActivity customTab = waitFor(CustomTabActivity.class);
mActivityTestRule.waitUntilIdle(customTab);
Assert.assertTrue(
mActivityTestRule.runJavaScriptCodeInCurrentTab("document.body.textContent")
......@@ -169,7 +181,7 @@ public class WebappNavigationTest {
otherPageUrl, mActivityTestRule.getActivity().getActivityTab().getUrl());
Assert.assertSame(
mActivityTestRule.getActivity(), activityListener.getMostRecentActivity());
mActivityTestRule.getActivity(), ApplicationStatus.getLastTrackedFocusedActivity());
}
@Test
......@@ -187,13 +199,52 @@ public class WebappNavigationTest {
mActivityTestRule.getActivity().getActivityTab(), "myTestAnchorId",
R.id.menu_id_open_in_chrome);
ChromeTabbedActivity tabbedChrome = activityListener.waitFor(ChromeTabbedActivity.class);
ChromeTabbedActivity tabbedChrome = waitFor(ChromeTabbedActivity.class);
mActivityTestRule.waitUntilIdle(tabbedChrome);
// Dropping the TLD as Google can redirect to a local site, so this could fail outside US.
Assert.assertTrue(tabbedChrome.getActivityTab().getUrl().startsWith("https://www.google."));
}
@Test
@SmallTest
@Feature({"Webapps"})
public void testRegularLinkToExternalApp() throws Exception {
runWebappActivityAndWaitForIdle(mActivityTestRule.createIntent());
InterceptNavigationDelegateImpl navigationDelegate =
mActivityTestRule.getActivity().getActivityTab().getInterceptNavigationDelegate();
addAnchor("testLink", YOUTUBE_URL, "_self");
DOMUtils.clickNode(
mActivityTestRule.getActivity().getActivityTab().getContentViewCore(), "testLink");
waitForExternalAppOrIntentPicker();
Assert.assertEquals(OverrideUrlLoadingResult.OVERRIDE_WITH_EXTERNAL_INTENT,
navigationDelegate.getLastOverrideUrlLoadingResultForTests());
}
@Test
@SmallTest
@Feature({"Webapps"})
public void testNewTabLinkToExternalApp() throws Exception {
runWebappActivityAndWaitForIdle(mActivityTestRule.createIntent());
addAnchor("testLink", YOUTUBE_URL, "_blank");
DOMUtils.clickNode(
mActivityTestRule.getActivity().getActivityTab().getContentViewCore(), "testLink");
// For _blank anchors, we open the CustomTab which does the redirecting if necessary.
CustomTabActivity customTab = waitFor(CustomTabActivity.class);
waitForExternalAppOrIntentPicker();
InterceptNavigationDelegateImpl navigationDelegate =
customTab.getActivityTab().getInterceptNavigationDelegate();
Assert.assertEquals(OverrideUrlLoadingResult.OVERRIDE_WITH_EXTERNAL_INTENT,
navigationDelegate.getLastOverrideUrlLoadingResultForTests());
}
private void runWebappActivityAndWaitForIdle(Intent intent) throws Exception {
mActivityTestRule.startWebappActivity(
intent.putExtra(ShortcutHelper.EXTRA_URL, mTestServer.getURL(WEB_APP_PATH)));
......@@ -203,7 +254,7 @@ public class WebappNavigationTest {
}
private CustomTabActivity assertCustomTabActivityLaunchedForOffOriginUrl() {
CustomTabActivity customTab = activityListener.waitFor(CustomTabActivity.class);
CustomTabActivity customTab = waitFor(CustomTabActivity.class);
mActivityTestRule.waitUntilIdle(customTab);
// Dropping the TLD as Google can redirect to a local site, so this could fail outside US.
......@@ -222,4 +273,46 @@ public class WebappNavigationTest {
+ "document.body.appendChild(aTag);",
id, url, target));
}
@SuppressWarnings("unchecked")
private <T extends Activity> T waitFor(final Class<T> expectedClass) {
final Activity[] holder = new Activity[1];
CriteriaHelper.pollUiThread(new Criteria() {
@Override
public boolean isSatisfied() {
holder[0] = ApplicationStatus.getLastTrackedFocusedActivity();
return holder[0] != null && expectedClass.isAssignableFrom(holder[0].getClass());
}
});
return (T) holder[0];
}
private void waitForExternalAppOrIntentPicker() {
final Callable<Boolean> callable = new Callable<Boolean>() {
@Override
public Boolean call() throws Exception {
return ApplicationStatus.getStateForApplication() == HAS_PAUSED_ACTIVITIES
|| ApplicationStatus.getStateForApplication() == HAS_STOPPED_ACTIVITIES
|| ApplicationStatus.getStateForApplication() == HAS_DESTROYED_ACTIVITIES;
}
};
CriteriaHelper.pollInstrumentationThread(new Criteria() {
@Override
public boolean isSatisfied() {
return ThreadUtils.runOnUiThreadBlockingNoException(callable);
}
@Override
public String getFailureReason() {
Activity lastFocusedActivity = ApplicationStatus.getLastTrackedFocusedActivity();
return String.format(
"Application state is: %d, most recent activity is %s and it is on URL %s",
ApplicationStatus.getStateForApplication(),
lastFocusedActivity.getClass().getSimpleName(),
lastFocusedActivity instanceof ChromeActivity
? ((ChromeActivity) lastFocusedActivity).getActivityTab().getUrl()
: "[not a ChromeActivity]");
}
}, CriteriaHelper.DEFAULT_MAX_TIME_TO_POLL * 2, CriteriaHelper.DEFAULT_POLLING_INTERVAL);
}
}
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