fix: remove memory leak when using webFrame and spell checker (#16770)

* fix: do not create native api::WebFrame in webFrame

When reloading a page without restarting renderer process (for example
sandbox mode), the blink::WebFrame is not destroyed, but api::WebFrame
is always recreated for the new page context. This leaves a leak of
api::WebFrame.

* fix: remove spell checker when page context is released

atom/renderer/api/atom_api_web_frame.cc

@@ -2,9 +2,10 @@
 // Use of this source code is governed by the MIT license that can be
 // found in the LICENSE file.
 
-#include "atom/renderer/api/atom_api_web_frame.h"
+#include <memory>
-
+#include <string>
 #include <utility>
+#include <vector>
 
 #include "atom/common/api/api_messages.h"
 #include "atom/common/api/event_emitter_caller.h"
@@ -112,15 +113,15 @@ class ScriptExecutionCallback : public blink::WebScriptExecutionCallback {
   DISALLOW_COPY_AND_ASSIGN(ScriptExecutionCallback);
 };
 
-class FrameSpellChecker : public content::RenderFrameVisitor {
+class FrameSetSpellChecker : public content::RenderFrameVisitor {
  public:
-  explicit FrameSpellChecker(SpellCheckClient* spell_check_client,
+  FrameSetSpellChecker(SpellCheckClient* spell_check_client,
-                             content::RenderFrame* main_frame)
+                       content::RenderFrame* main_frame)
-      : spell_check_client_(spell_check_client), main_frame_(main_frame) {}
+      : spell_check_client_(spell_check_client), main_frame_(main_frame) {
-  ~FrameSpellChecker() override {
+    content::RenderFrame::ForEach(this);
-    spell_check_client_ = nullptr;
+    main_frame->GetWebFrame()->SetSpellCheckPanelHostClient(spell_check_client);
-    main_frame_ = nullptr;
   }
+
   bool Visit(content::RenderFrame* render_frame) override {
     auto* view = render_frame->GetRenderView();
     if (view->GetMainRenderFrame() == main_frame_ ||
@@ -133,95 +134,129 @@ class FrameSpellChecker : public content::RenderFrameVisitor {
  private:
   SpellCheckClient* spell_check_client_;
   content::RenderFrame* main_frame_;
-  DISALLOW_COPY_AND_ASSIGN(FrameSpellChecker);
+
+  DISALLOW_COPY_AND_ASSIGN(FrameSetSpellChecker);
+};
+
+class SpellCheckerHolder : public content::RenderFrameObserver {
+ public:
+  // Find existing holder for the |render_frame|.
+  static SpellCheckerHolder* FromRenderFrame(
+      content::RenderFrame* render_frame) {
+    for (auto* holder : instances_) {
+      if (holder->render_frame() == render_frame)
+        return holder;
+    }
+    return nullptr;
+  }
+
+  SpellCheckerHolder(content::RenderFrame* render_frame,
+                     std::unique_ptr<SpellCheckClient> spell_check_client)
+      : content::RenderFrameObserver(render_frame),
+        spell_check_client_(std::move(spell_check_client)) {
+    DCHECK(!FromRenderFrame(render_frame));
+    instances_.insert(this);
+  }
+
+  ~SpellCheckerHolder() final { instances_.erase(this); }
+
+  void UnsetAndDestroy() {
+    FrameSetSpellChecker set_spell_checker(nullptr, render_frame());
+    delete this;
+  }
+
+  // RenderFrameObserver implementation.
+  void OnDestruct() final {
+    // Since we delete this in WillReleaseScriptContext, this method is unlikely
+    // to be called, but override anyway since I'm not sure if there are some
+    // corner cases.
+    //
+    // Note that while there are two "delete this", it is totally fine as the
+    // observer unsubscribes automatically in destructor and the other one won't
+    // be called.
+    //
+    // Also note that we should not call UnsetAndDestroy here, as the render
+    // frame is going to be destroyed.
+    delete this;
+  }
+
+  void WillReleaseScriptContext(v8::Local<v8::Context> context,
+                                int world_id) final {
+    // Unset spell checker when the script context is going to be released, as
+    // the spell check implementation lives there.
+    UnsetAndDestroy();
+  }
+
+ private:
+  static std::set<SpellCheckerHolder*> instances_;
+
+  std::unique_ptr<SpellCheckClient> spell_check_client_;
 };
 
 }  // namespace
 
-class AtomWebFrameObserver : public content::RenderFrameObserver {
+// static
- public:
+std::set<SpellCheckerHolder*> SpellCheckerHolder::instances_;
-  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 SetName(v8::Local<v8::Value> window, const std::string& name) {
-  void OnDestruct() final {
+  GetRenderFrame(window)->GetWebFrame()->SetName(
-    spell_check_client_.reset();
+      blink::WebString::FromUTF8(name));
-    // 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);
 }
 
-WebFrame::WebFrame(v8::Isolate* isolate, blink::WebLocalFrame* blink_frame)
+double SetZoomLevel(v8::Local<v8::Value> window, double level) {
-    : web_frame_(blink_frame) {
-  Init(isolate);
-}
-
-WebFrame::~WebFrame() {}
-
-void WebFrame::SetName(const std::string& name) {
-  web_frame_->SetName(blink::WebString::FromUTF8(name));
-}
-
-double WebFrame::SetZoomLevel(double level) {
   double result = 0.0;
-  content::RenderFrame* render_frame =
+  content::RenderFrame* render_frame = GetRenderFrame(window);
-      content::RenderFrame::FromWebFrame(web_frame_);
   render_frame->Send(new AtomFrameHostMsg_SetTemporaryZoomLevel(
       render_frame->GetRoutingID(), level, &result));
   return result;
 }
 
-double WebFrame::GetZoomLevel() const {
+double GetZoomLevel(v8::Local<v8::Value> window) {
   double result = 0.0;
-  content::RenderFrame* render_frame =
+  content::RenderFrame* render_frame = GetRenderFrame(window);
-      content::RenderFrame::FromWebFrame(web_frame_);
   render_frame->Send(
       new AtomFrameHostMsg_GetZoomLevel(render_frame->GetRoutingID(), &result));
   return result;
 }
 
-double WebFrame::SetZoomFactor(double factor) {
+double SetZoomFactor(v8::Local<v8::Value> window, double factor) {
   return blink::WebView::ZoomLevelToZoomFactor(
-      SetZoomLevel(blink::WebView::ZoomFactorToZoomLevel(factor)));
+      SetZoomLevel(window, blink::WebView::ZoomFactorToZoomLevel(factor)));
 }
 
-double WebFrame::GetZoomFactor() const {
+double GetZoomFactor(v8::Local<v8::Value> window) {
-  return blink::WebView::ZoomLevelToZoomFactor(GetZoomLevel());
+  return blink::WebView::ZoomLevelToZoomFactor(GetZoomLevel(window));
 }
 
-void WebFrame::SetVisualZoomLevelLimits(double min_level, double max_level) {
+void SetVisualZoomLevelLimits(v8::Local<v8::Value> window,
-  web_frame_->View()->SetDefaultPageScaleLimits(min_level, max_level);
+                              double min_level,
-  web_frame_->View()->SetIgnoreViewportTagScaleLimits(true);
+                              double max_level) {
+  blink::WebFrame* web_frame = GetRenderFrame(window)->GetWebFrame();
+  web_frame->View()->SetDefaultPageScaleLimits(min_level, max_level);
+  web_frame->View()->SetIgnoreViewportTagScaleLimits(true);
 }
 
-void WebFrame::SetLayoutZoomLevelLimits(double min_level, double max_level) {
+void SetLayoutZoomLevelLimits(v8::Local<v8::Value> window,
-  web_frame_->View()->ZoomLimitsChanged(min_level, max_level);
+                              double min_level,
+                              double max_level) {
+  blink::WebFrame* web_frame = GetRenderFrame(window)->GetWebFrame();
+  web_frame->View()->ZoomLimitsChanged(min_level, max_level);
 }
 
-void WebFrame::AllowGuestViewElementDefinition(
+void AllowGuestViewElementDefinition(v8::Isolate* isolate,
-    v8::Local<v8::Object> context,
+                                     v8::Local<v8::Value> window,
-    v8::Local<v8::Function> register_cb) {
+                                     v8::Local<v8::Object> context,
-  v8::HandleScope handle_scope(isolate());
+                                     v8::Local<v8::Function> register_cb) {
+  v8::HandleScope handle_scope(isolate);
   v8::Context::Scope context_scope(context->CreationContext());
   blink::WebCustomElement::EmbedderNamesAllowedScope embedder_names_scope;
-  web_frame_->RequestExecuteV8Function(context->CreationContext(), register_cb,
+  GetRenderFrame(context)->GetWebFrame()->RequestExecuteV8Function(
-                                       v8::Null(isolate()), 0, nullptr,
+      context->CreationContext(), register_cb, v8::Null(isolate), 0, nullptr,
-                                       nullptr);
+      nullptr);
 }
 
-int WebFrame::GetWebFrameId(v8::Local<v8::Value> content_window) {
+int GetWebFrameId(v8::Local<v8::Value> window,
+                  v8::Local<v8::Value> content_window) {
   // Get the WebLocalFrame before (possibly) executing any user-space JS while
   // getting the |params|. We track the status of the RenderFrame via an
   // observer in case it is deleted during user code execution.
@@ -240,9 +275,10 @@ int WebFrame::GetWebFrameId(v8::Local<v8::Value> content_window) {
   return render_frame->GetRoutingID();
 }
 
-void WebFrame::SetSpellCheckProvider(mate::Arguments* args,
+void SetSpellCheckProvider(mate::Arguments* args,
-                                     const std::string& language,
+                           v8::Local<v8::Value> window,
-                                     v8::Local<v8::Object> provider) {
+                           const std::string& language,
+                           v8::Local<v8::Object> provider) {
   auto context = args->isolate()->GetCurrentContext();
   if (!provider->Has(context, mate::StringToV8(args->isolate(), "spellCheck"))
            .ToChecked()) {
@@ -250,44 +286,61 @@ void WebFrame::SetSpellCheckProvider(mate::Arguments* args,
     return;
   }
 
-  auto spell_check_client =
+  // Remove the old client.
-      std::make_unique<SpellCheckClient>(language, args->isolate(), provider);
+  content::RenderFrame* render_frame = GetRenderFrame(window);
+  auto* existing = SpellCheckerHolder::FromRenderFrame(render_frame);
+  if (existing)
+    existing->UnsetAndDestroy();
+
   // 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_);
+  auto spell_check_client =
-  FrameSpellChecker spell_checker(spell_check_client.get(), render_frame);
+      std::make_unique<SpellCheckClient>(language, args->isolate(), provider);
-  content::RenderFrame::ForEach(&spell_checker);
+  FrameSetSpellChecker spell_checker(spell_check_client.get(), render_frame);
-  web_frame_->SetSpellCheckPanelHostClient(spell_check_client.get());
+
-  new AtomWebFrameObserver(render_frame, std::move(spell_check_client));
+  // Attach the spell checker to RenderFrame.
+  new SpellCheckerHolder(render_frame, std::move(spell_check_client));
 }
 
-void WebFrame::InsertText(const std::string& text) {
+void InsertText(v8::Local<v8::Value> window, const std::string& text) {
-  web_frame_->FrameWidget()->GetActiveWebInputMethodController()->CommitText(
+  blink::WebFrame* web_frame = GetRenderFrame(window)->GetWebFrame();
-      blink::WebString::FromUTF8(text),
+  if (web_frame->IsWebLocalFrame()) {
-      blink::WebVector<blink::WebImeTextSpan>(), blink::WebRange(), 0);
+    web_frame->ToWebLocalFrame()
+        ->FrameWidget()
+        ->GetActiveWebInputMethodController()
+        ->CommitText(blink::WebString::FromUTF8(text),
+                     blink::WebVector<blink::WebImeTextSpan>(),
+                     blink::WebRange(), 0);
+  }
 }
 
-void WebFrame::InsertCSS(const std::string& css) {
+void InsertCSS(v8::Local<v8::Value> window, const std::string& css) {
-  web_frame_->GetDocument().InsertStyleSheet(blink::WebString::FromUTF8(css));
+  blink::WebFrame* web_frame = GetRenderFrame(window)->GetWebFrame();
+  if (web_frame->IsWebLocalFrame()) {
+    web_frame->ToWebLocalFrame()->GetDocument().InsertStyleSheet(
+        blink::WebString::FromUTF8(css));
+  }
 }
 
-void WebFrame::ExecuteJavaScript(const base::string16& code,
+void ExecuteJavaScript(mate::Arguments* args,
-                                 mate::Arguments* args) {
+                       v8::Local<v8::Value> window,
+                       const base::string16& code) {
   bool has_user_gesture = false;
   args->GetNext(&has_user_gesture);
   ScriptExecutionCallback::CompletionCallback completion_callback;
   args->GetNext(&completion_callback);
   std::unique_ptr<blink::WebScriptExecutionCallback> callback(
       new ScriptExecutionCallback(completion_callback));
-  web_frame_->RequestExecuteScriptAndReturnValue(
+  GetRenderFrame(window)->GetWebFrame()->RequestExecuteScriptAndReturnValue(
       blink::WebScriptSource(blink::WebString::FromUTF16(code)),
       has_user_gesture, callback.release());
 }
 
-void WebFrame::ExecuteJavaScriptInIsolatedWorld(
+void ExecuteJavaScriptInIsolatedWorld(
+    mate::Arguments* args,
+    v8::Local<v8::Value> window,
     int world_id,
-    const std::vector<mate::Dictionary>& scripts,
+    const std::vector<mate::Dictionary>& scripts) {
-    mate::Arguments* args) {
   std::vector<blink::WebScriptSource> sources;
 
   for (const auto& script : scripts) {
@@ -319,182 +372,132 @@ void WebFrame::ExecuteJavaScriptInIsolatedWorld(
   std::unique_ptr<blink::WebScriptExecutionCallback> callback(
       new ScriptExecutionCallback(completion_callback));
 
-  web_frame_->RequestExecuteScriptInIsolatedWorld(
+  GetRenderFrame(window)->GetWebFrame()->RequestExecuteScriptInIsolatedWorld(
       world_id, &sources.front(), sources.size(), has_user_gesture,
       scriptExecutionType, callback.release());
 }
 
-void WebFrame::SetIsolatedWorldSecurityOrigin(int world_id,
+void SetIsolatedWorldSecurityOrigin(v8::Local<v8::Value> window,
-                                              const std::string& origin_url) {
+                                    int world_id,
-  web_frame_->SetIsolatedWorldSecurityOrigin(
+                                    const std::string& origin_url) {
+  GetRenderFrame(window)->GetWebFrame()->SetIsolatedWorldSecurityOrigin(
       world_id, blink::WebSecurityOrigin::CreateFromString(
                     blink::WebString::FromUTF8(origin_url)));
 }
 
-void WebFrame::SetIsolatedWorldContentSecurityPolicy(
+void SetIsolatedWorldContentSecurityPolicy(v8::Local<v8::Value> window,
-    int world_id,
+                                           int world_id,
-    const std::string& security_policy) {
+                                           const std::string& security_policy) {
-  web_frame_->SetIsolatedWorldContentSecurityPolicy(
+  GetRenderFrame(window)->GetWebFrame()->SetIsolatedWorldContentSecurityPolicy(
       world_id, blink::WebString::FromUTF8(security_policy));
 }
 
-void WebFrame::SetIsolatedWorldHumanReadableName(int world_id,
+void SetIsolatedWorldHumanReadableName(v8::Local<v8::Value> window,
-                                                 const std::string& name) {
+                                       int world_id,
-  web_frame_->SetIsolatedWorldHumanReadableName(
+                                       const std::string& name) {
+  GetRenderFrame(window)->GetWebFrame()->SetIsolatedWorldHumanReadableName(
       world_id, blink::WebString::FromUTF8(name));
 }
 
-// static
+blink::WebCache::ResourceTypeStats GetResourceUsage(v8::Isolate* isolate) {
-mate::Handle<WebFrame> WebFrame::Create(v8::Isolate* isolate) {
-  return mate::CreateHandle(isolate, new WebFrame(isolate));
-}
-
-blink::WebCache::ResourceTypeStats WebFrame::GetResourceUsage(
-    v8::Isolate* isolate) {
   blink::WebCache::ResourceTypeStats stats;
   blink::WebCache::GetResourceTypeStats(&stats);
   return stats;
 }
 
-void WebFrame::ClearCache(v8::Isolate* isolate) {
+void ClearCache(v8::Isolate* isolate) {
   isolate->IdleNotificationDeadline(0.5);
   blink::WebCache::Clear();
   base::MemoryPressureListener::NotifyMemoryPressure(
       base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL);
 }
 
-v8::Local<v8::Value> WebFrame::Opener() const {
+v8::Local<v8::Value> FindFrameByRoutingId(v8::Isolate* isolate,
-  blink::WebFrame* frame = web_frame_->Opener();
+                                          v8::Local<v8::Value> window,
-  if (frame && frame->IsWebLocalFrame())
+                                          int routing_id) {
-    return mate::CreateHandle(isolate(),
-                              new WebFrame(isolate(), frame->ToWebLocalFrame()))
-        .ToV8();
-  else
-    return v8::Null(isolate());
-}
-
-v8::Local<v8::Value> WebFrame::Parent() const {
-  blink::WebFrame* frame = web_frame_->Parent();
-  if (frame && frame->IsWebLocalFrame())
-    return mate::CreateHandle(isolate(),
-                              new WebFrame(isolate(), frame->ToWebLocalFrame()))
-        .ToV8();
-  else
-    return v8::Null(isolate());
-}
-
-v8::Local<v8::Value> WebFrame::Top() const {
-  blink::WebFrame* frame = web_frame_->Top();
-  if (frame && frame->IsWebLocalFrame())
-    return mate::CreateHandle(isolate(),
-                              new WebFrame(isolate(), frame->ToWebLocalFrame()))
-        .ToV8();
-  else
-    return v8::Null(isolate());
-}
-
-v8::Local<v8::Value> WebFrame::FirstChild() const {
-  blink::WebFrame* frame = web_frame_->FirstChild();
-  if (frame && frame->IsWebLocalFrame())
-    return mate::CreateHandle(isolate(),
-                              new WebFrame(isolate(), frame->ToWebLocalFrame()))
-        .ToV8();
-  else
-    return v8::Null(isolate());
-}
-
-v8::Local<v8::Value> WebFrame::NextSibling() const {
-  blink::WebFrame* frame = web_frame_->NextSibling();
-  if (frame && frame->IsWebLocalFrame())
-    return mate::CreateHandle(isolate(),
-                              new WebFrame(isolate(), frame->ToWebLocalFrame()))
-        .ToV8();
-  else
-    return v8::Null(isolate());
-}
-
-v8::Local<v8::Value> WebFrame::GetFrameForSelector(
-    const std::string& selector) const {
-  blink::WebElement element = web_frame_->GetDocument().QuerySelector(
-      blink::WebString::FromUTF8(selector));
-  blink::WebLocalFrame* element_frame =
-      blink::WebLocalFrame::FromFrameOwnerElement(element);
-  if (element_frame)
-    return mate::CreateHandle(isolate(), new WebFrame(isolate(), element_frame))
-        .ToV8();
-  else
-    return v8::Null(isolate());
-}
-
-v8::Local<v8::Value> WebFrame::FindFrameByName(const std::string& name) const {
-  blink::WebLocalFrame* local_frame =
-      web_frame_->FindFrameByName(blink::WebString::FromUTF8(name))
-          ->ToWebLocalFrame();
-  if (local_frame)
-    return mate::CreateHandle(isolate(), new WebFrame(isolate(), local_frame))
-        .ToV8();
-  else
-    return v8::Null(isolate());
-}
-
-v8::Local<v8::Value> WebFrame::FindFrameByRoutingId(int routing_id) const {
   content::RenderFrame* render_frame =
       content::RenderFrame::FromRoutingID(routing_id);
-  blink::WebLocalFrame* local_frame = nullptr;
   if (render_frame)
-    local_frame = render_frame->GetWebFrame();
+    return render_frame->GetWebFrame()->MainWorldScriptContext()->Global();
-  if (local_frame)
-    return mate::CreateHandle(isolate(), new WebFrame(isolate(), local_frame))
-        .ToV8();
   else
-    return v8::Null(isolate());
+    return v8::Null(isolate);
 }
 
-v8::Local<v8::Value> WebFrame::RoutingId() const {
+v8::Local<v8::Value> GetOpener(v8::Isolate* isolate,
-  int routing_id = content::RenderFrame::GetRoutingIdForWebFrame(web_frame_);
+                               v8::Local<v8::Value> window) {
-  return v8::Number::New(isolate(), routing_id);
+  blink::WebFrame* frame = GetRenderFrame(window)->GetWebFrame()->Opener();
+  if (frame && frame->IsWebLocalFrame())
+    return frame->ToWebLocalFrame()->MainWorldScriptContext()->Global();
+  else
+    return v8::Null(isolate);
 }
 
-// static
+// Don't name it as GetParent, Windows has API with same name.
-void WebFrame::BuildPrototype(v8::Isolate* isolate,
+v8::Local<v8::Value> GetFrameParent(v8::Isolate* isolate,
-                              v8::Local<v8::FunctionTemplate> prototype) {
+                                    v8::Local<v8::Value> window) {
-  prototype->SetClassName(mate::StringToV8(isolate, "WebFrame"));
+  blink::WebFrame* frame = GetRenderFrame(window)->GetWebFrame()->Parent();
-  mate::ObjectTemplateBuilder(isolate, prototype->PrototypeTemplate())
+  if (frame && frame->IsWebLocalFrame())
-      .SetMethod("setName", &WebFrame::SetName)
+    return frame->ToWebLocalFrame()->MainWorldScriptContext()->Global();
-      .SetMethod("setZoomLevel", &WebFrame::SetZoomLevel)
+  else
-      .SetMethod("getZoomLevel", &WebFrame::GetZoomLevel)
+    return v8::Null(isolate);
-      .SetMethod("setZoomFactor", &WebFrame::SetZoomFactor)
+}
-      .SetMethod("getZoomFactor", &WebFrame::GetZoomFactor)
+
-      .SetMethod("setVisualZoomLevelLimits",
+v8::Local<v8::Value> GetTop(v8::Isolate* isolate, v8::Local<v8::Value> window) {
-                 &WebFrame::SetVisualZoomLevelLimits)
+  blink::WebFrame* frame = GetRenderFrame(window)->GetWebFrame()->Top();
-      .SetMethod("setLayoutZoomLevelLimits",
+  if (frame && frame->IsWebLocalFrame())
-                 &WebFrame::SetLayoutZoomLevelLimits)
+    return frame->ToWebLocalFrame()->MainWorldScriptContext()->Global();
-      .SetMethod("allowGuestViewElementDefinition",
+  else
-                 &WebFrame::AllowGuestViewElementDefinition)
+    return v8::Null(isolate);
-      .SetMethod("getWebFrameId", &WebFrame::GetWebFrameId)
+}
-      .SetMethod("setSpellCheckProvider", &WebFrame::SetSpellCheckProvider)
+
-      .SetMethod("insertText", &WebFrame::InsertText)
+v8::Local<v8::Value> GetFirstChild(v8::Isolate* isolate,
-      .SetMethod("insertCSS", &WebFrame::InsertCSS)
+                                   v8::Local<v8::Value> window) {
-      .SetMethod("executeJavaScript", &WebFrame::ExecuteJavaScript)
+  blink::WebFrame* frame = GetRenderFrame(window)->GetWebFrame()->FirstChild();
-      .SetMethod("executeJavaScriptInIsolatedWorld",
+  if (frame && frame->IsWebLocalFrame())
-                 &WebFrame::ExecuteJavaScriptInIsolatedWorld)
+    return frame->ToWebLocalFrame()->MainWorldScriptContext()->Global();
-      .SetMethod("setIsolatedWorldSecurityOrigin",
+  else
-                 &WebFrame::SetIsolatedWorldSecurityOrigin)
+    return v8::Null(isolate);
-      .SetMethod("setIsolatedWorldContentSecurityPolicy",
+}
-                 &WebFrame::SetIsolatedWorldContentSecurityPolicy)
+
-      .SetMethod("setIsolatedWorldHumanReadableName",
+v8::Local<v8::Value> GetNextSibling(v8::Isolate* isolate,
-                 &WebFrame::SetIsolatedWorldHumanReadableName)
+                                    v8::Local<v8::Value> window) {
-      .SetMethod("getResourceUsage", &WebFrame::GetResourceUsage)
+  blink::WebFrame* frame = GetRenderFrame(window)->GetWebFrame()->NextSibling();
-      .SetMethod("clearCache", &WebFrame::ClearCache)
+  if (frame && frame->IsWebLocalFrame())
-      .SetMethod("getFrameForSelector", &WebFrame::GetFrameForSelector)
+    return frame->ToWebLocalFrame()->MainWorldScriptContext()->Global();
-      .SetMethod("findFrameByName", &WebFrame::FindFrameByName)
+  else
-      .SetProperty("opener", &WebFrame::Opener)
+    return v8::Null(isolate);
-      .SetProperty("parent", &WebFrame::Parent)
+}
-      .SetProperty("top", &WebFrame::Top)
+
-      .SetProperty("firstChild", &WebFrame::FirstChild)
+v8::Local<v8::Value> GetFrameForSelector(v8::Isolate* isolate,
-      .SetProperty("nextSibling", &WebFrame::NextSibling)
+                                         v8::Local<v8::Value> window,
-      .SetProperty("routingId", &WebFrame::RoutingId)
+                                         const std::string& selector) {
-      .SetMethod("findFrameByRoutingId", &WebFrame::FindFrameByRoutingId);
+  blink::WebElement element =
+      GetRenderFrame(window)->GetWebFrame()->GetDocument().QuerySelector(
+          blink::WebString::FromUTF8(selector));
+  if (element.IsNull())  // not found
+    return v8::Null(isolate);
+
+  blink::WebFrame* frame = blink::WebFrame::FromFrameOwnerElement(element);
+  if (frame && frame->IsWebLocalFrame())
+    return frame->ToWebLocalFrame()->MainWorldScriptContext()->Global();
+  else
+    return v8::Null(isolate);
+}
+
+v8::Local<v8::Value> FindFrameByName(v8::Isolate* isolate,
+                                     v8::Local<v8::Value> window,
+                                     const std::string& name) {
+  blink::WebFrame* frame =
+      GetRenderFrame(window)->GetWebFrame()->FindFrameByName(
+          blink::WebString::FromUTF8(name));
+  if (frame && frame->IsWebLocalFrame())
+    return frame->ToWebLocalFrame()->MainWorldScriptContext()->Global();
+  else
+    return v8::Null(isolate);
+}
+
+int GetRoutingId(v8::Local<v8::Value> window) {
+  return GetRenderFrame(window)->GetRoutingID();
 }
 
 }  // namespace api
@@ -503,18 +506,47 @@ void WebFrame::BuildPrototype(v8::Isolate* isolate,
 
 namespace {
 
-using atom::api::WebFrame;
-
 void Initialize(v8::Local<v8::Object> exports,
                 v8::Local<v8::Value> unused,
                 v8::Local<v8::Context> context,
                 void* priv) {
+  using namespace atom::api;  // NOLINT(build/namespaces)
+
   v8::Isolate* isolate = context->GetIsolate();
   mate::Dictionary dict(isolate, exports);
-  dict.Set("webFrame", WebFrame::Create(isolate));
+  dict.SetMethod("setName", &SetName);
-  dict.Set(
+  dict.SetMethod("setZoomLevel", &SetZoomLevel);
-      "WebFrame",
+  dict.SetMethod("getZoomLevel", &GetZoomLevel);
-      WebFrame::GetConstructor(isolate)->GetFunction(context).ToLocalChecked());
+  dict.SetMethod("setZoomFactor", &SetZoomFactor);
+  dict.SetMethod("getZoomFactor", &GetZoomFactor);
+  dict.SetMethod("setVisualZoomLevelLimits", &SetVisualZoomLevelLimits);
+  dict.SetMethod("setLayoutZoomLevelLimits", &SetLayoutZoomLevelLimits);
+  dict.SetMethod("allowGuestViewElementDefinition",
+                 &AllowGuestViewElementDefinition);
+  dict.SetMethod("getWebFrameId", &GetWebFrameId);
+  dict.SetMethod("setSpellCheckProvider", &SetSpellCheckProvider);
+  dict.SetMethod("insertText", &InsertText);
+  dict.SetMethod("insertCSS", &InsertCSS);
+  dict.SetMethod("executeJavaScript", &ExecuteJavaScript);
+  dict.SetMethod("executeJavaScriptInIsolatedWorld",
+                 &ExecuteJavaScriptInIsolatedWorld);
+  dict.SetMethod("setIsolatedWorldSecurityOrigin",
+                 &SetIsolatedWorldSecurityOrigin);
+  dict.SetMethod("setIsolatedWorldContentSecurityPolicy",
+                 &SetIsolatedWorldContentSecurityPolicy);
+  dict.SetMethod("setIsolatedWorldHumanReadableName",
+                 &SetIsolatedWorldHumanReadableName);
+  dict.SetMethod("getResourceUsage", &GetResourceUsage);
+  dict.SetMethod("clearCache", &ClearCache);
+  dict.SetMethod("_findFrameByRoutingId", &FindFrameByRoutingId);
+  dict.SetMethod("_getFrameForSelector", &GetFrameForSelector);
+  dict.SetMethod("_findFrameByName", &FindFrameByName);
+  dict.SetMethod("_getOpener", &GetOpener);
+  dict.SetMethod("_getParent", &GetFrameParent);
+  dict.SetMethod("_getTop", &GetTop);
+  dict.SetMethod("_getFirstChild", &GetFirstChild);
+  dict.SetMethod("_getNextSibling", &GetNextSibling);
+  dict.SetMethod("_getRoutingId", &GetRoutingId);
 }
 
 }  // namespace

atom/renderer/api/atom_api_web_frame.h

@@ -1,103 +0,0 @@
-// Copyright (c) 2014 GitHub, Inc.
-// Use of this source code is governed by the MIT license that can be
-// found in the LICENSE file.
-
-#ifndef ATOM_RENDERER_API_ATOM_API_WEB_FRAME_H_
-#define ATOM_RENDERER_API_ATOM_API_WEB_FRAME_H_
-
-#include <memory>
-#include <string>
-#include <vector>
-
-#include "native_mate/handle.h"
-#include "native_mate/wrappable.h"
-#include "third_party/blink/public/platform/web_cache.h"
-
-namespace blink {
-class WebLocalFrame;
-}
-
-namespace mate {
-class Dictionary;
-class Arguments;
-}  // namespace mate
-
-namespace atom {
-
-namespace api {
-
-class WebFrame : public mate::Wrappable<WebFrame> {
- public:
-  static mate::Handle<WebFrame> Create(v8::Isolate* isolate);
-
-  static void BuildPrototype(v8::Isolate* isolate,
-                             v8::Local<v8::FunctionTemplate> prototype);
-
- private:
-  explicit WebFrame(v8::Isolate* isolate);
-  explicit WebFrame(v8::Isolate* isolate, blink::WebLocalFrame* blink_frame);
-  ~WebFrame() override;
-
-  void SetName(const std::string& name);
-
-  double SetZoomLevel(double level);
-  double GetZoomLevel() const;
-  double SetZoomFactor(double factor);
-  double GetZoomFactor() const;
-
-  void SetVisualZoomLevelLimits(double min_level, double max_level);
-  void SetLayoutZoomLevelLimits(double min_level, double max_level);
-
-  void AllowGuestViewElementDefinition(v8::Local<v8::Object> context,
-                                       v8::Local<v8::Function> register_cb);
-  int GetWebFrameId(v8::Local<v8::Value> content_window);
-
-  // Set the provider that will be used by SpellCheckClient for spell check.
-  void SetSpellCheckProvider(mate::Arguments* args,
-                             const std::string& language,
-                             v8::Local<v8::Object> provider);
-
-  // Editing.
-  void InsertText(const std::string& text);
-  void InsertCSS(const std::string& css);
-
-  // Executing scripts.
-  void ExecuteJavaScript(const base::string16& code, mate::Arguments* args);
-  void ExecuteJavaScriptInIsolatedWorld(
-      int world_id,
-      const std::vector<mate::Dictionary>& scripts,
-      mate::Arguments* args);
-
-  // Isolated world related methods
-  void SetIsolatedWorldSecurityOrigin(int world_id,
-                                      const std::string& origin_url);
-  void SetIsolatedWorldContentSecurityPolicy(
-      int world_id,
-      const std::string& security_policy);
-  void SetIsolatedWorldHumanReadableName(int world_id, const std::string& name);
-
-  // Resource related methods
-  blink::WebCache::ResourceTypeStats GetResourceUsage(v8::Isolate* isolate);
-  void ClearCache(v8::Isolate* isolate);
-
-  // Frame navigation
-  v8::Local<v8::Value> Opener() const;
-  v8::Local<v8::Value> Parent() const;
-  v8::Local<v8::Value> Top() const;
-  v8::Local<v8::Value> FirstChild() const;
-  v8::Local<v8::Value> NextSibling() const;
-  v8::Local<v8::Value> GetFrameForSelector(const std::string& selector) const;
-  v8::Local<v8::Value> FindFrameByName(const std::string& name) const;
-  v8::Local<v8::Value> FindFrameByRoutingId(int routing_id) const;
-  v8::Local<v8::Value> RoutingId() const;
-
-  blink::WebLocalFrame* web_frame_;
-
-  DISALLOW_COPY_AND_ASSIGN(WebFrame);
-};
-
-}  // namespace api
-
-}  // namespace atom
-
-#endif  // ATOM_RENDERER_API_ATOM_API_WEB_FRAME_H_

filenames.gni

@@ -644,7 +644,6 @@ filenames = {
     "atom/renderer/api/atom_api_spell_check_client.cc",
     "atom/renderer/api/atom_api_spell_check_client.h",
     "atom/renderer/api/atom_api_web_frame.cc",
-    "atom/renderer/api/atom_api_web_frame.h",
     "atom/renderer/atom_autofill_agent.cc",
     "atom/renderer/atom_autofill_agent.h",
     "atom/renderer/atom_render_frame_observer.cc",

lib/renderer/api/web-frame.js

@@ -1,13 +1,67 @@
 'use strict'
 
 const { EventEmitter } = require('events')
-const { webFrame, WebFrame } = process.atomBinding('web_frame')
+const binding = process.atomBinding('web_frame')
 
-// WebFrame is an EventEmitter.
+class WebFrame extends EventEmitter {
-Object.setPrototypeOf(WebFrame.prototype, EventEmitter.prototype)
+  constructor (context) {
-EventEmitter.call(webFrame)
+    super()
 
-// Lots of webview would subscribe to webFrame's events.
+    this.context = context
-webFrame.setMaxListeners(0)
+    // Lots of webview would subscribe to webFrame's events.
+    this.setMaxListeners(0)
+  }
 
-module.exports = webFrame
+  findFrameByRoutingId (...args) {
+    return getWebFrame(binding._findFrameByRoutingId(this.context, ...args))
+  }
+
+  getFrameForSelector (...args) {
+    return getWebFrame(binding._getFrameForSelector(this.context, ...args))
+  }
+
+  findFrameByName (...args) {
+    return getWebFrame(binding._findFrameByName(this.context, ...args))
+  }
+
+  get opener () {
+    return getWebFrame(binding._getOpener(this.context))
+  }
+
+  get parent () {
+    return getWebFrame(binding._getParent(this.context))
+  }
+
+  get top () {
+    return getWebFrame(binding._getTop(this.context))
+  }
+
+  get firstChild () {
+    return getWebFrame(binding._getFirstChild(this.context))
+  }
+
+  get nextSibling () {
+    return getWebFrame(binding._getNextSibling(this.context))
+  }
+
+  get routingId () {
+    return binding._getRoutingId(this.context)
+  }
+}
+
+// Populate the methods.
+for (const name in binding) {
+  if (!name.startsWith('_')) { // some methods are manully populated above
+    WebFrame.prototype[name] = function (...args) {
+      return binding[name](this.context, ...args)
+    }
+  }
+}
+
+// Helper to return WebFrame or null depending on context.
+// TODO(zcbenz): Consider returning same WebFrame for the same frame.
+function getWebFrame (context) {
+  return context ? new WebFrame(context) : null
+}
+
+module.exports = new WebFrame(window)

spec/api-web-frame-spec.js

@@ -56,4 +56,28 @@ describe('webFrame module', function () {
     expect(words).to.deep.equal(['spleling', 'test'])
     expect(callback).to.be.true()
   })
+
+  it('top is self for top frame', () => {
+    expect(webFrame.top.context).to.equal(webFrame.context)
+  })
+
+  it('opener is null for top frame', () => {
+    expect(webFrame.opener).to.be.null()
+  })
+
+  it('firstChild is null for top frame', () => {
+    expect(webFrame.firstChild).to.be.null()
+  })
+
+  it('getFrameForSelector() does not crash when not found', () => {
+    expect(webFrame.getFrameForSelector('unexist-selector')).to.be.null()
+  })
+
+  it('findFrameByName() does not crash when not found', () => {
+    expect(webFrame.findFrameByName('unexist-name')).to.be.null()
+  })
+
+  it('findFrameByRoutingId() does not crash when not found', () => {
+    expect(webFrame.findFrameByRoutingId(-1)).to.be.null()
+  })
 })