IDL compiler: AudioBuffer interfaces

Source/bindings/templates/interface.cpp

Index: Source/bindings/templates/interface.cpp
diff --git a/Source/bindings/templates/interface.cpp b/Source/bindings/templates/interface.cpp
index 6da556407f9643d017c75e1e983ae2a7afd06fc7..b2c25719437544e69fcecf84df015e9dc043faaa 100644
--- a/Source/bindings/templates/interface.cpp
+++ b/Source/bindings/templates/interface.cpp
@@ -710,6 +710,12 @@ v8::Handle<v8::Object> {{v8_class}}::createWrapper(PassRefPtr<{{cpp_class}}> imp
     if (UNLIKELY(wrapper.IsEmpty()))
         return wrapper;
 
+    {% if is_audio_buffer %}
+    for (unsigned i = 0, n = impl->numberOfChannels(); i < n; i++) {
+        Float32Array* channelData = impl->getChannelData(i);
+        channelData->buffer()->setDeallocationObserver(V8ArrayBufferDeallocationObserver::instanceTemplate());

[haraken, 2014/01/09 08:52:58]

I don't fully understand why this code is written here. It seems to me that
deallocationObserver should be set when we allocate a C++ AudioBuffer object not
when we allocate a wrapper of the C++ object.

Would you cc AudioBuffer people?

[Nils Barth (inactive), 2014/01/09 11:24:45]

Will do!

[Nils Barth (inactive), 2014/01/10 01:37:10]

Hi Raymond,
We're currently rewriting the IDL compiler (bindings generator) in Python, and
we're checking anything puzzling as we go.

haraken had a question about why there is special-case code for AudioBuffer
here, namely why we're setting the deallocation observer in the bindings (when
we create a wrapper), rather than when we constructor the underlying C++ object
in Blink.

Any insights?
Thanks!

On 2014/01/09 08:52:58, haraken wrote:

[Ken Russell (switch to Gerrit), 2014/01/10 02:31:31]

+dslomov

On 2014/01/10 01:37:10, Nils Barth wrote:
I'm not sure -- dslomov@ probably knows better -- but I suspect the reason is
that we don't want to set deallocation observers for all ArrayBuffer instances,
because doing so will hugely slow down the new V8 implementation of typed
arrays.

[Nils Barth (inactive), 2014/01/14 07:39:45]

*ping* Dmitry: could you PTAL? Thanks!

[Dmitry Lomov (no reviews), 2014/01/14 08:37:58]

Yes, sorry, slipped through the cracks.
We only setDeallocationObservers on array buffers that are held by some object
in JavaScript heap. The contract is, roughly, "if ArrayBuffer has a deallocation
observer then there is an object in V8 heap that holds it (maybe indirectly)".
The reason why it is important to only setDeallocationObservers only when array
buffer becomes exposed to JS is that on setting the observer V8 gets notified
that its heap now references the certain amount of external memory. If you set
the observer in the constructor, V8 will get notified when the AudioBuffer is
created, not when it is exposed to JS.

Does this make sense?

[haraken, 2014/01/14 09:27:06]

What we want is to notify V8 the fact that Blink allocated a large amount of
memory, not the fact that something is exposed to JS, isn't it? In that sense, I
thought that we should notify the fact to V8 when we allocate the ArrayBuffer,
not when we expose the ArrayBuffer to JS.

+    }
+    {% endif %}
     installPerContextEnabledProperties(wrapper, impl.get(), isolate);
     {% set wrapper_configuration = 'WrapperConfiguration::Dependent'
                                    if (has_visit_dom_wrapper or