Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fixed Content Capture workflow when service package is updated.

Browse Source 
The workflow already handles the case where the service dies and the framework re-establishes
the binder connection, but that didn't work when it died because the package was being updated.

This CL fixes this scenario by gracefully pausing the existing sessions before the package is
updated, then resuming them afterwards.

Test: manual verification
Test: atest CtsContentCaptureServiceTestCases # sanity check minus flakiness

Bug: 126266412
Fixes: 129072171

Change-Id: Ibc6b723c7bc08b4f920436f365d6006bc8fac7b6
This commit is contained in:
Felipe Leme
2019-03-21 13:57:45 -07:00
parent c9a92abca4
commit 2f28843249
9 changed files with 185 additions and 84 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
core/java/android/service/contentcapture/ContentCaptureService.java
View File

@@ -367,7 +367,6 @@ public abstract class ContentCaptureService extends Service {
stateFlags = initialState;
} else {
stateFlags |= ContentCaptureSession.STATE_DISABLED;
}
setClientState(clientReceiver, stateFlags, mClientInterface.asBinder());
}

9
core/java/android/view/contentcapture/ContentCaptureSession.java
View File

@@ -134,12 +134,19 @@ public abstract class ContentCaptureSession implements AutoCloseable {
*/
public static final int STATE_SERVICE_DIED = 0x400;
/**
* Session is disabled because the service package is being udpated.
*
* @hide
*/
public static final int STATE_SERVICE_UPDATING = 0x800;
/**
* Session is enabled, after the service died and came back to live.
*
* @hide
*/
public static final int STATE_SERVICE_RESURRECTED = 0x800;
public static final int STATE_SERVICE_RESURRECTED = 0x1000;
private static final int INITIAL_CHILDREN_CAPACITY = 5;

3
core/java/android/view/contentcapture/MainContentCaptureSession.java
View File

@@ -230,7 +230,8 @@ public final class MainContentCaptureSession extends ContentCaptureSession {
/**
* Callback from {@code system_server} after call to
* {@link IContentCaptureManager#startSession(IBinder, ComponentName, String, int, IBinder)}
* {@link IContentCaptureManager#startSession(IBinder, ComponentName, String, int,
* IResultReceiver)}.
*
* @param resultCode session state
* @param binder handle to {@code IContentCaptureDirectManager}

6
core/java/com/android/internal/infra/AbstractRemoteService.java
View File

@@ -481,7 +481,11 @@ public abstract class AbstractRemoteService<S extends AbstractRemoteService<S, I
@Override
public String toString() {
return getClass().getSimpleName() + "[" + mComponentName + "]";
return getClass().getSimpleName() + "[" + mComponentName
+ " " + System.identityHashCode(this)
+ (mService != null ? " (bound)" : " (unbound)")
+ (mDestroyed ? " (destroyed)" : "")
+ "]";
}
/**

16
services/contentcapture/java/com/android/server/contentcapture/ContentCaptureManagerService.java
View File

@@ -166,6 +166,22 @@ public final class ContentCaptureManagerService extends
service.destroyLocked();
}
@Override // from AbstractMasterSystemService
protected void onServicePackageUpdatingLocked(int userId) {
final ContentCapturePerUserService service = getServiceForUserLocked(userId);
if (service != null) {
service.onPackageUpdatingLocked();
}
}
@Override // from AbstractMasterSystemService
protected void onServicePackageUpdatedLocked(@UserIdInt int userId) {
final ContentCapturePerUserService service = getServiceForUserLocked(userId);
if (service != null) {
service.onPackageUpdatedLocked();
}
}
@Override // from AbstractMasterSystemService
protected void enforceCallingPermissionForManagement() {
getContext().enforceCallingPermission(MANAGE_CONTENT_CAPTURE, mTag);

83
services/contentcapture/java/com/android/server/contentcapture/ContentCapturePerUserService.java
View File

@@ -74,7 +74,7 @@ final class ContentCapturePerUserService
AbstractPerUserSystemService<ContentCapturePerUserService, ContentCaptureManagerService>
implements ContentCaptureServiceCallbacks {
private static final String TAG = ContentCaptureManagerService.class.getSimpleName();
private static final String TAG = ContentCapturePerUserService.class.getSimpleName();
@GuardedBy("mLock")
private final ArrayMap<String, ContentCaptureServerSession> mSessions =
@@ -87,7 +87,8 @@ final class ContentCapturePerUserService
* master's cache (for example, because a temporary service was set).
*/
@GuardedBy("mLock")
private RemoteContentCaptureService mRemoteService;
@Nullable
RemoteContentCaptureService mRemoteService;
private final ContentCaptureServiceRemoteCallback mRemoteServiceCallback =
new ContentCaptureServiceRemoteCallback();
@@ -135,6 +136,10 @@ final class ContentCapturePerUserService
}
if (!disabled) {
if (mMaster.debug) {
Slog.d(TAG, "updateRemoteService(): creating new remote service for "
+ serviceComponentName);
}
mRemoteService = new RemoteContentCaptureService(mMaster.getContext(),
ContentCaptureService.SERVICE_INTERFACE, serviceComponentName,
mRemoteServiceCallback, mUserId, this, mMaster.isBindInstantServiceAllowed(),
@@ -182,20 +187,40 @@ final class ContentCapturePerUserService
}
mZombie = false;
final int numSessions = mSessions.size();
if (mMaster.debug) {
Slog.d(TAG, "Ressurrecting remote service (" + mRemoteService + ") on "
+ numSessions + " sessions");
}
for (int i = 0; i < numSessions; i++) {
final ContentCaptureServerSession session = mSessions.valueAt(i);
session.resurrectLocked();
}
resurrectSessionsLocked();
}
}
}
private void resurrectSessionsLocked() {
final int numSessions = mSessions.size();
if (mMaster.debug) {
Slog.d(TAG, "Ressurrecting remote service (" + mRemoteService + ") on "
+ numSessions + " sessions");
}
for (int i = 0; i < numSessions; i++) {
final ContentCaptureServerSession session = mSessions.valueAt(i);
session.resurrectLocked();
}
}
void onPackageUpdatingLocked() {
final int numSessions = mSessions.size();
if (mMaster.debug) {
Slog.d(TAG, "Pausing " + numSessions + " sessions while package is updating");
}
for (int i = 0; i < numSessions; i++) {
final ContentCaptureServerSession session = mSessions.valueAt(i);
session.pauseLocked();
}
}
void onPackageUpdatedLocked() {
updateRemoteServiceLocked(!isEnabledLocked());
resurrectSessionsLocked();
}
// TODO(b/119613670): log metrics
@GuardedBy("mLock")
public void startSessionLocked(@NonNull IBinder activityToken,
@@ -274,9 +299,12 @@ final class ContentCapturePerUserService
return;
}
// Make sure service is bound, just in case the initial connection failed somehow
mRemoteService.ensureBoundLocked();
final ContentCaptureServerSession newSession = new ContentCaptureServerSession(
activityToken, this, mRemoteService, componentName, clientReceiver, taskId,
displayId, sessionId, uid, flags);
activityToken, this, componentName, clientReceiver, taskId, displayId, sessionId,
uid, flags);
if (mMaster.verbose) {
Slog.v(TAG, "startSession(): new session for "
+ ComponentName.flattenToShortString(componentName) + " and id " + sessionId);
@@ -290,21 +318,6 @@ final class ContentCapturePerUserService
return mWhitelistHelper.isWhitelisted(componentName);
}
/**
* @throws IllegalArgumentException if packages or components are empty.
*/
private void setWhitelist(@Nullable List<String> packages,
@Nullable List<ComponentName> components) {
// TODO(b/122595322): add CTS test for when it's null
synchronized (mLock) {
if (mMaster.verbose) {
Slog.v(TAG, "whitelisting packages: " + packages + " and activities: "
+ components);
}
mWhitelistHelper.setWhitelist(packages, components);
}
}
// TODO(b/119613670): log metrics
@GuardedBy("mLock")
public void finishSessionLocked(@NonNull String sessionId) {
@@ -518,12 +531,16 @@ final class ContentCapturePerUserService
@Override
public void setContentCaptureWhitelist(List<String> packages,
List<ComponentName> activities) {
// TODO(b/122595322): add CTS test for when it's null
if (mMaster.verbose) {
Slog.v(TAG, "setContentCaptureWhitelist(packages=" + packages + ", activities="
+ activities + ")");
Slog.v(TAG, "setContentCaptureWhitelist(" + (packages == null
? "null_packages" : packages.size() + " packages")
+ ", " + (activities == null
? "null_activities" : activities.size() + " activities") + ")");
}
synchronized (mLock) {
mWhitelistHelper.setWhitelist(packages, activities);
}
setWhitelist(packages, activities);
// TODO(b/119613670): log metrics
}

57
services/contentcapture/java/com/android/server/contentcapture/ContentCaptureServerSession.java
View File

@@ -15,6 +15,12 @@
*/
package com.android.server.contentcapture;
import static android.service.contentcapture.ContentCaptureService.setClientState;
import static android.view.contentcapture.ContentCaptureSession.STATE_ACTIVE;
import static android.view.contentcapture.ContentCaptureSession.STATE_DISABLED;
import static android.view.contentcapture.ContentCaptureSession.STATE_SERVICE_RESURRECTED;
import static android.view.contentcapture.ContentCaptureSession.STATE_SERVICE_UPDATING;
import android.annotation.NonNull;
import android.content.ComponentName;
import android.os.IBinder;
@@ -23,7 +29,6 @@ import android.service.contentcapture.SnapshotData;
import android.util.LocalLog;
import android.util.Slog;
import android.view.contentcapture.ContentCaptureContext;
import android.view.contentcapture.ContentCaptureSession;
import android.view.contentcapture.ContentCaptureSessionId;
import com.android.internal.annotations.GuardedBy;
@@ -38,7 +43,6 @@ final class ContentCaptureServerSession {
final IBinder mActivityToken;
private final ContentCapturePerUserService mService;
private final RemoteContentCaptureService mRemoteService;
// NOTE: this is the "internal" context (like package and taskId), not the explicit content
// set by apps - those are only send to the ContentCaptureService.
@@ -61,15 +65,13 @@ final class ContentCaptureServerSession {
private final int mUid;
ContentCaptureServerSession(@NonNull IBinder activityToken,
@NonNull ContentCapturePerUserService service,
@NonNull RemoteContentCaptureService remoteService,
@NonNull ComponentName appComponentName, @NonNull IResultReceiver sessionStateReceiver,
@NonNull ContentCapturePerUserService service, @NonNull ComponentName appComponentName,
@NonNull IResultReceiver sessionStateReceiver,
int taskId, int displayId, @NonNull String sessionId, int uid, int flags) {
mActivityToken = activityToken;
mService = service;
mId = Preconditions.checkNotNull(sessionId);
mUid = uid;
mRemoteService = remoteService;
mContentCaptureContext = new ContentCaptureContext(/* clientContext= */ null,
appComponentName, taskId, displayId, flags);
mSessionStateReceiver = sessionStateReceiver;
@@ -87,8 +89,12 @@ final class ContentCaptureServerSession {
*/
@GuardedBy("mLock")
public void notifySessionStartedLocked(@NonNull IResultReceiver clientReceiver) {
mRemoteService.onSessionStarted(mContentCaptureContext, mId, mUid, clientReceiver,
ContentCaptureSession.STATE_ACTIVE);
if (mService.mRemoteService == null) {
Slog.w(TAG, "notifySessionStartedLocked(): no remote service");
return;
}
mService.mRemoteService.onSessionStarted(mContentCaptureContext, mId, mUid, clientReceiver,
STATE_ACTIVE);
}
/**
@@ -101,7 +107,11 @@ final class ContentCaptureServerSession {
logHistory.log("snapshot: id=" + mId);
}
mRemoteService.onActivitySnapshotRequest(mId, snapshotData);
if (mService.mRemoteService == null) {
Slog.w(TAG, "sendActivitySnapshotLocked(): no remote service");
return;
}
mService.mRemoteService.onActivitySnapshotRequest(mId, snapshotData);
}
/**
@@ -134,7 +144,11 @@ final class ContentCaptureServerSession {
}
// TODO(b/111276913): must call client to set session as FINISHED_BY_SERVER
if (notifyRemoteService) {
mRemoteService.onSessionFinished(mId);
if (mService.mRemoteService == null) {
Slog.w(TAG, "destroyLocked(): no remote service");
return;
}
mService.mRemoteService.onSessionFinished(mId);
}
}
@@ -143,10 +157,27 @@ final class ContentCaptureServerSession {
*/
@GuardedBy("mLock")
public void resurrectLocked() {
mRemoteService.onSessionStarted(new ContentCaptureContext(mContentCaptureContext,
final RemoteContentCaptureService remoteService = mService.mRemoteService;
if (remoteService == null) {
Slog.w(TAG, "destroyLocked(: no remote service");
return;
}
if (mService.isVerbose()) {
Slog.v(TAG, "resurrecting " + mActivityToken + " on " + remoteService);
}
remoteService.onSessionStarted(new ContentCaptureContext(mContentCaptureContext,
ContentCaptureContext.FLAG_RECONNECTED), mId, mUid, mSessionStateReceiver,
ContentCaptureSession.STATE_ACTIVE
| ContentCaptureSession.STATE_SERVICE_RESURRECTED);
STATE_ACTIVE | STATE_SERVICE_RESURRECTED);
}
/**
* Called to pause the session while the service is being updated.
*/
@GuardedBy("mLock")
public void pauseLocked() {
if (mService.isVerbose()) Slog.v(TAG, "pausing " + mActivityToken);
setClientState(mSessionStateReceiver, STATE_DISABLED | STATE_SERVICE_UPDATING,
/* binder= */ null);
}
@GuardedBy("mLock")

6
services/contentcapture/java/com/android/server/contentcapture/RemoteContentCaptureService.java
View File

@@ -54,7 +54,7 @@ final class RemoteContentCaptureService
mIdleUnbindTimeoutMs = idleUnbindTimeoutMs;
// Bind right away, which will trigger a onConnected() on service's
scheduleBind();
ensureBoundLocked();
}
@Override // from AbstractRemoteService
@@ -89,6 +89,10 @@ final class RemoteContentCaptureService
}
}
public void ensureBoundLocked() {
scheduleBind();
}
/**
* Called by {@link ContentCaptureServerSession} to generate a call to the
* {@link RemoteContentCaptureService} to indicate the session was created.

88
services/core/java/com/android/server/infra/AbstractMasterSystemService.java
View File

@@ -131,14 +131,10 @@ public abstract class AbstractMasterSystemService<M extends AbstractMasterSystem
private final boolean mRefreshServiceOnPackageUpdate;
/**
* Name of the service's package that was active but then was removed because its package
* update.
*
* <p>It's a temporary state set / used by the {@link PackageMonitor} implementation, but
* defined here so it can be dumped.
* Name of the service packages whose APK are being updated, keyed by user id.
*/
@GuardedBy("mLock")
private String mLastActivePackageName;
private SparseArray<String> mUpdatingPackageNames;
/**
* Default constructor.
@@ -564,6 +560,20 @@ public abstract class AbstractMasterSystemService<M extends AbstractMasterSystem
return service;
}
/**
* Called before the package that provides the service for the given user is being updated.
*/
protected void onServicePackageUpdatingLocked(@UserIdInt int userId) {
if (verbose) Slog.v(mTag, "onServicePackageUpdatingLocked(" + userId + ")");
}
/**
* Called after the package that provides the service for the given user is being updated.
*/
protected void onServicePackageUpdatedLocked(@UserIdInt int userId) {
if (verbose) Slog.v(mTag, "onServicePackageUpdated(" + userId + ")");
}
/**
* Called after the service is removed from the cache.
*/
@@ -602,8 +612,10 @@ public abstract class AbstractMasterSystemService<M extends AbstractMasterSystem
final int size = mServicesCache.size();
pw.print(prefix); pw.print("Debug: "); pw.print(realDebug);
pw.print(" Verbose: "); pw.println(realVerbose);
pw.print(" Refresh on package update: "); pw.println(mRefreshServiceOnPackageUpdate);
pw.print(" Last active service on update: "); pw.println(mLastActivePackageName);
pw.print("Refresh on package update: "); pw.println(mRefreshServiceOnPackageUpdate);
if (mUpdatingPackageNames != null) {
pw.print("Packages being updated: "); pw.println(mUpdatingPackageNames);
}
if (mServiceNameResolver != null) {
pw.print(prefix); pw.print("Name resolver: ");
mServiceNameResolver.dumpShort(pw); pw.println();
@@ -644,39 +656,49 @@ public abstract class AbstractMasterSystemService<M extends AbstractMasterSystem
final PackageMonitor monitor = new PackageMonitor() {
@Override
public void onPackageUpdateStarted(String packageName, int uid) {
public void onPackageUpdateStarted(@NonNull String packageName, int uid) {
if (verbose) Slog.v(mTag, "onPackageUpdateStarted(): " + packageName);
final String activePackageName = getActiveServicePackageNameLocked();
if (!packageName.equals(activePackageName)) return;
final int userId = getChangingUserId();
synchronized (mLock) {
final String activePackageName = getActiveServicePackageNameLocked();
if (packageName.equals(activePackageName)) {
final int userId = getChangingUserId();
if (mRefreshServiceOnPackageUpdate) {
if (debug) {
Slog.d(mTag, "Removing service for user " + userId
+ " because package " + activePackageName
+ " is being updated");
}
mLastActivePackageName = activePackageName;
removeCachedServiceLocked(userId);
} else {
if (debug) {
Slog.d(mTag, "Holding service for user " + userId
+ " while package " + activePackageName
+ " is being updated");
}
if (mUpdatingPackageNames == null) {
mUpdatingPackageNames = new SparseArray<String>(mServicesCache.size());
}
mUpdatingPackageNames.put(userId, packageName);
onServicePackageUpdatingLocked(userId);
if (mRefreshServiceOnPackageUpdate) {
if (debug) {
Slog.d(mTag, "Removing service for user " + userId + " because package "
+ activePackageName + " is being updated");
}
removeCachedServiceLocked(userId);
} else {
if (debug) {
Slog.d(mTag, "Holding service for user " + userId + " while package "
+ activePackageName + " is being updated");
}
}
}
}
@Override
public void onPackageUpdateFinished(String packageName, int uid) {
public void onPackageUpdateFinished(@NonNull String packageName, int uid) {
if (verbose) Slog.v(mTag, "onPackageUpdateFinished(): " + packageName);
final int userId = getChangingUserId();
synchronized (mLock) {
String activePackageName = getActiveServicePackageNameLocked();
if (activePackageName == null) {
activePackageName = mLastActivePackageName;
mLastActivePackageName = null;
}
if (!packageName.equals(activePackageName)) {
final String activePackageName = mUpdatingPackageNames == null ? null
: mUpdatingPackageNames.get(userId);
if (packageName.equals(activePackageName)) {
if (mUpdatingPackageNames != null) {
mUpdatingPackageNames.remove(userId);
if (mUpdatingPackageNames.size() == 0) {
mUpdatingPackageNames = null;
}
}
onServicePackageUpdatedLocked(userId);
} else {
handlePackageUpdateLocked(packageName);
}
}