customtabs: Don't resurrect a "kept alive" process that died.

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

Index: chrome/android/java/src/org/chromium/chrome/browser/customtabs/ClientManager.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/customtabs/ClientManager.java b/chrome/android/java/src/org/chromium/chrome/browser/customtabs/ClientManager.java
index 9c56a1042b320ae513fd0dc0561421e8b11082f3..a9e74f4045df8508735c14ee38d626b07b02b0dc 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/customtabs/ClientManager.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/customtabs/ClientManager.java
@@ -59,6 +59,55 @@ class ClientManager {
     /** To be called when a client gets disconnected. */
     public interface DisconnectCallback { public void run(CustomTabsSessionToken session); }
 
+    private static class KeepAliveServiceConnection implements ServiceConnection {
+        private final Context mContext;
+        private final Intent mServiceIntent;
+        private boolean mHasDied;
+        private boolean mIsBound;
+
+        public KeepAliveServiceConnection(Context context, Intent serviceIntent) {
+            mContext = context;
+            mServiceIntent = serviceIntent;
+        }
+

[Yusuf, 2017/02/15 17:43:52]

javadoc

[Benoit L, 2017/02/16 10:25:52]

Done.

+        public boolean connect() {
+            if (mIsBound) return true;
+            // If the remote process died at some point, it doesn't make sense to resurrect it.
+            if (mHasDied) return false;
+
+            boolean ok;
+            try {
+                ok = mContext.bindService(mServiceIntent, this, Context.BIND_AUTO_CREATE);
+            } catch (SecurityException e) {
+                return false;
+            }
+            mIsBound = ok;
+            return ok;
+        }
+

[Yusuf, 2017/02/15 17:43:52]

javadoc

[Benoit L, 2017/02/16 10:25:52]

Done.

+        public void disconnect() {
+            if (mIsBound) {
+                mContext.unbindService(this);

[Yusuf, 2017/02/15 17:43:51]

Will unbind still work if we are disconnected? When onServiceDisconnected calls
this won't it crash saying it is already not bound? Or binding and being
connected are technically different?

[Benoit L, 2017/02/16 10:25:52]

Yes, but you cannot unbind twice, otherwise the framework will raise an
exception. When the remote service dies but the application is still bound to
it, the framework restarts it automatically.

+                mIsBound = false;
+            }
+        }
+
+        @Override
+        public void onServiceConnected(ComponentName name, IBinder service) {}
+
+        @Override
+        public void onServiceDisconnected(ComponentName name) {
+            if (mIsBound) {
+                // The remote process has died. This typically happens if the system is low enough
+                // on memory to kill one of the last process on the "kill list". In this case, we
+                // shouldn't resurrect the process (which happens with BIND_AUTO_CREATE) because
+                // that could create a "restart/kill" loop.
+                mHasDied = true;
+                disconnect();
+            }
+        }
+    }
+
     /** Per-session values. */
     private static class SessionParams {
         public final int uid;
@@ -71,7 +120,7 @@ class ClientManager {
         private boolean mShouldHideDomain;
         private boolean mShouldPrerenderOnCellular;
         private boolean mShouldSendNavigationInfo;
-        private ServiceConnection mKeepAliveConnection;
+        private KeepAliveServiceConnection mKeepAliveConnection;
         private String mPredictedUrl;
         private long mLastMayLaunchUrlTimestamp;
 
@@ -90,11 +139,11 @@ class ClientManager {
             return packageList[0];
         }
 
-        public ServiceConnection getKeepAliveConnection() {
+        public KeepAliveServiceConnection getKeepAliveConnection() {
             return mKeepAliveConnection;
         }
 
-        public void setKeepAliveConnection(ServiceConnection serviceConnection) {
+        public void setKeepAliveConnection(KeepAliveServiceConnection serviceConnection) {
             mKeepAliveConnection = serviceConnection;
         }
 
@@ -402,28 +451,20 @@ class ClientManager {
         SessionParams params = mSessionParams.get(session);
         if (params == null) return false;
 
-        String packageName = intent.getComponent().getPackageName();
-        PackageManager pm = mContext.getApplicationContext().getPackageManager();
-        // Only binds to the application associated to this session.
-        if (!Arrays.asList(pm.getPackagesForUid(params.uid)).contains(packageName)) return false;
-        Intent serviceIntent = new Intent().setComponent(intent.getComponent());
-        // This ServiceConnection doesn't handle disconnects. This is on
-        // purpose, as it occurs when the remote process has died. Since the
-        // only use of this connection is to keep the application alive,
-        // re-connecting would just re-create the process, but the application
-        // state has been lost at that point, the callbacks invalidated, etc.
-        ServiceConnection connection = new ServiceConnection() {
-            @Override
-            public void onServiceConnected(ComponentName name, IBinder service) {}
-            @Override
-            public void onServiceDisconnected(ComponentName name) {}
-        };
-        boolean ok;
-        try {
-            ok = mContext.bindService(serviceIntent, connection, Context.BIND_AUTO_CREATE);
-        } catch (SecurityException e) {
-            return false;
+        KeepAliveServiceConnection connection = params.getKeepAliveConnection();
+
+        if (connection == null) {
+            String packageName = intent.getComponent().getPackageName();
+            PackageManager pm = mContext.getApplicationContext().getPackageManager();
+            // Only binds to the application associated to this session.
+            if (!Arrays.asList(pm.getPackagesForUid(params.uid)).contains(packageName)) {
+                return false;
+            }
+            Intent serviceIntent = new Intent().setComponent(intent.getComponent());
+            connection = new KeepAliveServiceConnection(mContext, serviceIntent);
         }
+
+        boolean ok = connection.connect();
         if (ok) params.setKeepAliveConnection(connection);
         return ok;
     }
@@ -432,9 +473,8 @@ class ClientManager {
     public synchronized void dontKeepAliveForSession(CustomTabsSessionToken session) {
         SessionParams params = mSessionParams.get(session);
         if (params == null || params.getKeepAliveConnection() == null) return;
-        ServiceConnection connection = params.getKeepAliveConnection();
-        params.setKeepAliveConnection(null);
-        mContext.unbindService(connection);
+        KeepAliveServiceConnection connection = params.getKeepAliveConnection();
+        connection.disconnect();
     }
 
     /** See {@link RequestThrottler#isPrerenderingAllowed()} */