[remoteobjects] Pass annotation class to RemoteObjectImpl

This CL passes the annotation class from JavascriptInjectorImpl to
RemoteObjectRegistry when the object id of the named object or
transient object is generated, and we get it from
RemoteObjectRegistry when RemoteObjectImpl is created.
The transient object should inherit the annotation class from the
parent's one.

Bug: 1105940
Change-Id: Ica8625113b5d4caf32c3e51a947179d1fb6b4514
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2397974
Commit-Queue: Miyoung Shin <myid.shin@igalia.com>
Reviewed-by: Oksana Zhuravlova <oksamyt@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#816537}

content/public/android/java/src/org/chromium/content/browser/JavascriptInjectorImpl.java

@@ -96,7 +96,7 @@ public class JavascriptInjectorImpl implements JavascriptInjector, UserData {
 
         assert mUseMojo != null;
         if (mUseMojo) {
-            mInjector.addInterface(object, name);
+            mInjector.addInterface(object, name, requiredAnnotation);
         } else if (mNativePtr != 0) {
             mInjectedObjects.put(name, new Pair<Object, Class>(object, requiredAnnotation));
             JavascriptInjectorImplJni.get().addInterface(

content/public/android/java/src/org/chromium/content/browser/remoteobjects/RemoteObjectHostImpl.java

@@ -9,7 +9,6 @@ import org.chromium.blink.mojom.RemoteObjectHost;
 import org.chromium.mojo.bindings.InterfaceRequest;
 import org.chromium.mojo.system.MojoException;
 
-import java.lang.annotation.Annotation;
 import java.lang.ref.WeakReference;
 
 /**
@@ -28,13 +27,6 @@ import java.lang.ref.WeakReference;
  * Internals.
  */
 class RemoteObjectHostImpl implements RemoteObjectHost {
-    /**
-     * Annotation required on all exposed methods.
-     * If null, no annotation is required.
-     * In practice, this is usually {@link android.webkit.JavascriptInterface}.
-     */
-    private final Class<? extends Annotation> mSafeAnnotationClass;
-
     /**
      * Auditor passed on to {@link RemoteObjectImpl}.
      * Should not hold any strong references that may lead to the contents.
@@ -49,10 +41,8 @@ class RemoteObjectHostImpl implements RemoteObjectHost {
 
     private final boolean mAllowInspection;
 
-    RemoteObjectHostImpl(Class<? extends Annotation> safeAnnotationClass,
-            RemoteObjectImpl.Auditor auditor, RemoteObjectRegistry registry,
+    RemoteObjectHostImpl(RemoteObjectImpl.Auditor auditor, RemoteObjectRegistry registry,
             boolean allowInspection) {
-        mSafeAnnotationClass = safeAnnotationClass;
         mAuditor = auditor;
         mRegistry = new WeakReference<>(registry);
         mAllowInspection = allowInspection;
@@ -69,8 +59,8 @@ class RemoteObjectHostImpl implements RemoteObjectHost {
             if (target == null) {
                 return;
             }
-            RemoteObjectImpl impl = new RemoteObjectImpl(
-                    target, mSafeAnnotationClass, mAuditor, registry, mAllowInspection);
+            RemoteObjectImpl impl = new RemoteObjectImpl(target,
+                    registry.getSafeAnnotationClass(target), mAuditor, registry, mAllowInspection);
             RemoteObject.MANAGER.bind(impl, request);
         }
     }

content/public/android/java/src/org/chromium/content/browser/remoteobjects/RemoteObjectImpl.java

@@ -54,7 +54,7 @@ class RemoteObjectImpl implements RemoteObject {
      * These identifiers must not collide.
      */
     interface ObjectIdAllocator {
-        int getObjectId(Object object);
+        int getObjectId(Object object, Class<? extends Annotation> safeAnnotationClass);
         Object getObjectById(int id);
     }
 
@@ -79,6 +79,13 @@ class RemoteObjectImpl implements RemoteObject {
      */
     private final WeakReference<Object> mTarget;
 
+    /**
+     * Annotation required on all exposed methods.
+     * If null, no annotation is required.
+     * In practice, this is usually {@link android.webkit.JavascriptInterface}.
+     */
+    private final Class<? extends Annotation> mSafeAnnotationClass;
+
     /**
      * Allocates IDs for other Java objects.
      *
@@ -105,6 +112,7 @@ class RemoteObjectImpl implements RemoteObject {
     public RemoteObjectImpl(Object target, Class<? extends Annotation> safeAnnotationClass,
             Auditor auditor, ObjectIdAllocator objectIdAllocator, boolean allowInspection) {
         mTarget = new WeakReference<>(target);
+        mSafeAnnotationClass = safeAnnotationClass;
         mAuditor = auditor;
         mObjectIdAllocator = new WeakReference<>(objectIdAllocator);
         mAllowInspection = allowInspection;
@@ -202,8 +210,8 @@ class RemoteObjectImpl implements RemoteObject {
             return;
         }
 
-        RemoteInvocationResult mojoResult =
-                convertResult(result, method.getReturnType(), objectIdAllocator);
+        RemoteInvocationResult mojoResult = convertResult(
+                result, method.getReturnType(), objectIdAllocator, mSafeAnnotationClass);
         callback.call(mojoResult);
     }
 
@@ -383,8 +391,8 @@ class RemoteObjectImpl implements RemoteObject {
         }
     }
 
-    private static RemoteInvocationResult convertResult(
-            Object result, Class<?> returnType, ObjectIdAllocator objectIdAllocator) {
+    private static RemoteInvocationResult convertResult(Object result, Class<?> returnType,
+            ObjectIdAllocator objectIdAllocator, Class<? extends Annotation> safeAnnotationClass) {
         // Methods returning arrays should not be called (for legacy reasons).
         assert !returnType.isArray();
 
@@ -413,7 +421,7 @@ class RemoteObjectImpl implements RemoteObject {
         } else if (result == null) {
             resultValue.setSingletonValue(SingletonJavaScriptValue.NULL);
         } else {
-            int objectId = objectIdAllocator.getObjectId(result);
+            int objectId = objectIdAllocator.getObjectId(result, safeAnnotationClass);
             resultValue.setObjectId(objectId);
         }
         RemoteInvocationResult mojoResult = new RemoteInvocationResult();

content/public/android/java/src/org/chromium/content/browser/remoteobjects/RemoteObjectInjector.java

@@ -31,13 +31,11 @@ public final class RemoteObjectInjector extends WebContentsObserver {
      */
     private static class RemoteObjectGatewayHelper {
         public RemoteObjectGateway.Proxy gateway;
-        public RemoteObjectHostImpl host;
         public RemoteObjectRegistry registry;
 
-        public RemoteObjectGatewayHelper(RemoteObjectGateway.Proxy newGateway,
-                RemoteObjectHostImpl newHost, RemoteObjectRegistry newRegistry) {
+        public RemoteObjectGatewayHelper(
+                RemoteObjectGateway.Proxy newGateway, RemoteObjectRegistry newRegistry) {
             gateway = newGateway;
-            host = newHost;
             registry = newRegistry;
         }
     }
@@ -71,7 +69,8 @@ public final class RemoteObjectInjector extends WebContentsObserver {
         }
     }
 
-    public void addInterface(Object object, String name) {
+    public void addInterface(
+            Object object, String name, Class<? extends Annotation> requiredAnnotation) {
         WebContents webContents = mWebContents.get();
         if (webContents == null) return;
 
@@ -85,9 +84,6 @@ public final class RemoteObjectInjector extends WebContentsObserver {
             removeInterface(name);
         }
 
-        // TODO(crbug.com/1105935): find a way to make requiredAnnotation available to
-        // mRemoteObjectHost when JavascriptInjectorImpl calls addInterface().
-        Class<? extends Annotation> requiredAnnotation = null;
         mInjectedObjects.put(name, new Pair<>(object, requiredAnnotation));
 
         // TODO(crbug.com/1105935): the objects need to be injected into all frames, not just the
@@ -113,9 +109,9 @@ public final class RemoteObjectInjector extends WebContentsObserver {
 
     private void addInterfaceForFrame(RenderFrameHost frameHost, String name, Object object,
             Class<? extends Annotation> requiredAnnotation) {
-        RemoteObjectGatewayHelper helper =
-                getRemoteObjectGatewayHelperForFrame(frameHost, requiredAnnotation);
-        helper.gateway.addNamedObject(name, helper.registry.getObjectId(object));
+        RemoteObjectGatewayHelper helper = getRemoteObjectGatewayHelperForFrame(frameHost);
+        helper.gateway.addNamedObject(
+                name, helper.registry.getObjectId(object, requiredAnnotation));
     }
 
     private void removeInterfaceForFrame(RenderFrameHost frameHost, String name, Object object) {
@@ -127,7 +123,7 @@ public final class RemoteObjectInjector extends WebContentsObserver {
     }
 
     private RemoteObjectGatewayHelper getRemoteObjectGatewayHelperForFrame(
-            RenderFrameHost frameHost, Class<? extends Annotation> requiredAnnotation) {
+            RenderFrameHost frameHost) {
         // Only create one instance of RemoteObjectHostImpl per frame and store it in a map so it is
         // reused in future calls.
         if (!mRemoteObjectGatewayHelpers.containsKey(frameHost)) {
@@ -135,7 +131,7 @@ public final class RemoteObjectInjector extends WebContentsObserver {
 
             // Construct a RemoteObjectHost implementation.
             RemoteObjectHostImpl host = new RemoteObjectHostImpl(
-                    requiredAnnotation, new RemoteObjectAuditorImpl(), registry, mAllowInspection);
+                    new RemoteObjectAuditorImpl(), registry, mAllowInspection);
 
             RemoteObjectGatewayFactory factory = frameHost.getRemoteInterfaces().getInterface(
                     RemoteObjectGatewayFactory.MANAGER);
@@ -144,7 +140,7 @@ public final class RemoteObjectInjector extends WebContentsObserver {
                     RemoteObjectGateway.MANAGER.getInterfaceRequest(CoreImpl.getInstance());
             factory.createRemoteObjectGateway(host, result.second);
             mRemoteObjectGatewayHelpers.put(
-                    frameHost, new RemoteObjectGatewayHelper(result.first, host, registry));
+                    frameHost, new RemoteObjectGatewayHelper(result.first, registry));
         }
 
         return mRemoteObjectGatewayHelpers.get(frameHost);

content/public/android/java/src/org/chromium/content/browser/remoteobjects/RemoteObjectRegistry.java

@@ -6,6 +6,7 @@ package org.chromium.content.browser.remoteobjects;
 
 import android.util.SparseArray;
 
+import java.lang.annotation.Annotation;
 import java.util.IdentityHashMap;
 import java.util.Map;
 import java.util.Set;
@@ -18,11 +19,11 @@ import java.util.Set;
  */
 final class RemoteObjectRegistry implements RemoteObjectImpl.ObjectIdAllocator {
     private final Set<? super RemoteObjectRegistry> mRetainingSet;
-
     private static class Entry {
-        Entry(int id, Object object) {
+        Entry(int id, Object object, Class<? extends Annotation> safeAnnotationClass) {
             this.id = id;
             this.object = object;
+            this.safeAnnotationClass = safeAnnotationClass;
         }
 
         // The ID assigned to the object, which can be used by the renderer
@@ -37,6 +38,10 @@ final class RemoteObjectRegistry implements RemoteObjectImpl.ObjectIdAllocator {
         //   * wrapper objects in the renderer (removed when it closes the pipe)
         //   * names assigned to the object by developer Java code
         public int referenceCount = 1;
+
+        // The annotation class annotated to the object, which can be exposed
+        // to JavaScript code.
+        public Class<? extends Annotation> safeAnnotationClass;
     }
 
     private final SparseArray<Entry> mEntriesById = new SparseArray<>();
@@ -53,8 +58,14 @@ final class RemoteObjectRegistry implements RemoteObjectImpl.ObjectIdAllocator {
         assert removed;
     }
 
+    public synchronized Class<? extends Annotation> getSafeAnnotationClass(Object object) {
+        Entry entry = mEntriesByObject.get(object);
+        return entry != null ? entry.safeAnnotationClass : null;
+    }
+
     @Override
-    public synchronized int getObjectId(Object object) {
+    public synchronized int getObjectId(
+            Object object, Class<? extends Annotation> safeAnnotationClass) {
         Entry entry = mEntriesByObject.get(object);
         if (entry != null) {
             entry.referenceCount++;
@@ -63,7 +74,7 @@ final class RemoteObjectRegistry implements RemoteObjectImpl.ObjectIdAllocator {
 
         int newId = mNextId++;
         assert newId >= 0;
-        entry = new Entry(newId, object);
+        entry = new Entry(newId, object, safeAnnotationClass);
         mEntriesById.put(newId, entry);
         mEntriesByObject.put(object, entry);
         return newId;

content/public/android/javatests/src/org/chromium/content/browser/JavaBridgeBasicsTest.java

@@ -806,7 +806,7 @@ public class JavaBridgeBasicsTest {
     @Test
     @SmallTest
     @Feature({"AndroidWebView", "Android-JavaBridge"})
-    @UseMethodParameter(JavaBridgeActivityTestRule.LegacyTestParams.class)
+    @UseMethodParameter(JavaBridgeActivityTestRule.MojoTestParams.class)
     public void testAllowOnlyAnnotatedMethods(boolean useMojo) throws Throwable {
         mActivityTestRule.injectObjectAndReload(new Object() {
             @JavascriptInterface
@@ -839,7 +839,7 @@ public class JavaBridgeBasicsTest {
     @Test
     @SmallTest
     @Feature({"AndroidWebView", "Android-JavaBridge"})
-    @UseMethodParameter(JavaBridgeActivityTestRule.LegacyTestParams.class)
+    @UseMethodParameter(JavaBridgeActivityTestRule.MojoTestParams.class)
     public void testAnnotationRequirementRetainsPropertyAcrossObjects(boolean useMojo)
             throws Throwable {
         class Test {
@@ -891,7 +891,7 @@ public class JavaBridgeBasicsTest {
     @Test
     @SmallTest
     @Feature({"AndroidWebView", "Android-JavaBridge"})
-    @UseMethodParameter(JavaBridgeActivityTestRule.LegacyTestParams.class)
+    @UseMethodParameter(JavaBridgeActivityTestRule.MojoTestParams.class)
     public void testAnnotationDoesNotGetInherited(boolean useMojo) throws Throwable {
         class Base {
             @JavascriptInterface
@@ -922,7 +922,7 @@ public class JavaBridgeBasicsTest {
     @Test
     @SmallTest
     @Feature({"AndroidWebView", "Android-JavaBridge"})
-    @UseMethodParameter(JavaBridgeActivityTestRule.LegacyTestParams.class)
+    @UseMethodParameter(JavaBridgeActivityTestRule.MojoTestParams.class)
     public void testCustomAnnotationRestriction(boolean useMojo) throws Throwable {
         class Test {
             @TestAnnotation

content/public/android/javatests/src/org/chromium/content/browser/remoteobjects/RemoteObjectHostImplTest.java

@@ -77,7 +77,7 @@ public final class RemoteObjectHostImplTest {
     @SmallTest
     @Feature({"AndroidWebView", "Android-JavaBridge"})
     public void testClosesPipeIfObjectDoesNotExist() {
-        RemoteObjectHostImpl host = new RemoteObjectHostImpl(TestJavascriptInterface.class,
+        RemoteObjectHostImpl host = new RemoteObjectHostImpl(
                 /* auditor */ null, mRegistry, /* allowInspection */ true);
 
         Pair<RemoteObject.Proxy, InterfaceRequest<RemoteObject>> result =
@@ -112,9 +112,9 @@ public final class RemoteObjectHostImplTest {
             @TestJavascriptInterface
             public void frobnicate() {}
         };
-        int id = mRegistry.getObjectId(o);
+        int id = mRegistry.getObjectId(o, TestJavascriptInterface.class);
 
-        RemoteObjectHostImpl host = new RemoteObjectHostImpl(TestJavascriptInterface.class,
+        RemoteObjectHostImpl host = new RemoteObjectHostImpl(
                 /* auditor */ null, mRegistry, /* allowInspection */ true);
 
         Pair<RemoteObject.Proxy, InterfaceRequest<RemoteObject>> result =
@@ -140,9 +140,9 @@ public final class RemoteObjectHostImplTest {
     @Feature({"AndroidWebView", "Android-JavaBridge"})
     public void testRelease() {
         Object o = new Object();
-        int id = mRegistry.getObjectId(o);
+        int id = mRegistry.getObjectId(o, TestJavascriptInterface.class);
 
-        RemoteObjectHostImpl host = new RemoteObjectHostImpl(TestJavascriptInterface.class,
+        RemoteObjectHostImpl host = new RemoteObjectHostImpl(
                 /* auditor */ null, mRegistry, /* allowInspection */ true);
 
         Assert.assertSame(o, mRegistry.getObjectById(id));
@@ -154,7 +154,7 @@ public final class RemoteObjectHostImplTest {
     @SmallTest
     @Feature({"AndroidWebView", "Android-JavaBridge"})
     public void testClose() {
-        RemoteObjectHostImpl host = new RemoteObjectHostImpl(TestJavascriptInterface.class,
+        RemoteObjectHostImpl host = new RemoteObjectHostImpl(
                 /* auditor */ null, mRegistry, /* allowInspection */ true);
         Assert.assertThat(mRegistry, isIn(mRetainingSet));
         host.close();

content/public/android/junit/src/org/chromium/content/browser/remoteobjects/RemoteObjectImplTest.java

@@ -777,7 +777,7 @@ public final class RemoteObjectImplTest {
             }
         };
 
-        when(mIdAllocator.getObjectId(foo)).thenReturn(42);
+        when(mIdAllocator.getObjectId(foo, TestJavascriptInterface.class)).thenReturn(42);
 
         RemoteObject remoteObject = newRemoteObjectImpl(target, TestJavascriptInterface.class);
         RemoteObject.InvokeMethodResponse response = mock(RemoteObject.InvokeMethodResponse.class);

content/public/android/junit/src/org/chromium/content/browser/remoteobjects/RemoteObjectRegistryTest.java

@@ -38,7 +38,7 @@ public final class RemoteObjectRegistryTest {
         Set<RemoteObjectRegistry> retainingSet = new HashSet<>();
         RemoteObjectRegistry registry = new RemoteObjectRegistry(retainingSet);
         Object o = new Object();
-        int id = registry.getObjectId(o);
+        int id = registry.getObjectId(o, null);
         Assert.assertSame(o, registry.getObjectById(id));
     }
 
@@ -48,8 +48,8 @@ public final class RemoteObjectRegistryTest {
         Set<RemoteObjectRegistry> retainingSet = new HashSet<>();
         RemoteObjectRegistry registry = new RemoteObjectRegistry(retainingSet);
         Object o = new Object();
-        int id = registry.getObjectId(o);
-        Assert.assertEquals(id, registry.getObjectId(o));
+        int id = registry.getObjectId(o, null);
+        Assert.assertEquals(id, registry.getObjectId(o, null));
     }
 
     @Test
@@ -58,9 +58,9 @@ public final class RemoteObjectRegistryTest {
         Set<RemoteObjectRegistry> retainingSet = new HashSet<>();
         RemoteObjectRegistry registry = new RemoteObjectRegistry(retainingSet);
         Object o = new Object();
-        int id = registry.getObjectId(o);
+        int id = registry.getObjectId(o, null);
         registry.unrefObjectById(id);
-        int id2 = registry.getObjectId(o);
+        int id2 = registry.getObjectId(o, null);
         Assert.assertSame(o, registry.getObjectById(id2));
     }