Reduce ServiceWatcher thrashing

ServiceWatcher forced a rebind everytime something about the connected
package changed. This causes a lot of thrashing, as every time a
component in the bound package changes (which happens a lot in Google
Play services for instance), the service was completely rebound. This
was likely initially necessary because there was no way to detect when a
binding failed completely. Current OS versions support onBindingDied()
to detect this case however, so it should no longer be necessary to
rebind everytime anything changes.

Also fixes NPE crash in LocationProviderManager when debug logging is
enabled.

Bug: 163076048
Bug: 164035101
Test: manual
Change-Id: Ida4b8c8774f68d63ab319ed342055ff050e39f95
This commit is contained in:
Soonil Nagarkar
2020-08-12 22:02:36 -07:00
parent f13bc4679a
commit 8e81109b2f
2 changed files with 20 additions and 33 deletions

View File

@@ -237,25 +237,14 @@ public class ServiceWatcher implements ServiceConnection {
}
new PackageMonitor() {
@Override
public void onPackageUpdateFinished(String packageName, int uid) {
ServiceWatcher.this.onPackageChanged(packageName);
}
@Override
public void onPackageAdded(String packageName, int uid) {
ServiceWatcher.this.onPackageChanged(packageName);
}
@Override
public void onPackageRemoved(String packageName, int uid) {
ServiceWatcher.this.onPackageChanged(packageName);
}
@Override
public boolean onPackageChanged(String packageName, int uid, String[] components) {
ServiceWatcher.this.onPackageChanged(packageName);
return super.onPackageChanged(packageName, uid, components);
return true;
}
@Override
public void onSomePackagesChanged() {
onBestServiceChanged(false);
}
}.register(mContext, UserHandle.ALL, true, mHandler);
@@ -320,7 +309,7 @@ public class ServiceWatcher implements ServiceConnection {
if (!mTargetService.equals(ServiceInfo.NONE)) {
if (D) {
Log.i(TAG, "[" + mIntent.getAction() + "] unbinding from " + mTargetService);
Log.d(TAG, "[" + mIntent.getAction() + "] unbinding from " + mTargetService);
}
mContext.unbindService(this);
@@ -335,9 +324,7 @@ public class ServiceWatcher implements ServiceConnection {
Preconditions.checkState(mTargetService.component != null);
if (D) {
Log.i(TAG, getLogPrefix() + " binding to " + mTargetService);
}
Log.i(TAG, getLogPrefix() + " binding to " + mTargetService);
Intent bindIntent = new Intent(mIntent).setComponent(mTargetService.component);
if (!mContext.bindServiceAsUser(bindIntent, this,
@@ -355,7 +342,7 @@ public class ServiceWatcher implements ServiceConnection {
Preconditions.checkState(mBinder == null);
if (D) {
Log.i(TAG, getLogPrefix() + " connected to " + component.toShortString());
Log.d(TAG, getLogPrefix() + " connected to " + component.toShortString());
}
mBinder = binder;
@@ -379,7 +366,7 @@ public class ServiceWatcher implements ServiceConnection {
}
if (D) {
Log.i(TAG, getLogPrefix() + " disconnected from " + component.toShortString());
Log.d(TAG, getLogPrefix() + " disconnected from " + component.toShortString());
}
mBinder = null;
@@ -392,13 +379,16 @@ public class ServiceWatcher implements ServiceConnection {
public final void onBindingDied(ComponentName component) {
Preconditions.checkState(Looper.myLooper() == mHandler.getLooper());
if (D) {
Log.i(TAG, getLogPrefix() + " " + component.toShortString() + " died");
}
Log.i(TAG, getLogPrefix() + " " + component.toShortString() + " died");
onBestServiceChanged(true);
}
@Override
public final void onNullBinding(ComponentName component) {
Log.e(TAG, getLogPrefix() + " " + component.toShortString() + " has null binding");
}
void onUserSwitched(@UserIdInt int userId) {
mCurrentUserId = userId;
onBestServiceChanged(false);
@@ -410,11 +400,6 @@ public class ServiceWatcher implements ServiceConnection {
}
}
void onPackageChanged(String packageName) {
// force a rebind if the changed package was the currently connected package
onBestServiceChanged(packageName.equals(mTargetService.getPackageName()));
}
/**
* Runs the given function asynchronously if and only if currently connected. Suppresses any
* RemoteException thrown during execution.

View File

@@ -229,7 +229,7 @@ class LocationProviderManager extends
// we cache these values because checking/calculating on the fly is more expensive
private boolean mPermitted;
private boolean mForeground;
@Nullable private LocationRequest mProviderLocationRequest;
private LocationRequest mProviderLocationRequest;
private boolean mIsUsingHighPower;
protected Registration(LocationRequest request, CallerIdentity identity,
@@ -244,6 +244,8 @@ class LocationProviderManager extends
} else {
mWorkSource = identity.addToWorkSource(null);
}
mProviderLocationRequest = super.getRequest();
}
@GuardedBy("mLock")
@@ -313,7 +315,7 @@ class LocationProviderManager extends
@Override
public final LocationRequest getRequest() {
return Objects.requireNonNull(mProviderLocationRequest);
return mProviderLocationRequest;
}
public final boolean isForeground() {