V8PerIsolateData::getToStringName() and V8PerIsolateData::getToStringTemplate() are not used at all. We can remove it and refactor the code around them.
Created attachment 159621 [details] Patch
Comment on attachment 159621 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=159621&action=review > Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:2964 > - desc->Set(getToStringName(), getToStringTemplate()); > + desc->Set(v8::String::NewSymbol("toString"), v8::Persistent<v8::FunctionTemplate>::New(v8::FunctionTemplate::New(constructorToString))); This doesn't leak the persistent handle? m_toStringTemplate used to make us reuse the handle. I guess we only do this when configuration the template, so it's bounded in any case.
(In reply to comment #2) > > + desc->Set(v8::String::NewSymbol("toString"), v8::Persistent<v8::FunctionTemplate>::New(v8::FunctionTemplate::New(constructorToString))); > > This doesn't leak the persistent handle? m_toStringTemplate used to make us reuse the handle. I guess we only do this when configuration the template, so it's bounded in any case. Good point. Maybe the real problem is that the current code fails to store the created persistent handle to V8PerIsolateData? In the current code (and in the uploaded patch), every time getToStringTemplate() is called, a new persistent handle is created. We should cache the persistent handle to V8PerIsolateData.
Created attachment 159645 [details] Patch
(In reply to comment #3) > Good point. Maybe the real problem is that the current code fails to store the created persistent handle to V8PerIsolateData? In the current code (and in the uploaded patch), every time getToStringTemplate() is called, a new persistent handle is created. We should cache the persistent handle to V8PerIsolateData. Uploaded the patch to fix the issue.
Comment on attachment 159645 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=159645&action=review > Source/WebCore/bindings/v8/V8Binding.cpp:-354 > - v8::Persistent<v8::FunctionTemplate>& toStringTemplate = V8PerIsolateData::current()->toStringTemplate(); > - if (toStringTemplate.IsEmpty()) > - toStringTemplate = v8::Persistent<v8::FunctionTemplate>::New(v8::FunctionTemplate::New(constructorToString)); I mean, this used to store the persistent handle because toStringTemplate is a non-const reference. However, it's not very clear code! :) > Source/WebCore/bindings/v8/V8PerIsolateData.h:55 > +v8::Handle<v8::Value> constructorOfToString(const v8::Arguments&); Should we make this a static function in V8PerIsolateData? That way we could make it private.
(In reply to comment #6) > > Source/WebCore/bindings/v8/V8PerIsolateData.h:55 > > +v8::Handle<v8::Value> constructorOfToString(const v8::Arguments&); > > Should we make this a static function in V8PerIsolateData? That way we could make it private. Fixed and landed. Thanks.
Committed r126237: <http://trac.webkit.org/changeset/126237>