Fixed global object leak

test/cctest/test-object-observe.cc

Index: test/cctest/test-object-observe.cc
diff --git a/test/cctest/test-object-observe.cc b/test/cctest/test-object-observe.cc
index 4c32d44dcb3fa5d4c624068327b593b76c840e77..54fa9f9620a35056d794b392efe3ab4f9ecbfd43 100644
--- a/test/cctest/test-object-observe.cc
+++ b/test/cctest/test-object-observe.cc
@@ -242,7 +242,6 @@ TEST(GlobalObjectObservation) {
   LocalContext context(isolate.GetIsolate());
   HandleScope scope(isolate.GetIsolate());
   Handle<Object> global_proxy = context->Global();
-  Handle<Object> inner_global = global_proxy->GetPrototype().As<Object>();
   CompileRun(
       "var records = [];"
       "var global = this;"
@@ -251,21 +250,9 @@ TEST(GlobalObjectObservation) {
   CHECK_EQ(1, CompileRun("records.length")->Int32Value());
   CHECK(global_proxy->StrictEquals(CompileRun("records[0].object")));
 
-  // Detached, mutating the proxy has no effect.
-  context->DetachGlobal();
-  CompileRun("global.bar = 'goodbye';");
-  CHECK_EQ(1, CompileRun("records.length")->Int32Value());
-
-  // Mutating the global object directly still has an effect...
-  CompileRun("this.bar = 'goodbye';");
-  CHECK_EQ(2, CompileRun("records.length")->Int32Value());
-  CHECK(inner_global->StrictEquals(CompileRun("records[1].object")));

[adamk, 2013/12/03 21:46:12]

Seems like we still want to have a test for what happens if you cause a mutation
to the global (via, e.g., a closure); we want to make sure Object.observe
doesn't leak the global object in ChangeRecord.object.

Can you at least leave a TODO here to test that case (if adding such a test now
is too burdensome)?

-
-  // Reattached, back to global proxy.
-  context->ReattachGlobal(global_proxy);
   CompileRun("global.baz = 'again';");
-  CHECK_EQ(3, CompileRun("records.length")->Int32Value());
-  CHECK(global_proxy->StrictEquals(CompileRun("records[2].object")));
+  CHECK_EQ(2, CompileRun("records.length")->Int32Value());
+  CHECK(global_proxy->StrictEquals(CompileRun("records[1].object")));
 
   // Attached to a different context, should not leak mutations
   // to the old context.
@@ -273,15 +260,13 @@ TEST(GlobalObjectObservation) {
   {
     LocalContext context2(isolate.GetIsolate());
     context2->DetachGlobal();
-    context2->ReattachGlobal(global_proxy);
     CompileRun(
         "var records2 = [];"
         "Object.observe(this, function(r) { [].push.apply(records2, r) });"
         "this.bat = 'context2';");
-    CHECK_EQ(1, CompileRun("records2.length")->Int32Value());
-    CHECK(global_proxy->StrictEquals(CompileRun("records2[0].object")));
+    CHECK_EQ(0, CompileRun("records2.length")->Int32Value());
   }
-  CHECK_EQ(3, CompileRun("records.length")->Int32Value());
+  CHECK_EQ(2, CompileRun("records.length")->Int32Value());
 
   // Attaching by passing to Context::New
   {
@@ -295,7 +280,7 @@ TEST(GlobalObjectObservation) {
     CHECK_EQ(1, CompileRun("records3.length")->Int32Value());
     CHECK(global_proxy->StrictEquals(CompileRun("records3[0].object")));
   }
-  CHECK_EQ(3, CompileRun("records.length")->Int32Value());
+  CHECK_EQ(2, CompileRun("records.length")->Int32Value());
 }