Rework MediaQueryMatcher to batch up listener notification

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 d76b5bbec1599f0b80432c1c55c7184e98a28e46..3a1b9345b3907ea8ec6bb7837f78ea56d0eff2f4 100644
--- a/Source/core/css/MediaQueryList.cpp
+++ b/Source/core/css/MediaQueryList.cpp
@@ -27,21 +27,26 @@
 
 namespace WebCore {
 
-PassRefPtrWillBeRawPtr<MediaQueryList> MediaQueryList::create(PassRefPtrWillBeRawPtr<MediaQueryMatcher> vector, PassRefPtrWillBeRawPtr<MediaQuerySet> media, bool matches)
+PassRefPtrWillBeRawPtr<MediaQueryList> MediaQueryList::create(PassRefPtrWillBeRawPtr<MediaQueryMatcher> matcher, PassRefPtrWillBeRawPtr<MediaQuerySet> media)
 {
-    return adoptRefWillBeNoop(new MediaQueryList(vector, media, matches));
+    return adoptRefWillBeNoop(new MediaQueryList(matcher, media));
 }
 
-MediaQueryList::MediaQueryList(PassRefPtrWillBeRawPtr<MediaQueryMatcher> vector, PassRefPtrWillBeRawPtr<MediaQuerySet> media, bool matches)
-    : m_matcher(vector)
+MediaQueryList::MediaQueryList(PassRefPtrWillBeRawPtr<MediaQueryMatcher> matcher, PassRefPtrWillBeRawPtr<MediaQuerySet> media)
+    : m_matcher(matcher)
     , m_media(media)
-    , m_evaluationRound(m_matcher->evaluationRound())
-    , m_changeRound(m_evaluationRound - 1) // m_evaluationRound and m_changeRound initial values must be different.
-    , m_matches(matches)
+    , m_matchesDirty(true)
+    , m_matches(false)
 {
+    m_matcher->addMediaQueryList(this);
+    updateMatches();
 }
 
-DEFINE_EMPTY_DESTRUCTOR_WILL_BE_REMOVED(MediaQueryList)
+MediaQueryList::~MediaQueryList()
+{
+    // FIXME: Figure out how to deal with this in oilpan.

[Mads Ager (chromium), 2014/06/26 06:06:22]

Right, this will not work for the oilpan build since m_matcher can be dead in
the same GC cycle and could therefore already be destructed. If we cannot avoid
tie together the destruction and unregistration of the object we should use
Oilpan weakness.

In order to do that: guard this with #if !ENABLE(OILPAN) and update the
MediaQueryMatcher collection to use RawPtrWillBeWeakMember instead of
RawPtrWillBeMember. Then oilpan will remove the list from the collection when it
dies automatically.

[cbiesinger, 2014/06/26 20:27:59]

Done.

+    m_matcher->removeMediaQueryList(this);
+}
 
 String MediaQueryList::media() const
 {
@@ -53,7 +58,8 @@ void MediaQueryList::addListener(PassRefPtrWillBeRawPtr<MediaQueryListListener>
     if (!listener)
         return;
 
-    m_matcher->addListener(listener, this);
+    listener->setMediaQueryList(this);
+    m_listeners.add(listener);
 }
 
 void MediaQueryList::removeListener(PassRefPtrWillBeRawPtr<MediaQueryListListener> listener)
@@ -61,31 +67,47 @@ void MediaQueryList::removeListener(PassRefPtrWillBeRawPtr<MediaQueryListListene
     if (!listener)
         return;
 
-    m_matcher->removeListener(listener.get(), this);
+    RefPtrWillBeRawPtr<MediaQueryList> protect(this);
+    listener->clearMediaQueryList();
+
+    for (ListenerList::iterator i = m_listeners.begin(), end = m_listeners.end(); i != end; ++i) {
+        // 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 (**i == *listener.get()) {
+            m_listeners.remove(i);
+            break;
+        }
+    }
 }
 
-bool MediaQueryList::evaluate(MediaQueryEvaluator* evaluator)
+void MediaQueryList::documentDetached()
 {
-    if (m_evaluationRound != m_matcher->evaluationRound() && evaluator)
-        setMatches(evaluator->eval(m_media.get()));
-    return m_changeRound == m_matcher->evaluationRound();
+    m_listeners.clear();
 }
 
-void MediaQueryList::setMatches(bool newValue)
+void MediaQueryList::mediaFeaturesChanged(WillBeHeapVector<RefPtrWillBeMember<MediaQueryListListener> >* toNotify)
 {
-    m_evaluationRound = m_matcher->evaluationRound();
-
-    if (newValue == m_matches)
-        return;
+    m_matchesDirty = true;
+    if (updateMatches()) {

[esprehn, 2014/06/26 05:43:24]

early return.

if (!updateMatches())
  return;
for (...)
  toNotify->append(*i);

then you don't need any braces at all.

[cbiesinger, 2014/06/26 20:27:59]

Done.

+        for (ListenerList::const_iterator i = m_listeners.begin(), end = m_listeners.end(); i != end; ++i) {
+            toNotify->append(*i);
+        }
+    }
+}
 
-    m_matches = newValue;
-    m_changeRound = m_evaluationRound;
+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()
 {
-    if (m_evaluationRound != m_matcher->evaluationRound())
-        setMatches(m_matcher->evaluate(m_media.get()));
+    updateMatches();
     return m_matches;
 }
 
@@ -93,6 +115,7 @@ void MediaQueryList::trace(Visitor* visitor)
 {
     visitor->trace(m_matcher);
     visitor->trace(m_media);
+    visitor->trace(m_listeners);
 }
 
 }