fix: correctly destroy spellcheck client (#16525)

* fix: Destroy spellcheck client * Address review comments
electron · Jan 29, 2019 · ae85864 · ae85864
1 parent 5cc0539
commit ae85864
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 9 deletions.
diff --git a/atom/renderer/api/atom_api_web_frame.cc b/atom/renderer/api/atom_api_web_frame.cc
@@ -4,6 +4,8 @@
 
 #include "atom/renderer/api/atom_api_web_frame.h"
 
+#include <utility>
+
 #include "atom/common/api/api_messages.h"
 #include "atom/common/api/event_emitter_caller.h"
 #include "atom/common/native_mate_converters/blink_converter.h"
@@ -137,6 +139,26 @@ class FrameSpellChecker : public content::RenderFrameVisitor {
 
 }  // namespace
 
+class AtomWebFrameObserver : public content::RenderFrameObserver {
+ public:
+  explicit AtomWebFrameObserver(
+      content::RenderFrame* render_frame,
+      std::unique_ptr<SpellCheckClient> spell_check_client)
+      : content::RenderFrameObserver(render_frame),
+        spell_check_client_(std::move(spell_check_client)) {}
+  ~AtomWebFrameObserver() final {}
+
+  // RenderFrameObserver implementation.
+  void OnDestruct() final {
+    spell_check_client_.reset();
+    // Frame observers should delete themselves
+    delete this;
+  }
+
+ private:
+  std::unique_ptr<SpellCheckClient> spell_check_client_;
+};
+
 WebFrame::WebFrame(v8::Isolate* isolate)
     : web_frame_(blink::WebLocalFrame::FrameForCurrentContext()) {
   Init(isolate);
@@ -229,15 +251,15 @@ void WebFrame::SetSpellCheckProvider(mate::Arguments* args,
     return;
   }
 
-  auto client =
+  auto spell_check_client =
       std::make_unique<SpellCheckClient>(language, args->isolate(), provider);
   // Set spellchecker for all live frames in the same process or
   // in the sandbox mode for all live sub frames to this WebFrame.
-  FrameSpellChecker spell_checker(
-      client.get(), content::RenderFrame::FromWebFrame(web_frame_));
+  auto* render_frame = content::RenderFrame::FromWebFrame(web_frame_);
+  FrameSpellChecker spell_checker(spell_check_client.get(), render_frame);
   content::RenderFrame::ForEach(&spell_checker);
-  spell_check_client_.swap(client);
-  web_frame_->SetSpellCheckPanelHostClient(spell_check_client_.get());
+  web_frame_->SetSpellCheckPanelHostClient(spell_check_client.get());
+  new AtomWebFrameObserver(render_frame, std::move(spell_check_client));
 }
 
 void WebFrame::RegisterURLSchemeAsBypassingCSP(const std::string& scheme) {

diff --git a/atom/renderer/api/atom_api_web_frame.h b/atom/renderer/api/atom_api_web_frame.h
@@ -26,8 +26,6 @@ namespace atom {
 
 namespace api {
 
-class SpellCheckClient;
-
 class WebFrame : public mate::Wrappable<WebFrame> {
  public:
   static mate::Handle<WebFrame> Create(v8::Isolate* isolate);
@@ -97,8 +95,6 @@ class WebFrame : public mate::Wrappable<WebFrame> {
   v8::Local<v8::Value> FindFrameByRoutingId(int routing_id) const;
   v8::Local<v8::Value> RoutingId() const;
 
-  std::unique_ptr<SpellCheckClient> spell_check_client_;
-
   blink::WebLocalFrame* web_frame_;
 
   DISALLOW_COPY_AND_ASSIGN(WebFrame);