[wrapper-tracing] Fix dispatching of TraceWrapperv8Reference

third_party/WebKit/Source/bindings/core/v8/TraceWrapperV8Reference.h

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef TraceWrapperV8Reference_h
+#define TraceWrapperV8Reference_h
+
+#include "bindings/core/v8/ScriptWrappableVisitor.h"
+
+namespace blink {
+
+/**
+ * TraceWrapperV8Reference is used to trace from Blink to V8. The reference is
+ * weak
+ * unless traced.

[haraken, 2016/11/07 14:27:58]

If the wrapper tracing is disabled, the reference is a weak ScopedPersistent.
Otherwise, the reference is (strongly) traced by the wrapper tracing.

[Michael Lippautz, 2016/11/07 15:07:15]

Done.

+ *
+ * TODO(mlippautz): Once shipped, create a separate type with a more
+ * appropriate handle type than v8::Persistent. The handle should have regular
+ * tracing semantics, i.e., only hold strongly on to an object if reached
+ * through tracing.
+ */
+template <typename T>
+class TraceWrapperV8Reference /*: public ScopedPersistent<T> */ {

[haraken, 2016/11/07 14:27:58]

Remove the comment.

[Michael Lippautz, 2016/11/07 15:07:15]

Done.

+ public:
+  explicit TraceWrapperV8Reference(void* parent) : m_parent(parent) {}
+
+  TraceWrapperV8Reference(v8::Isolate* isolate,
+                          void* parent,
+                          v8::Local<T> handle)
+      : m_handle(isolate, handle), m_parent(parent) {

[haraken, 2016/11/07 14:27:58]

It would be better to use internalSet. Then you can remove the below write
barrier.

[Michael Lippautz, 2016/11/07 15:07:15]

Done.

+    m_handle.SetWeak();

[haraken, 2016/11/07 14:27:58]

Can we use SetPhantom?

[Michael Lippautz, 2016/11/07 15:07:15]

SetWeak() without callbacks implements the phantom semantics. 

(It's exactly what ScopedPersistent::setPhantom() does.)

+    ScriptWrappableVisitor::writeBarrier(m_parent, &cast<v8::Value>());
+  }
+
+  ~TraceWrapperV8Reference() { clear(); }
+
+  void set(v8::Isolate* isolate, v8::Local<T> handle) {
+    internalSet(isolate, handle);
+    m_handle.SetWeak();

[haraken, 2016/11/07 14:27:58]

Can we use SetPhantom?

[Michael Lippautz, 2016/11/07 15:07:15]

See above.

+  }
+
+  template <typename P>
+  void set(v8::Isolate* isolate,
+           v8::Local<T> handle,
+           P* parameters,
+           void (*callback)(const v8::WeakCallbackInfo<P>&),
+           v8::WeakCallbackType type = v8::WeakCallbackType::kParameter) {
+    internalSet(isolate, handle);
+    m_handle.SetWeak(parameters, callback, type);
+  }
+
+  ALWAYS_INLINE v8::Local<T> newLocal(v8::Isolate* isolate) const {
+    return v8::Local<T>::New(isolate, m_handle);
+  }
+
+  bool isEmpty() const { return m_handle.IsEmpty(); }
+  void clear() { m_handle.Reset(); }
+  ALWAYS_INLINE v8::Persistent<T>& get() { return m_handle; }
+
+  void setReference(const v8::Persistent<v8::Object>& parent,
+                    v8::Isolate* isolate) {
+    isolate->SetReference(parent, m_handle);

[haraken, 2016/11/07 14:27:58]

This should not be called if the wrapper tracing is enabled. Can we add an
DCHECK about it?

[Michael Lippautz, 2016/11/07 15:07:15]

Done.

+  }
+
+  template <typename S>
+  const TraceWrapperV8Reference<S>& cast() const {

[Michael Lippautz, 2016/11/07 10:36:13]

This cast is not safe when we have vtable + templates (as we would just
reinterpret the vtable pointer).

However, if we want to avoid this cast, we need 3 dispatchers for markWrapper
(v8::Object, v8::Value, v8::Function), and potentially even more in future.

Since we anyways planned to untangle TraceWrapperV8Reference from
ScopedPersistent I think this is the better way to go.

wdyt?

[haraken, 2016/11/07 14:27:58]

Sounds reasonable to me.

+    return reinterpret_cast<const TraceWrapperV8Reference<S>&>(
+        const_cast<const TraceWrapperV8Reference<T>&>(*this));
+  }
+
+ private:
+  inline void internalSet(v8::Isolate* isolate, v8::Local<T> handle) {
+    m_handle.Reset(isolate, handle);
+    ScriptWrappableVisitor::writeBarrier(m_parent, &cast<v8::Value>());
+  }
+
+  v8::Persistent<T> m_handle;
+  void* m_parent;
+};
+
+}  // namespace blink
+
+#endif  // TraceWrapperV8Reference_h