Fix ScriptState leak in ScriptPromiseProperty unittests.

Source/bindings/core/v8/ScriptPromisePropertyTest.cpp

Index: Source/bindings/core/v8/ScriptPromisePropertyTest.cpp
diff --git a/Source/bindings/core/v8/ScriptPromisePropertyTest.cpp b/Source/bindings/core/v8/ScriptPromisePropertyTest.cpp
index 58b81c37d03539ee1aa85ef8a512c606adb2b835..0d6f95941e77e5147e5ab221ec08ebbc80ce7ed7 100644
--- a/Source/bindings/core/v8/ScriptPromisePropertyTest.cpp
+++ b/Source/bindings/core/v8/ScriptPromisePropertyTest.cpp
@@ -126,14 +126,12 @@ public:
         : m_page(DummyPageHolder::create(IntSize(1, 1)))
     {
         v8::HandleScope handleScope(isolate());
-        m_otherScriptState = ScriptStateForTesting::create(v8::Context::New(isolate()), DOMWrapperWorld::create(isolate(), 1));
+        m_otherScriptState = ScriptStateForTesting::create(v8::Context::New(isolate()), DOMWrapperWorld::ensureIsolatedWorld(isolate(), 1, -1));
     }
 
     virtual ~ScriptPromisePropertyTestBase()
     {
-        m_page.clear();
-        gc();
-        Heap::collectAllGarbage();
+        ScriptPromisePropertyTestBase::destroyContext();
     }
 
     Document& document() { return m_page->document(); }
@@ -147,9 +145,12 @@ public:
     virtual void destroyContext()
     {
         m_page.clear();
-        m_otherScriptState.clear();
+        if (m_otherScriptState) {
+            m_otherScriptState->disposePerContextData();
+            m_otherScriptState = nullptr;
+        }
         gc();
-        Heap::collectGarbage(ThreadState::HeapPointersOnStack, ThreadState::GCWithSweep, Heap::ForcedGC);
+        Heap::collectAllGarbage();

[sof, 2015/08/31 06:44:40]

Incidental to this CL, but having the name of this Heap entry point indicate
that it doesn't perform conservative GCs would be helpful, arguably.

[haraken, 2015/08/31 06:46:42]

Yeah, a better idea would be to call V8GCController::collectGarbage multiple
times in gc(). And don't call Heap::collectAllGarbage() here.

[sof, 2015/08/31 06:50:22]

You have to supply the "force" flag in V8GCController::collectGarbage() somehow
though, which isn't done currently.

[haraken, 2015/08/31 06:55:52]

V8GCController::collectGarbage calls window.gc() and I guess the window.gc()
passes the forced flag somehow... (I can't find the call path at the moment
though.)

[sof, 2015/08/31 06:58:34]

This is a unit test -- isn't "gc()" mapping to what v8 provides via --expose-gc
?

[sof, 2015/08/31 07:03:46]

Which maps to v8/src/extensions/gc-extension.*

i.e., it will force conservative Oilpan GCs as part of running. Which means that
the use of collectAllGarbage() here is redundant + the addition of the
Persistent<>s in two tests aren't needed. Right?

[yhirano, 2015/08/31 07:10:43]

Once I was told that relying on the conservative GC might make tests flaky. I
want to ensure the holder wrapper which was stored in
ScriptPromisePropertyGarbageCollectedTest::m_holder is collected.

     }
 
     void gc() { V8GCController::collectGarbage(v8::Isolate::GetCurrent()); }
@@ -361,7 +362,7 @@ TEST_F(ScriptPromisePropertyGarbageCollectedTest, Reject_RejectsScriptPromise)
 
 TEST_F(ScriptPromisePropertyGarbageCollectedTest, Promise_DeadContext)
 {
-    Property* property = this->property();
+    Persistent<Property> property = this->property();
     property->resolve(new GarbageCollectedScriptWrappable("value"));
     EXPECT_EQ(Property::Resolved, property->state());
 
@@ -372,7 +373,7 @@ TEST_F(ScriptPromisePropertyGarbageCollectedTest, Promise_DeadContext)
 
 TEST_F(ScriptPromisePropertyGarbageCollectedTest, Resolve_DeadContext)
 {
-    Property* property = this->property();
+    Persistent<Property> property = this->property();
 
     {
         ScriptState::Scope scope(mainScriptState());