Split PostMessageHandler from PostMessageServiceConnection

Stop inheriting from it and instead use the two classes separately, with
PostMessageServiceConnection abstracted out of PostMessageHandler behind
the PostMessageBackend interface added in change 1299016.

This means we now store a PostMessageServiceConnection in SessionParams
and call to it instead of PostMessageHandler for behaviour relating to
the service connection.

The result of this change is that PostMessageHandler can be used without
a service connection, allowing us to use it directly from CustomTabActivity
in a future CL.

Bug: 898840
Change-Id: Ifd06ee0a803ecd82c350d7b5466bf0a0ee35440b
Reviewed-on: https://chromium-review.googlesource.com/c/1299143
Commit-Queue: David Barker <barkerd@google.com>
Reviewed-by: Michael van Ouwerkerk <mvanouwerkerk@chromium.org>
Reviewed-by: Peter Conn <peconn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603516}

DEPS

@@ -670,7 +670,7 @@ deps = {
   },
 
   'src/third_party/custom_tabs_client/src': {
-      'url': Var('chromium_git') + '/custom-tabs-client.git' + '@' + '873e69c103480c501fafdacf06e90febadeeff9c',
+      'url': Var('chromium_git') + '/custom-tabs-client.git' + '@' + 'c813ed8fcc4ece1838dba8dddf6d8ca39d6c6785',
       'condition': 'checkout_android',
   },
 

chrome/android/java/src/org/chromium/chrome/browser/browserservices/PostMessageHandler.java

@@ -4,12 +4,10 @@
 
 package org.chromium.chrome.browser.browserservices;
 
-import android.content.Context;
 import android.net.Uri;
-import android.support.annotation.NonNull;
 import android.support.customtabs.CustomTabsService;
 import android.support.customtabs.CustomTabsSessionToken;
-import android.support.customtabs.PostMessageServiceConnection;
+import android.support.customtabs.PostMessageBackend;
 
 import org.chromium.base.ContextUtils;
 import org.chromium.base.ThreadUtils;
@@ -24,42 +22,31 @@ import org.chromium.content_public.browser.WebContentsObserver;
 /**
  * A class that handles postMessage communications with a designated {@link CustomTabsSessionToken}.
  */
