fix: disable contextBridge object identity caching (#21803)

* fix: disable contextBridge object identity caching

* cleanup

* chore: make non-const references raw pointers

* fix: zero-param constructors are not explicit

* refactor: use base::LinkedList

Co-authored-by: Jeremy Apthorp <[email protected]>

filenames.gni

@@ -559,8 +559,10 @@ filenames = {
     "shell/common/v8_value_converter.cc",
     "shell/common/v8_value_converter.h",
     "shell/common/world_ids.h",
-    "shell/renderer/api/context_bridge/render_frame_context_bridge_store.cc",
-    "shell/renderer/api/context_bridge/render_frame_context_bridge_store.h",
+    "shell/renderer/api/context_bridge/object_cache.cc",
+    "shell/renderer/api/context_bridge/object_cache.h",
+    "shell/renderer/api/context_bridge/render_frame_function_store.cc",
+    "shell/renderer/api/context_bridge/render_frame_function_store.h",
     "shell/renderer/api/electron_api_context_bridge.cc",
     "shell/renderer/api/electron_api_context_bridge.h",
     "shell/renderer/api/electron_api_renderer_ipc.cc",

shell/renderer/api/context_bridge/object_cache.cc

@@ -0,0 +1,65 @@
+// Copyright (c) 2020 Slack Technologies, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#include "shell/renderer/api/context_bridge/object_cache.h"
+
+#include <utility>
+
+#include "shell/common/api/remote/object_life_monitor.h"
+
+namespace electron {
+
+namespace api {
+
+namespace context_bridge {
+
+ObjectCachePairNode::ObjectCachePairNode(ObjectCachePair&& pair) {
+  this->pair = std::move(pair);
+}
+
+ObjectCachePairNode::~ObjectCachePairNode() = default;
+
+ObjectCache::ObjectCache() {}
+ObjectCache::~ObjectCache() = default;
+
+void ObjectCache::CacheProxiedObject(v8::Local<v8::Value> from,
+                                     v8::Local<v8::Value> proxy_value) {
+  if (from->IsObject() && !from->IsNullOrUndefined()) {
+    auto obj = v8::Local<v8::Object>::Cast(from);
+    int hash = obj->GetIdentityHash();
+
+    auto* node = new ObjectCachePairNode(std::make_pair(from, proxy_value));
+    proxy_map_[hash].Append(node);
+  }
+}
+
+v8::MaybeLocal<v8::Value> ObjectCache::GetCachedProxiedObject(
+    v8::Local<v8::Value> from) const {
+  if (!from->IsObject() || from->IsNullOrUndefined())
+    return v8::MaybeLocal<v8::Value>();
+
+  auto obj = v8::Local<v8::Object>::Cast(from);
+  int hash = obj->GetIdentityHash();
+  auto iter = proxy_map_.find(hash);
+  if (iter == proxy_map_.end())
+    return v8::MaybeLocal<v8::Value>();
+
+  auto& list = iter->second;
+  for (auto* node = list.head(); node != list.end(); node = node->next()) {
+    auto& pair = node->value()->pair;
+    auto from_cmp = pair.first;
+    if (from_cmp == from) {
+      if (pair.second.IsEmpty())
+        return v8::MaybeLocal<v8::Value>();
+      return pair.second;
+    }
+  }
+  return v8::MaybeLocal<v8::Value>();
+}
+
+}  // namespace context_bridge
+
+}  // namespace api
+
+}  // namespace electron

shell/renderer/api/context_bridge/object_cache.h

@@ -0,0 +1,53 @@
+// Copyright (c) 2020 Slack Technologies, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#ifndef SHELL_RENDERER_API_CONTEXT_BRIDGE_OBJECT_CACHE_H_
+#define SHELL_RENDERER_API_CONTEXT_BRIDGE_OBJECT_CACHE_H_
+
+#include <map>
+#include <utility>
+
+#include "base/containers/linked_list.h"
+#include "content/public/renderer/render_frame.h"
+#include "content/public/renderer/render_frame_observer.h"
+#include "shell/renderer/electron_render_frame_observer.h"
+#include "third_party/blink/public/web/web_local_frame.h"
+
+namespace electron {
+
+namespace api {
+
+namespace context_bridge {
+
+using ObjectCachePair = std::pair<v8::Local<v8::Value>, v8::Local<v8::Value>>;
+
+struct ObjectCachePairNode : public base::LinkNode<ObjectCachePairNode> {
+  explicit ObjectCachePairNode(ObjectCachePair&& pair);
+  ~ObjectCachePairNode();
+
+  ObjectCachePair pair;
+};
+
+class ObjectCache final {
+ public:
+  ObjectCache();
+  ~ObjectCache();
+
+  void CacheProxiedObject(v8::Local<v8::Value> from,
+                          v8::Local<v8::Value> proxy_value);
+  v8::MaybeLocal<v8::Value> GetCachedProxiedObject(
+      v8::Local<v8::Value> from) const;
+
+ private:
+  // object_identity ==> [from_value, proxy_value]
+  std::map<int, base::LinkedList<ObjectCachePairNode>> proxy_map_;
+};
+
+}  // namespace context_bridge
+
+}  // namespace api
+
+}  // namespace electron
+
+#endif  // SHELL_RENDERER_API_CONTEXT_BRIDGE_OBJECT_CACHE_H_

shell/renderer/api/context_bridge/render_frame_context_bridge_store.cc

@@ -1,155 +0,0 @@
-// Copyright (c) 2019 Slack Technologies, Inc.
-// Use of this source code is governed by the MIT license that can be
-// found in the LICENSE file.
-
-#include "shell/renderer/api/context_bridge/render_frame_context_bridge_store.h"
-
-#include <utility>
-
-#include "shell/common/api/remote/object_life_monitor.h"
-
-namespace electron {
-
-namespace api {
-
-namespace context_bridge {
-
-namespace {
-
-class CachedProxyLifeMonitor final : public ObjectLifeMonitor {
- public:
-  static void BindTo(v8::Isolate* isolate,
-                     v8::Local<v8::Object> target,
-                     base::WeakPtr<RenderFramePersistenceStore> store,
-                     WeakGlobalPairNode* node,
-                     int hash) {
-    new CachedProxyLifeMonitor(isolate, target, store, node, hash);
-  }
-
- protected:
-  CachedProxyLifeMonitor(v8::Isolate* isolate,
-                         v8::Local<v8::Object> target,
-                         base::WeakPtr<RenderFramePersistenceStore> store,
-                         WeakGlobalPairNode* node,
-                         int hash)
-      : ObjectLifeMonitor(isolate, target),
-        store_(store),
-        node_(node),
-        hash_(hash) {}
-
-  void RunDestructor() override {
-    if (!store_)
-      return;
-
-    if (node_->detached) {
-      delete node_;
-      return;
-    }
-    if (node_->prev) {
-      node_->prev->next = node_->next;
-      node_->prev = nullptr;
-    }
-    if (node_->next) {
-      node_->next->prev = node_->prev;
-      node_->next = nullptr;
-    }
-    if (!node_->prev && !node_->next) {
-      // Must be a single length linked list
-      store_->proxy_map()->erase(hash_);
-    }
-    node_->detached = true;
-  }
-
- private:
-  base::WeakPtr<RenderFramePersistenceStore> store_;
-  WeakGlobalPairNode* node_;
-  int hash_;
-};
-
-}  // namespace
-
-std::map<int32_t, RenderFramePersistenceStore*>& GetStoreMap() {
-  static base::NoDestructor<std::map<int32_t, RenderFramePersistenceStore*>>
-      store_map;
-  return *store_map;
-}
-
-WeakGlobalPairNode::WeakGlobalPairNode(WeakGlobalPair pair) {
-  this->pair = std::move(pair);
-}
-
-WeakGlobalPairNode::~WeakGlobalPairNode() {}
-
-RenderFramePersistenceStore::RenderFramePersistenceStore(
-    content::RenderFrame* render_frame)
-    : content::RenderFrameObserver(render_frame),
-      routing_id_(render_frame->GetRoutingID()) {}
-
-RenderFramePersistenceStore::~RenderFramePersistenceStore() = default;
-
-void RenderFramePersistenceStore::OnDestruct() {
-  GetStoreMap().erase(routing_id_);
-  delete this;
-}
-
-void RenderFramePersistenceStore::CacheProxiedObject(
-    v8::Local<v8::Value> from,
-    v8::Local<v8::Value> proxy_value) {
-  if (from->IsObject() && !from->IsNullOrUndefined()) {
-    auto obj = v8::Local<v8::Object>::Cast(from);
-    int hash = obj->GetIdentityHash();
-    auto global_from = v8::Global<v8::Value>(v8::Isolate::GetCurrent(), from);
-    auto global_proxy =
-        v8::Global<v8::Value>(v8::Isolate::GetCurrent(), proxy_value);
-    // Do not retain
-    global_from.SetWeak();
-    global_proxy.SetWeak();
-    auto iter = proxy_map_.find(hash);
-    auto* node = new WeakGlobalPairNode(
-        std::make_tuple(std::move(global_from), std::move(global_proxy)));
-    CachedProxyLifeMonitor::BindTo(v8::Isolate::GetCurrent(), obj,
-                                   weak_factory_.GetWeakPtr(), node, hash);
-    CachedProxyLifeMonitor::BindTo(v8::Isolate::GetCurrent(),
-                                   v8::Local<v8::Object>::Cast(proxy_value),
-                                   weak_factory_.GetWeakPtr(), node, hash);
-    if (iter == proxy_map_.end()) {
-      proxy_map_.emplace(hash, node);
-    } else {
-      WeakGlobalPairNode* target = iter->second;
-      while (target->next) {
-        target = target->next;
-      }
-      target->next = node;
-      node->prev = target;
-    }
-  }
-}
-
-v8::MaybeLocal<v8::Value> RenderFramePersistenceStore::GetCachedProxiedObject(
-    v8::Local<v8::Value> from) {
-  if (!from->IsObject() || from->IsNullOrUndefined())
-    return v8::MaybeLocal<v8::Value>();
-
-  auto obj = v8::Local<v8::Object>::Cast(from);
-  int hash = obj->GetIdentityHash();
-  auto iter = proxy_map_.find(hash);
-  if (iter == proxy_map_.end())
-    return v8::MaybeLocal<v8::Value>();
-  WeakGlobalPairNode* target = iter->second;
-  while (target) {
-    auto from_cmp = std::get<0>(target->pair).Get(v8::Isolate::GetCurrent());
-    if (from_cmp == from) {
-      if (std::get<1>(target->pair).IsEmpty())
-        return v8::MaybeLocal<v8::Value>();
-      return std::get<1>(target->pair).Get(v8::Isolate::GetCurrent());
-    }
-    target = target->next;
-  }
-  return v8::MaybeLocal<v8::Value>();
-}
-
-}  // namespace context_bridge
-
-}  // namespace api
-
-}  // namespace electron

shell/renderer/api/context_bridge/render_frame_context_bridge_store.h

@@ -1,80 +0,0 @@
-// Copyright (c) 2019 Slack Technologies, Inc.
-// Use of this source code is governed by the MIT license that can be
-// found in the LICENSE file.
-
-#ifndef SHELL_RENDERER_API_CONTEXT_BRIDGE_RENDER_FRAME_CONTEXT_BRIDGE_STORE_H_
-#define SHELL_RENDERER_API_CONTEXT_BRIDGE_RENDER_FRAME_CONTEXT_BRIDGE_STORE_H_
-
-#include <map>
-#include <tuple>
-
-#include "content/public/renderer/render_frame.h"
-#include "content/public/renderer/render_frame_observer.h"
-#include "shell/renderer/electron_render_frame_observer.h"
-#include "third_party/blink/public/web/web_local_frame.h"
-
-namespace electron {
-
-namespace api {
-
-namespace context_bridge {
-
-using FunctionContextPair =
-    std::tuple<v8::Global<v8::Function>, v8::Global<v8::Context>>;
-
-using WeakGlobalPair = std::tuple<v8::Global<v8::Value>, v8::Global<v8::Value>>;
-
-struct WeakGlobalPairNode {
-  explicit WeakGlobalPairNode(WeakGlobalPair pair_);
-  ~WeakGlobalPairNode();
-  WeakGlobalPair pair;
-  bool detached = false;
-  struct WeakGlobalPairNode* prev = nullptr;
-  struct WeakGlobalPairNode* next = nullptr;
-};
-
-class RenderFramePersistenceStore final : public content::RenderFrameObserver {
- public:
-  explicit RenderFramePersistenceStore(content::RenderFrame* render_frame);
-  ~RenderFramePersistenceStore() override;
-
-  // RenderFrameObserver implementation.
-  void OnDestruct() override;
-
-  size_t take_func_id() { return next_func_id_++; }
-
-  std::map<size_t, FunctionContextPair>& functions() { return functions_; }
-  std::map<int, WeakGlobalPairNode*>* proxy_map() { return &proxy_map_; }
-
-  void CacheProxiedObject(v8::Local<v8::Value> from,
-                          v8::Local<v8::Value> proxy_value);
-  v8::MaybeLocal<v8::Value> GetCachedProxiedObject(v8::Local<v8::Value> from);
-
-  base::WeakPtr<RenderFramePersistenceStore> GetWeakPtr() {
-    return weak_factory_.GetWeakPtr();
-  }
-
- private:
-  // func_id ==> { function, owning_context }
-  std::map<size_t, FunctionContextPair> functions_;
-  size_t next_func_id_ = 1;
-
-  // proxy maps are weak globals, i.e. these are not retained beyond
-  // there normal JS lifetime.  You must check IsEmpty()
-
-  const int32_t routing_id_;
-
-  // object_identity ==> [from_value, proxy_value]
-  std::map<int, WeakGlobalPairNode*> proxy_map_;
-  base::WeakPtrFactory<RenderFramePersistenceStore> weak_factory_{this};
-};
-
-std::map<int32_t, RenderFramePersistenceStore*>& GetStoreMap();
-
-}  // namespace context_bridge
-
-}  // namespace api
-
-}  // namespace electron
-
-#endif  // SHELL_RENDERER_API_CONTEXT_BRIDGE_RENDER_FRAME_CONTEXT_BRIDGE_STORE_H_

shell/renderer/api/context_bridge/render_frame_function_store.cc

@@ -0,0 +1,39 @@
+// Copyright (c) 2019 Slack Technologies, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#include "shell/renderer/api/context_bridge/render_frame_function_store.h"
+
+#include <utility>
+
+#include "shell/common/api/remote/object_life_monitor.h"
+
+namespace electron {
+
+namespace api {
+
+namespace context_bridge {
+
+std::map<int32_t, RenderFrameFunctionStore*>& GetStoreMap() {
+  static base::NoDestructor<std::map<int32_t, RenderFrameFunctionStore*>>
+      store_map;
+  return *store_map;
+}
+
+RenderFrameFunctionStore::RenderFrameFunctionStore(
+    content::RenderFrame* render_frame)
+    : content::RenderFrameObserver(render_frame),
+      routing_id_(render_frame->GetRoutingID()) {}
+
+RenderFrameFunctionStore::~RenderFrameFunctionStore() = default;
+
+void RenderFrameFunctionStore::OnDestruct() {
+  GetStoreMap().erase(routing_id_);
+  delete this;
+}
+
+}  // namespace context_bridge
+
+}  // namespace api
+
+}  // namespace electron

shell/renderer/api/context_bridge/render_frame_function_store.h

@@ -0,0 +1,59 @@
+// Copyright (c) 2019 Slack Technologies, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#ifndef SHELL_RENDERER_API_CONTEXT_BRIDGE_RENDER_FRAME_FUNCTION_STORE_H_
+#define SHELL_RENDERER_API_CONTEXT_BRIDGE_RENDER_FRAME_FUNCTION_STORE_H_
+
+#include <map>
+#include <tuple>
+
+#include "content/public/renderer/render_frame.h"
+#include "content/public/renderer/render_frame_observer.h"
+#include "shell/renderer/electron_render_frame_observer.h"
+#include "third_party/blink/public/web/web_local_frame.h"
+
+namespace electron {
+
+namespace api {
+
+namespace context_bridge {
+
+using FunctionContextPair =
+    std::tuple<v8::Global<v8::Function>, v8::Global<v8::Context>>;
+
+class RenderFrameFunctionStore final : public content::RenderFrameObserver {
+ public:
+  explicit RenderFrameFunctionStore(content::RenderFrame* render_frame);
+  ~RenderFrameFunctionStore() override;
+
+  // RenderFrameObserver implementation.
+  void OnDestruct() override;
+
+  size_t take_func_id() { return next_func_id_++; }
+
+  std::map<size_t, FunctionContextPair>& functions() { return functions_; }
+
+  base::WeakPtr<RenderFrameFunctionStore> GetWeakPtr() {
+    return weak_factory_.GetWeakPtr();
+  }
+
+ private:
+  // func_id ==> { function, owning_context }
+  std::map<size_t, FunctionContextPair> functions_;
+  size_t next_func_id_ = 1;
+
+  const int32_t routing_id_;
+
+  base::WeakPtrFactory<RenderFrameFunctionStore> weak_factory_{this};
+};
+
+std::map<int32_t, RenderFrameFunctionStore*>& GetStoreMap();
+
+}  // namespace context_bridge
+
+}  // namespace api
+
+}  // namespace electron
+
+#endif  // SHELL_RENDERER_API_CONTEXT_BRIDGE_RENDER_FRAME_FUNCTION_STORE_H_

shell/renderer/api/electron_api_context_bridge.cc

@@ -21,7 +21,7 @@
 #include "shell/common/gin_helper/promise.h"
 #include "shell/common/node_includes.h"
 #include "shell/common/world_ids.h"
-#include "shell/renderer/api/context_bridge/render_frame_context_bridge_store.h"
+#include "shell/renderer/api/context_bridge/render_frame_function_store.h"
 #include "third_party/blink/public/web/web_local_frame.h"
 
 namespace electron {
@@ -47,11 +47,11 @@ content::RenderFrame* GetRenderFrame(const v8::Local<v8::Object>& value) {
   return content::RenderFrame::FromWebFrame(frame);
 }
 
-context_bridge::RenderFramePersistenceStore* GetOrCreateStore(
+context_bridge::RenderFrameFunctionStore* GetOrCreateStore(
     content::RenderFrame* render_frame) {
   auto it = context_bridge::GetStoreMap().find(render_frame->GetRoutingID());
   if (it == context_bridge::GetStoreMap().end()) {
-    auto* store = new context_bridge::RenderFramePersistenceStore(render_frame);
+    auto* store = new context_bridge::RenderFrameFunctionStore(render_frame);
     context_bridge::GetStoreMap().emplace(render_frame->GetRoutingID(), store);
     return store;
   }
@@ -113,7 +113,7 @@ class FunctionLifeMonitor final : public ObjectLifeMonitor {
   static void BindTo(
       v8::Isolate* isolate,
       v8::Local<v8::Object> target,
-      base::WeakPtr<context_bridge::RenderFramePersistenceStore> store,
+      base::WeakPtr<context_bridge::RenderFrameFunctionStore> store,
       size_t func_id) {
     new FunctionLifeMonitor(isolate, target, store, func_id);
   }
@@ -122,7 +122,7 @@ class FunctionLifeMonitor final : public ObjectLifeMonitor {
   FunctionLifeMonitor(
       v8::Isolate* isolate,
       v8::Local<v8::Object> target,
-      base::WeakPtr<context_bridge::RenderFramePersistenceStore> store,
+      base::WeakPtr<context_bridge::RenderFrameFunctionStore> store,
       size_t func_id)
       : ObjectLifeMonitor(isolate, target), store_(store), func_id_(func_id) {}
   ~FunctionLifeMonitor() override = default;
@@ -134,7 +134,7 @@ class FunctionLifeMonitor final : public ObjectLifeMonitor {
   }
 
  private:
-  base::WeakPtr<context_bridge::RenderFramePersistenceStore> store_;
+  base::WeakPtr<context_bridge::RenderFrameFunctionStore> store_;
   size_t func_id_;
 };
 
@@ -144,7 +144,8 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
     v8::Local<v8::Context> source_context,
     v8::Local<v8::Context> destination_context,
     v8::Local<v8::Value> value,
-    context_bridge::RenderFramePersistenceStore* store,
+    context_bridge::RenderFrameFunctionStore* store,
+    context_bridge::ObjectCache* object_cache,
     int recursion_depth) {
   if (recursion_depth >= kMaxRecursion) {
     v8::Context::Scope source_scope(source_context);
@@ -158,7 +159,7 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
     }
   }
   // Check Cache
-  auto cached_value = store->GetCachedProxiedObject(value);
+  auto cached_value = object_cache->GetCachedProxiedObject(value);
   if (!cached_value.IsEmpty()) {
     return cached_value;
   }
@@ -182,7 +183,7 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
       FunctionLifeMonitor::BindTo(destination_context->GetIsolate(),
                                   v8::Local<v8::Object>::Cast(proxy_func),
                                   store->GetWeakPtr(), func_id);
-      store->CacheProxiedObject(value, proxy_func);
+      object_cache->CacheProxiedObject(value, proxy_func);
       return v8::MaybeLocal<v8::Value>(proxy_func);
     }
   }
@@ -202,11 +203,13 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
              v8::Isolate* isolate,
              v8::Global<v8::Context> global_source_context,
              v8::Global<v8::Context> global_destination_context,
-             context_bridge::RenderFramePersistenceStore* store,
+             context_bridge::RenderFrameFunctionStore* store,
              v8::Local<v8::Value> result) {
-            auto val = PassValueToOtherContext(
-                global_source_context.Get(isolate),
-                global_destination_context.Get(isolate), result, store, 0);
+            context_bridge::ObjectCache object_cache;
+            auto val =
+                PassValueToOtherContext(global_source_context.Get(isolate),
+                                        global_destination_context.Get(isolate),
+                                        result, store, &object_cache, 0);
             if (!val.IsEmpty())
               proxied_promise->Resolve(val.ToLocalChecked());
             delete proxied_promise;
@@ -221,11 +224,13 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
              v8::Isolate* isolate,
              v8::Global<v8::Context> global_source_context,
              v8::Global<v8::Context> global_destination_context,
-             context_bridge::RenderFramePersistenceStore* store,
+             context_bridge::RenderFrameFunctionStore* store,
              v8::Local<v8::Value> result) {
-            auto val = PassValueToOtherContext(
-                global_source_context.Get(isolate),
-                global_destination_context.Get(isolate), result, store, 0);
+            context_bridge::ObjectCache object_cache;
+            auto val =
+                PassValueToOtherContext(global_source_context.Get(isolate),
+                                        global_destination_context.Get(isolate),
+                                        result, store, &object_cache, 0);
             if (!val.IsEmpty())
               proxied_promise->Reject(val.ToLocalChecked());
             delete proxied_promise;
@@ -243,7 +248,7 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
           v8::Local<v8::Function>::Cast(
               gin::ConvertToV8(destination_context->GetIsolate(), catch_cb))));
 
-      store->CacheProxiedObject(value, proxied_promise_handle);
+      object_cache->CacheProxiedObject(value, proxied_promise_handle);
       return v8::MaybeLocal<v8::Value>(proxied_promise_handle);
     }
   }
