Fixed global object leak

test/cctest/test-object-observe.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 224 matching lines @@
   CompileRun("obj.foo = 'bar'");
   CHECK(CompileRun("ran")->BooleanValue());
 }
 
 
 TEST(GlobalObjectObservation) {
   HarmonyIsolate isolate;
   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;"
       "Object.observe(global, function(r) { [].push.apply(records, r) });"
       "global.foo = 'hello';");
   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';");
+  CompileRun("global.baz = 'again';");
   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(global_proxy->StrictEquals(CompileRun("records[1].object")));
 
   // Attached to a different context, should not leak mutations
   // to the old context.
   context->DetachGlobal();
   {
     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
   {
     // Delegates to Context::New
     LocalContext context3(
         isolate.GetIsolate(), NULL, Handle<ObjectTemplate>(), global_proxy);
     CompileRun(
         "var records3 = [];"
         "Object.observe(this, function(r) { [].push.apply(records3, r) });"
         "this.qux = 'context3';");
     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());
 }
 
 
 struct RecordExpectation {
   Handle<Value> object;
   const char* type;
   const char* name;
   Handle<Value> old_value;
 };
 
@@ skipping 451 matching lines @@
              "var observer = function(r) { records = r };"
              "Object.observe(obj, observer);");
   Handle<Value> obj =
       context->Global()->Get(String::NewFromUtf8(isolate.GetIsolate(), "obj"));
   Handle<Object>::Cast(obj)
       ->SetHiddenValue(String::NewFromUtf8(isolate.GetIsolate(), "foo"),
                        Null(isolate.GetIsolate()));
   CompileRun("");  // trigger delivery
   CHECK(CompileRun("records")->IsNull());
 }