From b9687849bbc70f11ccd52d0d10dcbcd07f2ffeb2 Mon Sep 17 00:00:00 2001 From: Felipe Leme Date: Mon, 17 Dec 2018 12:22:29 -0800 Subject: Optimizes the Content Capture workflow by calling the service directly. Initially, the ContentCaptureManager (in the app) was calling the IContentCaptureManager (on system server) for everything, even to pass the list of captured events, which caused 2 IPCs for each batch of events (i.e., from app to system_server, then from system_service to service). This CL optimizes the workflow by getting rid of the "middle man" and sending the events from the app to the service directly, which the system_server only calling the service to notify when the view starts and finishes (and passing the UID in the former so the servier can validate the sendEvents() calls). Bug: 119220549 Test: atest CtsContentCaptureServiceTestCases Change-Id: I6c08dccf755605320ac37cbc9424132e5455a594 --- Android.bp | 1 + api/system-current.txt | 2 +- .../ContentCaptureEventsRequest.java | 12 +- .../contentcapture/ContentCaptureService.java | 154 +++++++++++++++---- .../contentcapture/IContentCaptureService.aidl | 12 +- .../view/contentcapture/ContentCaptureSession.java | 169 +++++++++++++++++---- .../IContentCaptureDirectManager.aidl | 30 ++++ .../contentcapture/IContentCaptureManager.aidl | 10 +- .../ContentCaptureManagerService.java | 23 +-- .../ContentCapturePerUserService.java | 90 +++-------- .../ContentCaptureServerSession.java | 33 ++-- .../RemoteContentCaptureService.java | 23 ++- 12 files changed, 362 insertions(+), 197 deletions(-) create mode 100644 core/java/android/view/contentcapture/IContentCaptureDirectManager.aidl diff --git a/Android.bp b/Android.bp index 1b81306b3699..763139d311cc 100644 --- a/Android.bp +++ b/Android.bp @@ -357,6 +357,7 @@ java_defaults { "core/java/android/view/autofill/IAutoFillManagerClient.aidl", "core/java/android/view/autofill/IAugmentedAutofillManagerClient.aidl", "core/java/android/view/autofill/IAutofillWindowPresenter.aidl", + "core/java/android/view/contentcapture/IContentCaptureDirectManager.aidl", "core/java/android/view/contentcapture/IContentCaptureManager.aidl", "core/java/android/view/IApplicationToken.aidl", "core/java/android/view/IAppTransitionAnimationSpecsFuture.aidl", diff --git a/api/system-current.txt b/api/system-current.txt index 8f5ccd1f71f9..caee914f6d67 100644 --- a/api/system-current.txt +++ b/api/system-current.txt @@ -5017,7 +5017,7 @@ package android.service.contentcapture { method public final java.util.Set getContentCaptureDisabledActivities(); method public final java.util.Set getContentCaptureDisabledPackages(); method public void onActivitySnapshot(android.view.contentcapture.ContentCaptureSessionId, android.service.contentcapture.SnapshotData); - method public abstract void onContentCaptureEventsRequest(android.view.contentcapture.ContentCaptureSessionId, android.service.contentcapture.ContentCaptureEventsRequest); + method public void onContentCaptureEventsRequest(android.view.contentcapture.ContentCaptureSessionId, android.service.contentcapture.ContentCaptureEventsRequest); method public void onCreateContentCaptureSession(android.view.contentcapture.ContentCaptureContext, android.view.contentcapture.ContentCaptureSessionId); method public void onDestroyContentCaptureSession(android.view.contentcapture.ContentCaptureSessionId); method public final void setActivityContentCaptureEnabled(android.content.ComponentName, boolean); diff --git a/core/java/android/service/contentcapture/ContentCaptureEventsRequest.java b/core/java/android/service/contentcapture/ContentCaptureEventsRequest.java index df58f52dc59c..028180dd3bf0 100644 --- a/core/java/android/service/contentcapture/ContentCaptureEventsRequest.java +++ b/core/java/android/service/contentcapture/ContentCaptureEventsRequest.java @@ -17,6 +17,7 @@ package android.service.contentcapture; import android.annotation.NonNull; import android.annotation.SystemApi; +import android.content.pm.ParceledListSlice; import android.os.Parcel; import android.os.Parcelable; import android.view.contentcapture.ContentCaptureEvent; @@ -31,10 +32,10 @@ import java.util.List; @SystemApi public final class ContentCaptureEventsRequest implements Parcelable { - private final List mEvents; + private final ParceledListSlice mEvents; /** @hide */ - public ContentCaptureEventsRequest(@NonNull List events) { + public ContentCaptureEventsRequest(@NonNull ParceledListSlice events) { mEvents = events; } @@ -43,7 +44,7 @@ public final class ContentCaptureEventsRequest implements Parcelable { */ @NonNull public List getEvents() { - return mEvents; + return mEvents.getList(); } @Override @@ -53,7 +54,7 @@ public final class ContentCaptureEventsRequest implements Parcelable { @Override public void writeToParcel(Parcel parcel, int flags) { - parcel.writeTypedList(mEvents, flags); + parcel.writeParcelable(mEvents, flags); } public static final Parcelable.Creator CREATOR = @@ -61,8 +62,7 @@ public final class ContentCaptureEventsRequest implements Parcelable { @Override public ContentCaptureEventsRequest createFromParcel(Parcel parcel) { - return new ContentCaptureEventsRequest(parcel - .createTypedArrayList(ContentCaptureEvent.CREATOR)); + return new ContentCaptureEventsRequest(parcel.readParcelable(null)); } @Override diff --git a/core/java/android/service/contentcapture/ContentCaptureService.java b/core/java/android/service/contentcapture/ContentCaptureService.java index 58848fce43d6..953ccf1e4575 100644 --- a/core/java/android/service/contentcapture/ContentCaptureService.java +++ b/core/java/android/service/contentcapture/ContentCaptureService.java @@ -24,15 +24,27 @@ import android.annotation.SystemApi; import android.app.Service; import android.content.ComponentName; import android.content.Intent; +import android.content.pm.ParceledListSlice; +import android.os.Binder; +import android.os.Bundle; import android.os.Handler; import android.os.IBinder; import android.os.Looper; import android.os.RemoteException; +import android.util.ArrayMap; import android.util.Log; +import android.util.Slog; import android.view.contentcapture.ContentCaptureContext; import android.view.contentcapture.ContentCaptureEvent; +import android.view.contentcapture.ContentCaptureManager; +import android.view.contentcapture.ContentCaptureSession; import android.view.contentcapture.ContentCaptureSessionId; +import android.view.contentcapture.IContentCaptureDirectManager; +import com.android.internal.os.IResultReceiver; + +import java.io.FileDescriptor; +import java.io.PrintWriter; import java.util.List; import java.util.Set; @@ -63,39 +75,54 @@ public abstract class ContentCaptureService extends Service { private Handler mHandler; - private final IContentCaptureService mInterface = new IContentCaptureService.Stub() { + /** + * Binder that receives calls from the system server. + */ + private final IContentCaptureService mServerInterface = new IContentCaptureService.Stub() { @Override - public void onSessionLifecycle(ContentCaptureContext context, String sessionId) - throws RemoteException { - if (context != null) { - mHandler.sendMessage( - obtainMessage(ContentCaptureService::handleOnCreateSession, - ContentCaptureService.this, context, sessionId)); - } else { - mHandler.sendMessage( - obtainMessage(ContentCaptureService::handleOnDestroySession, - ContentCaptureService.this, sessionId)); - } + public void onSessionStarted(ContentCaptureContext context, String sessionId, int uid, + IResultReceiver clientReceiver) { + mHandler.sendMessage(obtainMessage(ContentCaptureService::handleOnCreateSession, + ContentCaptureService.this, context, sessionId, uid, clientReceiver)); } @Override - public void onContentCaptureEventsRequest(String sessionId, - ContentCaptureEventsRequest request) { + public void onActivitySnapshot(String sessionId, SnapshotData snapshotData) { mHandler.sendMessage( - obtainMessage(ContentCaptureService::handleOnContentCaptureEventsRequest, - ContentCaptureService.this, sessionId, request)); + obtainMessage(ContentCaptureService::handleOnActivitySnapshot, + ContentCaptureService.this, sessionId, snapshotData)); + } + @Override + public void onSessionFinished(String sessionId) { + mHandler.sendMessage(obtainMessage(ContentCaptureService::handleFinishSession, + ContentCaptureService.this, sessionId)); } + }; + + /** + * Binder that receives calls from the app. + */ + private final IContentCaptureDirectManager mClientInterface = + new IContentCaptureDirectManager.Stub() { @Override - public void onActivitySnapshot(String sessionId, SnapshotData snapshotData) { - mHandler.sendMessage( - obtainMessage(ContentCaptureService::handleOnActivitySnapshot, - ContentCaptureService.this, sessionId, snapshotData)); + public void sendEvents(String sessionId, + @SuppressWarnings("rawtypes") ParceledListSlice events) { + mHandler.sendMessage(obtainMessage(ContentCaptureService::handleSendEvents, + ContentCaptureService.this, sessionId, Binder.getCallingUid(), events)); } }; + /** + * List of sessions per UID. + * + *

This map is populated when an session is started, which is called by the system server + * and can be trusted. Then subsequent calls made by the app are verified against this map. + */ + private final ArrayMap mSessionsByUid = new ArrayMap<>(); + @CallSuper @Override public void onCreate() { @@ -107,7 +134,7 @@ public abstract class ContentCaptureService extends Service { @Override public final IBinder onBind(Intent intent) { if (SERVICE_INTERFACE.equals(intent.getAction())) { - return mInterface.asBinder(); + return mServerInterface.asBinder(); } Log.w(TAG, "Tried to bind to wrong intent (should be " + SERVICE_INTERFACE + ": " + intent); return null; @@ -196,8 +223,10 @@ public abstract class ContentCaptureService extends Service { * @param sessionId the session's Id * @param request the events */ - public abstract void onContentCaptureEventsRequest(@NonNull ContentCaptureSessionId sessionId, - @NonNull ContentCaptureEventsRequest request); + public void onContentCaptureEventsRequest(@NonNull ContentCaptureSessionId sessionId, + @NonNull ContentCaptureEventsRequest request) { + if (VERBOSE) Log.v(TAG, "onContentCaptureEventsRequest(id=" + sessionId + ")"); + } /** * Notifies the service of {@link SnapshotData snapshot data} associated with a session. @@ -212,10 +241,22 @@ public abstract class ContentCaptureService extends Service { * Destroys the content capture session. * * @param sessionId the id of the session to destroy - */ + * */ public void onDestroyContentCaptureSession(@NonNull ContentCaptureSessionId sessionId) { - if (VERBOSE) { - Log.v(TAG, "onDestroyContentCaptureSession(id=" + sessionId + ")"); + if (VERBOSE) Log.v(TAG, "onDestroyContentCaptureSession(id=" + sessionId + ")"); + } + + @Override + @CallSuper + protected void dump(FileDescriptor fd, PrintWriter pw, String[] args) { + final int size = mSessionsByUid.size(); + pw.print("Number sessions: "); pw.println(size); + if (size > 0) { + final String prefix = " "; + for (int i = 0; i < size; i++) { + pw.print(prefix); pw.print(mSessionsByUid.keyAt(i)); + pw.print(": uid="); pw.println(mSessionsByUid.valueAt(i)); + } } } @@ -223,13 +264,19 @@ public abstract class ContentCaptureService extends Service { // so we don't need to create a temporary InteractionSessionId for each event. private void handleOnCreateSession(@NonNull ContentCaptureContext context, - @NonNull String sessionId) { + @NonNull String sessionId, int uid, IResultReceiver clientReceiver) { + mSessionsByUid.put(sessionId, uid); onCreateContentCaptureSession(context, new ContentCaptureSessionId(sessionId)); + setClientState(clientReceiver, ContentCaptureSession.STATE_ACTIVE, + mClientInterface.asBinder()); } - private void handleOnContentCaptureEventsRequest(@NonNull String sessionId, - @NonNull ContentCaptureEventsRequest request) { - onContentCaptureEventsRequest(new ContentCaptureSessionId(sessionId), request); + private void handleSendEvents(@NonNull String sessionId, int uid, + @NonNull ParceledListSlice events) { + if (handleIsRightCallerFor(sessionId, uid)) { + onContentCaptureEventsRequest(new ContentCaptureSessionId(sessionId), + new ContentCaptureEventsRequest(events)); + } } private void handleOnActivitySnapshot(@NonNull String sessionId, @@ -237,7 +284,52 @@ public abstract class ContentCaptureService extends Service { onActivitySnapshot(new ContentCaptureSessionId(sessionId), snapshotData); } - private void handleOnDestroySession(@NonNull String sessionId) { + private void handleFinishSession(@NonNull String sessionId) { + mSessionsByUid.remove(sessionId); onDestroyContentCaptureSession(new ContentCaptureSessionId(sessionId)); } + + /** + * Checks if the given {@code uid} owns the session. + */ + private boolean handleIsRightCallerFor(@NonNull String sessionId, int uid) { + final Integer rightUid = mSessionsByUid.get(sessionId); + if (rightUid == null) { + if (VERBOSE) Log.v(TAG, "No session for " + sessionId); + // Just ignore, as the session could have finished + return false; + } + if (rightUid != uid) { + Log.e(TAG, "invalid call from UID " + uid + ": session " + sessionId + " belongs to " + + rightUid); + //TODO(b/111276913): log metrics as this could be a malicious app forging a sessionId + return false; + } + return true; + + } + + /** + * Sends the state of the {@link ContentCaptureManager} in the cleint app. + * + * @param clientReceiver receiver in the client app. + * @param binder handle to the {@code IContentCaptureDirectManager} object that resides in the + * service. + * @hide + */ + public static void setClientState(@NonNull IResultReceiver clientReceiver, + int sessionStatus, @Nullable IBinder binder) { + try { + final Bundle extras; + if (binder != null) { + extras = new Bundle(); + extras.putBinder(ContentCaptureSession.EXTRA_BINDER, binder); + } else { + extras = null; + } + clientReceiver.send(sessionStatus, extras); + } catch (RemoteException e) { + Slog.w(TAG, "Error async reporting result to client: " + e); + } + } } diff --git a/core/java/android/service/contentcapture/IContentCaptureService.aidl b/core/java/android/service/contentcapture/IContentCaptureService.aidl index 8167be9e6838..20e8e99d4ce1 100644 --- a/core/java/android/service/contentcapture/IContentCaptureService.aidl +++ b/core/java/android/service/contentcapture/IContentCaptureService.aidl @@ -16,10 +16,11 @@ package android.service.contentcapture; -import android.service.contentcapture.ContentCaptureEventsRequest; import android.service.contentcapture.SnapshotData; import android.view.contentcapture.ContentCaptureContext; +import com.android.internal.os.IResultReceiver; + import java.util.List; /** @@ -28,11 +29,8 @@ import java.util.List; * @hide */ oneway interface IContentCaptureService { - - // Called when session is created (context not null) or destroyed (context null) - void onSessionLifecycle(in ContentCaptureContext context, String sessionId); - - void onContentCaptureEventsRequest(String sessionId, in ContentCaptureEventsRequest request); - + void onSessionStarted(in ContentCaptureContext context, String sessionId, int uid, + in IResultReceiver clientReceiver); + void onSessionFinished(String sessionId); void onActivitySnapshot(String sessionId, in SnapshotData snapshotData); } diff --git a/core/java/android/view/contentcapture/ContentCaptureSession.java b/core/java/android/view/contentcapture/ContentCaptureSession.java index 632955d335cf..f411cf751b47 100644 --- a/core/java/android/view/contentcapture/ContentCaptureSession.java +++ b/core/java/android/view/contentcapture/ContentCaptureSession.java @@ -27,9 +27,11 @@ import android.annotation.NonNull; import android.annotation.Nullable; import android.content.ComponentName; import android.content.Context; +import android.content.pm.ParceledListSlice; import android.os.Bundle; import android.os.Handler; import android.os.IBinder; +import android.os.IBinder.DeathRecipient; import android.os.RemoteException; import android.os.SystemClock; import android.util.Log; @@ -45,6 +47,8 @@ import dalvik.system.CloseGuard; import java.io.PrintWriter; import java.util.ArrayList; +import java.util.Collections; +import java.util.List; import java.util.UUID; import java.util.concurrent.atomic.AtomicBoolean; @@ -99,6 +103,13 @@ public final class ContentCaptureSession implements AutoCloseable { */ public static final int STATE_DISABLED = 3; + /** + * Session is disabled because its id already existed on server. + * + * @hide + */ + public static final int STATE_DISABLED_DUPLICATED_ID = 4; + /** * Handler message used to flush the buffer. */ @@ -116,6 +127,13 @@ public final class ContentCaptureSession implements AutoCloseable { // TODO(b/121044064): use settings private static final int FLUSHING_FREQUENCY_MS = 5_000; + + /** + * Name of the {@link IResultReceiver} extra used to pass the binder interface to the service. + * @hide + */ + public static final String EXTRA_BINDER = "binder"; + private final CloseGuard mCloseGuard = CloseGuard.get(); @NonNull @@ -127,8 +145,21 @@ public final class ContentCaptureSession implements AutoCloseable { @NonNull private final Handler mHandler; + /** + * Interface to the system_server binder object - it's only used to start the session (and + * notify when the session is finished). + */ @Nullable - private final IContentCaptureManager mService; + private final IContentCaptureManager mSystemServerInterface; + + /** + * Direct interface to the service binder object - it's used to send the events, including the + * last ones (when the session is finished) + */ + @Nullable + private IContentCaptureDirectManager mDirectServiceInterface; + @Nullable + private DeathRecipient mDirectServiceVulture; @Nullable private final String mId = UUID.randomUUID().toString(); @@ -165,11 +196,11 @@ public final class ContentCaptureSession implements AutoCloseable { /** @hide */ protected ContentCaptureSession(@NonNull Context context, @NonNull Handler handler, - @Nullable IContentCaptureManager service, @NonNull AtomicBoolean disabled, + @Nullable IContentCaptureManager systemServerInterface, @NonNull AtomicBoolean disabled, @Nullable ContentCaptureContext clientContext) { mContext = context; mHandler = handler; - mService = service; + mSystemServerInterface = systemServerInterface; mDisabled = disabled; mClientContext = clientContext; mCloseGuard.open("destroy"); @@ -227,6 +258,8 @@ public final class ContentCaptureSession implements AutoCloseable { Log.v(TAG, "destroy(): state=" + getStateAsString(mState) + ", mId=" + mId); } + flush(); + mHandler.sendMessage(obtainMessage(ContentCaptureSession::handleDestroySession, this)); mCloseGuard.close(); } @@ -267,11 +300,20 @@ public final class ContentCaptureSession implements AutoCloseable { final int flags = 0; // TODO(b/111276913): get proper flags try { - mService.startSession(mContext.getUserId(), mApplicationToken, componentName, - mId, mClientContext, flags, new IResultReceiver.Stub() { + mSystemServerInterface.startSession(mContext.getUserId(), mApplicationToken, + componentName, mId, mClientContext, flags, new IResultReceiver.Stub() { @Override public void send(int resultCode, Bundle resultData) { - handleSessionStarted(resultCode); + IBinder binder = null; + if (resultData != null) { + binder = resultData.getBinder(EXTRA_BINDER); + if (binder == null) { + Log.wtf(TAG, "No " + EXTRA_BINDER + " extra result"); + handleResetState(); + return; + } + } + handleSessionStarted(resultCode, binder); } }); } catch (RemoteException e) { @@ -280,12 +322,38 @@ public final class ContentCaptureSession implements AutoCloseable { } } - private void handleSessionStarted(int resultCode) { + /** + * Callback from {@code system_server} after call to + * {@link IContentCaptureManager#startSession(int, IBinder, ComponentName, String, + * ContentCaptureContext, int, IResultReceiver)}. + * + * @param resultCode session state + * @param binder handle to {@link IContentCaptureDirectManager} + */ + private void handleSessionStarted(int resultCode, @Nullable IBinder binder) { mState = resultCode; - mDisabled.set(mState == STATE_DISABLED); + if (binder != null) { + mDirectServiceInterface = IContentCaptureDirectManager.Stub.asInterface(binder); + mDirectServiceVulture = () -> { + Log.w(TAG, "Destroying session " + mId + " because service died"); + destroy(); + }; + try { + binder.linkToDeath(mDirectServiceVulture, 0); + } catch (RemoteException e) { + Log.w(TAG, "Failed to link to death on " + binder + ": " + e); + } + } + if (resultCode == STATE_DISABLED || resultCode == STATE_DISABLED_DUPLICATED_ID) { + mDisabled.set(true); + handleResetSession(/* resetState= */ false); + } else { + mDisabled.set(false); + } if (VERBOSE) { Log.v(TAG, "handleSessionStarted() result: code=" + resultCode + ", id=" + mId - + ", state=" + getStateAsString(mState) + ", disabled=" + mDisabled.get()); + + ", state=" + getStateAsString(mState) + ", disabled=" + mDisabled.get() + + ", binder=" + binder); } } @@ -307,7 +375,7 @@ public final class ContentCaptureSession implements AutoCloseable { final boolean bufferEvent = numberEvents < MAX_BUFFER_SIZE; if (bufferEvent && !forceFlush) { - handleScheduleFlush(); + handleScheduleFlush(/* checkExisting= */ true); return; } @@ -331,8 +399,8 @@ public final class ContentCaptureSession implements AutoCloseable { handleForceFlush(); } - private void handleScheduleFlush() { - if (mHandler.hasMessages(MSG_FLUSH)) { + private void handleScheduleFlush(boolean checkExisting) { + if (checkExisting && mHandler.hasMessages(MSG_FLUSH)) { // "Renew" the flush message by removing the previous one mHandler.removeMessages(MSG_FLUSH); } @@ -356,52 +424,80 @@ public final class ContentCaptureSession implements AutoCloseable { private void handleForceFlush() { if (mEvents == null) return; + if (mDirectServiceInterface == null) { + Log.w(TAG, "handleForceFlush(): client not available yet"); + if (!mHandler.hasMessages(MSG_FLUSH)) { + handleScheduleFlush(/* checkExisting= */ false); + } + return; + } + final int numberEvents = mEvents.size(); try { if (DEBUG) { Log.d(TAG, "Flushing " + numberEvents + " event(s) for " + getActivityDebugName()); } mHandler.removeMessages(MSG_FLUSH); - mService.sendEvents(mContext.getUserId(), mId, mEvents); - // TODO(b/111276913): decide whether we should clear or set it to null, as each has - // its own advantages: clearing will save extra allocations while the session is - // active, while setting to null would save memory if there's no more event coming. - mEvents.clear(); + + final ParceledListSlice events = handleClearEvents(); + mDirectServiceInterface.sendEvents(mId, events); } catch (RemoteException e) { Log.w(TAG, "Error sending " + numberEvents + " for " + getActivityDebugName() + ": " + e); } } + /** + * Resets the buffer and return a {@link ParceledListSlice} with the previous events. + */ + @NonNull + private ParceledListSlice handleClearEvents() { + // NOTE: we must save a reference to the current mEvents and then set it to to null, + // otherwise clearing it would clear it in the receiving side if the service is also local. + final List events = mEvents == null + ? Collections.emptyList() + : mEvents; + mEvents = null; + return new ParceledListSlice<>(events); + } + private void handleDestroySession() { //TODO(b/111276913): right now both the ContentEvents and lifecycle sessions are sent // to system_server, so it's ok to call both in sequence here. But once we split // them so the events are sent directly to the service, we need to make sure they're // sent in order. - try { - if (DEBUG) { - Log.d(TAG, "Destroying session (ctx=" + mContext + ", id=" + mId + ") with " - + (mEvents == null ? 0 : mEvents.size()) + " event(s) for " - + getActivityDebugName()); - } + if (DEBUG) { + Log.d(TAG, "Destroying session (ctx=" + mContext + ", id=" + mId + ") with " + + (mEvents == null ? 0 : mEvents.size()) + " event(s) for " + + getActivityDebugName()); + } - mService.finishSession(mContext.getUserId(), mId, mEvents); + try { + mSystemServerInterface.finishSession(mContext.getUserId(), mId); } catch (RemoteException e) { - Log.e(TAG, "Error destroying session " + mId + " for " + getActivityDebugName() - + ": " + e); - } finally { - handleResetState(); + Log.e(TAG, "Error destroying system-service session " + mId + " for " + + getActivityDebugName() + ": " + e); } } + private void handleResetState() { + handleResetSession(/* resetState= */ true); + } + // TODO(b/111276913): once we support multiple sessions, we might need to move some of these // clearings out. - private void handleResetState() { - mState = STATE_UNKNOWN; + private void handleResetSession(boolean resetState) { + if (resetState) { + mState = STATE_UNKNOWN; + } mContentCaptureSessionId = null; mApplicationToken = null; mComponentName = null; mEvents = null; + if (mDirectServiceInterface != null) { + mDirectServiceInterface.asBinder().unlinkToDeath(mDirectServiceVulture, 0); + } + mDirectServiceInterface = null; mHandler.removeMessages(MSG_FLUSH); } @@ -492,15 +588,20 @@ public final class ContentCaptureSession implements AutoCloseable { } private boolean isContentCaptureEnabled() { - return mService != null && !mDisabled.get(); + return mSystemServerInterface != null && !mDisabled.get(); } void dump(@NonNull String prefix, @NonNull PrintWriter pw) { pw.print(prefix); pw.print("id: "); pw.println(mId); pw.print(prefix); pw.print("mContext: "); pw.println(mContext); pw.print(prefix); pw.print("user: "); pw.println(mContext.getUserId()); - if (mService != null) { - pw.print(prefix); pw.print("mService: "); pw.println(mService); + if (mSystemServerInterface != null) { + pw.print(prefix); pw.print("mSystemServerInterface: "); + pw.println(mSystemServerInterface); + } + if (mDirectServiceInterface != null) { + pw.print(prefix); pw.print("mDirectServiceInterface: "); + pw.println(mDirectServiceInterface); } if (mClientContext != null) { // NOTE: we don't dump clientContent because it could have PII @@ -563,6 +664,8 @@ public final class ContentCaptureSession implements AutoCloseable { return "ACTIVE"; case STATE_DISABLED: return "DISABLED"; + case STATE_DISABLED_DUPLICATED_ID: + return "DISABLED_DUPLICATED_ID"; default: return "INVALID:" + state; } diff --git a/core/java/android/view/contentcapture/IContentCaptureDirectManager.aidl b/core/java/android/view/contentcapture/IContentCaptureDirectManager.aidl new file mode 100644 index 000000000000..145fc16707a1 --- /dev/null +++ b/core/java/android/view/contentcapture/IContentCaptureDirectManager.aidl @@ -0,0 +1,30 @@ +/* + * Copyright (C) 2018 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.view.contentcapture; + +import android.content.pm.ParceledListSlice; +import android.view.contentcapture.ContentCaptureEvent; + +/** + * Interface between an app (ContentCaptureManager / ContentCaptureSession) and the app providing + * the ContentCaptureService implementation. + * + * @hide + */ +oneway interface IContentCaptureDirectManager { + void sendEvents(in String sessionId, in ParceledListSlice events); +} diff --git a/core/java/android/view/contentcapture/IContentCaptureManager.aidl b/core/java/android/view/contentcapture/IContentCaptureManager.aidl index 2002c5c1fc9a..cbd37017038e 100644 --- a/core/java/android/view/contentcapture/IContentCaptureManager.aidl +++ b/core/java/android/view/contentcapture/IContentCaptureManager.aidl @@ -26,12 +26,14 @@ import com.android.internal.os.IResultReceiver; import java.util.List; /** - * {@hide} - */ + * Interface between an app (ContentCaptureManager / ContentCaptureSession) and the system-server + * implementation service (ContentCaptureManagerService). + * + * @hide + */ oneway interface IContentCaptureManager { void startSession(int userId, IBinder activityToken, in ComponentName componentName, String sessionId, in ContentCaptureContext clientContext, int flags, in IResultReceiver result); - void finishSession(int userId, String sessionId, in List events); - void sendEvents(int userId, in String sessionId, in List events); + void finishSession(int userId, String sessionId); } diff --git a/services/contentcapture/java/com/android/server/contentcapture/ContentCaptureManagerService.java b/services/contentcapture/java/com/android/server/contentcapture/ContentCaptureManagerService.java index 10e713d9dabe..e8820ae4e32a 100644 --- a/services/contentcapture/java/com/android/server/contentcapture/ContentCaptureManagerService.java +++ b/services/contentcapture/java/com/android/server/contentcapture/ContentCaptureManagerService.java @@ -25,6 +25,7 @@ import android.annotation.UserIdInt; import android.app.ActivityManagerInternal; import android.content.ComponentName; import android.content.Context; +import android.os.Binder; import android.os.Bundle; import android.os.IBinder; import android.os.RemoteException; @@ -34,7 +35,6 @@ import android.os.UserHandle; import android.os.UserManager; import android.util.Slog; import android.view.contentcapture.ContentCaptureContext; -import android.view.contentcapture.ContentCaptureEvent; import android.view.contentcapture.IContentCaptureManager; import com.android.internal.annotations.GuardedBy; @@ -47,7 +47,6 @@ import com.android.server.infra.AbstractMasterSystemService; import java.io.FileDescriptor; import java.io.PrintWriter; import java.util.ArrayList; -import java.util.List; /** * A service used to observe the contents of the screen. @@ -182,30 +181,18 @@ public final class ContentCaptureManagerService extends synchronized (mLock) { final ContentCapturePerUserService service = getServiceForUserLocked(userId); service.startSessionLocked(activityToken, componentName, taskId, displayId, - sessionId, clientContext, flags, mAllowInstantService, result); + sessionId, Binder.getCallingUid(), clientContext, flags, + mAllowInstantService, result); } } @Override - public void sendEvents(@UserIdInt int userId, @NonNull String sessionId, - @NonNull List events) { + public void finishSession(@UserIdInt int userId, @NonNull String sessionId) { Preconditions.checkNotNull(sessionId); - Preconditions.checkNotNull(events); synchronized (mLock) { final ContentCapturePerUserService service = getServiceForUserLocked(userId); - service.sendEventsLocked(sessionId, events); - } - } - - @Override - public void finishSession(@UserIdInt int userId, @NonNull String sessionId, - @Nullable List events) { - Preconditions.checkNotNull(sessionId); - - synchronized (mLock) { - final ContentCapturePerUserService service = getServiceForUserLocked(userId); - service.finishSessionLocked(sessionId, events); + service.finishSessionLocked(sessionId); } } diff --git a/services/contentcapture/java/com/android/server/contentcapture/ContentCapturePerUserService.java b/services/contentcapture/java/com/android/server/contentcapture/ContentCapturePerUserService.java index 81309122d0ca..f21b0d810411 100644 --- a/services/contentcapture/java/com/android/server/contentcapture/ContentCapturePerUserService.java +++ b/services/contentcapture/java/com/android/server/contentcapture/ContentCapturePerUserService.java @@ -16,6 +16,8 @@ package com.android.server.contentcapture; +import static android.service.contentcapture.ContentCaptureService.setClientState; + import static com.android.server.wm.ActivityTaskManagerInternal.ASSIST_KEY_CONTENT; import static com.android.server.wm.ActivityTaskManagerInternal.ASSIST_KEY_DATA; import static com.android.server.wm.ActivityTaskManagerInternal.ASSIST_KEY_STRUCTURE; @@ -38,7 +40,6 @@ import android.service.contentcapture.SnapshotData; import android.util.ArrayMap; import android.util.Slog; import android.view.contentcapture.ContentCaptureContext; -import android.view.contentcapture.ContentCaptureEvent; import android.view.contentcapture.ContentCaptureSession; import com.android.internal.annotations.GuardedBy; @@ -48,7 +49,6 @@ import com.android.server.infra.FrameworkResourcesServiceNameResolver; import java.io.PrintWriter; import java.util.ArrayList; -import java.util.List; /** * Per-user instance of {@link ContentCaptureManagerService}. @@ -114,10 +114,10 @@ final class ContentCapturePerUserService @GuardedBy("mLock") public void startSessionLocked(@NonNull IBinder activityToken, @NonNull ComponentName componentName, int taskId, int displayId, - @NonNull String sessionId, @Nullable ContentCaptureContext clientContext, - int flags, boolean bindInstantServiceAllowed, @NonNull IResultReceiver resultReceiver) { + @NonNull String sessionId, int uid, @Nullable ContentCaptureContext clientContext, + int flags, boolean bindInstantServiceAllowed, @NonNull IResultReceiver clientReceiver) { if (!isEnabledLocked()) { - sendToClient(resultReceiver, ContentCaptureSession.STATE_DISABLED); + setClientState(clientReceiver, ContentCaptureSession.STATE_DISABLED, /* binder=*/ null); return; } final ComponentName serviceComponentName = getServiceComponentName(); @@ -131,35 +131,30 @@ final class ContentCapturePerUserService return; } - ContentCaptureServerSession session = mSessions.get(sessionId); - if (session != null) { - if (mMaster.debug) { - Slog.d(TAG, "startSession(): reusing session " + sessionId + " for " - + componentName); - } - // TODO(b/111276913): check if local ids match and decide what to do if they don't - // TODO(b/111276913): should we call session.notifySessionStartedLocked() again?? - // if not, move notifySessionStartedLocked() into session constructor - sendToClient(resultReceiver, ContentCaptureSession.STATE_ACTIVE); + final ContentCaptureServerSession existingSession = mSessions.get(sessionId); + if (existingSession != null) { + Slog.w(TAG, "startSession(id=" + existingSession + ", token=" + activityToken + + ": ignoring because it already exists for " + existingSession.mActivityToken); + setClientState(clientReceiver, ContentCaptureSession.STATE_DISABLED_DUPLICATED_ID, + /* binder=*/ null); return; } - session = new ContentCaptureServerSession(getContext(), mUserId, mLock, activityToken, - this, serviceComponentName, componentName, taskId, displayId, sessionId, - clientContext, flags, bindInstantServiceAllowed, mMaster.verbose); + final ContentCaptureServerSession newSession = new ContentCaptureServerSession(getContext(), + mUserId, mLock, activityToken, this, serviceComponentName, componentName, taskId, + displayId, sessionId, uid, clientContext, flags, bindInstantServiceAllowed, + mMaster.verbose); if (mMaster.verbose) { - Slog.v(TAG, "startSession(): new session for " + componentName + " and id " - + sessionId); + Slog.v(TAG, "startSession(): new session for " + + ComponentName.flattenToShortString(componentName) + " and id " + sessionId); } - mSessions.put(sessionId, session); - session.notifySessionStartedLocked(); - sendToClient(resultReceiver, ContentCaptureSession.STATE_ACTIVE); + mSessions.put(sessionId, newSession); + newSession.notifySessionStartedLocked(clientReceiver); } // TODO(b/111276913): log metrics @GuardedBy("mLock") - public void finishSessionLocked(@NonNull String sessionId, - @Nullable List events) { + public void finishSessionLocked(@NonNull String sessionId) { if (!isEnabledLocked()) { return; } @@ -171,41 +166,8 @@ final class ContentCapturePerUserService } return; } - if (events != null && !events.isEmpty()) { - // TODO(b/111276913): for now we're sending the events and the onDestroy() in 2 separate - // calls because it's not clear yet whether we'll change the manager to send events - // to the service directly (i.e., without passing through system server). Once we - // decide, we might need to split IContentCaptureManager.onSessionLifecycle() in 2 - // methods, one for start and another for finish (and passing the events to finish), - // otherwise the service might receive the 2 calls out of order. - session.sendEventsLocked(events); - } - if (mMaster.verbose) { - Slog.v(TAG, "finishSession(" + (events == null ? 0 : events.size()) + " events): " - + session); - } - session.removeSelfLocked(true); - } - - // TODO(b/111276913): need to figure out why some events are sent before session is started; - // probably because ContentCaptureManager is not buffering them until it gets the session back - @GuardedBy("mLock") - public void sendEventsLocked(@NonNull String sessionId, - @NonNull List events) { - if (!isEnabledLocked()) { - return; - } - final ContentCaptureServerSession session = mSessions.get(sessionId); - if (session == null) { - if (mMaster.verbose) { - Slog.v(TAG, "sendEvents(): no session for " + sessionId); - } - return; - } - if (mMaster.verbose) { - Slog.v(TAG, "sendEvents(): id=" + sessionId + ", events=" + events.size()); - } - session.sendEventsLocked(events); + if (mMaster.verbose) Slog.v(TAG, "finishSession(): id=" + sessionId); + session.removeSelfLocked(/* notifyRemoteService= */ true); } @GuardedBy("mLock") @@ -308,12 +270,4 @@ final class ContentCapturePerUserService } return null; } - - private static void sendToClient(@NonNull IResultReceiver resultReceiver, int value) { - try { - resultReceiver.send(value, null); - } catch (RemoteException e) { - Slog.w(TAG, "Error async reporting result to client: " + e); - } - } } diff --git a/services/contentcapture/java/com/android/server/contentcapture/ContentCaptureServerSession.java b/services/contentcapture/java/com/android/server/contentcapture/ContentCaptureServerSession.java index 181a2daa2467..ba98b95082f7 100644 --- a/services/contentcapture/java/com/android/server/contentcapture/ContentCaptureServerSession.java +++ b/services/contentcapture/java/com/android/server/contentcapture/ContentCaptureServerSession.java @@ -24,15 +24,14 @@ import android.service.contentcapture.ContentCaptureService; import android.service.contentcapture.SnapshotData; import android.util.Slog; import android.view.contentcapture.ContentCaptureContext; -import android.view.contentcapture.ContentCaptureEvent; import android.view.contentcapture.ContentCaptureSessionId; import com.android.internal.annotations.GuardedBy; +import com.android.internal.os.IResultReceiver; import com.android.internal.util.Preconditions; import com.android.server.contentcapture.RemoteContentCaptureService.ContentCaptureServiceCallbacks; import java.io.PrintWriter; -import java.util.List; final class ContentCaptureServerSession implements ContentCaptureServiceCallbacks { @@ -43,18 +42,28 @@ final class ContentCaptureServerSession implements ContentCaptureServiceCallback private final ContentCapturePerUserService mService; private final RemoteContentCaptureService mRemoteService; private final ContentCaptureContext mContentCaptureContext; + + /** + * Canonical session id. + */ private final String mId; + /** + * UID of the app whose contents is being captured. + */ + private final int mUid; + ContentCaptureServerSession(@NonNull Context context, int userId, @NonNull Object lock, @NonNull IBinder activityToken, @NonNull ContentCapturePerUserService service, @NonNull ComponentName serviceComponentName, @NonNull ComponentName appComponentName, - int taskId, int displayId, @NonNull String sessionId, + int taskId, int displayId, @NonNull String sessionId, int uid, @Nullable ContentCaptureContext clientContext, int flags, boolean bindInstantServiceAllowed, boolean verbose) { mLock = lock; mActivityToken = activityToken; mService = service; mId = Preconditions.checkNotNull(sessionId); + mUid = uid; mRemoteService = new RemoteContentCaptureService(context, ContentCaptureService.SERVICE_INTERFACE, serviceComponentName, userId, this, bindInstantServiceAllowed, verbose); @@ -73,15 +82,8 @@ final class ContentCaptureServerSession implements ContentCaptureServiceCallback * Notifies the {@link ContentCaptureService} that the service started. */ @GuardedBy("mLock") - public void notifySessionStartedLocked() { - mRemoteService.onSessionLifecycleRequest(mContentCaptureContext, mId); - } - - /** - * Notifies the {@link ContentCaptureService} of a batch of events. - */ - public void sendEventsLocked(@NonNull List events) { - mRemoteService.onContentCaptureEventsRequest(mId, events); + public void notifySessionStartedLocked(@NonNull IResultReceiver clientReceiver) { + mRemoteService.onSessionStarted(mContentCaptureContext, mId, mUid, clientReceiver); } /** @@ -118,11 +120,11 @@ final class ContentCaptureServerSession implements ContentCaptureServiceCallback @GuardedBy("mLock") public void destroyLocked(boolean notifyRemoteService) { if (mService.isVerbose()) { - Slog.v(TAG, "destroyLocked(notifyRemoteService=" + notifyRemoteService + ")"); + Slog.v(TAG, "destroy(notifyRemoteService=" + notifyRemoteService + ")"); } // TODO(b/111276913): must call client to set session as FINISHED_BY_SERVER if (notifyRemoteService) { - mRemoteService.onSessionLifecycleRequest(/* context= */ null, mId); + mRemoteService.onSessionFinished(mId); } } @@ -133,13 +135,14 @@ final class ContentCaptureServerSession implements ContentCaptureServiceCallback Slog.d(TAG, "onServiceDied() for " + mId); } synchronized (mLock) { - removeSelfLocked(/* notifyRemoteService= */ false); + removeSelfLocked(/* notifyRemoteService= */ true); } } @GuardedBy("mLock") public void dumpLocked(@NonNull String prefix, @NonNull PrintWriter pw) { pw.print(prefix); pw.print("id: "); pw.print(mId); pw.println(); + pw.print(prefix); pw.print("uid: "); pw.print(mUid); pw.println(); pw.print(prefix); pw.print("context: "); mContentCaptureContext.dump(pw); pw.println(); pw.print(prefix); pw.print("activity token: "); pw.println(mActivityToken); pw.print(prefix); pw.print("has autofill callback: "); diff --git a/services/contentcapture/java/com/android/server/contentcapture/RemoteContentCaptureService.java b/services/contentcapture/java/com/android/server/contentcapture/RemoteContentCaptureService.java index b4edf7e60bc4..b9b19435408f 100644 --- a/services/contentcapture/java/com/android/server/contentcapture/RemoteContentCaptureService.java +++ b/services/contentcapture/java/com/android/server/contentcapture/RemoteContentCaptureService.java @@ -20,17 +20,14 @@ import android.annotation.Nullable; import android.content.ComponentName; import android.content.Context; import android.os.IBinder; -import android.service.contentcapture.ContentCaptureEventsRequest; import android.service.contentcapture.IContentCaptureService; import android.service.contentcapture.SnapshotData; import android.text.format.DateUtils; import android.view.contentcapture.ContentCaptureContext; -import android.view.contentcapture.ContentCaptureEvent; +import com.android.internal.os.IResultReceiver; import com.android.server.infra.AbstractMultiplePendingRequestsRemoteService; -import java.util.List; - final class RemoteContentCaptureService extends AbstractMultiplePendingRequestsRemoteService { @@ -67,21 +64,19 @@ final class RemoteContentCaptureService /** * Called by {@link ContentCaptureServerSession} to generate a call to the - * {@link RemoteContentCaptureService} to indicate the session was created (when {@code context} - * is not {@code null} or destroyed (when {@code context} is {@code null}). + * {@link RemoteContentCaptureService} to indicate the session was created. */ - public void onSessionLifecycleRequest(@Nullable ContentCaptureContext context, - @NonNull String sessionId) { - scheduleAsyncRequest((s) -> s.onSessionLifecycle(context, sessionId)); + public void onSessionStarted(@Nullable ContentCaptureContext context, + @NonNull String sessionId, int uid, @NonNull IResultReceiver clientReceiver) { + scheduleAsyncRequest((s) -> s.onSessionStarted(context, sessionId, uid, clientReceiver)); } /** - * Called by {@link ContentCaptureServerSession} to send a batch of events to the service. + * Called by {@link ContentCaptureServerSession} to generate a call to the + * {@link RemoteContentCaptureService} to indicate the session was finished. */ - public void onContentCaptureEventsRequest(@NonNull String sessionId, - @NonNull List events) { - scheduleAsyncRequest((s) -> s.onContentCaptureEventsRequest(sessionId, - new ContentCaptureEventsRequest(events))); + public void onSessionFinished(@NonNull String sessionId) { + scheduleAsyncRequest((s) -> s.onSessionFinished(sessionId)); } /** -- cgit v1.2.3-59-g8ed1b