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

Moar Content capture improvements:

Browse Source 
- Drop Content Capture events when the session is disabled, otherwise sessions
  on this state might never finish.
- Fixed PII leak.
- Created helper class for common stuff.
- Improved logging.

Test: manually chenged VERBOSE to true, restarted system_server, and watched logcat - without
      this change, launcher's session would never die because it kept re-scheduling flush
      requests.
Test: atest CtsContentCaptureServiceTestCases
Bug: 122959591

Change-Id: Idcc1bbb12763d1cf98149164c68c2712834d5112
This commit is contained in:
Felipe Leme
2019-01-23 10:22:32 -08:00
parent f1cef97345
commit be002d89ec
5 changed files with 96 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
core/java/android/view/contentcapture/ContentCaptureEvent.java
View File

@@ -15,6 +15,8 @@
*/
package android.view.contentcapture;
import static android.view.contentcapture.ContentCaptureHelper.getSanitizedString;
import android.annotation.IntDef;
import android.annotation.NonNull;
import android.annotation.Nullable;
@@ -267,8 +269,7 @@ public final class ContentCaptureEvent implements Parcelable {
pw.print(", parentSessionId="); pw.print(mParentSessionId);
}
if (mText != null) {
// Cannot print content because could have PII
pw.print(", text="); pw.print(mText.length()); pw.print("_chars");
pw.print(", text="); pw.println(getSanitizedString(mText));
}
}
@@ -293,6 +294,9 @@ public final class ContentCaptureEvent implements Parcelable {
}
string.append(", id=").append(mNode.getAutofillId());
}
if (mText != null) {
string.append(", text=").append(getSanitizedString(mText));
}
return string.append(']').toString();
}

40
core/java/android/view/contentcapture/ContentCaptureHelper.java Normal file
View File

@@ -0,0 +1,40 @@
/*
* Copyright (C) 2019 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.annotation.Nullable;
/**
* Helpe class for this package.
*/
final class ContentCaptureHelper {
// TODO(b/121044306): define a way to dynamically set them(for example, using settings?)
static final boolean VERBOSE = false;
static final boolean DEBUG = true; // STOPSHIP if not set to false
/**
* Used to log text that could contain PII.
*/
@Nullable
public static String getSanitizedString(@Nullable CharSequence text) {
return text == null ? null : text.length() + "_chars";
}
private ContentCaptureHelper() {
throw new UnsupportedOperationException("contains only static methods");
}
}

6
core/java/android/view/contentcapture/ContentCaptureManager.java
View File

