Remove unnecessary queryIntentActivities from TabRedirectHandler

QueryIntentActivities is slow, and we were redundantly calling it with
the same intent we had already called it with.

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

chrome/android/java/src/org/chromium/chrome/browser/externalnav/ExternalNavigationHandler.java

@@ -688,10 +688,11 @@ public class ExternalNavigationHandler {
      * and "picking Chrome" is handled inside the support library.
      */
     private boolean shouldKeepIntentRedirectInChrome(ExternalNavigationParams params,
-            boolean incomingIntentRedirect, Intent targetIntent, boolean isExternalProtocol) {
+            boolean incomingIntentRedirect, List<ResolveInfo> resolvingInfos,
+            boolean isExternalProtocol) {
         if (params.getRedirectHandler() != null && incomingIntentRedirect && !isExternalProtocol
                 && !params.getRedirectHandler().isFromCustomTabIntent()
-                && !params.getRedirectHandler().hasNewResolver(targetIntent)) {
+                && !params.getRedirectHandler().hasNewResolver(resolvingInfos)) {
             if (DEBUG) Log.i(TAG, "Custom tab redirect no handled");
             return true;
         }
@@ -871,7 +872,7 @@ public class ExternalNavigationHandler {
         }
 
         if (shouldKeepIntentRedirectInChrome(
-                    params, incomingIntentRedirect, targetIntent, isExternalProtocol)) {
+                    params, incomingIntentRedirect, resolvingInfos, isExternalProtocol)) {
             return OverrideUrlLoadingResult.NO_OVERRIDE;
         }
 

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

@@ -23,7 +23,6 @@ import org.chromium.chrome.browser.customtabs.CustomTabIntentDataProvider;
 import org.chromium.chrome.browser.util.IntentUtils;
 import org.chromium.ui.base.PageTransition;
 
-import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
 
@@ -323,31 +322,24 @@ public class TabRedirectHandler extends EmptyTabObserver implements UserData {
         return mLastCommittedEntryIndexBeforeStartingNavigation;
     }
 
-    private static List<ComponentName> getIntentHandlers(Intent intent) {
-        List<ResolveInfo> list = PackageManagerUtils.queryIntentActivities(intent, 0);
-        List<ComponentName> nameList = new ArrayList<ComponentName>();
-        for (ResolveInfo r : list) {
-            nameList.add(new ComponentName(r.activityInfo.packageName, r.activityInfo.name));
-        }
-        return nameList;
-    }
-
     /**
      * @return whether |intent| has a new resolver against |mIntentHistory| or not.
      */
-    public boolean hasNewResolver(Intent intent) {
+    public boolean hasNewResolver(List<ResolveInfo> resolvingInfos) {
         if (mInitialIntent == null) {
-            return intent != null;
-        } else if (intent == null) {
-            return false;
+            return !resolvingInfos.isEmpty();
         }
 
-        List<ComponentName> newList = getIntentHandlers(intent);
         if (mCachedResolvers.isEmpty()) {
-            mCachedResolvers.addAll(getIntentHandlers(mInitialIntent));
+            for (ResolveInfo r : PackageManagerUtils.queryIntentActivities(mInitialIntent, 0)) {
+                mCachedResolvers.add(
+                        new ComponentName(r.activityInfo.packageName, r.activityInfo.name));
+            }
         }
-        for (ComponentName name : newList) {
-            if (!mCachedResolvers.contains(name)) {
+        if (resolvingInfos.size() > mCachedResolvers.size()) return true;
+        for (ResolveInfo r : resolvingInfos) {
+            if (!mCachedResolvers.contains(
+                        new ComponentName(r.activityInfo.packageName, r.activityInfo.name))) {
                 return true;
             }
         }

chrome/android/javatests/src/org/chromium/chrome/browser/tab/TabRedirectHandlerTest.java

@@ -20,6 +20,7 @@ import org.junit.runner.RunWith;
 
 import org.chromium.base.CommandLine;
 import org.chromium.base.ContextUtils;
+import org.chromium.base.PackageManagerUtils;
 import org.chromium.base.test.util.AdvancedMockContext;
 import org.chromium.base.test.util.Feature;
 import org.chromium.base.test.util.RetryOnFailure;
@@ -58,6 +59,10 @@ public class TabRedirectHandlerTest {
         ContextUtils.initApplicationContextForTests(new TestContext());
     }
 
+    private List<ResolveInfo> queryIntentActivities(Intent intent) {
+        return PackageManagerUtils.queryIntentActivities(intent, 0);
+    }
+
     @Test
     @SmallTest
     @Feature({"IntentHandling"})
@@ -70,9 +75,9 @@ public class TabRedirectHandlerTest {
         Assert.assertFalse(handler.isOnEffectiveIntentRedirectChain());
         handler.updateNewUrlLoading(TRANS_TYPE_OF_LINK_FROM_INTENT, true, false, 0, 0);
         Assert.assertTrue(handler.isOnEffectiveIntentRedirectChain());
-        Assert.assertFalse(handler.hasNewResolver(sMoblieYtIntent));
-        Assert.assertTrue(handler.hasNewResolver(sFooIntent));
-        Assert.assertFalse(handler.hasNewResolver(null));
+        Assert.assertFalse(handler.hasNewResolver(queryIntentActivities(sMoblieYtIntent)));
+        Assert.assertTrue(handler.hasNewResolver(queryIntentActivities(sFooIntent)));
+        Assert.assertFalse(handler.hasNewResolver(new ArrayList<ResolveInfo>()));
 
         Assert.assertTrue(handler.isOnNavigation());
         Assert.assertEquals(0, handler.getLastCommittedEntryIndexBeforeStartingNavigation());
@@ -90,9 +95,9 @@ public class TabRedirectHandlerTest {
         Assert.assertFalse(handler.isOnEffectiveIntentRedirectChain());
         handler.updateNewUrlLoading(PageTransition.LINK, false, false, 0, 1);
         Assert.assertTrue(handler.isOnEffectiveIntentRedirectChain());
-        Assert.assertFalse(handler.hasNewResolver(sMoblieYtIntent));
-        Assert.assertTrue(handler.hasNewResolver(sFooIntent));
-        Assert.assertFalse(handler.hasNewResolver(null));
+        Assert.assertFalse(handler.hasNewResolver(queryIntentActivities(sMoblieYtIntent)));
+        Assert.assertTrue(handler.hasNewResolver(queryIntentActivities(sFooIntent)));
+        Assert.assertFalse(handler.hasNewResolver(new ArrayList<ResolveInfo>()));
 
         Assert.assertTrue(handler.isOnNavigation());
         Assert.assertEquals(0, handler.getLastCommittedEntryIndexBeforeStartingNavigation());
@@ -110,9 +115,9 @@ public class TabRedirectHandlerTest {
         Assert.assertFalse(handler.isOnEffectiveIntentRedirectChain());
         handler.updateNewUrlLoading(PageTransition.FORM_SUBMIT, false, false, 0, 1);
         Assert.assertTrue(handler.isOnEffectiveIntentRedirectChain());
-        Assert.assertFalse(handler.hasNewResolver(sMoblieYtIntent));
-        Assert.assertTrue(handler.hasNewResolver(sFooIntent));
-        Assert.assertFalse(handler.hasNewResolver(null));
+        Assert.assertFalse(handler.hasNewResolver(queryIntentActivities(sMoblieYtIntent)));
+        Assert.assertTrue(handler.hasNewResolver(queryIntentActivities(sFooIntent)));
+        Assert.assertFalse(handler.hasNewResolver(new ArrayList<ResolveInfo>()));
 
         Assert.assertTrue(handler.isOnNavigation());
         Assert.assertEquals(0, handler.getLastCommittedEntryIndexBeforeStartingNavigation());
@@ -130,9 +135,9 @@ public class TabRedirectHandlerTest {
         Assert.assertFalse(handler.isOnEffectiveIntentRedirectChain());
         handler.updateNewUrlLoading(TRANS_TYPE_OF_LINK_FROM_INTENT, true, false, 0, 0);
         Assert.assertFalse(handler.isOnEffectiveIntentRedirectChain());
-        Assert.assertTrue(handler.hasNewResolver(sMoblieYtIntent));
-        Assert.assertTrue(handler.hasNewResolver(sFooIntent));
-        Assert.assertFalse(handler.hasNewResolver(null));
+        Assert.assertTrue(handler.hasNewResolver(queryIntentActivities(sMoblieYtIntent)));
+        Assert.assertTrue(handler.hasNewResolver(queryIntentActivities(sFooIntent)));
+        Assert.assertFalse(handler.hasNewResolver(new ArrayList<ResolveInfo>()));
 
         Assert.assertTrue(handler.isOnNavigation());
         Assert.assertEquals(0, handler.getLastCommittedEntryIndexBeforeStartingNavigation());
@@ -150,7 +155,7 @@ public class TabRedirectHandlerTest {
         Assert.assertFalse(handler.isOnEffectiveIntentRedirectChain());
         handler.updateNewUrlLoading(TRANS_TYPE_OF_LINK_FROM_INTENT, true, false, 0, 0);
         Assert.assertTrue(handler.isOnEffectiveIntentRedirectChain());
-        Assert.assertFalse(handler.hasNewResolver(sMoblieYtIntent));
+        Assert.assertFalse(handler.hasNewResolver(queryIntentActivities(sMoblieYtIntent)));
 
         Assert.assertTrue(handler.isOnNavigation());
         Assert.assertEquals(0, handler.getLastCommittedEntryIndexBeforeStartingNavigation());
@@ -158,9 +163,9 @@ public class TabRedirectHandlerTest {
         handler.clear();
         Assert.assertFalse(handler.isOnNavigation());
         Assert.assertFalse(handler.isOnEffectiveIntentRedirectChain());
-        Assert.assertTrue(handler.hasNewResolver(sMoblieYtIntent));
-        Assert.assertTrue(handler.hasNewResolver(sFooIntent));
-        Assert.assertFalse(handler.hasNewResolver(null));
+        Assert.assertTrue(handler.hasNewResolver(queryIntentActivities(sMoblieYtIntent)));
+        Assert.assertTrue(handler.hasNewResolver(queryIntentActivities(sFooIntent)));
+        Assert.assertFalse(handler.hasNewResolver(new ArrayList<ResolveInfo>()));
     }
 
     @Test
@@ -175,9 +180,9 @@ public class TabRedirectHandlerTest {
         Assert.assertFalse(handler.isOnEffectiveIntentRedirectChain());
         handler.updateNewUrlLoading(PageTransition.LINK, false, false, 0, 1);
         Assert.assertFalse(handler.isOnEffectiveIntentRedirectChain());
-        Assert.assertTrue(handler.hasNewResolver(sMoblieYtIntent));
-        Assert.assertTrue(handler.hasNewResolver(sFooIntent));
-        Assert.assertFalse(handler.hasNewResolver(null));
+        Assert.assertTrue(handler.hasNewResolver(queryIntentActivities(sMoblieYtIntent)));
+        Assert.assertTrue(handler.hasNewResolver(queryIntentActivities(sFooIntent)));
+        Assert.assertFalse(handler.hasNewResolver(new ArrayList<ResolveInfo>()));
 
         Assert.assertTrue(handler.isOnNavigation());
         Assert.assertEquals(0, handler.getLastCommittedEntryIndexBeforeStartingNavigation());
@@ -195,7 +200,7 @@ public class TabRedirectHandlerTest {
         Assert.assertFalse(handler.isOnEffectiveIntentRedirectChain());
         handler.updateNewUrlLoading(TRANS_TYPE_OF_LINK_FROM_INTENT, true, false, 0, 0);
         Assert.assertTrue(handler.isOnEffectiveIntentRedirectChain());
-        Assert.assertFalse(handler.hasNewResolver(sMoblieYtIntent));
+        Assert.assertFalse(handler.hasNewResolver(queryIntentActivities(sMoblieYtIntent)));
 
         Assert.assertTrue(handler.isOnNavigation());
         Assert.assertEquals(0, handler.getLastCommittedEntryIndexBeforeStartingNavigation());
@@ -204,9 +209,9 @@ public class TabRedirectHandlerTest {
         handler.updateNewUrlLoading(
                 PageTransition.LINK, false, true, SystemClock.elapsedRealtime(), 1);
         Assert.assertFalse(handler.isOnEffectiveIntentRedirectChain());
-        Assert.assertTrue(handler.hasNewResolver(sMoblieYtIntent));
-        Assert.assertTrue(handler.hasNewResolver(sFooIntent));
-        Assert.assertFalse(handler.hasNewResolver(null));
+        Assert.assertTrue(handler.hasNewResolver(queryIntentActivities(sMoblieYtIntent)));
+        Assert.assertTrue(handler.hasNewResolver(queryIntentActivities(sFooIntent)));
+        Assert.assertFalse(handler.hasNewResolver(new ArrayList<ResolveInfo>()));
 
         Assert.assertTrue(handler.isOnNavigation());
         Assert.assertEquals(1, handler.getLastCommittedEntryIndexBeforeStartingNavigation());