Make the MediaQueryList listener an EventListener

Source/core/css/MediaQueryList.cpp

 /*
  * Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies)
  *
  *  This library is free software; you can redistribute it and/or
  *  modify it under the terms of the GNU Library General Public
  *  License as published by the Free Software Foundation; either
  *  version 2 of the License, or (at your option) any later version.
  *
  *  This library is distributed in the hope that it will be useful,
  *  but WITHOUT ANY WARRANTY; without even the implied warranty of
  *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
  *  Library General Public License for more details.
  *
  *  You should have received a copy of the GNU Library General Public License
  *  along with this library; see the file COPYING.LIB.  If not, write to
  *  the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
  *  Boston, MA 02110-1301, USA.
  */
 
 #include "config.h"
 #include "core/css/MediaQueryList.h"
 
 #include "core/css/MediaList.h"
 #include "core/css/MediaQueryEvaluator.h"
 #include "core/css/MediaQueryListListener.h"
 #include "core/css/MediaQueryMatcher.h"
+#include "core/dom/Document.h"
 
 namespace blink {
 
 PassRefPtrWillBeRawPtr<MediaQueryList> MediaQueryList::create(ExecutionContext* context, PassRefPtrWillBeRawPtr<MediaQueryMatcher> matcher, PassRefPtrWillBeRawPtr<MediaQuerySet> media)
 {
     RefPtrWillBeRawPtr<MediaQueryList> list = adoptRefWillBeNoop(new MediaQueryList(context, matcher, media));
     list->suspendIfNeeded();
     return list.release();
 }
 
 MediaQueryList::MediaQueryList(ExecutionContext* context, PassRefPtrWillBeRawPtr<MediaQueryMatcher> matcher, PassRefPtrWillBeRawPtr<MediaQuerySet> media)
     : ActiveDOMObject(context)
     , m_matcher(matcher)
     , m_media(media)
     , m_matchesDirty(true)
     , m_matches(false)
 {
+    ScriptWrappable::init(this);
     m_matcher->addMediaQueryList(this);
     updateMatches();
 }
 
 MediaQueryList::~MediaQueryList()
 {
 #if !ENABLE(OILPAN)
     m_matcher->removeMediaQueryList(this);
 #endif
 }
 
 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;
 
     listener->setMediaQueryList(this);
     m_listeners.add(listener);
 }
 
-void MediaQueryList::removeListener(PassRefPtrWillBeRawPtr<MediaQueryListListener> listener)
+void MediaQueryList::removeMediaQueryListListener(PassRefPtrWillBeRawPtr<MediaQueryListListener> listener)
 {
     if (!listener)
         return;
 
     RefPtrWillBeRawPtr<MediaQueryList> protect(this);
     listener->clearMediaQueryList();
 
     for (ListenerList::iterator it = m_listeners.begin(), end = m_listeners.end(); it != end; ++it) {
         // We can't just use m_listeners.remove() here, because we get a new wrapper for the
         // listener callback every time. We have to use MediaQueryListListener::operator==.
         if (**it == *listener.get()) {
             m_listeners.remove(it);
             break;
         }
     }
 }
 
 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()
 {
     m_matchesDirty = false;
     if (m_matches != m_matcher->evaluate(m_media.get())) {
         m_matches = !m_matches;
         return true;
     }
     return false;
 }
 
 bool MediaQueryList::matches()
 {
     updateMatches();
     return m_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);
     visitor->trace(m_listeners);
 #endif
 }
 
+const AtomicString& MediaQueryList::interfaceName() const
+{
+    return EventTargetNames::MediaQueryList;
 }
+
+ExecutionContext* MediaQueryList::executionContext() const
+{
+    return ActiveDOMObject::executionContext();
+}
+
+}