@@ -15,6 +15,8 @@
*/
package android.view.contentcapture;
import static android.view.contentcapture.ContentCaptureHelper.VERBOSE;
import static com.android.internal.util.function.pooled.PooledLambda.obtainMessage;
import android.annotation.NonNull;
@@ -57,10 +59,6 @@ public final class ContentCaptureManager {
*/
private static final int SYNC_CALLS_TIMEOUT_MS = 5000;
// TODO(b/121044306): define a way to dynamically set them(for example, using settings?)
static final boolean VERBOSE = false;
static final boolean DEBUG = true; // STOPSHIP if not set to false
private final Object mLock = new Object();
@GuardedBy("mLock")

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

@@ -15,8 +15,8 @@
*/
package android.view.contentcapture;
import static android.view.contentcapture.ContentCaptureManager.DEBUG;
import static android.view.contentcapture.ContentCaptureManager.VERBOSE;
import static android.view.contentcapture.ContentCaptureHelper.DEBUG;
import static android.view.contentcapture.ContentCaptureHelper.VERBOSE;
import android.annotation.CallSuper;
import android.annotation.IntDef;
@@ -246,7 +246,7 @@ public abstract class ContentCaptureSession implements AutoCloseable {
public final void destroy() {
synchronized (mLock) {
if (mDestroyed) {
Log.e(TAG, "destroy(" + mId + "): already destroyed");
if (DEBUG) Log.d(TAG, "destroy(" + mId + "): already destroyed");
return;
}
mDestroyed = true;

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

@@ -20,8 +20,9 @@ import static android.view.contentcapture.ContentCaptureEvent.TYPE_SESSION_START
import static android.view.contentcapture.ContentCaptureEvent.TYPE_VIEW_APPEARED;
import static android.view.contentcapture.ContentCaptureEvent.TYPE_VIEW_DISAPPEARED;
import static android.view.contentcapture.ContentCaptureEvent.TYPE_VIEW_TEXT_CHANGED;
import static android.view.contentcapture.ContentCaptureManager.DEBUG;
import static android.view.contentcapture.ContentCaptureManager.VERBOSE;
import static android.view.contentcapture.ContentCaptureHelper.DEBUG;
import static android.view.contentcapture.ContentCaptureHelper.VERBOSE;
import static android.view.contentcapture.ContentCaptureHelper.getSanitizedString;
import static com.android.internal.util.function.pooled.PooledLambda.obtainMessage;
@@ -269,6 +270,7 @@ public final class MainContentCaptureSession extends ContentCaptureSession {
private void handleSendEvent(@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) {
// TODO(b/120494182): comment when this could happen (dialogs?)
Log.v(TAG, "handleSendEvent(" + getDebugState() + ", "
@@ -276,12 +278,16 @@ public final class MainContentCaptureSession extends ContentCaptureSession {
+ "): session not started yet");
return;
}
if (VERBOSE) Log.v(TAG, "handleSendEvent(" + getDebugState() + "): " + event);
if (mDisabled.get()) {
// This happens when the event was queued in the handler before the sesison was ready,
// then handleSessionStarted() returned and set it as disabled - we need to drop it,
// otherwise it will keep triggering handleScheduleFlush()
if (VERBOSE) Log.v(TAG, "handleSendEvent(): ignoring when disabled");
return;
}
if (mEvents == null) {
if (VERBOSE) {
Log.v(TAG, "handleSendEvent(" + getDebugState() + ", "
+ ContentCaptureEvent.getTypeAsString(eventType)
+ "): creating buffer for " + MAX_BUFFER_SIZE + " events");
Log.v(TAG, "handleSendEvent(): creating buffer for " + MAX_BUFFER_SIZE + " events");
}
mEvents = new ArrayList<>(MAX_BUFFER_SIZE);
}
@@ -296,8 +302,8 @@ public final class MainContentCaptureSession extends ContentCaptureSession {
if (lastEvent.getType() == TYPE_VIEW_TEXT_CHANGED
&& lastEvent.getId().equals(event.getId())) {
if (VERBOSE) {
Log.v(TAG, "Buffering VIEW_TEXT_CHANGED event, updated text = "
+ event.getText());
Log.v(TAG, "Buffering VIEW_TEXT_CHANGED event, updated text="
+ getSanitizedString(event.getText()));
}
lastEvent.setText(event.getText());
addEvent = false;
@@ -365,8 +371,20 @@ public final class MainContentCaptureSession extends ContentCaptureSession {
}
private void handleScheduleFlush(@FlushReason int reason, boolean checkExisting) {
if (VERBOSE) {
Log.v(TAG, "handleScheduleFlush(" + getDebugState(reason)
+ ", checkExisting=" + checkExisting);
}
if (!handleHasStarted()) {
Log.v(TAG, "handleScheduleFlush(" + getDebugState() + "): session not started yet");
if (VERBOSE) Log.v(TAG, "handleScheduleFlush(): session not started yet");
return;
}
if (mDisabled.get()) {
// Should not be called on this state, as handleSendEvent checks.
// But we rather add one if check and log than re-schedule and keep the session alive...
Log.e(TAG, "handleScheduleFlush(" + getDebugState(reason) + "): should not be called "
+ "when disabled. events=" + (mEvents == null ? null : mEvents.size()));
return;
}
if (checkExisting && mHandler.hasMessages(MSG_FLUSH)) {
@@ -375,8 +393,7 @@ public final class MainContentCaptureSession extends ContentCaptureSession {
}
mNextFlush = System.currentTimeMillis() + FLUSHING_FREQUENCY_MS;
if (VERBOSE) {
Log.v(TAG, "handleScheduleFlush(" + getDebugState()
+ ", reason=" + getflushReasonAsString(reason) + "): scheduled to flush in "
Log.v(TAG, "handleScheduleFlush(): scheduled to flush in "
+ FLUSHING_FREQUENCY_MS + "ms: " + TimeUtils.logTimeOfDay(mNextFlush));
}
mHandler.sendMessageDelayed(
@@ -395,11 +412,16 @@ public final class MainContentCaptureSession extends ContentCaptureSession {
private void handleForceFlush(@FlushReason int reason) {
if (mEvents == null) return;
if (mDisabled.get()) {
Log.e(TAG, "handleForceFlush(" + getDebugState(reason) + "): should not be when "
+ "disabled");
return;
}
if (mDirectServiceInterface == null) {
if (VERBOSE) {
Log.v(TAG, "handleForceFlush(" + getDebugState()
+ ", reason=" + getflushReasonAsString(reason)
+ "): hold your horses, client not ready: " + mEvents);
Log.v(TAG, "handleForceFlush(" + getDebugState(reason) + "): hold your horses, "
+ "client not ready: " + mEvents);
}
if (!mHandler.hasMessages(MSG_FLUSH)) {
handleScheduleFlush(reason, /* checkExisting= */ false);
@@ -410,8 +432,7 @@ public final class MainContentCaptureSession extends ContentCaptureSession {
final int numberEvents = mEvents.size();
final String reasonString = getflushReasonAsString(reason);
if (DEBUG) {
Log.d(TAG, "Flushing " + numberEvents + " event(s) for " + getDebugState()
+ ". Reason: " + reasonString);
Log.d(TAG, "Flushing " + numberEvents + " event(s) for " + getDebugState(reason));
}
// Logs reason, size, max size, idle timeout
final String logRecord = "r=" + reasonString + " s=" + numberEvents
@@ -592,7 +613,14 @@ public final class MainContentCaptureSession extends ContentCaptureSession {
: "act:" + mComponentName.flattenToShortString();
}
@NonNull
private String getDebugState() {
return getActivityName() + " (state=" + getStateAsString(mState) + ")";
return getActivityName() + " [state=" + getStateAsString(mState) + ", disabled="
+ mDisabled.get() + "]";
}
@NonNull
private String getDebugState(@FlushReason int reason) {
return getDebugState() + ", reason=" + getflushReasonAsString(reason);
}
}