customtabs: Extract a redirect endpoint, and maybe connect to it.

chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java

Index: chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java b/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java
index 8ca1690a9b6652ecf75ba6a447973935a513c692..b6281819b791a81e776b1587517ce42bc217ca50 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/customtabs/CustomTabsConnection.java
@@ -28,6 +28,7 @@ import android.util.Pair;
 import android.widget.RemoteViews;
 
 import org.chromium.base.CommandLine;
+import org.chromium.base.ContextUtils;
 import org.chromium.base.Log;
 import org.chromium.base.ThreadUtils;
 import org.chromium.base.TimeUtils;
@@ -56,6 +57,7 @@ import org.chromium.content.browser.ChildProcessLauncherHelper;
 import org.chromium.content_public.browser.LoadUrlParams;
 import org.chromium.content_public.browser.WebContents;
 import org.chromium.content_public.common.Referrer;
+import org.chromium.net.GURLUtils;
 
 import java.io.BufferedReader;
 import java.io.FileReader;
@@ -116,6 +118,9 @@ public class CustomTabsConnection {
     @VisibleForTesting
     static final int HIDDEN_TAB = 3;
 
+    @VisibleForTesting
+    static final String REDIRECT_ENDPOINT_KEY = "android.support.customtabs.REDIRECT_ENDPOINT";

[Yusuf, 2017/07/06 06:34:42]

some todo around moving this to support lib?

[Benoit L, 2017/07/07 11:30:56]

Done.

+
     private static AtomicReference<CustomTabsConnection> sInstance = new AtomicReference<>();
 
     /** Holds the parameters for the current speculation. */
@@ -346,15 +351,15 @@ public class CustomTabsConnection {
         return true;
     }
 
-    /** @return the URL converted to string, or null if it's invalid. */
-    private static String checkAndConvertUri(Uri uri) {
-        if (uri == null) return null;
+    /** @return the URL or null if it's invalid. */
+    private boolean isValid(Uri uri) {
+        if (uri == null) return false;
         // Don't do anything for unknown schemes. Not having a scheme is allowed, as we allow
         // "www.example.com".
         String scheme = uri.normalizeScheme().getScheme();
         boolean allowedScheme = scheme == null || scheme.equals("http") || scheme.equals("https");
-        if (!allowedScheme) return null;
-        return uri.toString();
+        if (!allowedScheme) return false;
+        return true;
     }
 
     /**
@@ -405,9 +410,8 @@ public class CustomTabsConnection {
             } catch (ClassCastException e) {
                 continue;
             }
-            String url = checkAndConvertUri(uri);
-            if (url != null) {
-                warmupManager.maybePreconnectUrlAndSubResources(profile, url);
+            if (isValid(uri)) {
+                warmupManager.maybePreconnectUrlAndSubResources(profile, uri.toString());
                 atLeastOneUrl = true;
             }
         }
@@ -430,7 +434,7 @@ public class CustomTabsConnection {
             final Bundle extras, final List<Bundle> otherLikelyBundles) {
         final boolean lowConfidence =
                 (url == null || TextUtils.isEmpty(url.toString())) && otherLikelyBundles != null;
-        final String urlString = checkAndConvertUri(url);
+        final String urlString = isValid(url) ? url.toString() : null;
         if (url != null && urlString == null && !lowConfidence) return false;
 
         // Things below need the browser process to be initialized.
@@ -725,6 +729,35 @@ public class CustomTabsConnection {
         return null;
     }
 
+    /**
+     * Called when an intent is handled by either an existing or a new CustomTabActivity.
+     *
+     * @param session Session extracted from the intent.
+     * @param url URL extracted from the intent.
+     * @param intent incoming intent.
+     */
+    void onHandledIntent(CustomTabsSessionToken session, String url, Intent intent) {
+        // For the preconnection to not be a no-op, we need more than just the native library.
+        Context context = ContextUtils.getApplicationContext();
+        if (!ChromeBrowserInitializer.getInstance(context).hasNativeInitializationCompleted()) {
+            return;
+        }
+        if (!ChromeFeatureList.isEnabled(ChromeFeatureList.CCT_REDIRECT_PRECONNECT)) return;
+
+        // Conditions:
+        // - There is a valid redirect endpoint.
+        // - The URL's origin is first party with respect to the app.
+        Uri redirectEndpoint = intent.getParcelableExtra(REDIRECT_ENDPOINT_KEY);

[Yusuf, 2017/07/06 06:34:41]

does it make sense to have this logic be in CustomTabIntentDataProvider?

[Benoit L, 2017/07/07 11:30:56]

This talks to ClientManager, and also I find it convenient to keep all the
"optimizations" in the same location.
wdyt?

+        if (redirectEndpoint == null || !isValid(redirectEndpoint)) return;
+
+        String origin = GURLUtils.getOrigin(url);
+        if (origin == null) return;
+        if (!mClientManager.isFirstPartyOriginForSession(session, Uri.parse(origin))) return;

[Yusuf, 2017/07/06 06:34:41]

Going back all the way to how we handle isValidOrigin, since this is independent
of current navigation and best effort, why not try to verify async as well? That
way we dont have to wait for the verifyOrigin support lib part to come to use
this, it would mostly improve and move that network request to earlier. Wdyt?

[Benoit L, 2017/07/07 11:30:55]

That would be a neat solution indeed.

However it would also likely cause complications, as in this case we would issue
the domain verification request at the same time as the navigations, and
essentially have a "race" between walking the redirects and getting the
validation of the origin.

If the two servers as as fast, then we don't gain anything, and end up with an
unused socket. Otherwise we might still see benefits.

I would err on the side of not doing it for now, as this would simplify metrics.
But I'm not firm on that.

+
+        WarmupManager.getInstance().maybePreconnectUrlAndSubResources(
+                Profile.getLastUsedProfile(), redirectEndpoint.toString());

[Yusuf, 2017/07/06 06:34:41]

getLastUsedProfile().getOriginalProfile() to avoid incognito.

[Benoit L, 2017/07/07 11:30:56]

Thanks!

Done.

+    }
+
     /** See {@link ClientManager#getReferrerForSession(CustomTabsSessionToken)} */
     public Referrer getReferrerForSession(CustomTabsSessionToken session) {
         return mClientManager.getReferrerForSession(session);