Address review comments

electron · Jan 24, 2019 · 0432d62 · 0432d62
1 parent ef7b674
commit 0432d62
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 9 deletions.
diff --git a/atom/renderer/api/atom_api_web_frame.cc b/atom/renderer/api/atom_api_web_frame.cc
@@ -149,7 +149,11 @@ class AtomWebFrameObserver : public content::RenderFrameObserver {
   ~AtomWebFrameObserver() final {}
 
   // RenderFrameObserver implementation.
-  void OnDestruct() final { spell_check_client_.reset(nullptr); }
+  void OnDestruct() final {
+    spell_check_client_.reset();
+    // Frame observers should delete themselves
+    delete this;
+  }
 
  private:
   std::unique_ptr<SpellCheckClient> spell_check_client_;
@@ -245,17 +249,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.
   auto* render_frame = content::RenderFrame::FromWebFrame(web_frame_);
-  FrameSpellChecker spell_checker(client.get(), render_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_observer_.reset(
-      new AtomWebFrameObserver(render_frame, std::move(spell_check_client_)));
+  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,7 +26,6 @@ namespace atom {
 
 namespace api {
 
-class SpellCheckClient;
 class AtomWebFrameObserver;
 
 class WebFrame : public mate::Wrappable<WebFrame> {
@@ -98,7 +97,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_;
   std::unique_ptr<AtomWebFrameObserver> web_frame_observer_;
 
   blink::WebLocalFrame* web_frame_;