Merge pull request #9148 from electron/share-render-frame-observer-with-sandbox

Refactor: Share AtomRenderFrameObserver with AtomSandboxedRendererClient

atom/renderer/atom_render_frame_observer.cc

@@ -0,0 +1,84 @@
+// Copyright (c) 2017 GitHub, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#include "atom/renderer/atom_render_frame_observer.h"
+
+#include "content/public/renderer/render_frame.h"
+#include "third_party/WebKit/public/web/WebDocument.h"
+#include "third_party/WebKit/public/web/WebLocalFrame.h"
+#include "third_party/WebKit/public/web/WebScriptSource.h"
+
+namespace atom {
+
+AtomRenderFrameObserver::AtomRenderFrameObserver(
+    content::RenderFrame* frame,
+    RendererClientBase* renderer_client)
+  : content::RenderFrameObserver(frame),
+    render_frame_(frame),
+    renderer_client_(renderer_client) {}
+
+void AtomRenderFrameObserver::DidClearWindowObject() {
+  renderer_client_->DidClearWindowObject(render_frame_);
+}
+
+void AtomRenderFrameObserver::DidCreateScriptContext(
+    v8::Handle<v8::Context> context,
+    int extension_group,
+    int world_id) {
+  if (ShouldNotifyClient(world_id))
+    renderer_client_->DidCreateScriptContext(context, render_frame_);
+
+  if (renderer_client_->isolated_world() && IsMainWorld(world_id)
+      && render_frame_->IsMainFrame()) {
+    CreateIsolatedWorldContext();
+    renderer_client_->SetupMainWorldOverrides(context);
+  }
+}
+
+void AtomRenderFrameObserver::WillReleaseScriptContext(
+    v8::Local<v8::Context> context,
+    int world_id) {
+  if (ShouldNotifyClient(world_id))
+    renderer_client_->WillReleaseScriptContext(context, render_frame_);
+}
+
+void AtomRenderFrameObserver::OnDestruct() {
+  delete this;
+}
+
+void AtomRenderFrameObserver::CreateIsolatedWorldContext() {
+  auto frame = render_frame_->GetWebFrame();
+
+  // This maps to the name shown in the context combo box in the Console tab
+  // of the dev tools.
+  frame->setIsolatedWorldHumanReadableName(
+      World::ISOLATED_WORLD,
+      blink::WebString::fromUTF8("Electron Isolated Context"));
+
+  // Setup document's origin policy in isolated world
+  frame->setIsolatedWorldSecurityOrigin(
+    World::ISOLATED_WORLD, frame->document().getSecurityOrigin());
+
+  // Create initial script context in isolated world
+  blink::WebScriptSource source("void 0");
+  frame->executeScriptInIsolatedWorld(
+      World::ISOLATED_WORLD, &source, 1, ExtensionGroup::MAIN_GROUP);
+}
+
+bool AtomRenderFrameObserver::IsMainWorld(int world_id) {
+  return world_id == World::MAIN_WORLD;
+}
+
+bool AtomRenderFrameObserver::IsIsolatedWorld(int world_id) {
+  return world_id == World::ISOLATED_WORLD;
+}
+
+bool AtomRenderFrameObserver::ShouldNotifyClient(int world_id) {
+  if (renderer_client_->isolated_world() && render_frame_->IsMainFrame())
+    return IsIsolatedWorld(world_id);
+  else
+    return IsMainWorld(world_id);
+}
+
+}  // namespace atom

atom/renderer/atom_render_frame_observer.h

@@ -0,0 +1,53 @@
+// Copyright (c) 2017 GitHub, Inc.
+// Use of this source code is governed by the MIT license that can be
+// found in the LICENSE file.
+
+#ifndef ATOM_RENDERER_ATOM_RENDER_FRAME_OBSERVER_H_
+#define ATOM_RENDERER_ATOM_RENDER_FRAME_OBSERVER_H_
+
+#include "atom/renderer/renderer_client_base.h"
+#include "content/public/renderer/render_frame_observer.h"
+
+namespace atom {
+
+enum World {
+  MAIN_WORLD = 0,
+  // Use a high number far away from 0 to not collide with any other world
+  // IDs created internally by Chrome.
+  ISOLATED_WORLD = 999
+};
+
+enum ExtensionGroup {
+  MAIN_GROUP = 1
+};
+
+// Helper class to forward the messages to the client.
+class AtomRenderFrameObserver : public content::RenderFrameObserver {
+ public:
+  AtomRenderFrameObserver(content::RenderFrame* frame,
+                          RendererClientBase* renderer_client);
+
+  // content::RenderFrameObserver:
+  void DidClearWindowObject() override;
+  void DidCreateScriptContext(v8::Handle<v8::Context> context,
+                              int extension_group,
+                              int world_id) override;
+  void WillReleaseScriptContext(v8::Local<v8::Context> context,
+                                int world_id) override;
+  void OnDestruct() override;
+
+ private:
+  bool ShouldNotifyClient(int world_id);
+  void CreateIsolatedWorldContext();
+  bool IsMainWorld(int world_id);
+  bool IsIsolatedWorld(int world_id);
+
+  content::RenderFrame* render_frame_;
+  RendererClientBase* renderer_client_;
+
+  DISALLOW_COPY_AND_ASSIGN(AtomRenderFrameObserver);
+};
+
+}  // namespace atom
+
+#endif  // ATOM_RENDERER_ATOM_RENDER_FRAME_OBSERVER_H_

