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

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

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 package org.chromium.chrome.browser.customtabs;
 
 import android.content.ComponentName;
 import android.content.Context;
 import android.content.Intent;
 import android.content.ServiceConnection;
@@ skipping 41 matching lines @@
     @VisibleForTesting static final int NO_SESSION_NO_WARMUP = 0;
     @VisibleForTesting static final int NO_SESSION_WARMUP = 1;
     @VisibleForTesting static final int SESSION_NO_WARMUP_ALREADY_CALLED = 2;
     @VisibleForTesting static final int SESSION_NO_WARMUP_NOT_CALLED = 3;
     @VisibleForTesting static final int SESSION_WARMUP = 4;
     @VisibleForTesting static final int SESSION_WARMUP_COUNT = 5;
 
     /** 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;
         public final DisconnectCallback disconnectCallback;
         public final String packageName;
         public final PostMessageHandler postMessageHandler;
         public boolean mIgnoreFragments;
         public boolean lowConfidencePrediction;
         public boolean highConfidencePrediction;
         private boolean mShouldHideDomain;
         private boolean mShouldPrerenderOnCellular;
         private boolean mShouldSendNavigationInfo;
-        private ServiceConnection mKeepAliveConnection;
+        private KeepAliveServiceConnection mKeepAliveConnection;
         private String mPredictedUrl;
         private long mLastMayLaunchUrlTimestamp;
 
         public SessionParams(Context context, int uid, DisconnectCallback callback,
                 PostMessageHandler postMessageHandler) {
             this.uid = uid;
             packageName = getPackageName(context, uid);
             disconnectCallback = callback;
             this.postMessageHandler = postMessageHandler;
         }
 
         private static String getPackageName(Context context, int uid) {
             PackageManager packageManager = context.getPackageManager();
             String[] packageList = packageManager.getPackagesForUid(uid);
             if (packageList.length != 1 || TextUtils.isEmpty(packageList[0])) return null;
             return packageList[0];
         }
 
-        public ServiceConnection getKeepAliveConnection() {
+        public KeepAliveServiceConnection getKeepAliveConnection() {
             return mKeepAliveConnection;
         }
 
-        public void setKeepAliveConnection(ServiceConnection serviceConnection) {
+        public void setKeepAliveConnection(KeepAliveServiceConnection serviceConnection) {
             mKeepAliveConnection = serviceConnection;
         }
 
         public void setPredictionMetrics(
                 String predictedUrl, long lastMayLaunchUrlTimestamp, boolean lowConfidence) {
             mPredictedUrl = predictedUrl;
             mLastMayLaunchUrlTimestamp = lastMayLaunchUrlTimestamp;
             highConfidencePrediction |= !TextUtils.isEmpty(predictedUrl);
             lowConfidencePrediction |= lowConfidence;
         }
@@ skipping 287 matching lines @@
 
     /** Tries to bind to a client to keep it alive, and returns true for success. */
     public synchronized boolean keepAliveForSession(CustomTabsSessionToken session, Intent intent) {
         // When an application is bound to a service, its priority is raised to
         // be at least equal to the application's one. This binds to a dummy
         // service (no calls to this service are made).
         if (intent == null || intent.getComponent() == null) return false;
         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;
     }
 
     /** Unbind from the KeepAlive service for a client. */
     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()} */
     public synchronized boolean isPrerenderingAllowed(int uid) {
         return RequestThrottler.getForUid(mContext, uid).isPrerenderingAllowed();
     }
 
     /** See {@link RequestThrottler#registerPrerenderRequest(String)} */
     public synchronized void registerPrerenderRequest(int uid, String url) {
         RequestThrottler.getForUid(mContext, uid).registerPrerenderRequest(url);
@@ skipping 25 matching lines @@
         SessionParams params = mSessionParams.get(session);
         if (params == null) return;
         mSessionParams.remove(session);
         if (params.postMessageHandler != null) {
             params.postMessageHandler.unbindFromContext(mContext);
         }
         if (params.disconnectCallback != null) params.disconnectCallback.run(session);
         mUidHasCalledWarmup.delete(params.uid);
     }
 }