[wrapper-tracing] Trace EventListener_s through EventListenerMap
Aligns behavior as closely as possible to Oilpan and remove the
manual write barrier when attaching event listeners.
Bug:
Change-Id: I39d421e53cd2779fdb97a0a3c996ebc63a8918a9
Reviewed-on: https://chromium-review.googlesource.com/618888
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#495275}diff --git a/third_party/WebKit/Source/bindings/core/v8/V8AbstractEventListener.h b/third_party/WebKit/Source/bindings/core/v8/V8AbstractEventListener.h
index a99866a..996fcc3 100644
--- a/third_party/WebKit/Source/bindings/core/v8/V8AbstractEventListener.h
+++ b/third_party/WebKit/Source/bindings/core/v8/V8AbstractEventListener.h
@@ -52,8 +52,7 @@
// Why does this matter?
// WebKit does not allow duplicated HTML event handlers of the same type,
// but ALLOWs duplicated non-HTML event handlers.
-class CORE_EXPORT V8AbstractEventListener : public EventListener,
- public TraceWrapperBase {
+class CORE_EXPORT V8AbstractEventListener : public EventListener {
public:
~V8AbstractEventListener() override;
diff --git a/third_party/WebKit/Source/core/dom/Node.cpp b/third_party/WebKit/Source/core/dom/Node.cpp
index 84702ed0..0481aff4 100644
--- a/third_party/WebKit/Source/core/dom/Node.cpp
+++ b/third_party/WebKit/Source/core/dom/Node.cpp
@@ -2002,7 +2002,8 @@
}
using EventTargetDataMap =
- PersistentHeapHashMap<WeakMember<Node>, Member<EventTargetData>>;
+ PersistentHeapHashMap<WeakMember<Node>,
+ TraceWrapperMember<EventTargetData>>;
static EventTargetDataMap& GetEventTargetDataMap() {
DEFINE_STATIC_LOCAL(EventTargetDataMap, map, ());
return map;
@@ -2639,6 +2640,7 @@
visitor->Trace(RareData());
visitor->Trace(tree_scope_);
+ // EventTargetData is traced through EventTargetDataMap.
EventTarget::Trace(visitor);
}
@@ -2648,6 +2650,8 @@
visitor->TraceWrappers(next_);
if (HasRareData())
visitor->TraceWrappersWithManualWriteBarrier(RareData());
+ visitor->TraceWrappersWithManualWriteBarrier(
+ const_cast<Node*>(this)->GetEventTargetData());
EventTarget::TraceWrappers(visitor);
}
diff --git a/third_party/WebKit/Source/core/events/EventListener.h b/third_party/WebKit/Source/core/events/EventListener.h
index 03045cc..fd1b362 100644
--- a/third_party/WebKit/Source/core/events/EventListener.h
+++ b/third_party/WebKit/Source/core/events/EventListener.h
@@ -22,6 +22,7 @@
#define EventListener_h
#include "core/CoreExport.h"
+#include "platform/bindings/ScriptWrappable.h"
#include "platform/heap/Handle.h"
#include "platform/wtf/text/WTFString.h"
@@ -31,7 +32,8 @@
class ExecutionContext;
class CORE_EXPORT EventListener
- : public GarbageCollectedFinalized<EventListener> {
+ : public GarbageCollectedFinalized<EventListener>,
+ public TraceWrapperBase {
public:
enum ListenerType {
kJSEventListenerType,
@@ -54,6 +56,7 @@
ListenerType GetType() const { return type_; }
DEFINE_INLINE_VIRTUAL_TRACE() {}
+ DEFINE_INLINE_VIRTUAL_TRACE_WRAPPERS() {}
protected:
explicit EventListener(ListenerType type) : type_(type) {}
diff --git a/third_party/WebKit/Source/core/events/EventListenerMap.cpp b/third_party/WebKit/Source/core/events/EventListenerMap.cpp
index fbf07c3..8d6e3bc 100644
--- a/third_party/WebKit/Source/core/events/EventListenerMap.cpp
+++ b/third_party/WebKit/Source/core/events/EventListenerMap.cpp
@@ -209,45 +209,13 @@
visitor->Trace(entries_);
}
-EventListenerIterator::EventListenerIterator(EventTarget* target)
- : map_(nullptr), entry_index_(0), index_(0) {
- DCHECK(target);
- EventTargetData* data = target->GetEventTargetData();
-
- if (!data)
- return;
-
- map_ = &data->event_listener_map;
-
-#if DCHECK_IS_ON()
- {
- MutexLocker locker(ActiveIteratorCountMutex());
- map_->active_iterator_count_++;
+DEFINE_TRACE_WRAPPERS(EventListenerMap) {
+ // Trace wrappers in entries_.
+ for (auto& entry : entries_) {
+ for (auto& listener : *entry.second) {
+ visitor->TraceWrappers(listener);
+ }
}
-#endif
-}
-
-#if DCHECK_IS_ON()
-EventListenerIterator::~EventListenerIterator() {
- if (map_) {
- MutexLocker locker(ActiveIteratorCountMutex());
- map_->active_iterator_count_--;
- }
-}
-#endif
-
-EventListener* EventListenerIterator::NextListener() {
- if (!map_)
- return nullptr;
-
- for (; entry_index_ < map_->entries_.size(); ++entry_index_) {
- EventListenerVector& listeners = *map_->entries_[entry_index_].second;
- if (index_ < listeners.size())
- return listeners[index_++].Listener();
- index_ = 0;
- }
-
- return nullptr;
}
} // namespace blink
diff --git a/third_party/WebKit/Source/core/events/EventListenerMap.h b/third_party/WebKit/Source/core/events/EventListenerMap.h
index ca45eb0..664e40ca 100644
--- a/third_party/WebKit/Source/core/events/EventListenerMap.h
+++ b/third_party/WebKit/Source/core/events/EventListenerMap.h
@@ -37,6 +37,7 @@
#include "core/events/AddEventListenerOptionsResolved.h"
#include "core/events/EventListenerOptions.h"
#include "core/events/RegisteredEventListener.h"
+#include "platform/bindings/ScriptWrappableVisitor.h"
#include "platform/wtf/Noncopyable.h"
#include "platform/wtf/text/AtomicStringHash.h"
@@ -73,6 +74,7 @@
void CopyEventListenersNotCreatedFromMarkupToTarget(EventTarget*);
DECLARE_TRACE();
+ DECLARE_TRACE_WRAPPERS();
private:
friend class EventListenerIterator;
@@ -90,26 +92,6 @@
#endif
};
-class EventListenerIterator {
- WTF_MAKE_NONCOPYABLE(EventListenerIterator);
- STACK_ALLOCATED();
-
- public:
- explicit EventListenerIterator(EventTarget*);
-#if DCHECK_IS_ON()
- ~EventListenerIterator();
-#endif
-
- EventListener* NextListener();
-
- private:
- // This cannot be a Member because it is pointing to a part of object.
- // TODO(haraken): Use Member<EventTarget> instead of EventListenerMap*.
- EventListenerMap* map_;
- unsigned entry_index_;
- unsigned index_;
-};
-
#if !DCHECK_IS_ON()
inline void EventListenerMap::CheckNoActiveIterators() {}
#endif
diff --git a/third_party/WebKit/Source/core/events/EventTarget.cpp b/third_party/WebKit/Source/core/events/EventTarget.cpp
index 42e75ff..ad67357 100644
--- a/third_party/WebKit/Source/core/events/EventTarget.cpp
+++ b/third_party/WebKit/Source/core/events/EventTarget.cpp
@@ -155,14 +155,8 @@
visitor->Trace(event_listener_map);
}
-DEFINE_TRACE_WRAPPERS(EventTarget) {
- EventListenerIterator iterator(const_cast<EventTarget*>(this));
- while (EventListener* listener = iterator.NextListener()) {
- if (listener->GetType() != EventListener::kJSEventListenerType)
- continue;
- visitor->TraceWrappersWithManualWriteBarrier(
- static_cast<V8AbstractEventListener*>(listener));
- }
+DEFINE_TRACE_WRAPPERS(EventTargetData) {
+ visitor->TraceWrappers(event_listener_map);
}
EventTarget::EventTarget() {}
@@ -357,10 +351,6 @@
bool added = EnsureEventTargetData().event_listener_map.Add(
event_type, listener, options, ®istered_listener);
if (added) {
- if (listener->GetType() == EventListener::kJSEventListenerType) {
- ScriptWrappableVisitor::WriteBarrier(
- static_cast<V8AbstractEventListener*>(listener));
- }
AddedEventListener(event_type, registered_listener);
}
return added;
diff --git a/third_party/WebKit/Source/core/events/EventTarget.h b/third_party/WebKit/Source/core/events/EventTarget.h
index 9be4bcd..5c8b23bd 100644
--- a/third_party/WebKit/Source/core/events/EventTarget.h
+++ b/third_party/WebKit/Source/core/events/EventTarget.h
@@ -79,11 +79,14 @@
~EventTargetData();
DECLARE_TRACE();
+ DECLARE_TRACE_WRAPPERS();
EventListenerMap event_listener_map;
std::unique_ptr<FiringEventIteratorVector> firing_event_iterators;
};
+DEFINE_TRAIT_FOR_TRACE_WRAPPERS(EventTargetData);
+
// All DOM event targets extend EventTarget. The spec is defined here:
// https://dom.spec.whatwg.org/#interface-eventtarget
// EventTarget objects allow us to add and remove an event
@@ -173,8 +176,7 @@
static DispatchEventResult GetDispatchEventResult(const Event&);
DEFINE_INLINE_VIRTUAL_TRACE() {}
-
- DECLARE_VIRTUAL_TRACE_WRAPPERS();
+ DEFINE_INLINE_VIRTUAL_TRACE_WRAPPERS() {}
virtual bool KeepEventInNode(Event*) { return false; }
@@ -234,6 +236,11 @@
EventTarget::Trace(visitor);
}
+ DEFINE_INLINE_VIRTUAL_TRACE_WRAPPERS() {
+ visitor->TraceWrappers(event_target_data_);
+ EventTarget::TraceWrappers(visitor);
+ }
+
protected:
EventTargetData* GetEventTargetData() final { return &event_target_data_; }
EventTargetData& EnsureEventTargetData() final { return event_target_data_; }
diff --git a/third_party/WebKit/Source/core/events/RegisteredEventListener.h b/third_party/WebKit/Source/core/events/RegisteredEventListener.h
index 6d8ea9e6..1909a0f 100644
--- a/third_party/WebKit/Source/core/events/RegisteredEventListener.h
+++ b/third_party/WebKit/Source/core/events/RegisteredEventListener.h
@@ -27,6 +27,7 @@
#include "core/events/AddEventListenerOptionsResolved.h"
#include "core/events/EventListener.h"
+#include "platform/bindings/TraceWrapperMember.h"
#include "platform/wtf/RefPtr.h"
namespace blink {
@@ -55,6 +56,7 @@
passive_specified_(options.PassiveSpecified()) {}
DEFINE_INLINE_TRACE() { visitor->Trace(listener_); }
+ DEFINE_INLINE_TRACE_WRAPPERS() { visitor->TraceWrappers(listener_); }
AddEventListenerOptionsResolved Options() const {
AddEventListenerOptionsResolved result;
@@ -108,7 +110,7 @@
}
private:
- Member<EventListener> listener_;
+ TraceWrapperMember<EventListener> listener_;
unsigned use_capture_ : 1;
unsigned passive_ : 1;
unsigned once_ : 1;
diff --git a/third_party/WebKit/Source/platform/bindings/ScriptWrappableVisitor.h b/third_party/WebKit/Source/platform/bindings/ScriptWrappableVisitor.h
index 439161f..cb745ad 100644
--- a/third_party/WebKit/Source/platform/bindings/ScriptWrappableVisitor.h
+++ b/third_party/WebKit/Source/platform/bindings/ScriptWrappableVisitor.h
@@ -170,13 +170,21 @@
// Member and |TraceWrappersWithManualWriteBarrier()|. See below.
template <typename T>
void TraceWrappers(const TraceWrapperMember<T>& t) const {
- TraceWrappers(t.Get());
+ MarkAndTraceWrappers(t.Get());
+ }
+
+ // Enable partial tracing of objects. This is used when tracing interior
+ // objects without their own header.
+ template <typename T>
+ void TraceWrappers(const T& traceable) const {
+ static_assert(sizeof(T), "T must be fully defined");
+ traceable.TraceWrappers(this);
}
// Only called from automatically generated bindings code.
template <typename T>
void TraceWrappersFromGeneratedCode(const T* traceable) const {
- TraceWrappers(traceable);
+ MarkAndTraceWrappers(traceable);
}
// Require all users of manual write barriers to make this explicit in their
@@ -185,17 +193,17 @@
// the field. Otherwise, the objects may be collected prematurely.
template <typename T>
void TraceWrappersWithManualWriteBarrier(const Member<T>& t) const {
- TraceWrappers(t.Get());
+ MarkAndTraceWrappers(t.Get());
}
template <typename T>
void TraceWrappersWithManualWriteBarrier(const WeakMember<T>& t) const {
- TraceWrappers(t.Get());
+ MarkAndTraceWrappers(t.Get());
}
template <typename T>
void TraceWrappersWithManualWriteBarrier(const T* traceable) const {
- TraceWrappers(traceable);
+ MarkAndTraceWrappers(traceable);
}
virtual void DispatchTraceWrappers(const TraceWrapperBase*) const;
@@ -230,7 +238,7 @@
}
template <typename T>
- void TraceWrappers(const T* traceable) const {
+ void MarkAndTraceWrappers(const T* traceable) const {
static_assert(sizeof(T), "T must be fully defined");
if (!traceable) {
