Merge pull request #154 from atom/safe-context

Wrap uv loop with web page context in renderer

common/api/atom_bindings.cc

@@ -16,13 +16,29 @@ namespace {
 
 static int kMaxCallStackSize = 200;  // Same with WebKit.
 
-static uv_async_t dummy_uv_handle;
+// Async handle to wake up uv loop.
+static uv_async_t g_dummy_uv_handle;
 
+// Async handle to execute the stored v8 callback.
+static uv_async_t g_callback_uv_handle;
+
+// Stored v8 callback, to be called by the async handler.
+RefCountedV8Function g_v8_callback;
+
+// Dummy class type that used for crashing the program.
 struct DummyClass { bool crash; };
 
+// Dummy async handler that does nothing.
 void UvNoOp(uv_async_t* handle, int status) {
 }
 
+// Async handler to execute the stored v8 callback.
+void UvOnCallback(uv_async_t* handle, int status) {
+  v8::HandleScope handle_scope(node_isolate);
+  v8::Handle<v8::Object> global = v8::Context::GetCurrent()->Global();
+  g_v8_callback->NewHandle()->Call(global, 0, NULL);
+}
+
 v8::Handle<v8::Object> DumpStackFrame(v8::Handle<v8::StackFrame> stack_frame) {
   v8::Local<v8::Object> result = v8::Object::New();
   result->Set(ToV8Value("line"), ToV8Value(stack_frame->GetLineNumber()));
@@ -44,7 +60,8 @@ v8::Handle<v8::Object> DumpStackFrame(v8::Handle<v8::StackFrame> stack_frame) {
 node::node_module_struct* GetBuiltinModule(const char *name, bool is_browser);
 
 AtomBindings::AtomBindings() {
-  uv_async_init(uv_default_loop(), &dummy_uv_handle, UvNoOp);
+  uv_async_init(uv_default_loop(), &g_dummy_uv_handle, UvNoOp);
+  uv_async_init(uv_default_loop(), &g_callback_uv_handle, UvOnCallback);
 }
 
 AtomBindings::~AtomBindings() {
@@ -58,6 +75,7 @@ void AtomBindings::BindTo(v8::Handle<v8::Object> process) {
   NODE_SET_METHOD(process, "activateUvLoop", ActivateUVLoop);
   NODE_SET_METHOD(process, "log", Log);
   NODE_SET_METHOD(process, "getCurrentStackTrace", GetCurrentStackTrace);
+  NODE_SET_METHOD(process, "scheduleCallback", ScheduleCallback);
 
   process->Get(v8::String::New("versions"))->ToObject()->
     Set(v8::String::New("atom-shell"), v8::String::New(ATOM_VERSION_STRING));
@@ -115,7 +133,7 @@ void AtomBindings::Crash(const v8::FunctionCallbackInfo<v8::Value>& args) {
 // static
 void AtomBindings::ActivateUVLoop(
     const v8::FunctionCallbackInfo<v8::Value>& args) {
-  uv_async_send(&dummy_uv_handle);
+  uv_async_send(&g_dummy_uv_handle);
 }
 
 // static
@@ -146,4 +164,12 @@ void AtomBindings::GetCurrentStackTrace(
   args.GetReturnValue().Set(result);
 }
 
+// static
+void AtomBindings::ScheduleCallback(
+    const v8::FunctionCallbackInfo<v8::Value>& args) {
+  if (!FromV8Arguments(args, &g_v8_callback))
+    return node::ThrowTypeError("Bad arguments");
+  uv_async_send(&g_callback_uv_handle);
+}
+
 }  // namespace atom

common/api/atom_bindings.h

@@ -26,6 +26,7 @@ class AtomBindings {
   static void Log(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void GetCurrentStackTrace(
       const v8::FunctionCallbackInfo<v8::Value>& args);
+  static void ScheduleCallback(const v8::FunctionCallbackInfo<v8::Value>& args);
 
   DISALLOW_COPY_AND_ASSIGN(AtomBindings);
 };

common/node_bindings.cc

@@ -66,6 +66,7 @@ NodeBindings::NodeBindings(bool is_browser)
       message_loop_(NULL),
       uv_loop_(uv_default_loop()),
       embed_closed_(false),
+      uv_env_(NULL),
       weak_factory_(this) {
 }
 
@@ -193,9 +194,13 @@ void NodeBindings::RunMessageLoop() {
 void NodeBindings::UvRunOnce() {
   DCHECK(!is_browser_ || BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  // Enter node context while dealing with uv events.
   v8::HandleScope handle_scope(node_isolate);
-  v8::Context::Scope context_scope(global_env->context());
+
+  // Enter node context while dealing with uv events, by default the global
+  // env would be used unless user specified another one (this happens for
+  // renderer process, which wraps the uv loop with web page context).
+  node::Environment* env = get_uv_env() ? get_uv_env() : global_env;
+  v8::Context::Scope context_scope(env->context());
 
   // Deal with uv events.
   int r = uv_run(uv_loop_, (uv_run_mode)(UV_RUN_ONCE | UV_RUN_NOWAIT));

common/node_bindings.h

@@ -38,6 +38,10 @@ class NodeBindings {
   // Do message loop integration.
   virtual void RunMessageLoop();
 
+  // Gets/sets the environment to wrap uv loop.
+  void set_uv_env(node::Environment* env) { uv_env_ = env; }
+  node::Environment* get_uv_env() const { return uv_env_; }
+
  protected:
   explicit NodeBindings(bool is_browser);
 
@@ -84,6 +88,9 @@ class NodeBindings {
   // Semaphore to wait for main loop in the embed thread.
   uv_sem_t embed_sem_;
 
+  // Environment that to wrap the uv loop.
+  node::Environment* uv_env_;
+
   base::WeakPtrFactory<NodeBindings> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(NodeBindings);

package.json

@@ -6,9 +6,11 @@
     "coffee-script": "~1.6.3",
     "coffeelint": "~0.6.1",
     "mocha": "~1.13.0",
+    "pathwatcher": "0.13.0",
     "walkdir": "~0.0.7",
     "runas": "0.3.0",
-    "formidable": "~1.0.14"
+    "formidable": "~1.0.14",
+    "temp": "~0.6.0"
   },
 
   "private": true,

renderer/atom_renderer_client.cc

@@ -4,6 +4,8 @@
 
 #include "renderer/atom_renderer_client.h"
 
+#include <algorithm>
+
 #include "common/node_bindings.h"
 #include "renderer/api/atom_renderer_bindings.h"
 #include "renderer/atom_render_view_observer.h"
@@ -51,10 +53,17 @@ void AtomRendererClient::DidCreateScriptContext(WebKit::WebFrame* frame,
   node_bindings_->RunMessageLoop();
 
   // Setup node environment for each window.
-  node_bindings_->CreateEnvironment(context);
+  node::Environment* env = node_bindings_->CreateEnvironment(context);
 
   // Add atom-shell extended APIs.
   atom_bindings_->BindToFrame(frame);
+
+  // Store the created environment.
+  web_page_envs_.push_back(env);
+
+  // Make uv loop being wrapped by window context.
+  if (node_bindings_->get_uv_env() == NULL)
+    node_bindings_->set_uv_env(env);
 }
 
 void AtomRendererClient::WillReleaseScriptContext(
@@ -67,7 +76,18 @@ void AtomRendererClient::WillReleaseScriptContext(
     return;
   }
 
+  // Clear the environment.
   env->Dispose();
+  web_page_envs_.erase(
+      std::remove(web_page_envs_.begin(), web_page_envs_.end(), env),
+      web_page_envs_.end());
+
+  // Wrap the uv loop with another environment.
+  if (env == node_bindings_->get_uv_env()) {
+    node::Environment* env = web_page_envs_.size() > 0 ? web_page_envs_[0] :
+                                                         NULL;
+    node_bindings_->set_uv_env(env);
+  }
 }
 
 }  // namespace atom

renderer/atom_renderer_client.h

@@ -5,8 +5,14 @@
 #ifndef ATOM_RENDERER_ATOM_RENDERER_CLIENT_H_
 #define ATOM_RENDERER_ATOM_RENDERER_CLIENT_H_
 
+#include <vector>
+
 #include "content/public/renderer/content_renderer_client.h"
 
+namespace node {
+class Environment;
+}
+
 namespace atom {
 
 class AtomRendererBindings;
@@ -30,6 +36,8 @@ class AtomRendererClient : public content::ContentRendererClient {
                                         v8::Handle<v8::Context>,
                                         int world_id) OVERRIDE;
 
+  std::vector<node::Environment*> web_page_envs_;
+
   scoped_ptr<NodeBindings> node_bindings_;
   scoped_ptr<AtomRendererBindings> atom_bindings_;
 

spec/modules-spec.coffee

@@ -1,9 +1,11 @@
 assert = require 'assert'
 fs     = require 'fs'
 path   = require 'path'
+temp   = require 'temp'
 
 describe 'third-party module', ->
   fixtures = path.join __dirname, 'fixtures'
+  temp.track()
 
   describe 'runas', ->
     it 'can be required in renderer', ->
@@ -15,3 +17,16 @@ describe 'third-party module', ->
       child.on 'message', (msg) ->
         assert.equal msg, 'ok'
         done()
+
+  describe 'pathwatcher', ->
+    it 'emits file events correctly', (done) ->
+      pathwatcher = require 'pathwatcher'
+      temp.mkdir 'dir', (err, dir) ->
+        assert err == null
+        file = path.join dir, 'file'
+        fs.writeFileSync file, 'content'
+        watcher = pathwatcher.watch file, (event) ->
+          assert.equal event, 'change'
+          watcher.close()
+          done()
+        fs.writeFileSync file, 'content2'

spec/node-spec.coffee

@@ -37,6 +37,11 @@ describe 'node feature', ->
         fs.readFile __filename, ->
           setTimeout done, 0
 
+    describe 'setTimeout in pure uv callback', ->
+      it 'does not crash', (done) ->
+        process.scheduleCallback ->
+          setTimeout done, 0
+
     describe 'throw error in node context', ->
       it 'gets caught', (done) ->
         error = new Error('boo!')