[go: nahoru, domu]

Check visibility store before dispatching observers.

This prevents apps from getting notifications of changes they don't
have access to.

Bug: 193494000
Test: AppSearchImplTest#testDispatchObserver*
Change-Id: Idef229b8968076e39358b4992f117bcabfac0d6f
diff --git a/appsearch/appsearch-local-storage/src/androidTest/java/androidx/appsearch/localstorage/AppSearchImplTest.java b/appsearch/appsearch-local-storage/src/androidTest/java/androidx/appsearch/localstorage/AppSearchImplTest.java
index 6ef0689..6fae9b6 100644
--- a/appsearch/appsearch-local-storage/src/androidTest/java/androidx/appsearch/localstorage/AppSearchImplTest.java
+++ b/appsearch/appsearch-local-storage/src/androidTest/java/androidx/appsearch/localstorage/AppSearchImplTest.java
@@ -2751,12 +2751,18 @@
         // Register an observer twice, on different packages.
         TestObserverCallback observer = new TestObserverCallback();
         mAppSearchImpl.addObserver(
-                /*observedPackage=*/mContext.getPackageName(),
+                /*listeningPackageName=*/mContext.getPackageName(),
+                /*listeningUid=*/Process.myUid(),
+                /*listeningPackageHasSystemAccess=*/false,
+                /*targetPackageName=*/mContext.getPackageName(),
                 new ObserverSpec.Builder().build(),
                 MoreExecutors.directExecutor(),
                 observer);
         mAppSearchImpl.addObserver(
-                /*observedPackage=*/fakePackage,
+                /*listeningPackageName=*/mContext.getPackageName(),
+                /*listeningUid=*/Process.myUid(),
+                /*listeningPackageHasSystemAccess=*/false,
+                /*targetPackageName=*/fakePackage,
                 new ObserverSpec.Builder().build(),
                 MoreExecutors.directExecutor(),
                 observer);
@@ -3341,4 +3347,259 @@
         assertThat(getResponse.getSchemaTypesNotDisplayedBySystem()).containsExactly("VisibleType");
         assertThat(getResponse.getVersion()).isEqualTo(1);
     }
