From 19cd8f3a022f4592be320c990f9d1e4c0f7dac1c Mon Sep 17 00:00:00 2001
From: loc <andy@slack-corp.com>
Date: Fri, 13 Dec 2019 10:13:04 -0800
Subject: [PATCH] fix: avoid contextBridge crash when RenderFrame address is
 reused (#21501)

* fix: avoid contextBridge crash when RenderFrame address is reused

Co-Authored-By: Jeremy Apthorp <nornagon@nornagon.net>

* make routing_id_ const
---
 shell/renderer/api/atom_api_context_bridge.cc      | 14 +++-----------
 .../render_frame_context_bridge_store.cc           | 10 +++++++++-
 .../render_frame_context_bridge_store.h            |  4 ++++
 3 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/shell/renderer/api/atom_api_context_bridge.cc b/shell/renderer/api/atom_api_context_bridge.cc
index 3a99aa4936be7..7d4fb97385dcf 100644
--- a/shell/renderer/api/atom_api_context_bridge.cc
+++ b/shell/renderer/api/atom_api_context_bridge.cc
@@ -47,20 +47,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
index cf46a69ebd40f..eb826bac75360 100644
--- 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
index 67d72d25ba1d6..0f37573451866 100644
--- 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