Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 4183a629 authored by Soonil Nagarkar's avatar Soonil Nagarkar Committed by Android (Google) Code Review
Browse files

Merge "Fix rebind issue in ServiceWatcher"

parents 68fe9164 ed5a2005
Loading
Loading
Loading
Loading
+1 −7
Original line number Diff line number Diff line
@@ -860,13 +860,7 @@ public class LocationManagerService extends ILocationManager.Stub {

                pw.increaseIndent();

                boolean enabled = isEnabled();
                pw.println("enabled=" + enabled);
                if (!enabled) {
                    pw.println("allowed=" + mProvider.getState().allowed);
                }

                pw.println("properties=" + mProvider.getState().properties);
                pw.println("enabled=" + isEnabled());
            }

            mProvider.dump(fd, pw, args);
+15 −4
Original line number Diff line number Diff line
@@ -341,7 +341,7 @@ public class ServiceWatcher implements ServiceConnection {
        Preconditions.checkState(Looper.myLooper() == mHandler.getLooper());

        if (D) {
            Log.i(TAG, getLogPrefix() + " connected to " + component);
            Log.i(TAG, getLogPrefix() + " connected to " + component.toShortString());
        }

        mBinder = binder;
@@ -350,12 +350,23 @@ public class ServiceWatcher implements ServiceConnection {
        }
    }

    @Override
    public void onBindingDied(ComponentName component) {
        Preconditions.checkState(Looper.myLooper() == mHandler.getLooper());

        if (D) {
            Log.i(TAG, getLogPrefix() + " " + component.toShortString() + " died");
        }

        onBestServiceChanged(true);
    }

    @Override
    public final void onServiceDisconnected(ComponentName component) {
        Preconditions.checkState(Looper.myLooper() == mHandler.getLooper());

        if (D) {
            Log.i(TAG, getLogPrefix() + " disconnected from " + component);
            Log.i(TAG, getLogPrefix() + " disconnected from " + component.toShortString());
        }

        mBinder = null;
@@ -383,8 +394,8 @@ public class ServiceWatcher implements ServiceConnection {
    }

    /**
     * Runs the given function asynchronously if currently connected. Suppresses any RemoteException
     * thrown during execution.
     * Runs the given function asynchronously if and only if currently connected. Suppresses any
     * RemoteException thrown during execution.
     */
    public final void runOnBinder(BinderRunner runner) {
        runOnHandler(() -> {
+59 −21
Original line number Diff line number Diff line
@@ -18,6 +18,8 @@ package com.android.server.location;

import static android.content.pm.PackageManager.MATCH_SYSTEM_ONLY;

import static com.android.internal.util.ConcurrentUtils.DIRECT_EXECUTOR;

import android.annotation.Nullable;
import android.content.ComponentName;
import android.content.Context;
@@ -29,6 +31,7 @@ import android.os.RemoteException;
import android.util.ArraySet;
import android.util.Log;

import com.android.internal.annotations.GuardedBy;
import com.android.internal.location.ILocationProvider;
import com.android.internal.location.ILocationProviderManager;
import com.android.internal.location.ProviderProperties;
@@ -71,7 +74,7 @@ public class LocationProviderProxy extends AbstractLocationProvider {
        @Override
        public void onSetAdditionalProviderPackages(List<String> packageNames) {
            int maxCount = Math.min(MAX_ADDITIONAL_PACKAGES, packageNames.size());
            ArraySet<String> allPackages = new ArraySet<>(maxCount);
            ArraySet<String> allPackages = new ArraySet<>(maxCount + 1);
            for (String packageName : packageNames) {
                if (packageNames.size() >= maxCount) {
                    return;
@@ -86,6 +89,11 @@ public class LocationProviderProxy extends AbstractLocationProvider {
                }
            }

            synchronized (mLock) {
                if (!mBound) {
                    return;
                }

                // add the binder package
                ComponentName service = mServiceWatcher.getBoundService().component;
                if (service != null) {
@@ -94,18 +102,27 @@ public class LocationProviderProxy extends AbstractLocationProvider {

                setPackageNames(allPackages);
            }
        }

        // executed on binder thread
        @Override
        public void onSetAllowed(boolean allowed) {
            synchronized (mLock) {
                if (mBound) {
                    setAllowed(allowed);
                }
            }
        }

        // executed on binder thread
        @Override
        public void onSetProperties(ProviderProperties properties) {
            synchronized (mLock) {
                if (mBound) {
                    setProperties(properties);
                }
            }
        }

        // executed on binder thread
        @Override
@@ -114,18 +131,27 @@ public class LocationProviderProxy extends AbstractLocationProvider {
        }
    };

    // also used to synchronized any state changes (setEnabled, setProperties, setState, etc)
    private final Object mLock = new Object();

    private final ServiceWatcher mServiceWatcher;

    @Nullable private ProviderRequest mRequest;
    @GuardedBy("mLock")
    private boolean mBound;
    @GuardedBy("mLock")
    private ProviderRequest mRequest;

    private LocationProviderProxy(Context context, String action, int enableOverlayResId,
            int nonOverlayPackageResId) {
        super(context, FgThread.getExecutor());
        // safe to use direct executor since none of our callbacks call back into any code above
        // this provider - they simply forward to the proxy service
        super(context, DIRECT_EXECUTOR);

        mServiceWatcher = new ServiceWatcher(context, FgThread.getHandler(), action, this::onBind,
                this::onUnbind, enableOverlayResId, nonOverlayPackageResId);

        mRequest = null;
        mBound = false;
        mRequest = ProviderRequest.EMPTY_REQUEST;
    }

    private boolean register() {
@@ -135,26 +161,35 @@ public class LocationProviderProxy extends AbstractLocationProvider {
    private void onBind(IBinder binder) throws RemoteException {
        ILocationProvider provider = ILocationProvider.Stub.asInterface(binder);

        synchronized (mLock) {
            mBound = true;

            provider.setLocationProviderManager(mManager);
            if (!mRequest.equals(ProviderRequest.EMPTY_REQUEST)) {
                provider.setRequest(mRequest, mRequest.workSource);
            }

            ComponentName service = mServiceWatcher.getBoundService().component;
            if (service != null) {
                setPackageNames(Collections.singleton(service.getPackageName()));
            }

        provider.setLocationProviderManager(mManager);

        if (mRequest != null) {
            provider.setRequest(mRequest, mRequest.workSource);
        }
    }

    private void onUnbind() {
        synchronized (mLock) {
            mBound = false;
            setState(State.EMPTY_STATE);
        }
    }

    @Override
    public void onSetRequest(ProviderRequest request) {
        mServiceWatcher.runOnBinder(binder -> {
        synchronized (mLock) {
            mRequest = request;
        }

        mServiceWatcher.runOnBinder(binder -> {
            ILocationProvider service = ILocationProvider.Stub.asInterface(binder);
            service.setRequest(request, request.workSource);
        });
@@ -179,5 +214,8 @@ public class LocationProviderProxy extends AbstractLocationProvider {
    @Override
    public void dump(FileDescriptor fd, PrintWriter pw, String[] args) {
        pw.println("service=" + mServiceWatcher);
        synchronized (mLock) {
            pw.println("bound=" + mBound);
        }
    }
}
+3 −0
Original line number Diff line number Diff line
@@ -233,6 +233,9 @@ public class MockableLocationProvider extends AbstractLocationProvider {
        AbstractLocationProvider provider;
        synchronized (mOwnerLock) {
            provider = mProvider;
            pw.println("allowed=" + getState().allowed);
            pw.println("properties=" + getState().properties);
            pw.println("packages=" + getState().providerPackageNames);
            pw.println("request=" + mRequest);
        }