+
+    @Test
+    public void testDispatchObserver_samePackage_noVisStore_accept() throws Exception {
+        mAppSearchImpl.setSchema(
+                mContext.getPackageName(),
+                "database1",
+                ImmutableList.of(new AppSearchSchema.Builder("Type1").build()),
+                /*visibilityDocuments=*/ Collections.emptyList(),
+                /*forceOverride=*/ false,
+                /*version=*/ 0,
+                /*setSchemaStatsBuilder=*/ null);
+
+        // Register an observer
+        TestObserverCallback observer = new TestObserverCallback();
+        mAppSearchImpl.addObserver(
+                /*listeningPackageName=*/mContext.getPackageName(),
+                Process.myUid(),
+                /*listeningPackageHasSystemAccess=*/false,
+                /*targetPackageName=*/mContext.getPackageName(),
+                new ObserverSpec.Builder().build(),
+                MoreExecutors.directExecutor(),
+                observer);
+
+        // Insert a valid doc
+        assertThat(observer.getSchemaChanges()).isEmpty();
+        assertThat(observer.getDocumentChanges()).isEmpty();
+        mAppSearchImpl.putDocument(
+                mContext.getPackageName(),
+                "database1",
+                new GenericDocument.Builder<>("namespace1", "id1", "Type1").build(),
+                /*logger=*/null);
+        assertThat(observer.getSchemaChanges()).isEmpty();
+        assertThat(observer.getDocumentChanges()).isEmpty();
+
+        // Dispatch notifications
+        mAppSearchImpl.dispatchAndClearChangeNotifications();
+        assertThat(observer.getSchemaChanges()).isEmpty();
+        assertThat(observer.getDocumentChanges()).containsExactly(
+                new DocumentChangeInfo(
+                        mContext.getPackageName(),
+                        "database1",
+                        "namespace1",
+                        "Type1",
+                        ImmutableSet.of("id1")));
+    }
+
+    @Test
+    public void testDispatchObserver_samePackage_withVisStore_accept() throws Exception {
+        // Make a visibility checker that rejects everything
+        final VisibilityChecker rejectChecker =
+                (packageName, prefixedSchema, callerUid, callerHasSystemAccess, visibilityStore)
+                        -> false;
+        mAppSearchImpl.close();
+        mAppSearchImpl = AppSearchImpl.create(
+                mAppSearchDir,
+                new UnlimitedLimitConfig(),
+                /*initStatsBuilder=*/null,
+                ALWAYS_OPTIMIZE,
+                rejectChecker);
+
+        mAppSearchImpl.setSchema(
+                mContext.getPackageName(),
+                "database1",
+                ImmutableList.of(new AppSearchSchema.Builder("Type1").build()),
+                /*visibilityDocuments=*/ Collections.emptyList(),
+                /*forceOverride=*/ false,
+                /*version=*/ 0,
+                /*setSchemaStatsBuilder=*/ null);
+
+        // Register an observer
+        TestObserverCallback observer = new TestObserverCallback();
+        mAppSearchImpl.addObserver(
+                /*listeningPackageName=*/mContext.getPackageName(),
+                Process.myUid(),
+                /*listeningPackageHasSystemAccess=*/false,
+                /*targetPackageName=*/mContext.getPackageName(),
+                new ObserverSpec.Builder().build(),
+                MoreExecutors.directExecutor(),
+                observer);
+
+        // Insert a valid doc
+        assertThat(observer.getSchemaChanges()).isEmpty();
+        assertThat(observer.getDocumentChanges()).isEmpty();
+        mAppSearchImpl.putDocument(
+                mContext.getPackageName(),
+                "database1",
+                new GenericDocument.Builder<>("namespace1", "id1", "Type1").build(),
+                /*logger=*/null);
+        assertThat(observer.getSchemaChanges()).isEmpty();
+        assertThat(observer.getDocumentChanges()).isEmpty();
+
+        // Dispatch notifications
+        mAppSearchImpl.dispatchAndClearChangeNotifications();
+        assertThat(observer.getSchemaChanges()).isEmpty();
+        assertThat(observer.getDocumentChanges()).containsExactly(
+                new DocumentChangeInfo(
+                        mContext.getPackageName(),
+                        "database1",
+                        "namespace1",
+                        "Type1",
+                        ImmutableSet.of("id1")));
+    }
+
+    @Test
+    public void testDispatchObserver_differentPackage_noVisStore_reject() throws Exception {
+        mAppSearchImpl.setSchema(
+                mContext.getPackageName(),
+                "database1",
+                ImmutableList.of(new AppSearchSchema.Builder("Type1").build()),
+                /*visibilityDocuments=*/ Collections.emptyList(),
+                /*forceOverride=*/ false,
+                /*version=*/ 0,
+                /*setSchemaStatsBuilder=*/ null);
+
+        // Register an observer from a simulated different package
+        TestObserverCallback observer = new TestObserverCallback();
+        mAppSearchImpl.addObserver(
+                /*listeningPackageName=*/"com.fake.Listening.package",
+                Process.myUid(),
+                /*listeningPackageHasSystemAccess=*/false,
+                /*targetPackageName=*/mContext.getPackageName(),
+                new ObserverSpec.Builder().build(),
+                MoreExecutors.directExecutor(),
+                observer);
+
+        // Insert a valid doc
+        assertThat(observer.getSchemaChanges()).isEmpty();
+        assertThat(observer.getDocumentChanges()).isEmpty();
+        mAppSearchImpl.putDocument(
+                mContext.getPackageName(),
+                "database1",
+                new GenericDocument.Builder<>("namespace1", "id1", "Type1").build(),
+                /*logger=*/null);
+        assertThat(observer.getSchemaChanges()).isEmpty();
+        assertThat(observer.getDocumentChanges()).isEmpty();
+
+        // Dispatch notifications
+        mAppSearchImpl.dispatchAndClearChangeNotifications();
+        assertThat(observer.getSchemaChanges()).isEmpty();
+        assertThat(observer.getDocumentChanges()).isEmpty();
+    }
+
+    @Test
+    public void testDispatchObserver_differentPackage_withVisStore_accept() throws Exception {
+        final String fakeListeningPackage = "com.fake.listening.package";
+        final int fakeListeningUid = 42;
+
+        // Make a visibility checker that allows only fakeListeningPackage.
+        final VisibilityChecker visibilityChecker =
+                (packageName, prefixedSchema, callerUid, callerHasSystemAccess, visibilityStore)
+                        -> callerUid == fakeListeningUid;
+        mAppSearchImpl.close();
+        mAppSearchImpl = AppSearchImpl.create(
+                mAppSearchDir,
+                new UnlimitedLimitConfig(),
+                /*initStatsBuilder=*/null,
+                ALWAYS_OPTIMIZE,
+                visibilityChecker);
+
+        mAppSearchImpl.setSchema(
+                mContext.getPackageName(),
+                "database1",
+                ImmutableList.of(new AppSearchSchema.Builder("Type1").build()),
+                /*visibilityDocuments=*/ Collections.emptyList(),
+                /*forceOverride=*/ false,
+                /*version=*/ 0,
+                /*setSchemaStatsBuilder=*/ null);
+
+        // Register an observer
+        TestObserverCallback observer = new TestObserverCallback();
+        mAppSearchImpl.addObserver(
+                fakeListeningPackage,
+                fakeListeningUid,
+                /*listeningPackageHasSystemAccess=*/false,
+                /*targetPackageName=*/mContext.getPackageName(),
+                new ObserverSpec.Builder().build(),
+                MoreExecutors.directExecutor(),
+                observer);
+
+        // Insert a valid doc
+        assertThat(observer.getSchemaChanges()).isEmpty();
+        assertThat(observer.getDocumentChanges()).isEmpty();
+        mAppSearchImpl.putDocument(
+                mContext.getPackageName(),
+                "database1",
+                new GenericDocument.Builder<>("namespace1", "id1", "Type1").build(),
+                /*logger=*/null);
+        assertThat(observer.getSchemaChanges()).isEmpty();
+        assertThat(observer.getDocumentChanges()).isEmpty();
+
+        // Dispatch notifications
+        mAppSearchImpl.dispatchAndClearChangeNotifications();
+        assertThat(observer.getSchemaChanges()).isEmpty();
+        assertThat(observer.getDocumentChanges()).containsExactly(
+                new DocumentChangeInfo(
+                        mContext.getPackageName(),
+                        "database1",
+                        "namespace1",
+                        "Type1",
+                        ImmutableSet.of("id1")));
+    }
+
+    @Test
+    public void testDispatchObserver_differentPackage_withVisStore_reject() throws Exception {
+        final String fakeListeningPackage = "com.fake.Listening.package";
+        final int fakeListeningUid = 42;
+
+        // Make a visibility checker that rejects everything.
+        final VisibilityChecker rejectChecker =
+                (packageName, prefixedSchema, callerUid, callerHasSystemAccess, visibilityStore)
+                        -> false;
+        mAppSearchImpl.close();
+        mAppSearchImpl = AppSearchImpl.create(
+                mAppSearchDir,
+                new UnlimitedLimitConfig(),
+                /*initStatsBuilder=*/null,
+                ALWAYS_OPTIMIZE,
+                rejectChecker);
+
+        mAppSearchImpl.setSchema(
+                mContext.getPackageName(),
+                "database1",
+                ImmutableList.of(new AppSearchSchema.Builder("Type1").build()),
+                /*visibilityDocuments=*/ Collections.emptyList(),
+                /*forceOverride=*/ false,
+                /*version=*/ 0,
+                /*setSchemaStatsBuilder=*/ null);
+
+        // Register an observer
+        TestObserverCallback observer = new TestObserverCallback();
+        mAppSearchImpl.addObserver(
+                fakeListeningPackage,
+                fakeListeningUid,
+                /*listeningPackageHasSystemAccess=*/false,
+                /*targetPackageName=*/mContext.getPackageName(),
+                new ObserverSpec.Builder().build(),
+                MoreExecutors.directExecutor(),
+                observer);
+
+        // Insert a doc
+        assertThat(observer.getSchemaChanges()).isEmpty();
+        assertThat(observer.getDocumentChanges()).isEmpty();
+        mAppSearchImpl.putDocument(
+                mContext.getPackageName(),
+                "database1",
+                new GenericDocument.Builder<>("namespace1", "id1", "Type1").build(),
+                /*logger=*/null);
+        assertThat(observer.getSchemaChanges()).isEmpty();
+        assertThat(observer.getDocumentChanges()).isEmpty();
+
+        // Dispatch notifications
+        mAppSearchImpl.dispatchAndClearChangeNotifications();
+        assertThat(observer.getSchemaChanges()).isEmpty();
+        assertThat(observer.getDocumentChanges()).isEmpty();
+    }
 }
