Changing the semantics of cross-frame eval to be compatible with Safari and Firefox.

Review URL: http://codereview.chromium.org/11601

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@781 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/runtime.cc

@@ -3956,29 +3956,8 @@ static Object* EvalContext() {
   Handle<Context> target = Top::global_context();
   if (caller->global_context() == *target) return *caller;
 
-  // Compute a function closure that captures the calling context. We
-  // need a function that has trivial scope info, since it is only
-  // used to hold the context chain together.
-  Handle<JSFunction> closure = Factory::NewFunction(Factory::empty_symbol(),
-                                                    Factory::undefined_value());
-  closure->set_context(*caller);
-
-  // Create a new adaptor context that has the target environment as
-  // the extension object. This enables the evaluated code to see both
-  // the current context with locals and everything and to see global
-  // variables declared in the target global object. Furthermore, any
-  // properties introduced with 'var' will be added to the target
-  // global object because it is the extension object.
-  Handle<Context> adaptor =
-    Factory::NewFunctionContext(Context::MIN_CONTEXT_SLOTS, closure);
-  adaptor->set_extension(target->global());
-  return *adaptor;
-}
-
-
-static Object* Runtime_EvalReceiver(Arguments args) {
-  StackFrameLocator locator;
-  return locator.FindJavaScriptFrame(1)->receiver();
+  // Otherwise, use the global context from the other environment.
+  return *target;
 }
 
 

src/runtime.h

@@ -195,7 +195,6 @@ namespace v8 { namespace internal {
   F(GlobalPrint, 1) \
   \
   /* Eval */ \
-  F(EvalReceiver, 1) \
   F(GlobalReceiver, 1) \
   \
   F(SetProperty, -1 /* 3 or 4 */) \

src/v8natives.js

@@ -113,7 +113,7 @@ function GlobalEval(x) {
   var f = %CompileString(x, 0, true);
   if (!IS_FUNCTION(f)) return f;
 
-  return f.call(%EvalReceiver(this));
+  return f.call(this);
 }
 
 

test/cctest/test-api.cc

@@ -4018,6 +4018,22 @@ THREADED_TEST(FunctionDescriptorException) {
 }
 
 
+THREADED_TEST(Eval) {
+  v8::HandleScope scope;
+  LocalContext current;
+
+  // Test that un-aliased eval uses local context.
+  Local<Script> script =
+      Script::Compile(v8_str("foo = 0;"
+                             "(function() {"
+                             "  var foo = 2;"
+                             "  return eval('foo');"
+                             "})();"));
+  Local<Value> foo = script->Run();
+  CHECK_EQ(2, foo->Int32Value());
+}
+
+
 THREADED_TEST(CrossEval) {
   v8::HandleScope scope;
   LocalContext other;
@@ -4039,49 +4055,58 @@ THREADED_TEST(CrossEval) {
   CHECK(!current->Global()->Has(v8_str("foo")));
 
   // Check that writing to non-existing properties introduces them in
-  // the current context.
+  // the other context.
   script =
       Script::Compile(v8_str("other.eval('na = 1234')"));
   script->Run();
-  CHECK_EQ(1234, current->Global()->Get(v8_str("na"))->Int32Value());
-  CHECK(!other->Global()->Has(v8_str("na")));
+  CHECK_EQ(1234, other->Global()->Get(v8_str("na"))->Int32Value());
+  CHECK(!current->Global()->Has(v8_str("na")));
 
-  // Check that variables in current context are visible in other
-  // context. This must include local variables.
+  // Check that global variables in current context are not visible in other
+  // context.
+  v8::TryCatch try_catch;
   script =
-      Script::Compile(v8_str("var bar = 42;"
-                                      "(function() { "
-                                      "  var baz = 87;"
-                                      "  return other.eval('bar + baz');"
-                                      "})();"));
+      Script::Compile(v8_str("var bar = 42; other.eval('bar');"));
   Local<Value> result = script->Run();
-  CHECK_EQ(42 + 87, result->Int32Value());
+  CHECK(try_catch.HasCaught());
+  try_catch.Reset();
+
+  // Check that local variables in current context are not visible in other
+  // context.
+  script =
+      Script::Compile(v8_str("(function() { "
+                             "  var baz = 87;"
+                             "  return other.eval('baz');"
+                             "})();"));
+  result = script->Run();
+  CHECK(try_catch.HasCaught());
+  try_catch.Reset();
 
   // Check that global variables in the other environment are visible
   // when evaluting code.
   other->Global()->Set(v8_str("bis"), v8_num(1234));
   script = Script::Compile(v8_str("other.eval('bis')"));
   CHECK_EQ(1234, script->Run()->Int32Value());
+  CHECK(!try_catch.HasCaught());
 
-  // Check that the 'this' pointer isn't touched as a result of
-  // calling eval across environments.
-  script =
-      Script::Compile(v8_str("var t = this; other.eval('this == t')"));
+  // Check that the 'this' pointer points to the global object evaluating
+  // code.
+  other->Global()->Set(v8_str("t"), other->Global());
+  script = Script::Compile(v8_str("other.eval('this == t')"));
   result = script->Run();
-  CHECK(result->IsBoolean());
-  CHECK(result->BooleanValue());
+  CHECK(result->IsTrue());
+  CHECK(!try_catch.HasCaught());
 
-  // Check that doing a cross eval works from within a global
-  // with-statement.
+  // Check that variables introduced in with-statement are not visible in
+  // other context.
   script =
-      Script::Compile(v8_str("other.y = 1;"
-                                      "with({x:2}){other.eval('x+y')}"));
+      Script::Compile(v8_str("with({x:2}){other.eval('x')}"));
   result = script->Run();
-  CHECK_EQ(3, result->Int32Value());
+  CHECK(try_catch.HasCaught());
+  try_catch.Reset();
 
   // Check that you cannot use 'eval.call' with another object than the
   // current global object.
-  v8::TryCatch try_catch;
   script =
       Script::Compile(v8_str("other.y = 1; eval.call(other, 'y')"));
   result = script->Run();