@@ -270,7 +275,7 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
       for (size_t i = 0; i < length; i++) {
         auto value_for_array = PassValueToOtherContext(
             source_context, destination_context,
-            arr->Get(source_context, i).ToLocalChecked(), store,
+            arr->Get(source_context, i).ToLocalChecked(), store, object_cache,
             recursion_depth + 1);
         if (value_for_array.IsEmpty())
           return v8::MaybeLocal<v8::Value>();
@@ -280,7 +285,7 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
           return v8::MaybeLocal<v8::Value>();
         }
       }
-      store->CacheProxiedObject(value, cloned_arr);
+      object_cache->CacheProxiedObject(value, cloned_arr);
       return v8::MaybeLocal<v8::Value>(cloned_arr);
     }
   }
@@ -290,7 +295,7 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
     auto object_value = v8::Local<v8::Object>::Cast(value);
     auto passed_value =
         CreateProxyForAPI(object_value, source_context, destination_context,
-                          store, recursion_depth + 1);
+                          store, object_cache, recursion_depth + 1);
     if (passed_value.IsEmpty())
       return v8::MaybeLocal<v8::Value>();
     return v8::MaybeLocal<v8::Value>(passed_value.ToLocalChecked());
@@ -311,13 +316,13 @@ v8::MaybeLocal<v8::Value> PassValueToOtherContext(
   {
     v8::Local<v8::Value> cloned_value =
         gin::ConvertToV8(destination_context->GetIsolate(), ret);
-    store->CacheProxiedObject(value, cloned_value);
+    object_cache->CacheProxiedObject(value, cloned_value);
     return v8::MaybeLocal<v8::Value>(cloned_value);
   }
 }
 
 v8::Local<v8::Value> ProxyFunctionWrapper(
-    context_bridge::RenderFramePersistenceStore* store,
+    context_bridge::RenderFrameFunctionStore* store,
     size_t func_id,
     gin_helper::Arguments* args) {
   // Context the proxy function was called from
@@ -327,6 +332,7 @@ v8::Local<v8::Value> ProxyFunctionWrapper(
       std::get<1>(store->functions()[func_id]).Get(args->isolate());
 
   v8::Context::Scope func_owning_context_scope(func_owning_context);
+  context_bridge::ObjectCache object_cache;
   {
     v8::Local<v8::Function> func =
         (std::get<0>(store->functions()[func_id])).Get(args->isolate());
@@ -337,7 +343,7 @@ v8::Local<v8::Value> ProxyFunctionWrapper(
 
     for (auto value : original_args) {
       auto arg = PassValueToOtherContext(calling_context, func_owning_context,
-                                         value, store, 0);
+                                         value, store, &object_cache, 0);
       if (arg.IsEmpty())
         return v8::Undefined(args->isolate());
       proxied_args.push_back(arg.ToLocalChecked());
@@ -375,9 +381,9 @@ v8::Local<v8::Value> ProxyFunctionWrapper(
     if (maybe_return_value.IsEmpty())
       return v8::Undefined(args->isolate());
 
-    auto ret =
-        PassValueToOtherContext(func_owning_context, calling_context,
-                                maybe_return_value.ToLocalChecked(), store, 0);
+    auto ret = PassValueToOtherContext(func_owning_context, calling_context,
+                                       maybe_return_value.ToLocalChecked(),
+                                       store, &object_cache, 0);
     if (ret.IsEmpty())
       return v8::Undefined(args->isolate());
     return ret.ToLocalChecked();
@@ -388,12 +394,13 @@ v8::MaybeLocal<v8::Object> CreateProxyForAPI(
     const v8::Local<v8::Object>& api_object,
     const v8::Local<v8::Context>& source_context,
     const v8::Local<v8::Context>& destination_context,
-    context_bridge::RenderFramePersistenceStore* store,
+    context_bridge::RenderFrameFunctionStore* store,
+    context_bridge::ObjectCache* object_cache,
     int recursion_depth) {
   gin_helper::Dictionary api(source_context->GetIsolate(), api_object);
   gin_helper::Dictionary proxy =
       gin::Dictionary::CreateEmpty(destination_context->GetIsolate());
-  store->CacheProxiedObject(api.GetHandle(), proxy.GetHandle());
+  object_cache->CacheProxiedObject(api.GetHandle(), proxy.GetHandle());
   auto maybe_keys = api.GetHandle()->GetOwnPropertyNames(
       source_context,
       static_cast<v8::PropertyFilter>(v8::ONLY_ENUMERABLE | v8::SKIP_SYMBOLS),
@@ -419,7 +426,7 @@ v8::MaybeLocal<v8::Object> CreateProxyForAPI(
 
       auto passed_value =
           PassValueToOtherContext(source_context, destination_context, value,
-                                  store, recursion_depth + 1);
+                                  store, object_cache, recursion_depth + 1);
       if (passed_value.IsEmpty())
         return v8::MaybeLocal<v8::Object>();
       proxy.Set(key_str, passed_value.ToLocalChecked());
@@ -435,22 +442,6 @@ gin_helper::Dictionary DebugGC(gin_helper::Dictionary empty) {
   auto* store = GetOrCreateStore(render_frame);
   gin_helper::Dictionary ret = gin::Dictionary::CreateEmpty(empty.isolate());
   ret.Set("functionCount", store->functions().size());
-  auto* proxy_map = store->proxy_map();
-  ret.Set("objectCount", proxy_map->size() * 2);
-  int live_from = 0;
-  int live_proxy = 0;
-  for (auto iter = proxy_map->begin(); iter != proxy_map->end(); iter++) {
-    auto* node = iter->second;
-    while (node) {
-      if (!std::get<0>(node->pair).IsEmpty())
-        live_from++;
-      if (!std::get<1>(node->pair).IsEmpty())
-        live_proxy++;
-      node = node->next;
-    }
-  }
-  ret.Set("liveFromValues", live_from);
-  ret.Set("liveProxyValues", live_proxy);
   return ret;
 }
 #endif
@@ -460,7 +451,7 @@ void ExposeAPIInMainWorld(const std::string& key,
                           gin_helper::Arguments* args) {
   auto* render_frame = GetRenderFrame(api_object);
   CHECK(render_frame);
-  context_bridge::RenderFramePersistenceStore* store =
+  context_bridge::RenderFrameFunctionStore* store =
       GetOrCreateStore(render_frame);
   auto* frame = render_frame->GetWebFrame();
   CHECK(frame);
@@ -478,10 +469,11 @@ void ExposeAPIInMainWorld(const std::string& key,
   v8::Local<v8::Context> isolated_context =
       frame->WorldScriptContext(args->isolate(), WorldIDs::ISOLATED_WORLD_ID);
 
+  context_bridge::ObjectCache object_cache;
   v8::Context::Scope main_context_scope(main_context);
   {
-    v8::MaybeLocal<v8::Object> maybe_proxy =
-        CreateProxyForAPI(api_object, isolated_context, main_context, store, 0);
+    v8::MaybeLocal<v8::Object> maybe_proxy = CreateProxyForAPI(
+        api_object, isolated_context, main_context, store, &object_cache, 0);
     if (maybe_proxy.IsEmpty())
       return;
     auto proxy = maybe_proxy.ToLocalChecked();

shell/renderer/api/electron_api_context_bridge.h

@@ -5,6 +5,7 @@
 #ifndef SHELL_RENDERER_API_ELECTRON_API_CONTEXT_BRIDGE_H_
 #define SHELL_RENDERER_API_ELECTRON_API_CONTEXT_BRIDGE_H_
 
+#include "shell/renderer/api/context_bridge/object_cache.h"
 #include "v8/include/v8.h"
 
 namespace gin_helper {
@@ -16,11 +17,11 @@ namespace electron {
 namespace api {
 
 namespace context_bridge {
-class RenderFramePersistenceStore;
+class RenderFrameFunctionStore;
 }
 
 v8::Local<v8::Value> ProxyFunctionWrapper(
-    context_bridge::RenderFramePersistenceStore* store,
+    context_bridge::RenderFrameFunctionStore* store,
     size_t func_id,
     gin_helper::Arguments* args);
 
@@ -28,7 +29,8 @@ v8::MaybeLocal<v8::Object> CreateProxyForAPI(
     const v8::Local<v8::Object>& api_object,
     const v8::Local<v8::Context>& source_context,
     const v8::Local<v8::Context>& target_context,
-    context_bridge::RenderFramePersistenceStore* store,
+    context_bridge::RenderFrameFunctionStore* store,
+    context_bridge::ObjectCache* object_cache,
     int recursion_depth);
 
 }  // namespace api

spec-main/api-context-bridge-spec.ts

@@ -320,43 +320,6 @@ describe('contextBridge', () => {
         expect(result).to.deep.equal([135, 135, 135])
       })
 
-      it('it should follow expected simple rules of object identity', async () => {
-        await makeBindingWindow(() => {
-          const o: any = { value: 135 }
-          const sub = { thing: 7 }
-          o.a = sub
-          o.b = sub
-          contextBridge.exposeInMainWorld('example', {
-            o
-          })
-        })
-        const result = await callWithBindings((root: any) => {
-          return root.example.a === root.example.b
-        })
-        expect(result).to.equal(true)
-      })
-
-      it('it should follow expected complex rules of object identity', async () => {
-        await makeBindingWindow(() => {
-          let first: any = null
-          contextBridge.exposeInMainWorld('example', {
-            check: (arg: any) => {
-              if (first === null) {
-                first = arg
-              } else {
-                return first === arg
-              }
-            }
-          })
-        })
-        const result = await callWithBindings((root: any) => {
-          const o = { thing: 123 }
-          root.example.check(o)
-          return root.example.check(o)
-        })
-        expect(result).to.equal(true)
-      })
-
       // Can only run tests which use the GCRunner in non-sandboxed environments
       if (!useSandbox) {
         it('should release the global hold on methods sent across contexts', async () => {
@@ -377,133 +340,6 @@ describe('contextBridge', () => {
           })
           expect((await getGCInfo()).functionCount).to.equal(2)
         })
-
-        it('should release the global hold on objects sent across contexts when the object proxy is de-reffed', async () => {
-          await makeBindingWindow(() => {
-            require('electron').ipcRenderer.on('get-gc-info', e => e.sender.send('gc-info', (contextBridge as any).debugGC()))
-            let myObj: any
-            contextBridge.exposeInMainWorld('example', {
-              setObj: (o: any) => {
-                myObj = o
-              },
-              getObj: () => myObj
-            })
-          })
-          await callWithBindings(async (root: any) => {
-            root.GCRunner.run()
-          })
-          // Initial Setup
-          let info = await getGCInfo()
-          expect(info.liveFromValues).to.equal(3)
-          expect(info.liveProxyValues).to.equal(3)
-          expect(info.objectCount).to.equal(6)
-
-          // Create Reference
-          await callWithBindings(async (root: any) => {
-            root.x = { value: 123 }
-            root.example.setObj(root.x)
-            root.GCRunner.run()
-          })
-          info = await getGCInfo()
-          expect(info.liveFromValues).to.equal(4)
-          expect(info.liveProxyValues).to.equal(4)
-          expect(info.objectCount).to.equal(8)
-
-          // Release Reference
-          await callWithBindings(async (root: any) => {
-            root.example.setObj(null)
-            root.GCRunner.run()
-          })
-          info = await getGCInfo()
-          expect(info.liveFromValues).to.equal(3)
-          expect(info.liveProxyValues).to.equal(3)
-          expect(info.objectCount).to.equal(6)
-        })
-
-        it('should release the global hold on objects sent across contexts when the object source is de-reffed', async () => {
-          await makeBindingWindow(() => {
-            require('electron').ipcRenderer.on('get-gc-info', e => e.sender.send('gc-info', (contextBridge as any).debugGC()))
-            let myObj: any
-            contextBridge.exposeInMainWorld('example', {
-              setObj: (o: any) => {
-                myObj = o
-              },
-              getObj: () => myObj
-            })
-          })
-          await callWithBindings(async (root: any) => {
-            root.GCRunner.run()
-          })
-          // Initial Setup
-          let info = await getGCInfo()
-          expect(info.liveFromValues).to.equal(3)
-          expect(info.liveProxyValues).to.equal(3)
-          expect(info.objectCount).to.equal(6)
-
-          // Create Reference
-          await callWithBindings(async (root: any) => {
-            root.x = { value: 123 }
-            root.example.setObj(root.x)
-            root.GCRunner.run()
-          })
-          info = await getGCInfo()
-          expect(info.liveFromValues).to.equal(4)
-          expect(info.liveProxyValues).to.equal(4)
-          expect(info.objectCount).to.equal(8)
-
-          // Release Reference
-          await callWithBindings(async (root: any) => {
-            delete root.x
-            root.GCRunner.run()
-          })
-          info = await getGCInfo()
-          expect(info.liveFromValues).to.equal(3)
-          expect(info.liveProxyValues).to.equal(3)
-          expect(info.objectCount).to.equal(6)
-        })
-
-        it('should not crash when the object source is de-reffed AND the object proxy is de-reffed', async () => {
-          await makeBindingWindow(() => {
-            require('electron').ipcRenderer.on('get-gc-info', e => e.sender.send('gc-info', (contextBridge as any).debugGC()))
-            let myObj: any
-            contextBridge.exposeInMainWorld('example', {
-              setObj: (o: any) => {
-                myObj = o
-              },
-              getObj: () => myObj
-            })
-          })
-          await callWithBindings(async (root: any) => {
-            root.GCRunner.run()
-          })
-          // Initial Setup
-          let info = await getGCInfo()
-          expect(info.liveFromValues).to.equal(3)
-          expect(info.liveProxyValues).to.equal(3)
-          expect(info.objectCount).to.equal(6)
-
-          // Create Reference
-          await callWithBindings(async (root: any) => {
-            root.x = { value: 123 }
-            root.example.setObj(root.x)
-            root.GCRunner.run()
-          })
-          info = await getGCInfo()
-          expect(info.liveFromValues).to.equal(4)
-          expect(info.liveProxyValues).to.equal(4)
-          expect(info.objectCount).to.equal(8)
-
-          // Release Reference
-          await callWithBindings(async (root: any) => {
-            delete root.x
-            root.example.setObj(null)
-            root.GCRunner.run()
-          })
-          info = await getGCInfo()
-          expect(info.liveFromValues).to.equal(3)
-          expect(info.liveProxyValues).to.equal(3)
-          expect(info.objectCount).to.equal(6)
-        })
       }
 
       it('it should not let you overwrite existing exposed things', async () => {