From 26834b052402aa22fa1d5d5d791f18e82dfb390b Mon Sep 17 00:00:00 2001
From: Ryan Dahl <ry@tinyclouds.org>
Date: Thu, 8 Sep 2011 13:30:19 -0700
Subject: [PATCH] Revert "vm context with accessors"

This reverts commit 4527de8cba3bf5f107fcefcf43dbdaa308881ba4.

Causes segfault in test/message/undefined_reference_in_new_context.js
---
 lib/vm.js                                     | 13 -----
 src/node_script.cc                            | 57 +++++++------------
 .../test-vm-create-context-accessors.js       | 26 ---------
 3 files changed, 22 insertions(+), 74 deletions(-)
 delete mode 100644 test/simple/test-vm-create-context-accessors.js

diff --git a/lib/vm.js b/lib/vm.js
index 45f0f59150..d3348afac5 100644
--- a/lib/vm.js
+++ b/lib/vm.js
@@ -21,19 +21,6 @@
 
 var binding = process.binding('evals');
 
-binding.NodeScript._setCloneMethod(function(source, target) {
-  Object.getOwnPropertyNames(source).forEach(function(key) {
-    try {
-      var desc = Object.getOwnPropertyDescriptor(source, key);
-      if (desc.value === source) desc.value = target;
-
-      Object.defineProperty(target, key, desc);
-    } catch (e) {
-      // Catch sealed properties errors
-    }
-  });
-});
-
 exports.Script = binding.NodeScript;
 exports.createScript = function(code, ctx, name) {
   return new exports.Script(code, ctx, name);
diff --git a/src/node_script.cc b/src/node_script.cc
index 019597485f..4f4153caa9 100644
--- a/src/node_script.cc
+++ b/src/node_script.cc
@@ -37,11 +37,9 @@ using v8::TryCatch;
 using v8::String;
 using v8::Exception;
 using v8::Local;
-using v8::Null;
 using v8::Array;
 using v8::Persistent;
 using v8::Integer;
-using v8::Function;
 using v8::FunctionTemplate;
 
 
@@ -96,23 +94,10 @@ class WrappedScript : ObjectWrap {
   static Handle<Value> CompileRunInThisContext(const Arguments& args);
   static Handle<Value> CompileRunInNewContext(const Arguments& args);
 
-  static Handle<Value> SetCloneMethod(const Arguments& args);
-
   Persistent<Script> script_;
 };
 
 
-Persistent<Function> cloneObjectMethod;
-
-void CloneObject(Handle<Object> recv,
-                 Handle<Value> source, Handle<Value> target) {
-  HandleScope scope;
-
-  Handle<Value> args[] = {source, target};
-  cloneObjectMethod->Call(recv, 2, args);
-}
-
-
 void WrappedContext::Initialize(Handle<Object> target) {
   HandleScope scope;
 
@@ -192,10 +177,6 @@ void WrappedScript::Initialize(Handle<Object> target) {
                             "runInNewContext",
                             WrappedScript::RunInNewContext);
 
-  NODE_SET_PROTOTYPE_METHOD(constructor_template,
-                            "_setCloneMethod",
-                            WrappedScript::SetCloneMethod);
-
   NODE_SET_METHOD(constructor_template,
                   "createContext",
                   WrappedScript::CreateContext);
@@ -212,10 +193,6 @@ void WrappedScript::Initialize(Handle<Object> target) {
                   "runInNewContext",
                   WrappedScript::CompileRunInNewContext);
 
-  NODE_SET_METHOD(constructor_template,
-                  "_setCloneMethod",
-                  WrappedScript::SetCloneMethod);
-
   target->Set(String::NewSymbol("NodeScript"),
               constructor_template->GetFunction());
 }
@@ -248,8 +225,14 @@ Handle<Value> WrappedScript::CreateContext(const Arguments& args) {
 
   if (args.Length() > 0) {
     Local<Object> sandbox = args[0]->ToObject();
+    Local<Array> keys = sandbox->GetPropertyNames();
 
-    CloneObject(args.This(), sandbox, context);
+    for (uint32_t i = 0; i < keys->Length(); i++) {
+      Handle<String> key = keys->Get(Integer::New(i))->ToString();
+      Handle<Value> value = sandbox->Get(key);
+      if(value == sandbox) { value = context; }
+      context->Set(key, value);
+    }
   }
 
 
@@ -292,15 +275,6 @@ Handle<Value> WrappedScript::CompileRunInNewContext(const Arguments& args) {
     WrappedScript::EvalMachine<compileCode, newContext, returnResult>(args);
 }
 
-Handle<Value> WrappedScript::SetCloneMethod(const Arguments& args) {
-  HandleScope scope;
-
-  Local<Function> cloneObjectMethod_ = Local<Function>::Cast(args[0]);
-  cloneObjectMethod = Persistent<Function>::New(cloneObjectMethod_);
-
-  return scope.Close(Null());
-}
-
 
 template <WrappedScript::EvalInputFlags input_flag,
           WrappedScript::EvalContextFlags context_flag,
@@ -369,7 +343,14 @@ Handle<Value> WrappedScript::EvalMachine(const Arguments& args) {
 
     // Copy everything from the passed in sandbox (either the persistent
     // context for runInContext(), or the sandbox arg to runInNewContext()).
-    CloneObject(args.This(), sandbox, context->Global()->GetPrototype());
+    keys = sandbox->GetPropertyNames();
+
+    for (i = 0; i < keys->Length(); i++) {
+      Handle<String> key = keys->Get(Integer::New(i))->ToString();
+      Handle<Value> value = sandbox->Get(key);
+      if (value == sandbox) { value = context->Global(); }
+      context->Global()->Set(key, value);
+    }
   }
 
   // Catch errors
@@ -427,7 +408,13 @@ Handle<Value> WrappedScript::EvalMachine(const Arguments& args) {
 
   if (context_flag == userContext || context_flag == newContext) {
     // success! copy changes back onto the sandbox object.
-    CloneObject(args.This(), context->Global()->GetPrototype(), sandbox);
+    keys = context->Global()->GetPropertyNames();
+    for (i = 0; i < keys->Length(); i++) {
+      Handle<String> key = keys->Get(Integer::New(i))->ToString();
+      Handle<Value> value = context->Global()->Get(key);
+      if (value == context->Global()) { value = sandbox; }
+      sandbox->Set(key, value);
+    }
   }
 
   if (context_flag == newContext) {
diff --git a/test/simple/test-vm-create-context-accessors.js b/test/simple/test-vm-create-context-accessors.js
deleted file mode 100644
index 23fc935389..0000000000
--- a/test/simple/test-vm-create-context-accessors.js
+++ /dev/null
@@ -1,26 +0,0 @@
-var common = require('../common');
-var assert = require('assert');
-var vm = require('vm');
-
-var ctx = {};
-
-Object.defineProperty(ctx, 'getter', {
-  get: function() {
-    return 'ok';
-  }
-});
-
-var val;
-Object.defineProperty(ctx, 'setter', {
-  set: function(_val) {
-    val = _val;
-  },
-  get: function() {
-    return 'ok=' + val;
-  }
-});
-
-ctx = vm.createContext(ctx);
-
-var result = vm.runInContext('setter = "test";[getter,setter]', ctx);
-assert.deepEqual(result, ['ok', 'ok=test']);