InjectedScriptManager::CallbackData should be disposed when an associated worker thread terminates

Source/bindings/core/v8/custom/V8InjectedScriptManager.cpp

 /*
  * Copyright (C) 2012 Google Inc. 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
@@ skipping 28 matching lines @@
 #include "bindings/core/v8/V8InjectedScriptHost.h"
 #include "bindings/core/v8/V8ObjectConstructor.h"
 #include "bindings/core/v8/V8ScriptRunner.h"
 #include "bindings/core/v8/V8Window.h"
 #include "core/frame/LocalDOMWindow.h"
 #include "core/inspector/InjectedScriptHost.h"
 #include "wtf/RefPtr.h"
 
 namespace blink {
 
-struct InjectedScriptManager::CallbackData {
-    ScopedPersistent<v8::Object> handle;
-    RefPtrWillBePersistent<InjectedScriptHost> host;
-};
+InjectedScriptManager::CallbackData* InjectedScriptManager::createCallbackData(InjectedScriptManager* injectedScriptManager)
+{
+    OwnPtr<InjectedScriptManager::CallbackData> callbackData = adoptPtr(new InjectedScriptManager::CallbackData());
+    InjectedScriptManager::CallbackData* callbackDataPtr = callbackData.get();
+    callbackData->injectedScriptManager = injectedScriptManager;
+    m_callbackDataSet.add(callbackData.release());
+    return callbackDataPtr;
+}
 
-static v8::Local<v8::Object> createInjectedScriptHostV8Wrapper(InjectedScriptHost* host, v8::Isolate* isolate)
+void InjectedScriptManager::removeCallbackData(InjectedScriptManager::CallbackData* callbackData)
 {
-    v8::Local<v8::Function> function = V8InjectedScriptHost::domTemplate(isolate)->GetFunction();
-    if (function.IsEmpty()) {
-        // Return if allocation failed.
-        return v8::Local<v8::Object>();
-    }
-    v8::Local<v8::Object> instanceTemplate = V8ObjectConstructor::newInstance(isolate, function);
-    if (instanceTemplate.IsEmpty()) {
-        // Avoid setting the wrapper if allocation failed.
-        return v8::Local<v8::Object>();
-    }
-#if ENABLE(OILPAN)
-    V8DOMWrapper::setNativeInfoWithPersistentHandle(instanceTemplate, &V8InjectedScriptHost::wrapperTypeInfo, host, new Persistent<InjectedScriptHost>(host));
-#else
-    V8DOMWrapper::setNativeInfo(instanceTemplate, &V8InjectedScriptHost::wrapperTypeInfo, host);
-#endif
+    fprintf(stderr, "b %p\n", callbackData);

[aandrey, 2014/08/11 07:36:17]

this also got commited

[haraken, 2014/08/11 07:40:20]

oh, sorry. Let me fix it asap.

+    ASSERT(m_callbackDataSet.contains(callbackData));
+    fprintf(stderr, "c %p\n", callbackData);
+    m_callbackDataSet.remove(callbackData);
+    fprintf(stderr, "d %p\n", callbackData);
+}
+
+static v8::Local<v8::Object> createInjectedScriptHostV8Wrapper(PassRefPtrWillBeRawPtr<InjectedScriptHost> host, InjectedScriptManager* injectedScriptManager, v8::Handle<v8::Object> creationContext, v8::Isolate* isolate)
+{
+    ASSERT(host);
+
+    v8::Handle<v8::Object> wrapper = V8DOMWrapper::createWrapper(creationContext, &V8InjectedScriptHost::wrapperTypeInfo, V8InjectedScriptHost::toInternalPointer(host.get()), isolate);
+    if (UNLIKELY(wrapper.IsEmpty()))
+        return wrapper;
+
     // Create a weak reference to the v8 wrapper of InspectorBackend to deref
     // InspectorBackend when the wrapper is garbage collected.
-    InjectedScriptManager::CallbackData* data = new InjectedScriptManager::CallbackData;
-    data->host = host;
-    data->handle.set(isolate, instanceTemplate);
-    data->handle.setWeak(data, &InjectedScriptManager::setWeakCallback);
-    return instanceTemplate;
+    InjectedScriptManager::CallbackData* callbackData = injectedScriptManager->createCallbackData(injectedScriptManager);
+    callbackData->host = host.get();
+    callbackData->handle.set(isolate, wrapper);
+    callbackData->handle.setWeak(callbackData, &InjectedScriptManager::setWeakCallback);
+
+#if ENABLE(OILPAN)
+    V8DOMWrapper::setNativeInfoWithPersistentHandle(wrapper, &V8InjectedScriptHost::wrapperTypeInfo, V8InjectedScriptHost::toInternalPointer(host), &callbackData->host);
+#else
+    V8DOMWrapper::setNativeInfo(wrapper, &V8InjectedScriptHost::wrapperTypeInfo, V8InjectedScriptHost::toInternalPointer(host.get()));
+#endif
+    ASSERT(V8DOMWrapper::isDOMWrapper(wrapper));
+    return wrapper;
 }
 
 ScriptValue InjectedScriptManager::createInjectedScript(const String& scriptSource, ScriptState* inspectedScriptState, int id)
 {
     v8::Isolate* isolate = inspectedScriptState->isolate();
     ScriptState::Scope scope(inspectedScriptState);
 
     // Call custom code to create InjectedScripHost wrapper specific for the context
     // instead of calling toV8() that would create the
     // wrapper in the current context.
     // FIXME: make it possible to use generic bindings factory for InjectedScriptHost.
-    v8::Local<v8::Object> scriptHostWrapper = createInjectedScriptHostV8Wrapper(m_injectedScriptHost.get(), inspectedScriptState->isolate());
+    v8::Local<v8::Object> scriptHostWrapper = createInjectedScriptHostV8Wrapper(m_injectedScriptHost, this, inspectedScriptState->context()->Global(), inspectedScriptState->isolate());
     if (scriptHostWrapper.IsEmpty())
         return ScriptValue();
 
     // Inject javascript into the context. The compiled script is supposed to evaluate into
     // a single anonymous function(it's anonymous to avoid cluttering the global object with
     // inspector's stuff) the function is called a few lines below with InjectedScriptHost wrapper,
     // injected script id and explicit reference to the inspected global object. The function is expected
     // to create and configure InjectedScript instance that is going to be used by the inspector.
     v8::Local<v8::Value> value = V8ScriptRunner::compileAndRunInternalScript(v8String(isolate, scriptSource), isolate);
     ASSERT(!value.IsEmpty());
@@ skipping 14 matching lines @@
     v8::Handle<v8::Object> holder = V8Window::findInstanceInPrototypeChain(global, scriptState->isolate());
     if (holder.IsEmpty())
         return false;
     LocalFrame* frame = V8Window::toNative(holder)->frame();
 
     return BindingSecurity::shouldAllowAccessToFrame(scriptState->isolate(), frame, DoNotReportSecurityError);
 }
 
 void InjectedScriptManager::setWeakCallback(const v8::WeakCallbackData<v8::Object, InjectedScriptManager::CallbackData>& data)
 {
-    data.GetParameter()->handle.clear();
-    data.GetParameter()->host.clear();
-    delete data.GetParameter();
+    InjectedScriptManager::CallbackData* callbackData = data.GetParameter();
+    fprintf(stderr, "a %p\n", callbackData);
+    callbackData->injectedScriptManager->removeCallbackData(callbackData);
 }
 
 } // namespace blink