From d49d52c0ad936447c2f4b2668b620f32510e4762 Mon Sep 17 00:00:00 2001 From: Felipe Leme Date: Fri, 15 Feb 2019 09:48:20 -0800 Subject: ContentCaptureManager should be null when the feature is disabled. Test: manual verification using logcat and dumpsys Test: atest CtsContentCaptureServiceTestCases Fixes: 124475449 Change-Id: I699de127bf9e61ba187eb129c38d243d5fc93f1c --- core/java/android/app/SystemServiceRegistry.java | 6 ++- .../view/contentcapture/ContentCaptureManager.java | 44 ++++++++-------------- .../contentcapture/MainContentCaptureSession.java | 19 +++------- 3 files changed, 27 insertions(+), 42 deletions(-) diff --git a/core/java/android/app/SystemServiceRegistry.java b/core/java/android/app/SystemServiceRegistry.java index cca8bd00b646..688af3931116 100644 --- a/core/java/android/app/SystemServiceRegistry.java +++ b/core/java/android/app/SystemServiceRegistry.java @@ -1137,7 +1137,11 @@ final class SystemServiceRegistry { IBinder b = ServiceManager .getService(Context.CONTENT_CAPTURE_MANAGER_SERVICE); IContentCaptureManager service = IContentCaptureManager.Stub.asInterface(b); - return new ContentCaptureManager(outerContext, service); + if (service != null) { + // When feature is disabled, we return a null manager to apps so the + // performance impact is practically zero + return new ContentCaptureManager(outerContext, service); + } } return null; }}); diff --git a/core/java/android/view/contentcapture/ContentCaptureManager.java b/core/java/android/view/contentcapture/ContentCaptureManager.java index 634443d78b49..0157d2634b39 100644 --- a/core/java/android/view/contentcapture/ContentCaptureManager.java +++ b/core/java/android/view/contentcapture/ContentCaptureManager.java @@ -39,15 +39,8 @@ import com.android.internal.util.SyncResultReceiver; import java.io.PrintWriter; -/* - * 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 mEnabled, which is called at the beginning - * of every method. - */ /** - * TODO(b/123577059): add javadocs / implement + * TODO(b/123577059): add javadocs / mention it can be null */ @SystemService(Context.CONTENT_CAPTURE_MANAGER_SERVICE) public final class ContentCaptureManager { @@ -90,7 +83,7 @@ public final class ContentCaptureManager { @NonNull private final Context mContext; - @Nullable + @NonNull private final IContentCaptureManager mService; // Flags used for starting session. @@ -107,11 +100,11 @@ public final class ContentCaptureManager { /** @hide */ public ContentCaptureManager(@NonNull Context context, - @Nullable IContentCaptureManager service) { - mContext = Preconditions.checkNotNull(context, "context cannot be null"); + @NonNull IContentCaptureManager service) { if (VERBOSE) Log.v(TAG, "Constructor for " + context.getPackageName()); + mContext = Preconditions.checkNotNull(context, "context cannot be null"); + mService = Preconditions.checkNotNull(service, "service cannot be null"); - mService = service; // TODO(b/119220549): we might not even need a handler, as the IPCs are oneway. But if we // do, then we should optimize it to run the tests after the Choreographer finishes the most // important steps of the frame. @@ -199,8 +192,6 @@ public final class ContentCaptureManager { * */ public boolean isContentCaptureEnabled() { - if (mService == null) return false; - final MainContentCaptureSession mainSession; synchronized (mLock) { mainSession = mMainSession; @@ -242,8 +233,6 @@ public final class ContentCaptureManager { @SystemApi @TestApi public boolean isContentCaptureFeatureEnabled() { - if (mService == null) return false; - final SyncResultReceiver resultReceiver = new SyncResultReceiver(SYNC_CALLS_TIMEOUT_MS); final int resultCode; try { @@ -314,22 +303,21 @@ public final class ContentCaptureManager { /** @hide */ public void dump(String prefix, PrintWriter pw) { + pw.print(prefix); pw.println("ContentCaptureManager"); + final String prefix2 = prefix + " "; synchronized (mLock) { - pw.print(prefix); pw.println("ContentCaptureManager"); - pw.print(prefix); pw.print("isContentCaptureEnabled(): "); + pw.print(prefix2); pw.print("isContentCaptureEnabled(): "); pw.println(isContentCaptureEnabled()); - pw.print(prefix); pw.print("Context: "); pw.println(mContext); - pw.print(prefix); pw.print("User: "); pw.println(mContext.getUserId()); - if (mService != null) { - pw.print(prefix); pw.print("Service: "); pw.println(mService); - } - pw.print(prefix); pw.print("Flags: "); pw.println(mFlags); + pw.print(prefix2); pw.print("Context: "); pw.println(mContext); + pw.print(prefix2); pw.print("User: "); pw.println(mContext.getUserId()); + pw.print(prefix2); pw.print("Service: "); pw.println(mService); + pw.print(prefix2); pw.print("Flags: "); pw.println(mFlags); if (mMainSession != null) { - final String prefix2 = prefix + " "; - pw.print(prefix); pw.println("Main session:"); - mMainSession.dump(prefix2, pw); + final String prefix3 = prefix2 + " "; + pw.print(prefix2); pw.println("Main session:"); + mMainSession.dump(prefix3, pw); } else { - pw.print(prefix); pw.println("No sessions"); + pw.print(prefix2); pw.println("No sessions"); } } } diff --git a/core/java/android/view/contentcapture/MainContentCaptureSession.java b/core/java/android/view/contentcapture/MainContentCaptureSession.java index d949f45ba5e6..f0f2c49f31ee 100644 --- a/core/java/android/view/contentcapture/MainContentCaptureSession.java +++ b/core/java/android/view/contentcapture/MainContentCaptureSession.java @@ -105,14 +105,14 @@ public final class MainContentCaptureSession extends ContentCaptureSession { * Interface to the system_server binder object - it's only used to start the session (and * notify when the session is finished). */ - @Nullable // TODO(b/122959591): shoul never be null, we should make main session null instead + @NonNull 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 + @NonNull private IContentCaptureDirectManager mDirectServiceInterface; @Nullable private DeathRecipient mDirectServiceVulture; @@ -140,7 +140,7 @@ public final class MainContentCaptureSession extends ContentCaptureSession { /** @hide */ protected MainContentCaptureSession(@NonNull Context context, @NonNull ContentCaptureManager manager, @NonNull Handler handler, - @Nullable IContentCaptureManager systemServerInterface) { + @NonNull IContentCaptureManager systemServerInterface) { mContext = context; mManager = manager; mHandler = handler; @@ -193,8 +193,6 @@ public final class MainContentCaptureSession extends ContentCaptureSession { } try { - if (mSystemServerInterface == null) return; - mSystemServerInterface.startSession(mApplicationToken, component, mId, flags, new IResultReceiver.Stub() { @Override @@ -507,8 +505,6 @@ public final class MainContentCaptureSession extends ContentCaptureSession { } try { - if (mSystemServerInterface == null) return; - mSystemServerInterface.finishSession(mId); } catch (RemoteException e) { Log.e(TAG, "Error destroying system-service session " + mId + " for " @@ -628,12 +624,11 @@ public final class MainContentCaptureSession extends ContentCaptureSession { @Override void dump(@NonNull String prefix, @NonNull PrintWriter pw) { + super.dump(prefix, pw); + pw.print(prefix); pw.print("mContext: "); pw.println(mContext); pw.print(prefix); pw.print("user: "); pw.println(mContext.getUserId()); - if (mSystemServerInterface != null) { - pw.print(prefix); pw.print("mSystemServerInterface: "); - pw.println(mSystemServerInterface); - } + pw.print(prefix); pw.print("mSystemServerInterface: "); if (mDirectServiceInterface != null) { pw.print(prefix); pw.print("mDirectServiceInterface: "); pw.println(mDirectServiceInterface); @@ -667,8 +662,6 @@ public final class MainContentCaptureSession extends ContentCaptureSession { } pw.print(prefix); pw.println("flush history:"); mFlushHistory.reverseDump(/* fd= */ null, pw, /* args= */ null); pw.println(); - - super.dump(prefix, pw); } /** -- cgit v1.2.3-59-g8ed1b