diff --git a/appsearch/appsearch-local-storage/src/main/java/androidx/appsearch/localstorage/AppSearchImpl.java b/appsearch/appsearch-local-storage/src/main/java/androidx/appsearch/localstorage/AppSearchImpl.java
index b016a9c..6bc938da 100644
--- a/appsearch/appsearch-local-storage/src/main/java/androidx/appsearch/localstorage/AppSearchImpl.java
+++ b/appsearch/appsearch-local-storage/src/main/java/androidx/appsearch/localstorage/AppSearchImpl.java
@@ -771,7 +771,9 @@
                     databaseName,
                     document.getNamespace(),
                     document.getSchemaType(),
-                    document.getId());
+                    document.getId(),
+                    mVisibilityStoreLocked,
+                    mVisibilityCheckerLocked);
         } finally {
             mReadWriteLock.writeLock().unlock();
 
@@ -1442,7 +1444,13 @@
             // Prepare notifications
             if (schemaType != null) {
                 mObserverManager.onDocumentChange(
-                        packageName, databaseName, namespace, schemaType, documentId);
+                        packageName,
+                        databaseName,
+                        namespace,
+                        schemaType,
+                        documentId,
+                        mVisibilityStoreLocked,
+                        mVisibilityCheckerLocked);
             }
         } finally {
             mReadWriteLock.writeLock().unlock();
@@ -1604,7 +1612,9 @@
                             /*databaseName=*/ PrefixUtil.getDatabaseName(document.getNamespace()),
                             /*namespace=*/ PrefixUtil.removePrefix(document.getNamespace()),
                             /*schemaType=*/ PrefixUtil.removePrefix(document.getSchema()),
-                            document.getUri());
+                            document.getUri(),
+                            mVisibilityStoreLocked,
+                            mVisibilityCheckerLocked);
                 }
             }
 
