Address review comments

electron · Jan 24, 2019 · 7503954 · 7503954
1 parent ef67ca4
commit 7503954
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 13 deletions.
diff --git a/atom/renderer/api/atom_api_web_frame.cc b/atom/renderer/api/atom_api_web_frame.cc
@@ -147,7 +147,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_;
@@ -242,17 +246,15 @@ void WebFrame::SetSpellCheckProvider(mate::Arguments* args,
     return;
   }
 
-  auto client = std::make_unique<SpellCheckClient>(
+  auto spell_check_client = std::make_unique<SpellCheckClient>(
       language, auto_spell_correct_turned_on, 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,9 +26,6 @@ namespace atom {
 
 namespace api {
 
-class SpellCheckClient;
-class AtomWebFrameObserver;
-
 class WebFrame : public mate::Wrappable<WebFrame> {
  public:
   static mate::Handle<WebFrame> Create(v8::Isolate* isolate);
@@ -101,9 +98,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_;
 
   DISALLOW_COPY_AND_ASSIGN(WebFrame);