[remoteobjects] Call ReleaseObject on wrapper destruction

This change adds a remote call to RemoteObjectHost's ReleaseObject
method when the corresponding RemoteObject renderer instance is
destroyed. Also removes the entry from RemoteObjectGatewayImpl's map.

Bug: 794320
Change-Id: Ib9769ffac5a0cdc8c654164211c163c2ab889c57
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2090922Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Commit-Queue: Oksana Zhuravlova <oksamyt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#760109}

content/browser/frame_host/render_frame_host_impl_browsertest.cc

@@ -34,6 +34,7 @@
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/common/content_client.h"
 #include "content/public/common/content_features.h"
+#include "content/public/common/content_switches.h"
 #include "content/public/common/page_visibility_state.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/content_browser_test.h"
@@ -188,6 +189,11 @@ class RenderFrameHostImplBrowserTest : public ContentBrowserTest {
     host_resolver()->AddRule("*", "127.0.0.1");
     SetupCrossSiteRedirector(embedded_test_server());
     ASSERT_TRUE(embedded_test_server()->Start());
+
+    // TODO(https://crbug.com/794320): Remove this when the new Java Bridge code
+    // is integrated into WebView.
+    base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
+        switches::kJavaScriptFlags, "--expose_gc");
   }
   net::EmbeddedTestServer* https_server() { return &https_server_; }
 
@@ -4135,8 +4141,8 @@ IN_PROC_BROWSER_TEST_F(ContentBrowserTest,
   EXPECT_TRUE(web_contents->IsDocumentOnLoadCompletedInMainFrame());
 }
 
-// TODO(crbug.com/794320): the code below is temporary and will be removed when
-// Java Bridge is mojofied.
+// TODO(https://crbug.com/794320): the code below is temporary and will be
+// removed when Java Bridge is mojofied.
 #if defined(OS_ANDROID)
 
 struct ObjectData {
@@ -4230,8 +4236,8 @@ class MockObjectHost : public blink::mojom::RemoteObjectHost {
     }
   }
 
-  void ReleaseObject(int32_t) override {
-    // TODO(crbug.com/794320): implement this.
+  void ReleaseObject(int32_t object_id) override {
+    release_object_called_[object_id] = true;
   }
 
   mojo::PendingRemote<blink::mojom::RemoteObjectHost> GetRemote() {
@@ -4240,9 +4246,15 @@ class MockObjectHost : public blink::mojom::RemoteObjectHost {
 
   MockObject* GetMockObject() const { return mock_object_.get(); }
 
+  bool release_object_called_for_object(int32_t object_id) const {
+    return release_object_called_.at(object_id);
+  }
+
  private:
   mojo::Receiver<blink::mojom::RemoteObjectHost> receiver_{this};
   std::unique_ptr<MockObject> mock_object_;
+  std::map<int32_t, bool> release_object_called_{{kMainObject.id, false},
+                                                 {kInnerObject.id, false}};
 };
 
 class RemoteObjectInjector : public WebContentsObserver {
@@ -4283,7 +4295,7 @@ void SetupRemoteObjectInvocation(Shell* shell, const GURL& url) {
 }
 }  // namespace
 
-// TODO(crbug.com/794320): Remove this when the new Java Bridge code is
+// TODO(https://crbug.com/794320): Remove this when the new Java Bridge code is
 // integrated into WebView.
 // This test is a temporary way of verifying that the renderer part
 // works as expected.
@@ -4373,5 +4385,31 @@ IN_PROC_BROWSER_TEST_F(RenderFrameHostImplBrowserTest,
   EXPECT_NE(error.find(error_message), std::string::npos);
 }
 
+// Based on testReturnedObjectIsGarbageCollected.
+IN_PROC_BROWSER_TEST_F(RenderFrameHostImplBrowserTest, RemoteObjectRelease) {
+  GURL url(embedded_test_server()->GetURL("/empty.html"));
+
+  WebContents* web_contents = shell()->web_contents();
+  RemoteObjectInjector injector(web_contents);
+  SetupRemoteObjectInvocation(shell(), url);
+
+  EXPECT_EQ(
+      "object",
+      EvalJs(
+          web_contents,
+          "globalInner = testObject.getInnerObject(); typeof globalInner; "));
+
+  EXPECT_FALSE(injector.GetObjectHost().release_object_called_for_object(
+      kInnerObject.id));
+  EXPECT_EQ("object", EvalJs(web_contents, "gc(); typeof globalInner;"));
+  EXPECT_FALSE(injector.GetObjectHost().release_object_called_for_object(
+      kInnerObject.id));
+  EXPECT_EQ(
+      "undefined",
+      EvalJs(web_contents, "delete globalInner; gc(); typeof globalInner;"));
+  EXPECT_TRUE(injector.GetObjectHost().release_object_called_for_object(
+      kInnerObject.id));
+}
+
 #endif  // OS_ANDROID
 }  // namespace content

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

@@ -166,6 +166,14 @@ RemoteObject::RemoteObject(v8::Isolate* isolate,
       gateway_(gateway),
       object_id_(object_id) {}
 
+RemoteObject::~RemoteObject() {
+  // TODO(https://crbug.com/794320): if |this| outlives |gateway_|, the
+  // browser (RemoteObjectImpl.java) needs to handle object ID invalidation when
+  // the RemoteObject mojo pipe is closed.
+  if (gateway_)
+    gateway_->ReleaseObject(object_id_);
+}
+
 gin::ObjectTemplateBuilder RemoteObject::GetObjectTemplateBuilder(
     v8::Isolate* isolate) {
   return gin::Wrappable<RemoteObject>::GetObjectTemplateBuilder(isolate)

third_party/blink/renderer/modules/remote_objects/remote_object.h

@@ -25,7 +25,7 @@ class RemoteObject : public gin::Wrappable<RemoteObject>,
   // Not copyable or movable
   RemoteObject(const RemoteObject&) = delete;
   RemoteObject& operator=(const RemoteObject&) = delete;
-  ~RemoteObject() override = default;
+  ~RemoteObject() override;
 
   // gin::Wrappable.
   gin::ObjectTemplateBuilder GetObjectTemplateBuilder(

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

@@ -101,6 +101,16 @@ void RemoteObjectGatewayImpl::BindRemoteObjectReceiver(
   object_host_->GetObject(object_id, std::move(receiver));
 }
 
+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
 void RemoteObjectGatewayFactoryImpl::Create(
     LocalFrame* frame,

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

@@ -57,6 +57,7 @@ class MODULES_EXPORT RemoteObjectGatewayImpl
   void BindRemoteObjectReceiver(
       int32_t object_id,
       mojo::PendingReceiver<mojom::blink::RemoteObject>);
+  void ReleaseObject(int32_t object_id);
 
  private:
   // mojom::blink::RemoteObjectGateway