blink::createClosure() is used only in ScriptFunction and two tests.

The use cases in the two tests are simply wrong, and we should remove it.

ScriptFunction needs to create a different instance of function object for each C++ instance, so we shouldn't use v8::FunctionTemplate.

I think we should remove the use cases in the two tests, and also remove createClosure() and inline it in ScriptFunction::bindToV8Function().

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6e8f53e38e8a711e7662626b3dd3fa5fd0106701

commit 6e8f53e38e8a711e7662626b3dd3fa5fd0106701
Author: yukishiino <yukishiino@chromium.org>
Date: Fri Jul 22 09:13:50 2016

Removes abuse of createClosure() from ScriptPromiseTest.

createClosure() is abused in ScriptPromise{,Resolver}Test,
so, simply removes it.

BUG=626266

Review-Url: https://codereview.chromium.org/2144413004
Cr-Commit-Position: refs/heads/master@{#407109}

[modify] https://crrev.com/6e8f53e38e8a711e7662626b3dd3fa5fd0106701/third_party/WebKit/Source/bindings/core/v8/ScriptPromiseResolverTest.cpp
[modify] https://crrev.com/6e8f53e38e8a711e7662626b3dd3fa5fd0106701/third_party/WebKit/Source/bindings/core/v8/ScriptPromiseTest.cpp
[modify] https://crrev.com/6e8f53e38e8a711e7662626b3dd3fa5fd0106701/third_party/WebKit/Source/bindings/core/v8/V8BindingForTesting.cpp

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/19832e78567e1a69a9d1f99761494b54b9aa67fa

commit 19832e78567e1a69a9d1f99761494b54b9aa67fa
Author: yukishiino <yukishiino@chromium.org>
Date: Mon Jul 25 11:52:09 2016

binding: Removes createClosure().

createClosure is used only at ScriptFunction::bindToV8Function,
and it shouldn't be used widely because it doesn't create
a v8::FunctionTemplate or reuse it.  Thus, this CL removes it.

BUG=626266

Review-Url: https://codereview.chromium.org/2173843002
Cr-Commit-Position: refs/heads/master@{#407451}

[modify] https://crrev.com/19832e78567e1a69a9d1f99761494b54b9aa67fa/third_party/WebKit/Source/bindings/core/v8/ScriptFunction.cpp
[modify] https://crrev.com/19832e78567e1a69a9d1f99761494b54b9aa67fa/third_party/WebKit/Source/bindings/core/v8/ScriptFunction.h
[modify] https://crrev.com/19832e78567e1a69a9d1f99761494b54b9aa67fa/third_party/WebKit/Source/bindings/core/v8/V8Binding.h

I was misunderstanding the issue and I was totally missing Jochen's point.  I reopen the issue.

Hi Jochen and Toon,

It seems like that V8 is now able to collect unused FunctionTemplates.  Maybe something changed in V8?  As I tested, FunctionTemplates seem collectable as same as any other objects, not limited to v8::Function::New case, but also weak v8::Persistent<v8::FunctionTemplate> is collected.

Am I understanding the situation correctly?

They are reclaimed when unused, but not when the function is still live (obviously).

From the description of the issue it sounds like we create multiple similar closures using createClosure (e.g., same underlying callback). If that's the case, we should create a single FunctionTemplate for that callback first, and then create the closures from that. That way we can share the FunctionTemplate and SharedFunctionInfo on the V8 side, and only need to allocate a much thinner JSFunction.

I suppose leak is the wrong word. Rather bloats.

That makes sense.

In case of ScriptFunction, v8::Function::New is expected to be called only once.

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/bindings/core/v8/ScriptFunction.h?rcl=a1356693554cd0bdb5b1cc4bc406896346fa3159&l=73

So it shouldn't be a big problem so far.

Thanks a lot.  The explanation lets me get the correct understanding.