[wrapper-tracing] Bring remoteplayback up to date

third_party/WebKit/Source/bindings/templates/callback_function.h.tmpl

Index: third_party/WebKit/Source/bindings/templates/callback_function.h.tmpl
diff --git a/third_party/WebKit/Source/bindings/templates/callback_function.h.tmpl b/third_party/WebKit/Source/bindings/templates/callback_function.h.tmpl
index 52a6e0aa55e0bd3cc8418f1b068960df10605789..03e66568685f5141f462152918cd58d6d4650822 100644
--- a/third_party/WebKit/Source/bindings/templates/callback_function.h.tmpl
+++ b/third_party/WebKit/Source/bindings/templates/callback_function.h.tmpl
@@ -16,7 +16,8 @@ class ScriptState;
 class {{forward_declaration}};
 {% endfor %}
 
-class {{exported}}{{cpp_class}} final : public GarbageCollectedFinalized<{{cpp_class}}> {
+class {{exported}}{{cpp_class}} final : public GarbageCollectedFinalized<{{cpp_class}}>,
+                                        public TraceWrapperBase {

[Michael Lippautz, 2016/10/19 12:20:21]

If you think that the extra vtable ptr is too much, we would need to have
special handling in RemotePlayback::traceWrappers.

[haraken, 2016/10/19 12:32:38]

I don't think sizeof(callback function) really matters, so I think the vtable
ptr is fine.

However, would you help me understand why the callback function needs to inherit
from TraceWrapperBase? For example, we're tracing ElementRareData without making
ElementRareData TraceWrapperBase.

[Michael Lippautz, 2016/10/19 13:13:14]

Sure; ElementRareData and similar classes are part of the "special" cases in
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/heap/...

This macro generates the appropriate dispatchers. Ideally, all special cases
would just inherit from this TraceWrapperBase and we wouldn't need them.
However, I think for ElementRareData we want to save the vtable ptr. We could
revisit some of the cases and check whether they already have a vtable ptr and
if yes just inherit from TraceWrapperBase.

Additionally, we cannot use the special case macro for the callback in this CL
as the callback is part of modules, which ScriptWrappableVisitor and friends
(platform) cannot  see (linker).

[Marcel Hlopko, 2016/10/19 15:08:17]

To reiterate what Michael is saying, if a class is not inheriting from
ScriptWrappable, we need to tell somehow WrapperVisitor to trace it. Before we
only had WRAPPER_VISITOR_SPECIAL_CLASSES, where you had to add your class.
TraceWrapperBase solves the same problem, it marks the class so WrapperVisitor
traces it, but in a nicer way, because you do the change in your class, not in
some arcane WrapperVisitor.h file.

[haraken, 2016/10/19 15:31:43]

Thanks for the clarification -- yes, TraceWrappersBase definitely looks nicer.

I'm okay with handling ElementRareData (and a couple of classes where the object
size really matters) specially; e.g., keep using
WRAPPER_VISITOR_SPECIAL_CLASSES.

Also making ScriptWrappable inherit from TraceWrappersBase totally makes sense.

[Michael Lippautz, 2016/10/19 15:44:35]

Acknowledged.

 
 public:
     static {{cpp_class}}* create(v8::Isolate* isolate, v8::Local<v8::Function> callback)
@@ -27,6 +28,7 @@ public:
     ~{{cpp_class}}() = default;
 
     DECLARE_TRACE();
+    DECLARE_TRACE_WRAPPERS();
 
     bool call({{argument_declarations | join(', ')}});