atom/renderer/atom_renderer_client.cc

@@ -16,16 +16,15 @@
 #include "atom/common/node_bindings.h"
 #include "atom/common/options_switches.h"
 #include "atom/renderer/api/atom_api_renderer_ipc.h"
+#include "atom/renderer/atom_render_frame_observer.h"
 #include "atom/renderer/atom_render_view_observer.h"
 #include "atom/renderer/node_array_buffer_bridge.h"
 #include "atom/renderer/web_worker_observer.h"
 #include "base/command_line.h"
 #include "content/public/renderer/render_frame.h"
-#include "content/public/renderer/render_frame_observer.h"
 #include "native_mate/dictionary.h"
 #include "third_party/WebKit/public/web/WebDocument.h"
 #include "third_party/WebKit/public/web/WebLocalFrame.h"
-#include "third_party/WebKit/public/web/WebScriptSource.h"
 
 #include "atom/common/node_includes.h"
 
@@ -33,127 +32,6 @@ namespace atom {
 
 namespace {
 
-enum World {
-  MAIN_WORLD = 0,
-  // Use a high number far away from 0 to not collide with any other world
-  // IDs created internally by Chrome.
-  ISOLATED_WORLD = 999
-};
-
-enum ExtensionGroup {
-  MAIN_GROUP = 1
-};
-
-// Helper class to forward the messages to the client.
-class AtomRenderFrameObserver : public content::RenderFrameObserver {
- public:
-  AtomRenderFrameObserver(content::RenderFrame* frame,
-                          AtomRendererClient* renderer_client)
-      : content::RenderFrameObserver(frame),
-        render_frame_(frame),
-        renderer_client_(renderer_client) {}
-
-  // content::RenderFrameObserver:
-  void DidClearWindowObject() override {
-    renderer_client_->DidClearWindowObject(render_frame_);
-  }
-
-  void CreateIsolatedWorldContext() {
-    auto frame = render_frame_->GetWebFrame();
-
-    // This maps to the name shown in the context combo box in the Console tab
-    // of the dev tools.
-    frame->setIsolatedWorldHumanReadableName(
-        World::ISOLATED_WORLD,
-        blink::WebString::fromUTF8("Electron Isolated Context"));
-
-    // Setup document's origin policy in isolated world
-    frame->setIsolatedWorldSecurityOrigin(
-      World::ISOLATED_WORLD, frame->document().getSecurityOrigin());
-
-    // Create initial script context in isolated world
-    blink::WebScriptSource source("void 0");
-    frame->executeScriptInIsolatedWorld(
-        World::ISOLATED_WORLD, &source, 1, ExtensionGroup::MAIN_GROUP);
-  }
-
-  void SetupMainWorldOverrides(v8::Handle<v8::Context> context) {
-    // Setup window overrides in the main world context
-    v8::Isolate* isolate = context->GetIsolate();
-
-    // Wrap the bundle into a function that receives the binding object as
-    // an argument.
-    std::string bundle(node::isolated_bundle_data,
-        node::isolated_bundle_data + sizeof(node::isolated_bundle_data));
-    std::string wrapper = "(function (binding, require) {\n" + bundle + "\n})";
-    auto script = v8::Script::Compile(
-        mate::ConvertToV8(isolate, wrapper)->ToString());
-    auto func = v8::Handle<v8::Function>::Cast(
-        script->Run(context).ToLocalChecked());
-
-    auto binding = v8::Object::New(isolate);
-    api::Initialize(binding, v8::Null(isolate), context, nullptr);
-
-    // Pass in CLI flags needed to setup window
-    base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
-    mate::Dictionary dict(isolate, binding);
-    if (command_line->HasSwitch(switches::kGuestInstanceID))
-      dict.Set(options::kGuestInstanceID,
-               command_line->GetSwitchValueASCII(switches::kGuestInstanceID));
-    if (command_line->HasSwitch(switches::kOpenerID))
-      dict.Set(options::kOpenerID,
-               command_line->GetSwitchValueASCII(switches::kOpenerID));
-    dict.Set("hiddenPage", command_line->HasSwitch(switches::kHiddenPage));
-
-    v8::Local<v8::Value> args[] = { binding };
-    ignore_result(func->Call(context, v8::Null(isolate), 1, args));
-  }
-
-  bool IsMainWorld(int world_id) {
-    return world_id == World::MAIN_WORLD;
-  }
-
-  bool IsIsolatedWorld(int world_id) {
-    return world_id == World::ISOLATED_WORLD;
-  }
-
-  bool ShouldNotifyClient(int world_id) {
-    if (renderer_client_->isolated_world() && render_frame_->IsMainFrame())
-      return IsIsolatedWorld(world_id);
-    else
-      return IsMainWorld(world_id);
-  }
-
-  void DidCreateScriptContext(v8::Handle<v8::Context> context,
-                              int extension_group,
-                              int world_id) override {
-    if (ShouldNotifyClient(world_id))
-      renderer_client_->DidCreateScriptContext(context, render_frame_);
-
-    if (renderer_client_->isolated_world() && IsMainWorld(world_id)
-        && render_frame_->IsMainFrame()) {
-      CreateIsolatedWorldContext();
-      SetupMainWorldOverrides(context);
-    }
-  }
-
-  void WillReleaseScriptContext(v8::Local<v8::Context> context,
-                                int world_id) override {
-    if (ShouldNotifyClient(world_id))
-      renderer_client_->WillReleaseScriptContext(context, render_frame_);
-  }
-
-  void OnDestruct() override {
-    delete this;
-  }
-
- private:
-  content::RenderFrame* render_frame_;
-  AtomRendererClient* renderer_client_;
-
-  DISALLOW_COPY_AND_ASSIGN(AtomRenderFrameObserver);
-};
-
 bool IsDevToolsExtension(content::RenderFrame* render_frame) {
   return static_cast<GURL>(render_frame->GetWebFrame()->document().url())
       .SchemeIs("chrome-extension");
@@ -180,7 +58,6 @@ void AtomRendererClient::RenderThreadStarted() {
 
 void AtomRendererClient::RenderFrameCreated(
     content::RenderFrame* render_frame) {
-  new AtomRenderFrameObserver(render_frame, this);
   RendererClientBase::RenderFrameCreated(render_frame);
 }
 
@@ -189,12 +66,6 @@ void AtomRendererClient::RenderViewCreated(content::RenderView* render_view) {
   RendererClientBase::RenderViewCreated(render_view);
 }
 
-void AtomRendererClient::DidClearWindowObject(
-    content::RenderFrame* render_frame) {
-  // Make sure every page will get a script context created.
-  render_frame->GetWebFrame()->executeScript(blink::WebScriptSource("void 0"));
-}
-
 void AtomRendererClient::RunScriptsAtDocumentStart(
     content::RenderFrame* render_frame) {
   // Inform the document start pharse.
@@ -307,4 +178,38 @@ v8::Local<v8::Context> AtomRendererClient::GetContext(
     return frame->mainWorldScriptContext();
 }
 
+void AtomRendererClient::SetupMainWorldOverrides(
+    v8::Handle<v8::Context> context) {
+  // Setup window overrides in the main world context
+  v8::Isolate* isolate = context->GetIsolate();
+
+  // Wrap the bundle into a function that receives the binding object as
+  // an argument.
+  std::string bundle(node::isolated_bundle_data,
+      node::isolated_bundle_data + sizeof(node::isolated_bundle_data));
+  std::string wrapper = "(function (binding, require) {\n" + bundle + "\n})";
+  auto script = v8::Script::Compile(
+      mate::ConvertToV8(isolate, wrapper)->ToString());
+  auto func = v8::Handle<v8::Function>::Cast(
+      script->Run(context).ToLocalChecked());
+
+  auto binding = v8::Object::New(isolate);
+  api::Initialize(binding, v8::Null(isolate), context, nullptr);
+
+  // Pass in CLI flags needed to setup window
+  base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
+  mate::Dictionary dict(isolate, binding);
+  if (command_line->HasSwitch(switches::kGuestInstanceID))
+    dict.Set(options::kGuestInstanceID,
+             command_line->GetSwitchValueASCII(switches::kGuestInstanceID));
+  if (command_line->HasSwitch(switches::kOpenerID))
+    dict.Set(options::kOpenerID,
+             command_line->GetSwitchValueASCII(switches::kOpenerID));
+  dict.Set("hiddenPage", command_line->HasSwitch(switches::kHiddenPage));
+
+  v8::Local<v8::Value> args[] = { binding };
+  ignore_result(func->Call(context, v8::Null(isolate), 1, args));
+}
+
+
 }  // namespace atom

atom/renderer/atom_renderer_client.h

@@ -20,16 +20,19 @@ class AtomRendererClient : public RendererClientBase {
   AtomRendererClient();
   virtual ~AtomRendererClient();
 
-  void DidClearWindowObject(content::RenderFrame* render_frame);
-  void DidCreateScriptContext(
-      v8::Handle<v8::Context> context, content::RenderFrame* render_frame);
-  void WillReleaseScriptContext(
-      v8::Handle<v8::Context> context, content::RenderFrame* render_frame);
-
   // Get the context that the Electron API is running in.
   v8::Local<v8::Context> GetContext(
       blink::WebFrame* frame, v8::Isolate* isolate);
-  bool isolated_world() { return isolated_world_; }
+
+  // atom::RendererClientBase:
+  void DidCreateScriptContext(
+      v8::Handle<v8::Context> context,
+      content::RenderFrame* render_frame) override;
+  void WillReleaseScriptContext(
+      v8::Handle<v8::Context> context,
+      content::RenderFrame* render_frame) override;
+  void SetupMainWorldOverrides(v8::Handle<v8::Context> context) override;
+  bool isolated_world() override { return isolated_world_; }
 
  private:
   enum NodeIntegration {

atom/renderer/atom_sandboxed_renderer_client.cc

@@ -20,7 +20,6 @@
 #include "base/command_line.h"
 #include "chrome/renderer/printing/print_web_view_helper.h"
 #include "content/public/renderer/render_frame.h"
-#include "content/public/renderer/render_frame_observer.h"
 #include "content/public/renderer/render_view.h"
 #include "content/public/renderer/render_view_observer.h"
 #include "ipc/ipc_message_macros.h"
@@ -89,50 +88,6 @@ void InitializeBindings(v8::Local<v8::Object> binding,
   b.SetMethod("crash", AtomBindings::Crash);
 }
 
-class AtomSandboxedRenderFrameObserver : public content::RenderFrameObserver {
- public:
-  AtomSandboxedRenderFrameObserver(content::RenderFrame* frame,
-                                   AtomSandboxedRendererClient* renderer_client)
-      : content::RenderFrameObserver(frame),
-        render_frame_(frame),
-        world_id_(-1),
-        renderer_client_(renderer_client) {}
-
-  // content::RenderFrameObserver:
-  void DidClearWindowObject() override {
-    // Make sure every page will get a script context created.
-    render_frame_->GetWebFrame()->executeScript(
-        blink::WebScriptSource("void 0"));
-  }
-
-  void DidCreateScriptContext(v8::Handle<v8::Context> context,
-                              int extension_group,
-                              int world_id) override {
-    if (world_id_ != -1 && world_id_ != world_id)
-      return;
-    world_id_ = world_id;
-    renderer_client_->DidCreateScriptContext(context, render_frame_);
-  }
-
-  void WillReleaseScriptContext(v8::Local<v8::Context> context,
-                                int world_id) override {
-    if (world_id_ != world_id)
-      return;
-    renderer_client_->WillReleaseScriptContext(context, render_frame_);
-  }
-
-  void OnDestruct() override {
-    delete this;
-  }
-
- private:
-  content::RenderFrame* render_frame_;
-  int world_id_;
-  AtomSandboxedRendererClient* renderer_client_;
-
-  DISALLOW_COPY_AND_ASSIGN(AtomSandboxedRenderFrameObserver);
-};
-
 class AtomSandboxedRenderViewObserver : public AtomRenderViewObserver {
  public:
   AtomSandboxedRenderViewObserver(content::RenderView* render_view,
@@ -181,7 +136,6 @@ AtomSandboxedRendererClient::~AtomSandboxedRendererClient() {
 
 void AtomSandboxedRendererClient::RenderFrameCreated(
     content::RenderFrame* render_frame) {
-  new AtomSandboxedRenderFrameObserver(render_frame, this);
   RendererClientBase::RenderFrameCreated(render_frame);
 }
 

atom/renderer/atom_sandboxed_renderer_client.h

@@ -16,13 +16,18 @@ class AtomSandboxedRendererClient : public RendererClientBase {
   AtomSandboxedRendererClient();
   virtual ~AtomSandboxedRendererClient();
 
-  void DidCreateScriptContext(
-      v8::Handle<v8::Context> context, content::RenderFrame* render_frame);
-  void WillReleaseScriptContext(
-      v8::Handle<v8::Context> context, content::RenderFrame* render_frame);
   void InvokeIpcCallback(v8::Handle<v8::Context> context,
                          const std::string& callback_name,
                          std::vector<v8::Handle<v8::Value>> args);
+  // atom::RendererClientBase:
+  void DidCreateScriptContext(
+      v8::Handle<v8::Context> context,
+      content::RenderFrame* render_frame) override;
+  void WillReleaseScriptContext(
+      v8::Handle<v8::Context> context,
+      content::RenderFrame* render_frame) override;
+  void SetupMainWorldOverrides(v8::Handle<v8::Context> context) override { }
+  bool isolated_world() override { return false; }
   // content::ContentRendererClient:
   void RenderFrameCreated(content::RenderFrame*) override;
   void RenderViewCreated(content::RenderView*) override;

atom/renderer/renderer_client_base.cc

@@ -11,6 +11,7 @@
 #include "atom/common/color_util.h"
 #include "atom/common/native_mate_converters/value_converter.h"
 #include "atom/common/options_switches.h"
+#include "atom/renderer/atom_render_frame_observer.h"
 #include "atom/renderer/content_settings_observer.h"
 #include "atom/renderer/guest_view_container.h"
 #include "atom/renderer/preferences_manager.h"
@@ -27,6 +28,7 @@
 #include "third_party/WebKit/public/web/WebFrameWidget.h"
 #include "third_party/WebKit/public/web/WebKit.h"
 #include "third_party/WebKit/public/web/WebPluginParams.h"
+#include "third_party/WebKit/public/web/WebScriptSource.h"
 #include "third_party/WebKit/public/web/WebSecurityPolicy.h"
 
 #if defined(OS_MACOSX)
@@ -109,6 +111,7 @@ void RendererClientBase::RenderThreadStarted() {
 
 void RendererClientBase::RenderFrameCreated(
     content::RenderFrame* render_frame) {
+  new AtomRenderFrameObserver(render_frame, this);
   new PepperHelper(render_frame);
   new ContentSettingsObserver(render_frame);
   new printing::PrintWebViewHelper(render_frame);
@@ -149,6 +152,12 @@ void RendererClientBase::RenderViewCreated(content::RenderView* render_view) {
   }
 }
 
+void RendererClientBase::DidClearWindowObject(
+    content::RenderFrame* render_frame) {
+  // Make sure every page will get a script context created.
+  render_frame->GetWebFrame()->executeScript(blink::WebScriptSource("void 0"));
+}
+
 blink::WebSpeechSynthesizer* RendererClientBase::OverrideSpeechSynthesizer(
     blink::WebSpeechSynthesizerClient* client) {
   return new TtsDispatcher(client);

atom/renderer/renderer_client_base.h

@@ -19,6 +19,14 @@ class RendererClientBase : public content::ContentRendererClient {
   RendererClientBase();
   virtual ~RendererClientBase();
 
+  virtual void DidCreateScriptContext(
+      v8::Handle<v8::Context> context, content::RenderFrame* render_frame) = 0;
+  virtual void WillReleaseScriptContext(
+      v8::Handle<v8::Context> context, content::RenderFrame* render_frame) = 0;
+  virtual void DidClearWindowObject(content::RenderFrame* render_frame);
+  virtual void SetupMainWorldOverrides(v8::Handle<v8::Context> context) = 0;
+  virtual bool isolated_world() = 0;
+
  protected:
   void AddRenderBindings(v8::Isolate* isolate,
                          v8::Local<v8::Object> binding_object);

filenames.gypi

@@ -468,6 +468,8 @@
       '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_render_frame_observer.cc',
+      'atom/renderer/atom_render_frame_observer.h',
       'atom/renderer/atom_render_view_observer.cc',
       'atom/renderer/atom_render_view_observer.h',
       'atom/renderer/atom_renderer_client.cc',