Revert of Redirecting off-origin navigations in PWAs to CCT. (patchset …

…chromium#17 id:640001 of https://codereview.chromium.org/2829943002/ ) Reason for revert: Suspect of breaking Android Tests bot. First failure: https://uberchromegw.corp.google.com/i/chromium.linux/builders/Android%20Tests/builds/42115 Will reland if the revert does not fix the bot. Original issue's description: > Redirects off-origin navigations in an installed PWAs to a CustomTab. > > This is done in order to improve UX by getting rid of a "minibar" toolbar > (to be done in a follow-up patch) and bring more app-like feel to PWAs. > > In a long run this will help bring implementation of Minimal-UI, which requires > replacing "minibar" with a new "minimal-ui" toolbar. > > BUG=709889 > > Review-Url: https://codereview.chromium.org/2829943002 > Cr-Original-Commit-Position: refs/heads/master@{#474625} > Committed: https://chromium.googlesource.com/chromium/src/+/7c7e9e600d256af4d2af1dbf68b59b030be0ab25 > Review-Url: https://codereview.chromium.org/2829943002 > Cr-Commit-Position: refs/heads/master@{#474953} > Committed: https://chromium.googlesource.com/chromium/src/+/4c271c0deaf103d5612078b3d0d81847819c5d52 TBR=dominickn@chromium.org,yusufo@chromium.org,pkotwicz@chromium.org,tedchoc@chromium.org,piotrs@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=709889 Review-Url: https://codereview.chromium.org/2906043002 Cr-Commit-Position: refs/heads/master@{#474976}
Denger-Network · May 26, 2017 · 0ed1a20 · 0ed1a20
1 parent 2675d3c
commit 0ed1a20
Show file tree

Hide file tree

Showing 13 changed files with 27 additions and 391 deletions.
diff --git a/chrome/android/java/AndroidManifest.xml b/chrome/android/java/AndroidManifest.xml
@@ -486,7 +486,6 @@ by a child template that "extends" this file.
         <activity android:name="org.chromium.chrome.browser.webapps.WebappActivity"
             android:theme="@style/WebappTheme"
             android:label="@string/webapp_activity_title"
-            android:launchMode="singleTop"
             android:documentLaunchMode="intoExisting"
             android:persistableMode="persistNever"
             {{ self.supports_video_persistence() }}
@@ -518,7 +517,6 @@ by a child template that "extends" this file.
         <activity android:name="org.chromium.chrome.browser.webapps.WebApkActivity"
             android:theme="@style/WebappTheme"
             android:label="@string/webapp_activity_title"
-            android:launchMode="singleTop"
             android:documentLaunchMode="intoExisting"
             android:persistableMode="persistNever"
             {{ self.supports_video_persistence() }}

diff --git a/chrome/android/java/src/org/chromium/chrome/browser/tab/TabContextMenuItemDelegate.java b/chrome/android/java/src/org/chromium/chrome/browser/tab/TabContextMenuItemDelegate.java
@@ -226,9 +226,6 @@ public void onOpenInChrome(String linkUrl, String pageUrl) {
         Intent chromeIntent = new Intent(Intent.ACTION_VIEW, Uri.parse(linkUrl));
         chromeIntent.setPackage(mTab.getApplicationContext().getPackageName());
         chromeIntent.setFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
-        // For "Open in Chrome" from the context menu in FullscreenActivity we want to bypass
-        // CustomTab, and this flag ensures we open in TabbedChrome.
-        chromeIntent.putExtra(ChromeLauncherActivity.EXTRA_IS_ALLOWED_TO_RETURN_TO_PARENT, false);
 
         boolean activityStarted = false;
         if (pageUrl != null) {

diff --git a/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkActivity.java b/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebApkActivity.java
@@ -19,7 +19,6 @@
 import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.chrome.browser.tab.TabDelegateFactory;
 import org.chromium.chrome.browser.tab.TabRedirectHandler;
-import org.chromium.chrome.browser.util.UrlUtilities;
 import org.chromium.components.navigation_interception.NavigationParams;
 import org.chromium.webapk.lib.client.WebApkServiceConnectionManager;
 
@@ -58,7 +57,7 @@ protected TabDelegateFactory createTabDelegateFactory() {
         return new WebappDelegateFactory(this) {
             @Override
             public InterceptNavigationDelegateImpl createInterceptNavigationDelegate(Tab tab) {
-                return new WebappInterceptNavigationDelegate(WebApkActivity.this, tab) {
+                return new InterceptNavigationDelegateImpl(tab) {
                     @Override
                     public ExternalNavigationParams.Builder buildExternalNavigationParams(
                             NavigationParams navigationParams,
@@ -69,11 +68,6 @@ public ExternalNavigationParams.Builder buildExternalNavigationParams(
                         builder.setWebApkPackageName(getWebApkPackageName());
                         return builder;
                     }
-
-                    @Override
-                    protected boolean isUrlOutsideWebappScope(WebappInfo info, String url) {
-                        return !UrlUtilities.isUrlWithinScope(url, info.scopeUri().toString());
-                    }
                 };
             }
 

diff --git a/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDelegateFactory.java b/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappDelegateFactory.java
@@ -10,7 +10,6 @@
 import org.chromium.base.ContextUtils;
 import org.chromium.chrome.browser.ShortcutHelper;
 import org.chromium.chrome.browser.tab.BrowserControlsVisibilityDelegate;
-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.TabWebContentsDelegateAndroid;
@@ -73,9 +72,4 @@ public TabWebContentsDelegateAndroid createWebContentsDelegate(Tab tab) {
     public BrowserControlsVisibilityDelegate createBrowserControlsVisibilityDelegate(Tab tab) {
         return new WebappBrowserControlsDelegate(mActivity, tab);
     }
-
-    @Override
-    public InterceptNavigationDelegateImpl createInterceptNavigationDelegate(Tab tab) {
-        return new WebappInterceptNavigationDelegate(mActivity, tab);
-    }
 }
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappInfo.java b/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappInfo.java
@@ -10,7 +10,6 @@
 import android.text.TextUtils;
 
 import org.chromium.base.Log;
-import org.chromium.base.VisibleForTesting;
 import org.chromium.blink_public.platform.WebDisplayMode;
 import org.chromium.chrome.browser.ShortcutHelper;
 import org.chromium.chrome.browser.ShortcutSource;
@@ -325,9 +324,4 @@ public boolean isLaunchedFromHomescreen() {
         int source = source();
         return source != ShortcutSource.NOTIFICATION && source != ShortcutSource.EXTERNAL_INTENT;
     }
-
-    @VisibleForTesting
-    void setScopeForTest(String scope) {
-        mScopeUri = Uri.parse(scope);
-    }
 }
diff --git a/...droid/java/src/org/chromium/chrome/browser/webapps/WebappInterceptNavigationDelegate.java b/...droid/java/src/org/chromium/chrome/browser/webapps/WebappInterceptNavigationDelegate.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappLauncherActivity.java b/chrome/android/java/src/org/chromium/chrome/browser/webapps/WebappLauncherActivity.java
@@ -196,17 +196,19 @@ private Intent createWebappLaunchIntent(WebappInfo info, int source, boolean isW
         launchIntent.setData(Uri.parse(WebappActivity.WEBAPP_SCHEME + "://" + info.id()));
         launchIntent.setFlags(
                 Intent.FLAG_ACTIVITY_NEW_TASK | ApiCompatibilityUtils.getActivityNewDocumentFlag());
-        // Setting FLAG_ACTIVITY_CLEAR_TOP handles 2 edge cases:
-        // - If a legacy PWA is launching from a notification, we want to ensure that the URL being
-        // launched is the URL in the intent. If a paused WebappActivity exists for this id,
-        // then by default it will be focused and we have no way of sending the desired URL to
-        // it (the intent is swallowed). As a workaround, set the CLEAR_TOP flag to ensure that
-        // the existing Activity handles an update via onNewIntent().
-        // - If a WebAPK is having a CustomTabActivity on top of it in the same Task, and user
-        // clicks a link to takes them back to the scope of a WebAPK, we want to destroy the
-        // CustomTabActivity activity and go back to the WebAPK activity. It is intentional that
-        // Custom Tab will not be reachable with a back button.
-        launchIntent.setFlags(Intent.FLAG_ACTIVITY_CLEAR_TOP);
+
+        if (!isWebApk) {
+            // If this is launching from a notification, we want to ensure that the URL being
+            // launched is the URL in the intent. If a paused WebappActivity exists for this id,
+            // then by default it will be focused and we have no way of sending the desired URL to
+            // it (the intent is swallowed). As a workaround, set the CLEAR_TOP flag to ensure that
+            // the existing Activity is cleared and relaunched with this intent.
+            // TODO(dominickn): ideally, we want be able to route an intent to
+            // WebappActivity.onNewIntent instead of restarting the Activity.
+            if (source == ShortcutSource.NOTIFICATION) {
+                launchIntent.setFlags(Intent.FLAG_ACTIVITY_CLEAR_TOP);
+            }
+        }
         return launchIntent;
     }
 

diff --git a/chrome/android/java_sources.gni b/chrome/android/java_sources.gni
@@ -1199,7 +1199,6 @@ chrome_java_sources = [
   "java/src/org/chromium/chrome/browser/webapps/WebappDelegateFactory.java",
   "java/src/org/chromium/chrome/browser/webapps/WebappDirectoryManager.java",
   "java/src/org/chromium/chrome/browser/webapps/WebappInfo.java",
-  "java/src/org/chromium/chrome/browser/webapps/WebappInterceptNavigationDelegate.java",
   "java/src/org/chromium/chrome/browser/webapps/WebappLauncherActivity.java",
   "java/src/org/chromium/chrome/browser/webapps/WebappManagedActivity.java",
   "java/src/org/chromium/chrome/browser/webapps/WebappRegistry.java",
@@ -1649,7 +1648,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",
@@ -1658,7 +1656,6 @@ chrome_test_java_sources = [
   "javatests/src/org/chromium/chrome/browser/webapps/WebappDirectoryManagerTest.java",
   "javatests/src/org/chromium/chrome/browser/webapps/WebappInfoTest.java",
   "javatests/src/org/chromium/chrome/browser/webapps/WebappModeTest.java",
-  "javatests/src/org/chromium/chrome/browser/webapps/WebappNavigationTest.java",
   "javatests/src/org/chromium/chrome/browser/webapps/WebappSplashScreenBackgroundColorTest.java",
   "javatests/src/org/chromium/chrome/browser/webapps/WebappSplashScreenHomescreenIconTest.java",
   "javatests/src/org/chromium/chrome/browser/webapps/WebappSplashScreenIconTest.java",

diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/TopActivityListener.java b/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/TopActivityListener.java
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebApkIntegrationTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebApkIntegrationTest.java
@@ -9,7 +9,6 @@
 import android.support.test.filters.LargeTest;
 
 import org.junit.After;
-import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
@@ -19,10 +18,8 @@
 import org.chromium.base.test.util.Feature;
 import org.chromium.base.test.util.RetryOnFailure;
 import org.chromium.base.test.util.ScalableTimeout;
-import org.chromium.chrome.browser.ChromeActivity;
 import org.chromium.chrome.browser.ChromeSwitches;
 import org.chromium.chrome.browser.ShortcutHelper;
-import org.chromium.chrome.browser.customtabs.CustomTabActivity;
 import org.chromium.chrome.test.ChromeActivityTestRule;
 import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
 import org.chromium.chrome.test.util.ChromeTabUtils;
@@ -40,9 +37,6 @@ public class WebApkIntegrationTest {
     public final ChromeActivityTestRule<WebApkActivity> mActivityTestRule =
             new ChromeActivityTestRule<>(WebApkActivity.class);
 
-    @Rule
-    public final TopActivityListener activityListener = new TopActivityListener();
-
     private static final long STARTUP_TIMEOUT = ScalableTimeout.scaleTimeout(10000);
 
     private EmbeddedTestServer mTestServer;
@@ -54,13 +48,9 @@ public void startWebApkActivity(String webApkPackageName, final String startUrl)
         intent.putExtra(WebApkConstants.EXTRA_WEBAPK_PACKAGE_NAME, webApkPackageName);
         intent.putExtra(ShortcutHelper.EXTRA_URL, startUrl);
         intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
-
-        WebApkActivity webApkActivity =
+        mActivityTestRule.setActivity(
                 (WebApkActivity) InstrumentationRegistry.getInstrumentation().startActivitySync(
-                        intent);
-        webApkActivity.mWebappInfo.setScopeForTest(
-                startUrl.substring(0, startUrl.lastIndexOf('/') + 1));
-        mActivityTestRule.setActivity(webApkActivity);
+                        intent));
         InstrumentationRegistry.getInstrumentation().waitForIdleSync();
 
         CriteriaHelper.pollInstrumentationThread(new Criteria() {
@@ -104,41 +94,9 @@ public void tearDown() throws Exception {
     @LargeTest
     @Feature({"WebApk"})
     @RetryOnFailure
-    public void testLaunchAndNavigateOffOrigin() throws Exception {
+    public void testLaunch() throws InterruptedException {
         startWebApkActivity("org.chromium.webapk.test",
                 mTestServer.getURL("/chrome/test/data/android/test.html"));
         waitUntilSplashscreenHides();
-
-        WebApkActivity webApkActivity = mActivityTestRule.getActivity();
-        waitForFinishLoading(webApkActivity);
-
-        Assert.assertSame(webApkActivity, activityListener.getMostRecentActivity());
-        Assert.assertEquals(mTestServer.getURL("/chrome/test/data/android/test.html"),
-                webApkActivity.getActivityTab().getUrl());
-
-        // We navigate outside origin and expect Custom Tab to open on top of WebApkActivity.
-        mActivityTestRule.runJavaScriptCodeInCurrentTab(
-                "window.top.location = 'https://www.google.com/'");
-
-        activityListener.waitFor(CustomTabActivity.class);
-
-        final CustomTabActivity customTab =
-                (CustomTabActivity) activityListener.getMostRecentActivity();
-
-        waitForFinishLoading(customTab);
-
-        // Dropping the TLD as Google can redirect to a local site, so this could fail outside US.
-        Assert.assertTrue(customTab.getActivityTab().getUrl().startsWith("https://www.google."));
-    }
-
-    private void waitForFinishLoading(final ChromeActivity chromeActivity) {
-        CriteriaHelper.pollInstrumentationThread(new Criteria() {
-            @Override
-            public boolean isSatisfied() {
-                return chromeActivity.getActivityTab() != null
-                        && !chromeActivity.getActivityTab().isLoading();
-            }
-        }, STARTUP_TIMEOUT, CriteriaHelper.DEFAULT_POLLING_INTERVAL);
-        InstrumentationRegistry.getInstrumentation().waitForIdleSync();
     }
 }
diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappActivityTestRule.java b/chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebappActivityTestRule.java
@@ -15,7 +15,6 @@
 import org.junit.runners.model.Statement;
 
 import org.chromium.base.test.util.UrlUtils;
-import org.chromium.chrome.browser.ChromeActivity;
 import org.chromium.chrome.browser.ShortcutHelper;
 import org.chromium.chrome.test.ChromeActivityTestRule;
 import org.chromium.content.browser.test.util.Criteria;
@@ -117,21 +116,15 @@ public final void startWebappActivity(Intent intent) throws Exception {
     }
 
     /**
-     * Waits until any loads in progress of the activity under test have completed.
+     * Waits until any loads in progress have completed.
      */
-    protected void waitUntilIdle() {
-        waitUntilIdle(getActivity());
-    }
-
-    /**
-     * Waits until any loads in progress of a selected activity have completed.
-     */
-    protected void waitUntilIdle(final ChromeActivity activity) {
+    public void waitUntilIdle() {
         getInstrumentation().waitForIdleSync();
         CriteriaHelper.pollInstrumentationThread(new Criteria() {
             @Override
             public boolean isSatisfied() {
-                return activity.getActivityTab() != null && !activity.getActivityTab().isLoading();
+                return getActivity().getActivityTab() != null
+                        && !getActivity().getActivityTab().isLoading();
             }
         }, STARTUP_TIMEOUT, CriteriaHelper.DEFAULT_POLLING_INTERVAL);