-public class PostMessageHandler
-        extends PostMessageServiceConnection implements OriginVerificationListener {
+public class PostMessageHandler implements OriginVerificationListener {
     private final MessageCallback mMessageCallback;
+    private final PostMessageBackend mPostMessageBackend;
     private WebContents mWebContents;
-    private boolean mMessageChannelCreated;
-    private boolean mBoundToService;
     private MessagePort[] mChannel;
     private Uri mPostMessageUri;
-    private String mPackageName;
 
     /**
      * Basic constructor. Everytime the given {@link CustomTabsSessionToken} is associated with a
      * new {@link WebContents},
      * {@link PostMessageHandler#reset(WebContents)} should be called to
      * reset all internal state.
-     * @param session The {@link CustomTabsSessionToken} to establish the postMessage communication
-     *                with.
+     * @param postMessageBackend The {@link PostMessageBackend} to which updates about the channel
+     *                           and posted messages will be sent.
      */
-    public PostMessageHandler(CustomTabsSessionToken session) {
-        super(session);
+    public PostMessageHandler(PostMessageBackend postMessageBackend) {
+        mPostMessageBackend = postMessageBackend;
         mMessageCallback = new MessageCallback() {
             @Override
             public void onMessage(String message, MessagePort[] sentPorts) {
-                if (mBoundToService) postMessage(message, null);
+                mPostMessageBackend.onPostMessage(message, null);
             }
         };
     }
 
-    /**
-     * Sets the package name unique to the session.
-     * @param packageName The package name for the client app for the owning session.
-     */
-    public void setPackageName(@NonNull String packageName) {
-        mPackageName = packageName;
-    }
-
     /**
      * Resets the internal state of the handler, linking the associated
      * {@link CustomTabsSessionToken} with a new {@link WebContents} and the {@link Tab} that
@@ -70,7 +57,6 @@ public class PostMessageHandler
     public void reset(final WebContents webContents) {
         if (webContents == null || webContents.isDestroyed()) {
             disconnectChannel();
-            unbindFromContext(ContextUtils.getApplicationContext());
             return;
         }
         // Can't reset with the same web contents twice.
@@ -89,7 +75,6 @@ public class PostMessageHandler
                         && mChannel != null) {
                     webContents.removeObserver(this);
                     disconnectChannel();
-                    unbindFromContext(ContextUtils.getApplicationContext());
                     return;
                 }
                 mNavigatedOnce = true;
@@ -98,7 +83,6 @@ public class PostMessageHandler
             @Override
             public void renderProcessGone(boolean wasOomProtected) {
                 disconnectChannel();
-                unbindFromContext(ContextUtils.getApplicationContext());
             }
 
             @Override
@@ -109,16 +93,6 @@ public class PostMessageHandler
         };
     }
 
-    /**
-     * See
-     * {@link PostMessageServiceConnection#bindSessionToPostMessageService(Context, String)}.
-     * Attempts to bind with the package name set during initialization.
-     */
-    public boolean bindSessionToPostMessageService() {
-        return super.bindSessionToPostMessageService(
-                ContextUtils.getApplicationContext(), mPackageName);
-    }
-
     private void initializeWithWebContents(final WebContents webContents) {
         mChannel = webContents.createMessageChannel();
         mChannel[0].setMessageCallback(mMessageCallback, null);
@@ -126,8 +100,7 @@ public class PostMessageHandler
         webContents.postMessageToFrame(
                 null, "", mPostMessageUri.toString(), "", new MessagePort[] {mChannel[1]});
 
-        mMessageChannelCreated = true;
-        if (mBoundToService) notifyMessageChannelReady(null);
+        mPostMessageBackend.onNotifyMessageChannelReady(null);
     }
 
     private void disconnectChannel() {
@@ -135,6 +108,7 @@ public class PostMessageHandler
         mChannel[0].close();
         mChannel = null;
         mWebContents = null;
+        mPostMessageBackend.onDisconnectChannel(ContextUtils.getApplicationContext());
     }
 
     /**
@@ -173,22 +147,6 @@ public class PostMessageHandler
         return CustomTabsService.RESULT_SUCCESS;
     }
 
-    @Override
-    public void unbindFromContext(Context context) {
-        if (mBoundToService) super.unbindFromContext(context);
-    }
-
-    @Override
-    public void onPostMessageServiceConnected() {
-        mBoundToService = true;
-        if (mMessageChannelCreated) notifyMessageChannelReady(null);
-    }
-
-    @Override
-    public void onPostMessageServiceDisconnected() {
-        mBoundToService = false;
-    }
-
     @Override
     public void onOriginVerified(String packageName, Origin origin, boolean result,
             Boolean online) {
@@ -204,12 +162,4 @@ public class PostMessageHandler
     public Uri getPostMessageUriForTesting() {
         return mPostMessageUri;
     }
-
-    /**
-     * Cleans up any dependencies that this handler might have.
-     * @param context Context to use for unbinding if necessary.
-     */
-    public void cleanup(Context context) {
-        if (mBoundToService) super.unbindFromContext(context);
-    }
 }

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

@@ -20,6 +20,7 @@ import android.support.customtabs.CustomTabsCallback;
 import android.support.customtabs.CustomTabsService;
 import android.support.customtabs.CustomTabsService.Relation;
 import android.support.customtabs.CustomTabsSessionToken;
+import android.support.customtabs.PostMessageServiceConnection;
 import android.text.TextUtils;
 import android.util.SparseBooleanArray;
 
@@ -167,6 +168,7 @@ class ClientManager {
         private CustomTabsCallback mCustomTabsCallback;
         public final DisconnectCallback disconnectCallback;
         public final PostMessageHandler postMessageHandler;
+        public final PostMessageServiceConnection serviceConnection;
         public final Set<Origin> mLinkedOrigins = new HashSet<>();
         public OriginVerifier originVerifier;
         public boolean mIgnoreFragments;
@@ -186,13 +188,15 @@ class ClientManager {
         private boolean mShouldGetPageLoadMetrics;
 
         public SessionParams(Context context, int uid, CustomTabsCallback customTabsCallback,
-                             DisconnectCallback callback, PostMessageHandler postMessageHandler) {
+                DisconnectCallback callback, PostMessageHandler postMessageHandler,
+                PostMessageServiceConnection serviceConnection) {
             this.uid = uid;
             mPackageName = getPackageName(context, uid);
             mCustomTabsCallback = customTabsCallback;
             disconnectCallback = callback;
             this.postMessageHandler = postMessageHandler;
-            if (postMessageHandler != null) this.postMessageHandler.setPackageName(mPackageName);
+            this.serviceConnection = serviceConnection;
+            if (postMessageHandler != null) this.serviceConnection.setPackageName(mPackageName);
         }
 
         /**
@@ -297,13 +301,14 @@ class ClientManager {
      * @return true for success.
      */
     public synchronized boolean newSession(CustomTabsSessionToken session, int uid,
-            DisconnectCallback onDisconnect, @NonNull PostMessageHandler postMessageHandler) {
+            DisconnectCallback onDisconnect, @NonNull PostMessageHandler postMessageHandler,
+            @NonNull PostMessageServiceConnection serviceConnection) {
         if (session == null || session.getCallback() == null) return false;
         if (mSessionParams.containsKey(session)) {
             mSessionParams.get(session).setCustomTabsCallback(session.getCallback());
         } else {
             SessionParams params = new SessionParams(ContextUtils.getApplicationContext(), uid,
-                    session.getCallback(), onDisconnect, postMessageHandler);
+                    session.getCallback(), onDisconnect, postMessageHandler, serviceConnection);
             mSessionParams.put(session, params);
         }
 
@@ -427,12 +432,13 @@ class ClientManager {
     }
 
     /**
-     * See {@link PostMessageHandler#bindSessionToPostMessageService(Context, String)}.
+     * See {@link PostMessageServiceConnection#bindSessionToPostMessageService(Context, String)}.
      */
     public synchronized boolean bindToPostMessageServiceForSession(CustomTabsSessionToken session) {
         SessionParams params = mSessionParams.get(session);
         if (params == null) return false;
-        return params.postMessageHandler.bindSessionToPostMessageService();
+        return params.serviceConnection.bindSessionToPostMessageService(
+                ContextUtils.getApplicationContext());
     }
 
     /**
@@ -791,8 +797,8 @@ class ClientManager {
         SessionParams params = mSessionParams.get(session);
         if (params == null) return;
         mSessionParams.remove(session);
-        if (params.postMessageHandler != null) {
-            params.postMessageHandler.cleanup(ContextUtils.getApplicationContext());
+        if (params.serviceConnection != null) {
+            params.serviceConnection.cleanup(ContextUtils.getApplicationContext());
         }
         if (params.originVerifier != null) params.originVerifier.cleanUp();
         if (params.disconnectCallback != null) params.disconnectCallback.run(session);

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

@@ -23,6 +23,7 @@ import android.support.customtabs.CustomTabsCallback;
 import android.support.customtabs.CustomTabsIntent;
 import android.support.customtabs.CustomTabsService;
 import android.support.customtabs.CustomTabsSessionToken;
+import android.support.customtabs.PostMessageServiceConnection;
 import android.text.TextUtils;
 import android.widget.RemoteViews;
 
@@ -359,8 +360,10 @@ public class CustomTabsConnection {
                 cancelSpeculation(session);
             }
         };
-        PostMessageHandler handler = new PostMessageHandler(session);
-        return mClientManager.newSession(session, Binder.getCallingUid(), onDisconnect, handler);
+        PostMessageServiceConnection serviceConnection = new PostMessageServiceConnection(session);
+        PostMessageHandler handler = new PostMessageHandler(serviceConnection);
+        return mClientManager.newSession(
+                session, Binder.getCallingUid(), onDisconnect, handler, serviceConnection);
     }
 
     /**

chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/ClientManagerTest.java

@@ -9,6 +9,7 @@ import android.net.Uri;
 import android.os.Process;
 import android.support.customtabs.CustomTabsService;
 import android.support.customtabs.CustomTabsSessionToken;
+import android.support.customtabs.PostMessageServiceConnection;
 import android.support.test.InstrumentationRegistry;
 import android.support.test.filters.SmallTest;
 
@@ -94,7 +95,7 @@ public class ClientManagerTest {
     @Test
     @SmallTest
     public void testValidSessionNoWarmup() {
-        mClientManager.newSession(mSession, mUid, null, null);
+        mClientManager.newSession(mSession, mUid, null, null, null);
         Assert.assertEquals(ClientManager.CalledWarmup.SESSION_NO_WARMUP_NOT_CALLED,
                 mClientManager.getWarmupState(mSession));
     }
@@ -103,7 +104,7 @@ public class ClientManagerTest {
     @SmallTest
     public void testValidSessionOtherWarmup() {
         mClientManager.recordUidHasCalledWarmup(mUid + 1);
-        mClientManager.newSession(mSession, mUid, null, null);
+        mClientManager.newSession(mSession, mUid, null, null, null);
         Assert.assertEquals(ClientManager.CalledWarmup.SESSION_NO_WARMUP_ALREADY_CALLED,
                 mClientManager.getWarmupState(mSession));
     }
@@ -112,7 +113,7 @@ public class ClientManagerTest {
     @SmallTest
     public void testValidSessionWarmup() {
         mClientManager.recordUidHasCalledWarmup(mUid);
-        mClientManager.newSession(mSession, mUid, null, null);
+        mClientManager.newSession(mSession, mUid, null, null, null);
         Assert.assertEquals(
                 ClientManager.CalledWarmup.SESSION_WARMUP, mClientManager.getWarmupState(mSession));
     }
@@ -121,12 +122,12 @@ public class ClientManagerTest {
     @SmallTest
     public void testValidSessionWarmupSeveralCalls() {
         mClientManager.recordUidHasCalledWarmup(mUid);
-        mClientManager.newSession(mSession, mUid, null, null);
+        mClientManager.newSession(mSession, mUid, null, null, null);
         Assert.assertEquals(
                 ClientManager.CalledWarmup.SESSION_WARMUP, mClientManager.getWarmupState(mSession));
 
         CustomTabsSessionToken token = CustomTabsSessionToken.createMockSessionTokenForTesting();
-        mClientManager.newSession(token, mUid, null, null);
+        mClientManager.newSession(token, mUid, null, null, null);
         Assert.assertEquals(
                 ClientManager.CalledWarmup.SESSION_WARMUP, mClientManager.getWarmupState(token));
     }
@@ -135,7 +136,7 @@ public class ClientManagerTest {
     @SmallTest
     @RetryOnFailure
     public void testPredictionOutcomeSuccess() {
-        Assert.assertTrue(mClientManager.newSession(mSession, mUid, null, null));
+        Assert.assertTrue(mClientManager.newSession(mSession, mUid, null, null, null));
         Assert.assertTrue(
                 mClientManager.updateStatsAndReturnWhetherAllowed(mSession, mUid, URL, false));
         Assert.assertEquals(ClientManager.PredictionStatus.GOOD,
@@ -145,7 +146,7 @@ public class ClientManagerTest {
     @Test
     @SmallTest
     public void testPredictionOutcomeNoPrediction() {
-        Assert.assertTrue(mClientManager.newSession(mSession, mUid, null, null));
+        Assert.assertTrue(mClientManager.newSession(mSession, mUid, null, null, null));
         mClientManager.recordUidHasCalledWarmup(mUid);
         Assert.assertEquals(ClientManager.PredictionStatus.NONE,
                 mClientManager.getPredictionOutcome(mSession, URL));
@@ -154,7 +155,7 @@ public class ClientManagerTest {
     @Test
     @SmallTest
     public void testPredictionOutcomeBadPrediction() {
-        Assert.assertTrue(mClientManager.newSession(mSession, mUid, null, null));
+        Assert.assertTrue(mClientManager.newSession(mSession, mUid, null, null, null));
         Assert.assertTrue(
                 mClientManager.updateStatsAndReturnWhetherAllowed(mSession, mUid, URL, false));
         Assert.assertEquals(ClientManager.PredictionStatus.BAD,
@@ -164,7 +165,7 @@ public class ClientManagerTest {
     @Test
     @SmallTest
     public void testPredictionOutcomeIgnoreFragment() {
-        Assert.assertTrue(mClientManager.newSession(mSession, mUid, null, null));
+        Assert.assertTrue(mClientManager.newSession(mSession, mUid, null, null, null));
         Assert.assertTrue(
                 mClientManager.updateStatsAndReturnWhetherAllowed(mSession, mUid, URL, false));
         mClientManager.setIgnoreFragmentsForSession(mSession, true);
@@ -176,7 +177,9 @@ public class ClientManagerTest {
     @SmallTest
     public void testPostMessageOriginVerification() {
         final ClientManager cm = mClientManager;
-        Assert.assertTrue(cm.newSession(mSession, mUid, null, new PostMessageHandler(mSession)));
+        PostMessageServiceConnection serviceConnection = new PostMessageServiceConnection(mSession);
+        Assert.assertTrue(cm.newSession(mSession, mUid, null,
+                new PostMessageHandler(serviceConnection), serviceConnection));
         // Should always start with no origin.
         Assert.assertNull(cm.getPostMessageOriginForSessionForTesting(mSession));
 
@@ -223,7 +226,9 @@ public class ClientManagerTest {
     @SmallTest
     public void testPostMessageOriginDifferentRelations() {
         final ClientManager cm = mClientManager;
-        Assert.assertTrue(cm.newSession(mSession, mUid, null, new PostMessageHandler(mSession)));
+        PostMessageServiceConnection serviceConnection = new PostMessageServiceConnection(mSession);
+        Assert.assertTrue(cm.newSession(mSession, mUid, null,
+                new PostMessageHandler(serviceConnection), serviceConnection));
         // Should always start with no origin.
         Assert.assertNull(cm.getPostMessageOriginForSessionForTesting(mSession));
 
@@ -260,7 +265,9 @@ public class ClientManagerTest {
     @SmallTest
     public void testPostMessageOriginHttpNotAllowed() {
         final ClientManager cm = mClientManager;
-        Assert.assertTrue(cm.newSession(mSession, mUid, null, new PostMessageHandler(mSession)));
+        PostMessageServiceConnection serviceConnection = new PostMessageServiceConnection(mSession);
+        Assert.assertTrue(cm.newSession(mSession, mUid, null,
+                new PostMessageHandler(serviceConnection), serviceConnection));
         // Should always start with no origin.
         Assert.assertNull(cm.getPostMessageOriginForSessionForTesting(mSession));
 
@@ -291,7 +298,7 @@ public class ClientManagerTest {
         Context context = InstrumentationRegistry.getInstrumentation()
                                   .getTargetContext()
                                   .getApplicationContext();
-        Assert.assertTrue(mClientManager.newSession(mSession, mUid, null, null));
+        Assert.assertTrue(mClientManager.newSession(mSession, mUid, null, null, null));
 
         // Two low confidence in a row is OK.
         Assert.assertTrue(
@@ -334,7 +341,7 @@ public class ClientManagerTest {
         MetricsUtils.HistogramDelta bothDelta =
                 new MetricsUtils.HistogramDelta(name, ClientManager.MayLaunchUrlType.BOTH);
 
-        Assert.assertTrue(mClientManager.newSession(mSession, mUid, null, null));
+        Assert.assertTrue(mClientManager.newSession(mSession, mUid, null, null, null));
 
         // No prediction;
         mClientManager.registerLaunch(mSession, URL);

third_party/custom_tabs_client/BUILD.gn

@@ -73,6 +73,7 @@ android_library("custom_tabs_support_java") {
     "src/customtabs/src/android/support/customtabs/CustomTabsServiceConnection.java",
     "src/customtabs/src/android/support/customtabs/CustomTabsSession.java",
     "src/customtabs/src/android/support/customtabs/CustomTabsSessionToken.java",
+    "src/customtabs/src/android/support/customtabs/PostMessageBackend.java",
     "src/customtabs/src/android/support/customtabs/PostMessageService.java",
     "src/customtabs/src/android/support/customtabs/PostMessageServiceConnection.java",
     "src/customtabs/src/android/support/customtabs/TrustedWebUtils.java",