Removed the service number member from BaseChildProcessConnection.

In preparation for making the ChildProcessConnection simpler, removing
the service number member from  BaseChildProcessConnection.
It is pertaining to the allocator, not the connection.

Also replaced the LruCache used for the moderate binding pool in
BindingManagerImpl (which was relying on the service number from the
connection) with a LinkedList. It makes the code clearer since we were
only using the LruCache to evict old connections, not the cache part.

BUG=689758

Review-Url: https://codereview.chromium.org/2855323003
Cr-Commit-Position: refs/heads/master@{#469452}

content/public/android/java/src/org/chromium/content/browser/BaseChildProcessConnection.java

@@ -74,7 +74,7 @@ public abstract class BaseChildProcessConnection {
 
     /** Used to create specialization connection instances. */
     interface Factory {
-        BaseChildProcessConnection create(Context context, int number, boolean sandboxed,
+        BaseChildProcessConnection create(Context context, boolean sandboxed,
                 DeathCallback deathCallback, String serviceClassName,
                 Bundle childProcessCommonParameters, ChildProcessCreationParams creationParams);
     }
@@ -162,7 +162,6 @@ public abstract class BaseChildProcessConnection {
     // TODO(mnaganov): Get rid of it after the release of the next Android SDK.
     private static Boolean sNeedsExtrabindFlags[] = new Boolean[2];
     private final Context mContext;
-    private final int mServiceNumber;
     private final boolean mSandboxed;
     private final BaseChildProcessConnection.DeathCallback mDeathCallback;
     private final ComponentName mServiceName;
@@ -216,17 +215,16 @@ public abstract class BaseChildProcessConnection {
     // Process ID of the corresponding child process.
     private int mPid;
 
-    protected BaseChildProcessConnection(Context context, int number, boolean sandboxed,
+    protected BaseChildProcessConnection(Context context, boolean sandboxed,
             DeathCallback deathCallback, String serviceClassName,
             Bundle childProcessCommonParameters, ChildProcessCreationParams creationParams) {
         assert LauncherThread.runningOnLauncherThread();
         mContext = context;
-        mServiceNumber = number;
         mSandboxed = sandboxed;
         mDeathCallback = deathCallback;
         String packageName =
                 creationParams != null ? creationParams.getPackageName() : context.getPackageName();
-        mServiceName = new ComponentName(packageName, serviceClassName + mServiceNumber);
+        mServiceName = new ComponentName(packageName, serviceClassName);
         mChildProcessCommonParameters = childProcessCommonParameters;
         mCreationParams = creationParams;
     }
@@ -236,11 +234,6 @@ public abstract class BaseChildProcessConnection {
         return mContext;
     }
 
-    public final int getServiceNumber() {
-        assert LauncherThread.runningOnLauncherThread();
-        return mServiceNumber;
-    }
-
     public final boolean isSandboxed() {
         assert LauncherThread.runningOnLauncherThread();
         return mSandboxed;

content/public/android/java/src/org/chromium/content/browser/BindingManagerImpl.java

@@ -4,12 +4,9 @@
 
 package org.chromium.content.browser;
 
-import android.annotation.TargetApi;
 import android.content.ComponentCallbacks2;
 import android.content.Context;
 import android.content.res.Configuration;
-import android.os.Build;
-import android.util.LruCache;
 import android.util.SparseArray;
 
 import org.chromium.base.Log;
@@ -18,14 +15,14 @@ import org.chromium.base.ThreadUtils;
 import org.chromium.base.VisibleForTesting;
 import org.chromium.base.metrics.RecordHistogram;
 
-import java.util.Map;
+import java.util.LinkedList;
 
 /**
  * Manages oom bindings used to bound child services.
  * This object must only be accessed from the launcher thread.
  */
 class BindingManagerImpl implements BindingManager {
-    private static final String TAG = "cr.BindingManager";
+    private static final String TAG = "cr_BindingManager";
 
     // Low reduce ratio of moderate binding.
     private static final float MODERATE_BINDING_LOW_REDUCE_RATIO = 0.25f;
@@ -43,12 +40,15 @@ class BindingManagerImpl implements BindingManager {
     // createBindingManagerForTesting().
     private final boolean mIsLowMemoryDevice;
 
-    private static class ModerateBindingPool
-            extends LruCache<Integer, ManagedConnection> implements ComponentCallbacks2 {
+    private static class ModerateBindingPool implements ComponentCallbacks2 {
+        // Stores the connections in MRU order.
+        private final LinkedList<ManagedConnection> mConnections = new LinkedList<>();
+        private final int mMaxSize;
+
         private Runnable mDelayedClearer;
 
         public ModerateBindingPool(int maxSize) {
-            super(maxSize);
+            mMaxSize = maxSize;
         }
 
         @Override
@@ -57,8 +57,8 @@ class BindingManagerImpl implements BindingManager {
             LauncherThread.post(new Runnable() {
                 @Override
                 public void run() {
-                    Log.i(TAG, "onTrimMemory: level=%d, size=%d", level, size());
-                    if (size() <= 0) {
+                    Log.i(TAG, "onTrimMemory: level=%d, size=%d", level, mConnections.size());
+                    if (mConnections.isEmpty()) {
                         return;
                     }
                     if (level <= TRIM_MEMORY_RUNNING_MODERATE) {
@@ -69,7 +69,7 @@ class BindingManagerImpl implements BindingManager {
                         // This will be handled by |mDelayedClearer|.
                         return;
                     } else {
-                        evictAll();
+                        removeAllConnections();
                     }
                 }
             });
@@ -81,8 +81,8 @@ class BindingManagerImpl implements BindingManager {
             LauncherThread.post(new Runnable() {
                 @Override
                 public void run() {
-                    Log.i(TAG, "onLowMemory: evict %d bindings", size());
-                    evictAll();
+                    Log.i(TAG, "onLowMemory: evict %d bindings", mConnections.size());
+                    removeAllConnections();
                 }
             });
         }
@@ -90,25 +90,12 @@ class BindingManagerImpl implements BindingManager {
         @Override
         public void onConfigurationChanged(Configuration configuration) {}
 
-        @TargetApi(Build.VERSION_CODES.JELLY_BEAN_MR1)
         private void reduce(float reduceRatio) {
-            int oldSize = size();
+            int oldSize = mConnections.size();
             int newSize = (int) (oldSize * (1f - reduceRatio));
             Log.i(TAG, "Reduce connections from %d to %d", oldSize, newSize);
-            if (newSize == 0) {
-                evictAll();
-            } else if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.JELLY_BEAN_MR1) {
-                trimToSize(newSize);
-            } else {
-                // Entries will be removed from the front because snapshot() returns ones ordered
-                // from least recently accessed to most recently accessed.
-                int count = 0;
-                for (Map.Entry<Integer, ManagedConnection> entry : snapshot().entrySet()) {
-                    remove(entry.getKey());
-                    ++count;
-                    if (count == oldSize - newSize) break;
-                }
-            }
+            removeOldConnections(oldSize - newSize);
+            assert mConnections.size() == newSize;
         }
 
         void addConnection(ManagedConnection managedConnection) {
@@ -116,9 +103,9 @@ class BindingManagerImpl implements BindingManager {
             if (connection != null && connection.isSandboxed()) {
                 managedConnection.addModerateBinding();
                 if (connection.isModerateBindingBound()) {
-                    put(connection.getServiceNumber(), managedConnection);
+                    addConnectionImpl(managedConnection);
                 } else {
-                    remove(connection.getServiceNumber());
+                    removeConnectionImpl(managedConnection);
                 }
             }
         }
@@ -126,31 +113,60 @@ class BindingManagerImpl implements BindingManager {
         void removeConnection(ManagedConnection managedConnection) {
             ManagedChildProcessConnection connection = managedConnection.mConnection;
             if (connection != null && connection.isSandboxed()) {
-                remove(connection.getServiceNumber());
+                removeConnectionImpl(managedConnection);
             }
         }
 
-        @Override
-        protected void entryRemoved(boolean evicted, Integer key, ManagedConnection oldValue,
-                ManagedConnection newValue) {
-            if (oldValue != newValue) {
-                oldValue.removeModerateBinding();
+        void removeAllConnections() {
+            removeOldConnections(mConnections.size());
+        }
+
+        int size() {
+            return mConnections.size();
+        }
+
+        private void addConnectionImpl(ManagedConnection managedConnection) {
+            // Note that the size of connections is currently fairly small (20).
+            // If it became bigger we should consider using an alternate data structure so we don't
+            // have to traverse the list every time.
+
+            // Remove the connection if it's already in the list, we'll add it at the head.
+            mConnections.removeFirstOccurrence(managedConnection);
+            if (mConnections.size() == mMaxSize) {
+                // Make room for the connection we are about to add.
+                removeOldConnections(1);
+            }
+            mConnections.add(0, managedConnection);
+            assert mConnections.size() <= mMaxSize;
+        }
+
+        private void removeConnectionImpl(ManagedConnection managedConnection) {
+            int index = mConnections.indexOf(managedConnection);
+            if (index != -1) {
+                ManagedConnection connection = mConnections.remove(index);
+                connection.mConnection.removeModerateBinding();
+            }
+        }
+
+        private void removeOldConnections(int numberOfConnections) {
+            assert numberOfConnections <= mConnections.size();
+            for (int i = 0; i < numberOfConnections; i++) {
+                ManagedConnection connection = mConnections.removeLast();
+                connection.mConnection.removeModerateBinding();
             }
         }
 
         void onSentToBackground(final boolean onTesting) {
-            if (size() == 0) return;
+            if (mConnections.isEmpty()) return;
             mDelayedClearer = new Runnable() {
                 @Override
                 public void run() {
-                    if (mDelayedClearer == null) return;
-                    mDelayedClearer = null;
-                    Log.i(TAG, "Release moderate connections: %d", size());
+                    Log.i(TAG, "Release moderate connections: %d", mConnections.size());
                     if (!onTesting) {
                         RecordHistogram.recordCountHistogram(
-                                "Android.ModerateBindingCount", size());
+                                "Android.ModerateBindingCount", mConnections.size());
                     }
-                    evictAll();
+                    removeAllConnections();
                 }
             };
             LauncherThread.postDelayed(mDelayedClearer, MODERATE_BINDING_POOL_CLEARER_DELAY_MILLIS);
@@ -459,7 +475,7 @@ class BindingManagerImpl implements BindingManager {
         assert LauncherThread.runningOnLauncherThread();
         if (mModerateBindingPool != null) {
             Log.i(TAG, "Release all moderate bindings: %d", mModerateBindingPool.size());
-            mModerateBindingPool.evictAll();
+            mModerateBindingPool.removeAllConnections();
         }
     }
 }

content/public/android/java/src/org/chromium/content/browser/ChildConnectionAllocator.java

@@ -18,6 +18,7 @@ import org.chromium.content.app.PrivilegedProcessService;
 import org.chromium.content.app.SandboxedProcessService;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.HashMap;
 import java.util.LinkedList;
 import java.util.Map;
@@ -191,8 +192,9 @@ public class ChildConnectionAllocator {
         }
         int slot = mFreeConnectionIndices.remove(0);
         assert mChildProcessConnections[slot] == null;
-        mChildProcessConnections[slot] = mConnectionFactory.create(spawnData.getContext(), slot,
-                mInSandbox, deathCallback, mChildClassName, childProcessCommonParameters,
+        String serviceClassName = mChildClassName + slot;
+        mChildProcessConnections[slot] = mConnectionFactory.create(spawnData.getContext(),
+                mInSandbox, deathCallback, serviceClassName, childProcessCommonParameters,
                 spawnData.getCreationParams());
         Log.d(TAG, "Allocator allocated a connection, sandbox: %b, slot: %d", mInSandbox, slot);
         return mChildProcessConnections[slot];
@@ -201,15 +203,11 @@ public class ChildConnectionAllocator {
     // Also return the first ChildSpawnData in the pending queue, if any.
     public ChildSpawnData free(BaseChildProcessConnection connection) {
         assert LauncherThread.runningOnLauncherThread();
-        int slot = connection.getServiceNumber();
-        if (mChildProcessConnections[slot] != connection) {
-            int occupier = mChildProcessConnections[slot] == null
-                    ? -1
-                    : mChildProcessConnections[slot].getServiceNumber();
-            Log.e(TAG,
-                    "Unable to find connection to free in slot: %d "
-                            + "already occupied by service: %d",
-                    slot, occupier);
+        // mChildProcessConnections is relatively short (20 items at max at this point).
+        // We are better of iterating than caching in a map.
+        int slot = Arrays.asList(mChildProcessConnections).indexOf(connection);
+        if (slot == -1) {
+            Log.e(TAG, "Unable to find connection to free.");
             assert false;
         } else {
             mChildProcessConnections[slot] = null;

content/public/android/java/src/org/chromium/content/browser/ChildProcessLauncher.java

@@ -491,7 +491,8 @@ public class ChildProcessLauncher {
             String[] commandLine, int childProcessId, FileDescriptorInfo[] filesToBeMapped,
             final IBinder childProcessCallback, final LaunchCallback launchCallback) {
         assert LauncherThread.runningOnLauncherThread();
-        Log.d(TAG, "Setting up connection to process: slot=%d", connection.getServiceNumber());
+        Log.d(TAG, "Setting up connection to process, connection name=%s",
+                connection.getServiceName());
         BaseChildProcessConnection.ConnectionCallback connectionCallback =
                 new BaseChildProcessConnection.ConnectionCallback() {
                     @Override

content/public/android/java/src/org/chromium/content/browser/ImportantChildProcessConnection.java

@@ -16,21 +16,21 @@ import org.chromium.base.process_launcher.ChildProcessCreationParams;
 public class ImportantChildProcessConnection extends BaseChildProcessConnection {
     public static final Factory FACTORY = new BaseChildProcessConnection.Factory() {
         @Override
-        public BaseChildProcessConnection create(Context context, int number, boolean sandboxed,
+        public BaseChildProcessConnection create(Context context, boolean sandboxed,
                 DeathCallback deathCallback, String serviceClassName,
                 Bundle childProcessCommonParameters, ChildProcessCreationParams creationParams) {
-            return new ImportantChildProcessConnection(context, number, sandboxed, deathCallback,
+            return new ImportantChildProcessConnection(context, sandboxed, deathCallback,
                     serviceClassName, childProcessCommonParameters, creationParams);
         }
     };
 
     private final ChildServiceConnection mBinding;
 
-    private ImportantChildProcessConnection(Context context, int number, boolean sandboxed,
+    private ImportantChildProcessConnection(Context context, boolean sandboxed,
             DeathCallback deathCallback, String serviceClassName,
             Bundle childProcessCommonParameters, ChildProcessCreationParams creationParams) {
-        super(context, number, sandboxed, deathCallback, serviceClassName,
-                childProcessCommonParameters, creationParams);
+        super(context, sandboxed, deathCallback, serviceClassName, childProcessCommonParameters,
+                creationParams);
         int flags = Context.BIND_AUTO_CREATE | Context.BIND_IMPORTANT;
         if (shouldBindAsExportedService()) {
             flags |= Context.BIND_EXTERNAL_SERVICE;

content/public/android/java/src/org/chromium/content/browser/ManagedChildProcessConnection.java

@@ -21,11 +21,11 @@ public class ManagedChildProcessConnection extends BaseChildProcessConnection {
 
     public static final Factory FACTORY = new BaseChildProcessConnection.Factory() {
         @Override
-        public BaseChildProcessConnection create(Context context, int number, boolean sandboxed,
+        public BaseChildProcessConnection create(Context context, boolean sandboxed,
                 DeathCallback deathCallback, String serviceClassName,
                 Bundle childProcessCommonParameters, ChildProcessCreationParams creationParams) {
             assert LauncherThread.runningOnLauncherThread();
-            return new ManagedChildProcessConnection(context, number, sandboxed, deathCallback,
+            return new ManagedChildProcessConnection(context, sandboxed, deathCallback,
                     serviceClassName, childProcessCommonParameters, creationParams);
         }
     };
@@ -60,11 +60,11 @@ public class ManagedChildProcessConnection extends BaseChildProcessConnection {
     private boolean mUnbound;
 
     @VisibleForTesting
-    ManagedChildProcessConnection(Context context, int number, boolean sandboxed,
-            DeathCallback deathCallback, String serviceClassName,
-            Bundle childProcessCommonParameters, ChildProcessCreationParams creationParams) {
-        super(context, number, sandboxed, deathCallback, serviceClassName,
-                childProcessCommonParameters, creationParams);
+    ManagedChildProcessConnection(Context context, boolean sandboxed, DeathCallback deathCallback,
+            String serviceClassName, Bundle childProcessCommonParameters,
+            ChildProcessCreationParams creationParams) {
+        super(context, sandboxed, deathCallback, serviceClassName, childProcessCommonParameters,
+                creationParams);
 
         int initialFlags = Context.BIND_AUTO_CREATE;
         int extraBindFlags = shouldBindAsExportedService() ? Context.BIND_EXTERNAL_SERVICE : 0;

content/public/android/junit/src/org/chromium/content/browser/BindingManagerImplTest.java

@@ -67,9 +67,8 @@ public class BindingManagerImplTest {
          * connection is established: with initial binding bound and no strong binding.
          */
         private TestChildProcessConnection(int pid) {
-            super(null /* context */, pid /* number */, true /* sandboxed */,
-                    null /* deathCallback */, null /* serviceClassName */,
-                    null /* childProcessCommonParameters */,
+            super(null /* context */, true /* sandboxed */, null /* deathCallback */,
+                    "TestService" /* serviceClassName */, null /* childProcessCommonParameters */,
                     new ChildProcessCreationParams("org.chromium.test",
                             false /* isExternalService */, 0 /* libraryProcessType */,
                             false /* bindToCallerCheck */));
@@ -566,7 +565,7 @@ public class BindingManagerImplTest {
             }
 
             app.onTrimMemory(pair.first);
-            // Verify that some of moderate bindings drop.
+            // Verify that some of the moderate bindings have been dropped.
             for (int i = 0; i < connections.length; i++) {
                 Assert.assertEquals(
                         message, i >= pair.second, connections[i].isModerateBindingBound());

content/shell/android/shell_apk/src/org/chromium/content_shell_apk/ChildProcessLauncherTestUtils.java

@@ -79,13 +79,33 @@ public final class ChildProcessLauncherTestUtils {
         });
     }
 
+    // Retrieves the service number of the passed in connection from its service name, or -1 if the
+    // service number could not be determined.
+    public static int getConnectionServiceNumber(final BaseChildProcessConnection connection) {
+        String serviceName = getConnectionServiceName(connection);
+        // The service name ends up with the service number.
+        StringBuilder numberString = new StringBuilder();
+        for (int i = serviceName.length() - 1; i >= 0; i--) {
+            char c = serviceName.charAt(i);
+            if (!Character.isDigit(c)) {
+                break;
+            }
+            numberString.append(c);
+        }
+        try {
+            return Integer.decode(numberString.toString());
+        } catch (NumberFormatException nfe) {
+            return -1;
+        }
+    }
+
     // Retrieves the service number of the passed in connection on the launcher thread as to not
     // assert.
-    public static int getConnectionServiceNumber(final BaseChildProcessConnection connection) {
-        return runOnLauncherAndGetResult(new Callable<Integer>() {
+    public static String getConnectionServiceName(final BaseChildProcessConnection connection) {
+        return runOnLauncherAndGetResult(new Callable<String>() {
             @Override
-            public Integer call() {
-                return connection.getServiceNumber();
+            public String call() {
+                return connection.getServiceName().getClassName();
             }
         });
     }