fix: ensure persistence store still exists when GC runs (#21417)

Fix a bad access crash that happens when a render frame is deleted (window closed) and garbage collection runs afterward.
electron · Dec 6, 2019 · 6156254 · 6156254
1 parent d0e4bd3
commit 6156254
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 6 deletions.
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
@@ -20,7 +20,7 @@ class CachedProxyLifeMonitor final : public ObjectLifeMonitor {
  public:
   static void BindTo(v8::Isolate* isolate,
                      v8::Local<v8::Object> target,
-                     RenderFramePersistenceStore* store,
+                     base::WeakPtr<RenderFramePersistenceStore> store,
                      WeakGlobalPairNode* node,
                      int hash) {
     new CachedProxyLifeMonitor(isolate, target, store, node, hash);
@@ -29,7 +29,7 @@ class CachedProxyLifeMonitor final : public ObjectLifeMonitor {
  protected:
   CachedProxyLifeMonitor(v8::Isolate* isolate,
                          v8::Local<v8::Object> target,
-                         RenderFramePersistenceStore* store,
+                         base::WeakPtr<RenderFramePersistenceStore> store,
                          WeakGlobalPairNode* node,
                          int hash)
       : ObjectLifeMonitor(isolate, target),
@@ -38,6 +38,9 @@ class CachedProxyLifeMonitor final : public ObjectLifeMonitor {
         hash_(hash) {}
 
   void RunDestructor() override {
+    if (!store_)
+      return;
+
     if (node_->detached) {
       delete node_;
       return;
@@ -56,7 +59,7 @@ class CachedProxyLifeMonitor final : public ObjectLifeMonitor {
   }
 
  private:
-  RenderFramePersistenceStore* store_;
+  base::WeakPtr<RenderFramePersistenceStore> store_;
   WeakGlobalPairNode* node_;
   int hash_;
 };
@@ -98,11 +101,11 @@ void RenderFramePersistenceStore::CacheProxiedObject(
     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, this, node,
-                                   hash);
+    CachedProxyLifeMonitor::BindTo(v8::Isolate::GetCurrent(), obj,
+                                   weak_factory_.GetWeakPtr(), node, hash);
     CachedProxyLifeMonitor::BindTo(v8::Isolate::GetCurrent(),
                                    v8::Local<v8::Object>::Cast(proxy_value),
-                                   this, node, hash);
+                                   weak_factory_.GetWeakPtr(), node, hash);
     if (iter == proxy_map_.end()) {
       proxy_map_.emplace(hash, node);
     } else {

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
@@ -60,6 +60,7 @@ class RenderFramePersistenceStore final : public content::RenderFrameObserver {
 
   // object_identity ==> [from_value, proxy_value]
   std::map<int, WeakGlobalPairNode*> proxy_map_;
+  base::WeakPtrFactory<RenderFramePersistenceStore> weak_factory_{this};
 };
 
 }  // namespace context_bridge