Make the MediaQueryList listener an EventListener

Source/core/css/MediaQueryList.cpp

Index: Source/core/css/MediaQueryList.cpp
diff --git a/Source/core/css/MediaQueryList.cpp b/Source/core/css/MediaQueryList.cpp
index f21c99006e607f37c81dcf7ac24c23608039ddb1..b5ff9af4dde9556d418ef449a31e0aa1370c3e1b 100644
--- a/Source/core/css/MediaQueryList.cpp
+++ b/Source/core/css/MediaQueryList.cpp
@@ -24,6 +24,7 @@
 #include "core/css/MediaQueryEvaluator.h"
 #include "core/css/MediaQueryListListener.h"
 #include "core/css/MediaQueryMatcher.h"
+#include "core/dom/Document.h"
 
 namespace blink {
 
@@ -41,6 +42,7 @@ MediaQueryList::MediaQueryList(ExecutionContext* context, PassRefPtrWillBeRawPtr
     , m_matchesDirty(true)
     , m_matches(false)
 {
+    ScriptWrappable::init(this);
     m_matcher->addMediaQueryList(this);
     updateMatches();
 }
@@ -57,7 +59,17 @@ String MediaQueryList::media() const
     return m_media->mediaText();
 }
 
-void MediaQueryList::addListener(PassRefPtrWillBeRawPtr<MediaQueryListListener> listener)
+void MediaQueryList::addListener(PassRefPtr<EventListener> listener)
+{
+    addEventListener("change", listener, false);
+}
+
+void MediaQueryList::removeListener(PassRefPtr<EventListener> listener)
+{
+    removeEventListener("change", listener.get(), false);
+}
+
+void MediaQueryList::addMediaQueryListListener(PassRefPtrWillBeRawPtr<MediaQueryListListener> listener)

[haraken, 2014/07/20 05:14:49]

Just help me understand: Why do we want to use m_listeners for
addMediaQueryListListener/removeMediaQueryListListener whereas we redirect
addListener/removeListener to EventTarget::addEventListener/removeEventListener?

[cbiesinger, 2014/07/21 22:07:19]

I've been told that using DOM events for C++ listeners is discouraged;
consequently, I'm keeping a C++-specific way to add listeners. But I'm happy to
change that if it's fine to have C++ DOM event listeners.

[haraken, 2014/07/22 01:30:12]

I think Elliott would know better.

 {
     if (!listener)
         return;
@@ -66,7 +78,7 @@ void MediaQueryList::addListener(PassRefPtrWillBeRawPtr<MediaQueryListListener>
     m_listeners.add(listener);
 }
 
-void MediaQueryList::removeListener(PassRefPtrWillBeRawPtr<MediaQueryListListener> listener)
+void MediaQueryList::removeMediaQueryListListener(PassRefPtrWillBeRawPtr<MediaQueryListListener> listener)
 {
     if (!listener)
         return;
@@ -86,22 +98,24 @@ void MediaQueryList::removeListener(PassRefPtrWillBeRawPtr<MediaQueryListListene
 
 bool MediaQueryList::hasPendingActivity() const
 {
-    return m_listeners.size();
+    return m_listeners.size() || hasEventListeners(EventTypeNames::change);
 }
 
 void MediaQueryList::stop()
 {
     m_listeners.clear();
+    removeAllEventListeners();

[haraken, 2014/07/20 05:14:49]

This will remove all event listeners other than onchange listeners. Is it
expected?

[cbiesinger, 2014/07/21 22:07:19]

By my understanding of ActiveDOMEvent::stop that is expected, but I admit that I
don't understand that interface very well.

 }
 
-void MediaQueryList::mediaFeaturesChanged(WillBeHeapVector<RefPtrWillBeMember<MediaQueryListListener> >* listenersToNotify)
+bool MediaQueryList::mediaFeaturesChanged(WillBeHeapVector<RefPtrWillBeMember<MediaQueryListListener> >* listenersToNotify)
 {
     m_matchesDirty = true;
     if (!updateMatches())
-        return;
+        return false;
     for (ListenerList::const_iterator it = m_listeners.begin(), end = m_listeners.end(); it != end; ++it) {
         listenersToNotify->append(*it);
     }
+    return hasEventListeners(EventTypeNames::change);
 }
 
 bool MediaQueryList::updateMatches()
@@ -122,6 +136,7 @@ bool MediaQueryList::matches()
 
 void MediaQueryList::trace(Visitor* visitor)
 {
+    EventTargetWithInlineData::trace(visitor);

[haraken, 2014/07/20 05:14:49]

Nit: We normally make this a tail call of the trace() method.

[cbiesinger, 2014/07/21 22:07:19]

Done.

 #if ENABLE(OILPAN)
     visitor->trace(m_matcher);
     visitor->trace(m_media);
@@ -129,4 +144,14 @@ void MediaQueryList::trace(Visitor* visitor)
 #endif
 }
 
+const AtomicString& MediaQueryList::interfaceName() const
+{
+    return EventTargetNames::MediaQueryList;
+}
+
+ExecutionContext* MediaQueryList::executionContext() const
+{
+    return ActiveDOMObject::executionContext();
+}
+
 }