fix: avoid contextBridge crash when RenderFrame address is reused (#2…

…1514) * fix: avoid contextBridge crash when RenderFrame address is reused Co-Authored-By: Jeremy Apthorp <nornagon@nornagon.net> * make routing_id_ const
electron · Dec 13, 2019 · 1e7f26e · 1e7f26e
1 parent 383e757
commit 1e7f26e
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 12 deletions.
diff --git a/shell/renderer/api/atom_api_context_bridge.cc b/shell/renderer/api/atom_api_context_bridge.cc
@@ -34,20 +34,12 @@ content::RenderFrame* GetRenderFrame(const v8::Local<v8::Object>& value) {
   return content::RenderFrame::FromWebFrame(frame);
 }
 
-std::map<content::RenderFrame*, context_bridge::RenderFramePersistenceStore*>&
-GetStoreMap() {
-  static base::NoDestructor<std::map<
-      content::RenderFrame*, context_bridge::RenderFramePersistenceStore*>>
-      store_map;
-  return *store_map;
-}
-
 context_bridge::RenderFramePersistenceStore* GetOrCreateStore(
     content::RenderFrame* render_frame) {
-  auto it = GetStoreMap().find(render_frame);
-  if (it == GetStoreMap().end()) {
+  auto it = context_bridge::GetStoreMap().find(render_frame->GetRoutingID());
+  if (it == context_bridge::GetStoreMap().end()) {
     auto* store = new context_bridge::RenderFramePersistenceStore(render_frame);
-    GetStoreMap().emplace(render_frame, store);
+    context_bridge::GetStoreMap().emplace(render_frame->GetRoutingID(), store);
     return store;
   }
   return it->second;

diff --git a/shell/renderer/api/context_bridge/render_frame_context_bridge_store.cc b/shell/renderer/api/context_bridge/render_frame_context_bridge_store.cc
@@ -66,6 +66,12 @@ class CachedProxyLifeMonitor final : public ObjectLifeMonitor {
 
 }  // 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);
 }
@@ -78,11 +84,13 @@ WeakGlobalPairNode::~WeakGlobalPairNode() {
 
 RenderFramePersistenceStore::RenderFramePersistenceStore(
     content::RenderFrame* render_frame)
-    : content::RenderFrameObserver(render_frame) {}
+    : content::RenderFrameObserver(render_frame),
+      routing_id_(render_frame->GetRoutingID()) {}
 
 RenderFramePersistenceStore::~RenderFramePersistenceStore() = default;
 
 void RenderFramePersistenceStore::OnDestruct() {
+  GetStoreMap().erase(routing_id_);
   delete this;
 }
 

diff --git a/shell/renderer/api/context_bridge/render_frame_context_bridge_store.h b/shell/renderer/api/context_bridge/render_frame_context_bridge_store.h
@@ -58,11 +58,15 @@ class RenderFramePersistenceStore final : public content::RenderFrameObserver {
   // 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