@@ -2137,23 +2147,40 @@
 
     /**
      * Adds an {@link AppSearchObserverCallback} to monitor changes within the
-     * databases owned by {@code observedPackage} if they match the given
+     * databases owned by {@code targetPackageName} if they match the given
      * {@link androidx.appsearch.observer.ObserverSpec}.
      *
-     * <p>If the data owned by {@code observedPackage} is not visible to you, the registration call
-     * will succeed but no notifications will be dispatched. Notifications could start flowing later
-     * if {@code observedPackage} changes its schema visibility settings.
+     * <p>If the data owned by {@code targetPackageName} is not visible to you, the registration
+     * call will succeed but no notifications will be dispatched. Notifications could start flowing
+     * later if {@code targetPackageName} changes its schema visibility settings.
      *
-     * <p>If no package matching {@code observedPackage} exists on the system, the registration call
-     * will succeed but no notifications will be dispatched. Notifications could start flowing later
-     * if {@code observedPackage} is installed and starts indexing data.
+     * <p>If no package matching {@code targetPackageName} exists on the system, the registration
+     * call will succeed but no notifications will be dispatched. Notifications could start flowing
+     * later if {@code targetPackageName} is installed and starts indexing data.
      *
      * <p>Note that this method does not take the standard read/write lock that guards I/O, so it
      * will not queue behind I/O. Therefore it is safe to call from any thread including UI or
      * binder threads.
+     *
+     * @param listeningPackageName            The package name of the app that wants to receive
+     *                                        notifications.
+     * @param listeningUid                    The uid of the app that wants to receive
+     *                                        notifications.
+     * @param listeningPackageHasSystemAccess Whether the app that wants to receive notifications
+     *                                        has access to schema types marked 'visible to system'.
+     * @param targetPackageName               The package that owns the data the observer wants
+     *                                        to be notified for.
+     * @param spec                            Describes the kind of data changes the observer
+     *                                        should trigger for.
+     * @param executor                        The executor on which to trigger the observer callback
+     *                                        to deliver notifications.
+     * @param observer                        The callback to trigger on notifications.
      */
     public void addObserver(
-            @NonNull String observedPackage,
+            @NonNull String listeningPackageName,
+            int listeningUid,
+            boolean listeningPackageHasSystemAccess,
+            @NonNull String targetPackageName,
             @NonNull ObserverSpec spec,
             @NonNull Executor executor,
             @NonNull AppSearchObserverCallback observer) {
@@ -2161,12 +2188,19 @@
         // observers for types that don't exist. This is intentional because we notify for types
         // being created or removed. If we only registered observer for existing types, it would
         // be impossible to ever dispatch a notification of a type being added.
-        mObserverManager.addObserver(observedPackage, spec, executor, observer);
+        mObserverManager.addObserver(
+                listeningPackageName,
+                listeningUid,
+                listeningPackageHasSystemAccess,
+                targetPackageName,
+                spec,
+                executor,
+                observer);
     }
 
     /**
      * Removes an {@link AppSearchObserverCallback} from watching the databases owned by
-     * {@code observedPackage}.
+     * {@code targetPackageName}.
      *
      * <p>All observers which compare equal to the given observer via
      * {@link AppSearchObserverCallback#equals} are removed. This may be 0, 1, or many observers.
@@ -2176,8 +2210,8 @@
      * binder threads.
      */
     public void removeObserver(
-            @NonNull String observedPackage, @NonNull AppSearchObserverCallback observer) {
-        mObserverManager.removeObserver(observedPackage, observer);
+            @NonNull String targetPackageName, @NonNull AppSearchObserverCallback observer) {
+        mObserverManager.removeObserver(targetPackageName, observer);
     }
 
     /**
diff --git a/appsearch/appsearch-local-storage/src/main/java/androidx/appsearch/localstorage/GlobalSearchSessionImpl.java b/appsearch/appsearch-local-storage/src/main/java/androidx/appsearch/localstorage/GlobalSearchSessionImpl.java
index 42c88a9..3d569ca 100644
--- a/appsearch/appsearch-local-storage/src/main/java/androidx/appsearch/localstorage/GlobalSearchSessionImpl.java
+++ b/appsearch/appsearch-local-storage/src/main/java/androidx/appsearch/localstorage/GlobalSearchSessionImpl.java
@@ -130,35 +130,42 @@
 
     @Override
     public void addObserver(
-            @NonNull String observedPackage,
+            @NonNull String targetPackageName,
             @NonNull ObserverSpec spec,
             @NonNull Executor executor,
             @NonNull AppSearchObserverCallback observer) {
-        Preconditions.checkNotNull(observedPackage);
+        Preconditions.checkNotNull(targetPackageName);
         Preconditions.checkNotNull(spec);
         Preconditions.checkNotNull(executor);
         Preconditions.checkNotNull(observer);
         // LocalStorage does not support observing data from other packages.
-        if (!observedPackage.equals(mContext.getPackageName())) {
+        if (!targetPackageName.equals(mContext.getPackageName())) {
             throw new UnsupportedOperationException(
                     "Local storage implementation does not support receiving change notifications "
                             + "from other packages.");
         }
-        mAppSearchImpl.addObserver(observedPackage, spec, executor, observer);
+        mAppSearchImpl.addObserver(
+                /*listeningPackageName=*/mContext.getPackageName(),
+                /*listeningUid=*/Process.myUid(),
+                /*listeningPackageHasSystemAccess=*/false,
+                /*targetPackageName=*/targetPackageName,
+                spec,
+                executor,
+                observer);
     }
 
     @Override
     public void removeObserver(
-            @NonNull String observedPackage, @NonNull AppSearchObserverCallback observer) {
-        Preconditions.checkNotNull(observedPackage);
+            @NonNull String targetPackageName, @NonNull AppSearchObserverCallback observer) {
+        Preconditions.checkNotNull(targetPackageName);
         Preconditions.checkNotNull(observer);
         // LocalStorage does not support observing data from other packages.
-        if (!observedPackage.equals(mContext.getPackageName())) {
+        if (!targetPackageName.equals(mContext.getPackageName())) {
             throw new UnsupportedOperationException(
                     "Local storage implementation does not support receiving change notifications "
                             + "from other packages.");
         }
-        mAppSearchImpl.removeObserver(observedPackage, observer);
+        mAppSearchImpl.removeObserver(targetPackageName, observer);
     }
 
     @Override
diff --git a/appsearch/appsearch-local-storage/src/main/java/androidx/appsearch/localstorage/ObserverManager.java b/appsearch/appsearch-local-storage/src/main/java/androidx/appsearch/localstorage/ObserverManager.java
index 8c77fe8..764d44c 100644
--- a/appsearch/appsearch-local-storage/src/main/java/androidx/appsearch/localstorage/ObserverManager.java
+++ b/appsearch/appsearch-local-storage/src/main/java/androidx/appsearch/localstorage/ObserverManager.java
@@ -22,6 +22,10 @@
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 import androidx.annotation.RestrictTo;
+import androidx.appsearch.localstorage.util.PrefixUtil;
+import androidx.appsearch.localstorage.visibilitystore.VisibilityChecker;
+import androidx.appsearch.localstorage.visibilitystore.VisibilityStore;
+import androidx.appsearch.localstorage.visibilitystore.VisibilityUtil;
 import androidx.appsearch.observer.AppSearchObserverCallback;
 import androidx.appsearch.observer.DocumentChangeInfo;
 import androidx.appsearch.observer.ObserverSpec;
@@ -84,6 +88,10 @@
     }
 
     private static final class ObserverInfo {
+        /** The package which registered the observer. */
+        final String mListeningPackageName;
+        final int mListeningUid;
+        final boolean mListeningPackageHasSystemAccess;
         final ObserverSpec mObserverSpec;
         final Executor mExecutor;
         final AppSearchObserverCallback mObserver;
@@ -91,9 +99,15 @@
         volatile Map<DocumentChangeGroupKey, Set<String>> mDocumentChanges = new ArrayMap<>();
 
         ObserverInfo(
+                @NonNull String listeningPackageName,
+                int listeningUid,
+                boolean listeningPackageHasSystemAccess,
                 @NonNull ObserverSpec observerSpec,
                 @NonNull Executor executor,
                 @NonNull AppSearchObserverCallback observer) {
+            mListeningPackageName = Preconditions.checkNotNull(listeningPackageName);
+            mListeningUid = listeningUid;
+            mListeningPackageHasSystemAccess = listeningPackageHasSystemAccess;
             mObserverSpec = Preconditions.checkNotNull(observerSpec);
             mExecutor = Preconditions.checkNotNull(executor);
             mObserver = Preconditions.checkNotNull(observer);
@@ -110,42 +124,69 @@
 
     /**
      * Adds an {@link AppSearchObserverCallback} to monitor changes within the
-     * databases owned by {@code observedPackage} if they match the given
+     * databases owned by {@code targetPackageName} if they match the given
      * {@link androidx.appsearch.observer.ObserverSpec}.
      *
-     * <p>If the data owned by {@code observedPackage} is not visible to you, the registration call
-     * will succeed but no notifications will be dispatched. Notifications could start flowing later
-     * if {@code observedPackage} changes its schema visibility settings.
+     * <p>If the data owned by {@code targetPackageName} is not visible to you, the registration
+     * call will succeed but no notifications will be dispatched. Notifications could start flowing
+     * later if {@code targetPackageName} changes its schema visibility settings.
      *
-     * <p>If no package matching {@code observedPackage} exists on the system, the registration call
-     * will succeed but no notifications will be dispatched. Notifications could start flowing later
-     * if {@code observedPackage} is installed and starts indexing data.
+     * <p>If no package matching {@code targetPackageName} exists on the system, the registration
+     * call will succeed but no notifications will be dispatched. Notifications could start flowing
+     * later if {@code targetPackageName} is installed and starts indexing data.
+     *
+     * <p>Note that this method does not take the standard read/write lock that guards I/O, so it
+     * will not queue behind I/O. Therefore it is safe to call from any thread including UI or
+     * binder threads.
+     *
+     * @param listeningPackageName            The package name of the app that wants to receive
+     *                                        notifications.
+     * @param listeningUid                    The uid of the app that wants to receive
+     *                                        notifications.
+     * @param listeningPackageHasSystemAccess Whether the app that wants to receive notifications
+     *                                        has access to schema types marked 'visible to system'.
+     * @param targetPackageName               The package that owns the data the observer wants
+     *                                        to be notified for.
+     * @param spec                            Describes the kind of data changes the observer
+     *                                        should trigger for.
+     * @param executor                        The executor on which to trigger the observer callback
+     *                                        to deliver notifications.
+     * @param observer                        The callback to trigger on notifications.
      */
     public void addObserver(
-            @NonNull String observedPackage,
+            @NonNull String listeningPackageName,
+            int listeningUid,
+            boolean listeningPackageHasSystemAccess,
+            @NonNull String targetPackageName,
             @NonNull ObserverSpec spec,
             @NonNull Executor executor,
             @NonNull AppSearchObserverCallback observer) {
         synchronized (mLock) {
-            List<ObserverInfo> infos = mObserversLocked.get(observedPackage);
+            List<ObserverInfo> infos = mObserversLocked.get(targetPackageName);
             if (infos == null) {
                 infos = new ArrayList<>();
-                mObserversLocked.put(observedPackage, infos);
+                mObserversLocked.put(targetPackageName, infos);
             }
-            infos.add(new ObserverInfo(spec, executor, observer));
+            infos.add(new ObserverInfo(
+                    listeningPackageName,
+                    listeningUid,
+                    listeningPackageHasSystemAccess,
+                    spec,
+                    executor,
+                    observer));
         }
     }
 
     /**
      * Removes all observers that match via {@link AppSearchObserverCallback#equals} to the given
-     * observer from watching the observedPackage.
+     * observer from watching the targetPackageName.
      *
      * <p>Pending notifications queued for this observer, if any, are discarded.
      */
     public void removeObserver(
-            @NonNull String observedPackage, @NonNull AppSearchObserverCallback observer) {
+            @NonNull String targetPackageName, @NonNull AppSearchObserverCallback observer) {
         synchronized (mLock) {
-            List<ObserverInfo> infos = mObserversLocked.get(observedPackage);
+            List<ObserverInfo> infos = mObserversLocked.get(targetPackageName);
             if (infos == null) {
                 return;
             }
@@ -163,13 +204,20 @@
      *
      * <p>The notification will be queued in memory for later dispatch. You must call
      * {@link #dispatchAndClearPendingNotifications} to dispatch all such pending notifications.
+     *
+     * @param visibilityStore   Store for visibility information. If not provided, only access to
+     *                          own data will be allowed.
+     * @param visibilityChecker Checker for visibility access. If not provided, only access to own
+     *                          data will be allowed.
      */
     public void onDocumentChange(
             @NonNull String packageName,
             @NonNull String databaseName,
             @NonNull String namespace,
             @NonNull String schemaType,
-            @NonNull String documentId) {
+            @NonNull String documentId,
+            @Nullable VisibilityStore visibilityStore,
+            @Nullable VisibilityChecker visibilityChecker) {
         synchronized (mLock) {
             List<ObserverInfo> allObserverInfosForPackage = mObserversLocked.get(packageName);
             if (allObserverInfosForPackage == null || allObserverInfosForPackage.isEmpty()) {
@@ -179,18 +227,32 @@
             DocumentChangeGroupKey key = null;
             for (int i = 0; i < allObserverInfosForPackage.size(); i++) {
                 ObserverInfo observerInfo = allObserverInfosForPackage.get(i);
-                if (matchesSpec(schemaType, observerInfo.mObserverSpec)) {
-                    if (key == null) {
-                        key = new DocumentChangeGroupKey(
-                                packageName, databaseName, namespace, schemaType);
-                    }
-                    Set<String> changedDocumentIds = observerInfo.mDocumentChanges.get(key);
-                    if (changedDocumentIds == null) {
-                        changedDocumentIds = new ArraySet<>();
-                        observerInfo.mDocumentChanges.put(key, changedDocumentIds);
-                    }
-                    changedDocumentIds.add(documentId);
+                if (!matchesSpec(schemaType, observerInfo.mObserverSpec)) {
+                    continue;  // Observer doesn't want this notification
                 }
+                String prefixedSchema =
+                        PrefixUtil.createPrefix(packageName, databaseName)
+                                + schemaType;
+                if (!VisibilityUtil.isSchemaSearchableByCaller(
+                        /*callerPackageName=*/observerInfo.mListeningPackageName,
+                        observerInfo.mListeningUid,
+                        observerInfo.mListeningPackageHasSystemAccess,
+                        /*targetPackageName=*/packageName,
+                        /*prefixedSchema=*/prefixedSchema,
+                        visibilityStore,
+                        visibilityChecker)) {
+                    continue;  // Observer can't have this notification.
+                }
+                if (key == null) {
+                    key = new DocumentChangeGroupKey(
+                            packageName, databaseName, namespace, schemaType);
+                }
+                Set<String> changedDocumentIds = observerInfo.mDocumentChanges.get(key);
+                if (changedDocumentIds == null) {
+                    changedDocumentIds = new ArraySet<>();
+                    observerInfo.mDocumentChanges.put(key, changedDocumentIds);
+                }
+                changedDocumentIds.add(documentId);
             }
             mHasNotifications = true;
         }
@@ -287,12 +349,6 @@
     private static boolean matchesSpec(
             @NonNull String schemaType, @NonNull ObserverSpec observerSpec) {
         Set<String> schemaFilters = observerSpec.getFilterSchemas();
-        if (!schemaFilters.isEmpty() && !schemaFilters.contains(schemaType)) {
-            return false;
-        }
-        // TODO(b/193494000): We also need to check VisibilityStore to see if the observer is
-        //  allowed to access this type before granting access. Note if fixing this TODO makes the
-        //  method non-static we need to handle locking.
-        return true;
+        return schemaFilters.isEmpty() || schemaFilters.contains(schemaType);
     }
 }
diff --git a/appsearch/appsearch-platform-storage/src/main/java/androidx/appsearch/platformstorage/GlobalSearchSessionImpl.java b/appsearch/appsearch-platform-storage/src/main/java/androidx/appsearch/platformstorage/GlobalSearchSessionImpl.java
index ee295e1..22b8afe 100644
--- a/appsearch/appsearch-platform-storage/src/main/java/androidx/appsearch/platformstorage/GlobalSearchSessionImpl.java
+++ b/appsearch/appsearch-platform-storage/src/main/java/androidx/appsearch/platformstorage/GlobalSearchSessionImpl.java
@@ -123,11 +123,11 @@
     @BuildCompat.PrereleaseSdkCheck
     @Override
     public void addObserver(
-            @NonNull String observedPackage,
+            @NonNull String targetPackageName,
             @NonNull ObserverSpec spec,
             @NonNull Executor executor,
             @NonNull AppSearchObserverCallback observer) throws AppSearchException {
-        Preconditions.checkNotNull(observedPackage);
+        Preconditions.checkNotNull(targetPackageName);
         Preconditions.checkNotNull(spec);
         Preconditions.checkNotNull(executor);
         Preconditions.checkNotNull(observer);
@@ -140,7 +140,7 @@
         }
         try {
             mPlatformSession.addObserver(
-                    observedPackage,
+                    targetPackageName,
                     ObserverSpecToPlatformConverter.toPlatformObserverSpec(spec),
                     executor,
                     new android.app.appsearch.observer.AppSearchObserverCallback() {
@@ -171,8 +171,8 @@
 
     @Override
     public void removeObserver(
-            @NonNull String observedPackage, @NonNull AppSearchObserverCallback observer) {
-        Preconditions.checkNotNull(observedPackage);
+            @NonNull String targetPackageName, @NonNull AppSearchObserverCallback observer) {
+        Preconditions.checkNotNull(targetPackageName);
         Preconditions.checkNotNull(observer);
         // TODO(b/193494000): Implement removeObserver
         throw new UnsupportedOperationException("removeObserver not supported for platform yet");
diff --git a/appsearch/appsearch/src/main/java/androidx/appsearch/app/GlobalSearchSession.java b/appsearch/appsearch/src/main/java/androidx/appsearch/app/GlobalSearchSession.java
index d92226e..92b1733 100644
--- a/appsearch/appsearch/src/main/java/androidx/appsearch/app/GlobalSearchSession.java
+++ b/appsearch/appsearch/src/main/java/androidx/appsearch/app/GlobalSearchSession.java
@@ -119,21 +119,21 @@
 
     /**
      * Adds an {@link AppSearchObserverCallback} to monitor changes within the
-     * databases owned by {@code observedPackage} if they match the given
+     * databases owned by {@code targetPackageName} if they match the given
      * {@link androidx.appsearch.observer.ObserverSpec}.
      *
-     * <p>If the data owned by {@code observedPackage} is not visible to you, the registration call
-     * will succeed but no notifications will be dispatched. Notifications could start flowing later
-     * if {@code observedPackage} changes its schema visibility settings.
+     * <p>If the data owned by {@code targetPackageName} is not visible to you, the registration
+     * call will succeed but no notifications will be dispatched. Notifications could start flowing
+     * later if {@code targetPackageName} changes its schema visibility settings.
      *
-     * <p>If no package matching {@code observedPackage} exists on the system, the registration call
-     * will succeed but no notifications will be dispatched. Notifications could start flowing later
-     * if {@code observedPackage} is installed and starts indexing data.
+     * <p>If no package matching {@code targetPackageName} exists on the system, the registration
+     * call will succeed but no notifications will be dispatched. Notifications could start flowing
+     * later if {@code targetPackageName} is installed and starts indexing data.
      *
      * <p>This feature may not be available in all implementations. Check
      * {@link Features#GLOBAL_SEARCH_SESSION_ADD_REMOVE_OBSERVER} before calling this method.
      *
-     * @param observedPackage Package whose changes to monitor
+     * @param targetPackageName Package whose changes to monitor
      * @param spec            Specification of what types of changes to listen for
      * @param executor        Executor on which to call the {@code observer} callback methods.
      * @param observer        Callback to trigger when a schema or document changes
@@ -147,7 +147,7 @@
             name = Features.GLOBAL_SEARCH_SESSION_ADD_REMOVE_OBSERVER)
     // @exportToFramework:endStrip()
     void addObserver(
-            @NonNull String observedPackage,
+            @NonNull String targetPackageName,
             @NonNull ObserverSpec spec,
             @NonNull Executor executor,
             @NonNull AppSearchObserverCallback observer) throws AppSearchException;
@@ -156,7 +156,7 @@
      * Removes previously registered {@link AppSearchObserverCallback} instances from the system.
      *
      * <p>All instances of {@link AppSearchObserverCallback} which are registered to observe
-     * {@code observedPackage} and compare equal to the provided callback using
+     * {@code targetPackageName} and compare equal to the provided callback using
      * {@link AppSearchObserverCallback#equals} will be removed.
      *
      * <p>If no matching observers have been registered, this method has no effect. If multiple
@@ -165,8 +165,8 @@
      * <p>This feature may not be available in all implementations. Check
      * {@link Features#GLOBAL_SEARCH_SESSION_ADD_REMOVE_OBSERVER} before calling this method.
      *
-     * @param observedPackage Package in which the observers to be removed are registered
-     * @param observer        Callback to unregister
+     * @param targetPackageName Package which the observers to be removed are listening to.
+     * @param observer          Callback to unregister.
      * @throws UnsupportedOperationException if this feature is not available on this
      *                                       AppSearch implementation.
      */
@@ -176,7 +176,7 @@
             name = Features.GLOBAL_SEARCH_SESSION_ADD_REMOVE_OBSERVER)
     // @exportToFramework:endStrip()
     void removeObserver(
-            @NonNull String observedPackage, @NonNull AppSearchObserverCallback observer);
+            @NonNull String targetPackageName, @NonNull AppSearchObserverCallback observer);
 
     /** Closes the {@link GlobalSearchSession}. */
     @Override