Make ContentCapture calls directly from the UI thread.
Most calls are bufffered anyways, so by not using a handler we save time spent
on getting a pooled lambda and posting to the the handler.
The only "expensive" operation is flushing, but it makes a oneway binder call
so it might be fine (and if necessary, we can optimize it later).
Bug: 123307965
Test: atest CtsContentCaptureServiceTestCases
Change-Id: I7182c8e193f58fa000396fdb3003e771214bf79b
diff --git a/core/java/android/view/contentcapture/MainContentCaptureSession.java b/core/java/android/view/contentcapture/MainContentCaptureSession.java
index 0605fa3..eb945b5 100644
--- a/core/java/android/view/contentcapture/MainContentCaptureSession.java
+++ b/core/java/android/view/contentcapture/MainContentCaptureSession.java
@@ -26,10 +26,9 @@
import static android.view.contentcapture.ContentCaptureHelper.VERBOSE;
import static android.view.contentcapture.ContentCaptureHelper.getSanitizedString;
-import static com.android.internal.util.function.pooled.PooledLambda.obtainMessage;
-
import android.annotation.NonNull;
import android.annotation.Nullable;
+import android.annotation.UiThread;
import android.content.ComponentName;
import android.content.Context;
import android.content.pm.ParceledListSlice;
@@ -58,10 +57,6 @@
* <p>This session is created when the activity starts and finished when it stops; clients can use
* it to create children activities.
*
- * <p><b>NOTE: all methods in this class should return right away, or do the real work in a handler
- * thread. Hence, the only field that must be thread-safe is {@code mEnabled}, which is called at
- * the beginning of every method.
- *
* @hide
*/
public final class MainContentCaptureSession extends ContentCaptureSession {
@@ -168,46 +163,28 @@
*
* @hide
*/
- void start(@NonNull IBinder applicationToken, @NonNull ComponentName activityComponent,
+ @UiThread
+ void start(@NonNull IBinder token, @NonNull ComponentName component,
int flags) {
if (!isContentCaptureEnabled()) return;
if (VERBOSE) {
- Log.v(TAG, "start(): token=" + applicationToken + ", comp="
- + ComponentName.flattenToShortString(activityComponent));
+ Log.v(TAG, "start(): token=" + token + ", comp="
+ + ComponentName.flattenToShortString(component));
}
- mHandler.sendMessage(obtainMessage(MainContentCaptureSession::handleStartSession, this,
- applicationToken, activityComponent, flags));
- }
-
- @Override
- void flush(@FlushReason int reason) {
- mHandler.sendMessage(
- obtainMessage(MainContentCaptureSession::handleForceFlush, this, reason));
- }
-
- @Override
- void onDestroy() {
- mHandler.removeMessages(MSG_FLUSH);
- mHandler.sendMessage(
- obtainMessage(MainContentCaptureSession::handleDestroySession, this));
- }
-
- private void handleStartSession(@NonNull IBinder token, @NonNull ComponentName componentName,
- int flags) {
- if (handleHasStarted()) {
+ if (hasStarted()) {
// TODO(b/122959591): make sure this is expected (and when), or use Log.w
if (DEBUG) {
Log.d(TAG, "ignoring handleStartSession(" + token + "/"
- + ComponentName.flattenToShortString(componentName) + " while on state "
+ + ComponentName.flattenToShortString(component) + " while on state "
+ getStateAsString(mState));
}
return;
}
mState = STATE_WAITING_FOR_SERVER;
mApplicationToken = token;
- mComponentName = componentName;
+ mComponentName = component;
if (VERBOSE) {
Log.v(TAG, "handleStartSession(): token=" + token + ", act="
@@ -217,28 +194,36 @@
try {
if (mSystemServerInterface == null) return;
- mSystemServerInterface.startSession(mApplicationToken, componentName, mId, flags,
+ mSystemServerInterface.startSession(mApplicationToken, component, mId, flags,
new IResultReceiver.Stub() {
@Override
public void send(int resultCode, Bundle resultData) {
- IBinder binder = null;
+ final IBinder binder;
if (resultData != null) {
binder = resultData.getBinder(EXTRA_BINDER);
if (binder == null) {
Log.wtf(TAG, "No " + EXTRA_BINDER + " extra result");
- handleResetSession(STATE_DISABLED | STATE_INTERNAL_ERROR);
+ mHandler.post(() -> resetSession(
+ STATE_DISABLED | STATE_INTERNAL_ERROR));
return;
}
+ } else {
+ binder = null;
}
- handleSessionStarted(resultCode, binder);
+ mHandler.post(() -> onSessionStarted(resultCode, binder));
}
});
} catch (RemoteException e) {
- Log.w(TAG, "Error starting session for " + componentName.flattenToShortString() + ": "
- + e);
+ Log.w(TAG, "Error starting session for " + component.flattenToShortString() + ": " + e);
}
}
+ @Override
+ void onDestroy() {
+ mHandler.removeMessages(MSG_FLUSH);
+ mHandler.post(() -> destroySession());
+ }
+
/**
* Callback from {@code system_server} after call to
* {@link IContentCaptureManager#startSession(IBinder, ComponentName, String, int,
@@ -247,7 +232,8 @@
* @param resultCode session state
* @param binder handle to {@code IContentCaptureDirectManager}
*/
- private void handleSessionStarted(int resultCode, @Nullable IBinder binder) {
+ @UiThread
+ private void onSessionStarted(int resultCode, @Nullable IBinder binder) {
if (binder != null) {
mDirectServiceInterface = IContentCaptureDirectManager.Stub.asInterface(binder);
mDirectServiceVulture = () -> {
@@ -262,7 +248,7 @@
}
if ((resultCode & STATE_DISABLED) != 0) {
- handleResetSession(resultCode);
+ resetSession(resultCode);
} else {
mState = resultCode;
mDisabled.set(false);
@@ -274,14 +260,16 @@
}
}
- private void handleSendEvent(@NonNull ContentCaptureEvent event) {
- handleSendEvent(event, /* forceFlush= */ false);
+ @UiThread
+ private void sendEvent(@NonNull ContentCaptureEvent event) {
+ sendEvent(event, /* forceFlush= */ false);
}
- private void handleSendEvent(@NonNull ContentCaptureEvent event, boolean forceFlush) {
+ @UiThread
+ private void sendEvent(@NonNull ContentCaptureEvent event, boolean forceFlush) {
final int eventType = event.getType();
if (VERBOSE) Log.v(TAG, "handleSendEvent(" + getDebugState() + "): " + event);
- if (!handleHasStarted() && eventType != ContentCaptureEvent.TYPE_SESSION_STARTED) {
+ if (!hasStarted() && eventType != ContentCaptureEvent.TYPE_SESSION_STARTED) {
// TODO(b/120494182): comment when this could happen (dialogs?)
Log.v(TAG, "handleSendEvent(" + getDebugState() + ", "
+ ContentCaptureEvent.getTypeAsString(eventType)
@@ -342,7 +330,7 @@
final boolean bufferEvent = numberEvents < MAX_BUFFER_SIZE;
if (bufferEvent && !forceFlush) {
- handleScheduleFlush(FLUSH_REASON_IDLE_TIMEOUT, /* checkExisting= */ true);
+ scheduleFlush(FLUSH_REASON_IDLE_TIMEOUT, /* checkExisting= */ true);
return;
}
@@ -356,7 +344,7 @@
Log.d(TAG, "Closing session for " + getDebugState()
+ " after " + numberEvents + " delayed events");
}
- handleResetSession(STATE_DISABLED | STATE_NO_RESPONSE);
+ resetSession(STATE_DISABLED | STATE_NO_RESPONSE);
// TODO(b/111276913): blacklist activity / use special flag to indicate that
// when it's launched again
return;
@@ -373,19 +361,21 @@
flushReason = FLUSH_REASON_FULL;
}
- handleForceFlush(flushReason);
+ flush(flushReason);
}
- private boolean handleHasStarted() {
+ @UiThread
+ private boolean hasStarted() {
return mState != UNKNOWN_STATE;
}
- private void handleScheduleFlush(@FlushReason int reason, boolean checkExisting) {
+ @UiThread
+ private void scheduleFlush(@FlushReason int reason, boolean checkExisting) {
if (VERBOSE) {
Log.v(TAG, "handleScheduleFlush(" + getDebugState(reason)
+ ", checkExisting=" + checkExisting);
}
- if (!handleHasStarted()) {
+ if (!hasStarted()) {
if (VERBOSE) Log.v(TAG, "handleScheduleFlush(): session not started yet");
return;
}
@@ -406,20 +396,22 @@
Log.v(TAG, "handleScheduleFlush(): scheduled to flush in "
+ FLUSHING_FREQUENCY_MS + "ms: " + TimeUtils.logTimeOfDay(mNextFlush));
}
- mHandler.sendMessageDelayed(
- obtainMessage(MainContentCaptureSession::handleFlushIfNeeded, this, reason)
- .setWhat(MSG_FLUSH), FLUSHING_FREQUENCY_MS);
+ // Post using a Runnable directly to trim a few μs from PooledLambda.obtainMessage()
+ mHandler.postDelayed(() -> flushIfNeeded(reason), MSG_FLUSH, FLUSHING_FREQUENCY_MS);
}
- private void handleFlushIfNeeded(@FlushReason int reason) {
+ @UiThread
+ private void flushIfNeeded(@FlushReason int reason) {
if (mEvents == null || mEvents.isEmpty()) {
if (VERBOSE) Log.v(TAG, "Nothing to flush");
return;
}
- handleForceFlush(reason);
+ flush(reason);
}
- private void handleForceFlush(@FlushReason int reason) {
+ @Override
+ @UiThread
+ void flush(@FlushReason int reason) {
if (mEvents == null) return;
if (mDisabled.get()) {
@@ -434,7 +426,7 @@
+ "client not ready: " + mEvents);
}
if (!mHandler.hasMessages(MSG_FLUSH)) {
- handleScheduleFlush(reason, /* checkExisting= */ false);
+ scheduleFlush(reason, /* checkExisting= */ false);
}
return;
}
@@ -451,7 +443,7 @@
mFlushHistory.log(logRecord);
mHandler.removeMessages(MSG_FLUSH);
- final ParceledListSlice<ContentCaptureEvent> events = handleClearEvents();
+ final ParceledListSlice<ContentCaptureEvent> events = clearEvents();
mDirectServiceInterface.sendEvents(events);
} catch (RemoteException e) {
Log.w(TAG, "Error sending " + numberEvents + " for " + getDebugState()
@@ -463,7 +455,8 @@
* Resets the buffer and return a {@link ParceledListSlice} with the previous events.
*/
@NonNull
- private ParceledListSlice<ContentCaptureEvent> handleClearEvents() {
+ @UiThread
+ private ParceledListSlice<ContentCaptureEvent> clearEvents() {
// 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<ContentCaptureEvent> events = mEvents == null
@@ -473,7 +466,8 @@
return new ParceledListSlice<>(events);
}
- private void handleDestroySession() {
+ @UiThread
+ private void destroySession() {
if (DEBUG) {
Log.d(TAG, "Destroying session (ctx=" + mContext + ", id=" + mId + ") with "
+ (mEvents == null ? 0 : mEvents.size()) + " event(s) for "
@@ -492,7 +486,8 @@
// TODO(b/122454205): once we support multiple sessions, we might need to move some of these
// clearings out.
- private void handleResetSession(int newState) {
+ @UiThread
+ private void resetSession(int newState) {
if (VERBOSE) {
Log.v(TAG, "handleResetSession(" + getActivityName() + "): from "
+ getStateAsString(mState) + " to " + getStateAsString(newState));
@@ -545,28 +540,25 @@
// change should also get get rid of the "internalNotifyXXXX" methods above
void notifyChildSessionStarted(@NonNull String parentSessionId,
@NonNull String childSessionId, @NonNull ContentCaptureContext clientContext) {
- mHandler.sendMessage(obtainMessage(MainContentCaptureSession::handleSendEvent, this,
- new ContentCaptureEvent(childSessionId, TYPE_SESSION_STARTED)
- .setParentSessionId(parentSessionId)
- .setClientContext(clientContext),
- FORCE_FLUSH));
+ sendEvent(new ContentCaptureEvent(childSessionId, TYPE_SESSION_STARTED)
+ .setParentSessionId(parentSessionId).setClientContext(clientContext),
+ FORCE_FLUSH);
}
void notifyChildSessionFinished(@NonNull String parentSessionId,
@NonNull String childSessionId) {
- mHandler.sendMessage(obtainMessage(MainContentCaptureSession::handleSendEvent, this,
- new ContentCaptureEvent(childSessionId, TYPE_SESSION_FINISHED)
- .setParentSessionId(parentSessionId), FORCE_FLUSH));
+ sendEvent(new ContentCaptureEvent(childSessionId, TYPE_SESSION_FINISHED)
+ .setParentSessionId(parentSessionId), FORCE_FLUSH);
}
void notifyViewAppeared(@NonNull String sessionId, @NonNull ViewStructureImpl node) {
- mHandler.sendMessage(obtainMessage(MainContentCaptureSession::handleSendEvent, this,
- new ContentCaptureEvent(sessionId, TYPE_VIEW_APPEARED).setViewNode(node.mNode)));
+ sendEvent(new ContentCaptureEvent(sessionId, TYPE_VIEW_APPEARED)
+ .setViewNode(node.mNode));
}
void notifyViewDisappeared(@NonNull String sessionId, @NonNull AutofillId id) {
- mHandler.sendMessage(obtainMessage(MainContentCaptureSession::handleSendEvent, this,
- new ContentCaptureEvent(sessionId, TYPE_VIEW_DISAPPEARED).setAutofillId(id)));
+ sendEvent(
+ new ContentCaptureEvent(sessionId, TYPE_VIEW_DISAPPEARED).setAutofillId(id));
}
/** @hide */
@@ -578,27 +570,21 @@
} else {
event.setAutofillIds(ids);
}
-
- mHandler.sendMessage(
- obtainMessage(MainContentCaptureSession::handleSendEvent, this, event));
+ sendEvent(event);
}
void notifyViewTextChanged(@NonNull String sessionId, @NonNull AutofillId id,
@Nullable CharSequence text) {
- mHandler.sendMessage(obtainMessage(MainContentCaptureSession::handleSendEvent, this,
- new ContentCaptureEvent(sessionId, TYPE_VIEW_TEXT_CHANGED).setAutofillId(id)
- .setText(text)));
+ sendEvent(new ContentCaptureEvent(sessionId, TYPE_VIEW_TEXT_CHANGED).setAutofillId(id)
+ .setText(text));
}
void notifyInitialViewHierarchyEvent(@NonNull String sessionId, boolean started) {
if (started) {
- mHandler.sendMessage(obtainMessage(MainContentCaptureSession::handleSendEvent, this,
- new ContentCaptureEvent(sessionId, TYPE_INITIAL_VIEW_TREE_APPEARING)));
+ sendEvent(new ContentCaptureEvent(sessionId, TYPE_INITIAL_VIEW_TREE_APPEARING));
} else {
- mHandler.sendMessage(obtainMessage(MainContentCaptureSession::handleSendEvent, this,
- new ContentCaptureEvent(sessionId, TYPE_INITIAL_VIEW_TREE_APPEARED),
- FORCE_FLUSH));
-
+ sendEvent(new ContentCaptureEvent(sessionId, TYPE_INITIAL_VIEW_TREE_APPEARED),
+ FORCE_FLUSH);
}
}