[remoteobjects] Implement RemoteObjectInjector#removeInterface

This CL implements RemoteObjectInjector#removeInterface which
should call the RemoveNamedObject method on a RemoteObjectGateway
remote.

This CL changes to removing the unnamed object on only the browser
side to secure that the object is injected again on the renderer.
It is consistent with the old code, and  fixes
JavaBridgeBasicsTest#testRemovalNotReflectedUntilReload.

Bug: 1105925
Change-Id: I7a304dba05f4e09b190658f684c13632497481d0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2379270Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Oksana Zhuravlova <oksamyt@chromium.org>
Reviewed-by: Shimi Zhang <ctzsm@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Commit-Queue: Miyoung Shin <myid.shin@igalia.com>
Cr-Commit-Position: refs/heads/master@{#816520}

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

@@ -106,10 +106,15 @@ public class JavascriptInjectorImpl implements JavascriptInjector, UserData {
 
     @Override
     public void removeInterface(String name) {
-        mInjectedObjects.remove(name);
-        if (mNativePtr != 0) {
-            JavascriptInjectorImplJni.get().removeInterface(
-                    mNativePtr, JavascriptInjectorImpl.this, name);
+        assert mUseMojo != null;
+        if (mUseMojo) {
+            mInjector.removeInterface(name);
+        } else {
+            mInjectedObjects.remove(name);
+            if (mNativePtr != 0) {
+                JavascriptInjectorImplJni.get().removeInterface(
+                        mNativePtr, JavascriptInjectorImpl.this, name);
+            }
         }
     }
 

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

@@ -58,10 +58,6 @@ class RemoteObjectHostImpl implements RemoteObjectHost {
         mAllowInspection = allowInspection;
     }
 
-    public RemoteObjectRegistry getRegistry() {
-        return mRegistry.get();
-    }
-
     @Override
     public void getObject(int objectId, InterfaceRequest<RemoteObject> request) {
         try (InterfaceRequest<RemoteObject> autoClose = request) {

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

@@ -32,10 +32,13 @@ public final class RemoteObjectInjector extends WebContentsObserver {
     private static class RemoteObjectGatewayHelper {
         public RemoteObjectGateway.Proxy gateway;
         public RemoteObjectHostImpl host;
-        public RemoteObjectGatewayHelper(
-                RemoteObjectGateway.Proxy newGateway, RemoteObjectHostImpl newHost) {
+        public RemoteObjectRegistry registry;
+
+        public RemoteObjectGatewayHelper(RemoteObjectGateway.Proxy newGateway,
+                RemoteObjectHostImpl newHost, RemoteObjectRegistry newRegistry) {
             gateway = newGateway;
             host = newHost;
+            registry = newRegistry;
         }
     }
 
@@ -72,6 +75,16 @@ public final class RemoteObjectInjector extends WebContentsObserver {
         WebContents webContents = mWebContents.get();
         if (webContents == null) return;
 
+        Pair<Object, Class<? extends Annotation>> value = mInjectedObjects.get(name);
+
+        // Nothing to do if the named object already exists.
+        if (value != null && value.first == object) return;
+
+        if (value != null) {
+            // Remove existing name for replacement.
+            removeInterface(name);
+        }
+
         // TODO(crbug.com/1105935): find a way to make requiredAnnotation available to
         // mRemoteObjectHost when JavascriptInjectorImpl calls addInterface().
         Class<? extends Annotation> requiredAnnotation = null;
@@ -82,6 +95,18 @@ public final class RemoteObjectInjector extends WebContentsObserver {
         addInterfaceForFrame(webContents.getMainFrame(), name, object, requiredAnnotation);
     }
 
+    public void removeInterface(String name) {
+        WebContents webContents = mWebContents.get();
+        if (webContents == null) return;
+
+        Pair<Object, Class<? extends Annotation>> value = mInjectedObjects.remove(name);
+        if (value == null) return;
+
+        // TODO(crbug.com/1105935): the objects need to be removed from all frames, not just the
+        // main one.
+        removeInterfaceForFrame(webContents.getMainFrame(), name, value.first);
+    }
+
     public void setAllowInspection(boolean allow) {
         mAllowInspection = allow;
     }
@@ -90,7 +115,15 @@ public final class RemoteObjectInjector extends WebContentsObserver {
             Class<? extends Annotation> requiredAnnotation) {
         RemoteObjectGatewayHelper helper =
                 getRemoteObjectGatewayHelperForFrame(frameHost, requiredAnnotation);
-        helper.gateway.addNamedObject(name, helper.host.getRegistry().getObjectId(object));
+        helper.gateway.addNamedObject(name, helper.registry.getObjectId(object));
+    }
+
+    private void removeInterfaceForFrame(RenderFrameHost frameHost, String name, Object object) {
+        RemoteObjectGatewayHelper helper = mRemoteObjectGatewayHelpers.get(frameHost);
+        if (helper == null) return;
+
+        helper.gateway.removeNamedObject(name);
+        helper.registry.unrefObjectByObject(object);
     }
 
     private RemoteObjectGatewayHelper getRemoteObjectGatewayHelperForFrame(
@@ -111,7 +144,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));
+                    frameHost, new RemoteObjectGatewayHelper(result.first, host, registry));
         }
 
         return mRemoteObjectGatewayHelpers.get(frameHost);

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

@@ -83,7 +83,14 @@ final class RemoteObjectRegistry implements RemoteObjectImpl.ObjectIdAllocator {
     }
 
     public synchronized void unrefObjectById(int id) {
-        Entry entry = mEntriesById.get(id);
+        unrefObject(mEntriesById.get(id));
+    }
+
+    public synchronized void unrefObjectByObject(Object object) {
+        unrefObject(mEntriesByObject.get(object));
+    }
+
+    private synchronized void unrefObject(Entry entry) {
         if (entry == null) return;
 
         entry.referenceCount--;

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

@@ -173,7 +173,7 @@ public class JavaBridgeBasicsTest {
     @Test
     @SmallTest
     @Feature({"AndroidWebView", "Android-JavaBridge"})
-    @UseMethodParameter(JavaBridgeActivityTestRule.LegacyTestParams.class)
+    @UseMethodParameter(JavaBridgeActivityTestRule.MojoTestParams.class)
     public void testRemovalNotReflectedUntilReload(boolean useMojo) throws Throwable {
         mActivityTestRule.injectObjectAndReload(new Object() {
             public void method() {
@@ -330,7 +330,7 @@ public class JavaBridgeBasicsTest {
     @Test
     @SmallTest
     @Feature({"AndroidWebView", "Android-JavaBridge"})
-    @UseMethodParameter(JavaBridgeActivityTestRule.LegacyTestParams.class)
+    @UseMethodParameter(JavaBridgeActivityTestRule.MojoTestParams.class)
     public void testReplaceInjectedObject(boolean useMojo) throws Throwable {
         mActivityTestRule.injectObjectAndReload(new Object() {
             public void method() {
@@ -361,7 +361,7 @@ public class JavaBridgeBasicsTest {
     @Test
     @SmallTest
     @Feature({"AndroidWebView", "Android-JavaBridge"})
-    @UseMethodParameter(JavaBridgeActivityTestRule.LegacyTestParams.class)
+    @UseMethodParameter(JavaBridgeActivityTestRule.MojoTestParams.class)
     public void testReplaceInjectedObjectWithNullObjectIsIgnored(boolean useMojo) throws Throwable {
         mActivityTestRule.injectObjectAndReload(new Object(), "testObject");
         Assert.assertEquals("object", executeJavaScriptAndGetStringResult("typeof testObject"));
@@ -1039,7 +1039,7 @@ public class JavaBridgeBasicsTest {
     @Test
     @SmallTest
     @Feature({"AndroidWebView", "Android-JavaBridge"})
-    @UseMethodParameter(JavaBridgeActivityTestRule.LegacyTestParams.class)
+    @UseMethodParameter(JavaBridgeActivityTestRule.MojoTestParams.class)
     public void testReplaceJavascriptInterface(boolean useMojo) throws Throwable {
         class Test {
             public Test(int value) {

third_party/blink/renderer/modules/remote_objects/remote_object_gateway_impl.cc

@@ -113,12 +113,6 @@ void RemoteObjectGatewayImpl::BindRemoteObjectReceiver(
 
 void RemoteObjectGatewayImpl::ReleaseObject(int32_t object_id) {
   object_host_->ReleaseObject(object_id);
-  for (const auto& pair : named_objects_) {
-    if (pair.value == object_id) {
-      named_objects_.erase(pair.key);
-      break;
-    }
-  }
 }
 
 // static