cc: Making BeginFrameSources support multiple BeginFrameObservers.

cc/scheduler/begin_frame_source.cc

Index: cc/scheduler/begin_frame_source.cc
diff --git a/cc/scheduler/begin_frame_source.cc b/cc/scheduler/begin_frame_source.cc
index d9cadc83e726f7ceaca6fb32298c279b18c1ccb5..bba805f172e1452160fbcb5ae2b91da65536bfdd 100644
--- a/cc/scheduler/begin_frame_source.cc
+++ b/cc/scheduler/begin_frame_source.cc
@@ -61,13 +61,14 @@ void BeginFrameObserverBase::AsValueInto(
 
 // BeginFrameSourceBase ------------------------------------------------------
 BeginFrameSourceBase::BeginFrameSourceBase()
-    : observer_(NULL),
+    : observer_list_(),
       needs_begin_frames_(false),
       inside_as_value_into_(false) {
-  DCHECK(!observer_);
   DCHECK_EQ(inside_as_value_into_, false);
 }
 
+BeginFrameSourceBase::~BeginFrameSourceBase() {}
+
 bool BeginFrameSourceBase::NeedsBeginFrames() const {
   return needs_begin_frames_;
 }
@@ -85,34 +86,47 @@ void BeginFrameSourceBase::SetNeedsBeginFrames(bool needs_begin_frames) {
 }
 
 void BeginFrameSourceBase::AddObserver(BeginFrameObserver* obs) {
-  DEBUG_FRAMES("BeginFrameSourceBase::AddObserver",
-               "current observer",
-               observer_,
-               "to add observer",
-               obs);
-  DCHECK(!observer_);
-  observer_ = obs;
+  DEBUG_FRAMES("BeginFrameSourceBase::AddObserver", "has observers?",
+               (observer_list_).might_have_observers(), "to add observer", obs);
+  DCHECK(obs);
+  DCHECK(!observer_list_.HasObserver(obs));
+  observer_list_.AddObserver(obs);
 }
 
 void BeginFrameSourceBase::RemoveObserver(BeginFrameObserver* obs) {
-  DEBUG_FRAMES("BeginFrameSourceBase::RemoveObserver",
-               "current observer",
-               observer_,
-               "to remove observer",
+  DEBUG_FRAMES("BeginFrameSourceBase::RemoveObserver", "has observers?",
+               (observer_list_).might_have_observers(), "to remove observer",
                obs);
-  DCHECK_EQ(observer_, obs);
-  observer_ = NULL;
+  DCHECK(obs);
+  DCHECK(observer_list_.HasObserver(obs));
+  observer_list_.RemoveObserver(obs);
 }
 
 void BeginFrameSourceBase::CallOnBeginFrame(const BeginFrameArgs& args) {
-  DEBUG_FRAMES("BeginFrameSourceBase::CallOnBeginFrame",
-               "current observer",
-               observer_,
-               "args",
-               args.AsValue());
-  if (observer_) {
-    return observer_->OnBeginFrame(args);
-  }
+  DEBUG_FRAMES("BeginFrameSourceBase::CallOnBeginFrame", "has observers?",
+               observer_list_.might_have_observers(), "args", args.AsValue());
+  FOR_EACH_OBSERVER(BeginFrameObserver, observer_list_, OnBeginFrame(args));
+}
+
+base::ObserverList<const BeginFrameObserver>::Iterator
+BeginFrameSourceBase::getConstObserverListIterator() const {

[brianderson, 2015/09/04 19:48:18]

Please pay special attention here.

+  // TODO(mithro): Add a const iterator on base::ObserverList which does
+  // something similar to this. That should probably also DCHECK that
+  // AddObserver/RemoveObserver are never called.
+
+  // First we use reinterpret_cast to make an base::ObserverList containing
+  // *const Observers*. This means we can only call *const* methods on the
+  // Observer.
+  const base::ObserverList<const BeginFrameObserver>* a =
+      reinterpret_cast<const base::ObserverList<const BeginFrameObserver>*>(
+          &observer_list_);
+  // As AddObserver and RemoveObserver are *not* const methods, const methods
+  // on the Observer will (most) not be able to call them. This makes it
+  // "safe" to cast away the const on the ObserverList so we can iterate over
+  // it.
+  base::ObserverList<const BeginFrameObserver>* b =
+      const_cast<base::ObserverList<const BeginFrameObserver>*>(a);
+  return base::ObserverList<const BeginFrameObserver>::Iterator(b);
 }
 
 // Tracing support
@@ -124,17 +138,22 @@ void BeginFrameSourceBase::AsValueInto(
     dict->SetString("observer", "<loop detected>");
     return;
   }
+  base::AutoReset<bool> prevent_loops(const_cast<bool*>(&inside_as_value_into_),
+                                      true);
 
-  if (observer_) {
-    base::AutoReset<bool> prevent_loops(
-        const_cast<bool*>(&inside_as_value_into_), true);
-    dict->BeginDictionary("observer");
-    observer_->AsValueInto(dict);
-    dict->EndDictionary();
-  } else {
-    dict->SetString("observer", "NULL");
-  }
   dict->SetBoolean("needs_begin_frames", NeedsBeginFrames());
+  if (observer_list_.might_have_observers()) {
+    base::ObserverList<const BeginFrameObserver>::Iterator it =
+        getConstObserverListIterator();
+    const BeginFrameObserver* obs;
+    dict->BeginArray("observers");
+    while ((obs = it.GetNext()) != NULL) {
+      dict->BeginDictionary();
+      obs->AsValueInto(dict);
+      dict->EndDictionary();
+    }
+    dict->EndArray();
+  }
 }
 
 // BackToBackBeginFrameSource --------------------------------------------
@@ -385,10 +404,19 @@ void BeginFrameSourceMultiplexer::OnBeginFrame(const BeginFrameArgs& args) {
 
 const BeginFrameArgs BeginFrameSourceMultiplexer::LastUsedBeginFrameArgs()
     const {
-  if (observer_)
-    return observer_->LastUsedBeginFrameArgs();
-  else
-    return BeginFrameArgs();
+  BeginFrameArgs last;
+
+  if (observer_list_.might_have_observers()) {
+    base::ObserverList<const BeginFrameObserver>::Iterator it =
+        getConstObserverListIterator();
+    const BeginFrameObserver* obs;
+    while ((obs = it.GetNext()) != NULL) {
+      BeginFrameArgs obs_last = obs->LastUsedBeginFrameArgs();
+      if (obs_last.frame_time > last.frame_time)
+        last = obs_last;
+    }
+  }
+  return last;
 }
 
 // BeginFrameSource support
@@ -419,14 +447,9 @@ void BeginFrameSourceMultiplexer::DidFinishFrame(size_t remaining_frames) {
 void BeginFrameSourceMultiplexer::AsValueInto(
     base::trace_event::TracedValue* dict) const {
   dict->SetString("type", "BeginFrameSourceMultiplexer");
+  BeginFrameSourceBase::AsValueInto(dict);
 
   dict->SetInteger("minimum_interval_us", minimum_interval_.InMicroseconds());
-  if (observer_) {
-    dict->BeginDictionary("last_begin_frame_args");
-    observer_->LastUsedBeginFrameArgs().AsValueInto(dict);
-    dict->EndDictionary();
-  }
-
   if (active_source_) {
     dict->BeginDictionary("active_source");
     active_source_->AsValueInto(dict);
@@ -453,17 +476,15 @@ bool BeginFrameSourceMultiplexer::HasSource(BeginFrameSource* source) {
 
 bool BeginFrameSourceMultiplexer::IsIncreasing(const BeginFrameArgs& args) {
   DCHECK(args.IsValid());
-  if (!observer_)
-    return false;
 
   // If the last begin frame is invalid, then any new begin frame is valid.
-  if (!observer_->LastUsedBeginFrameArgs().IsValid())
+  if (!LastUsedBeginFrameArgs().IsValid())
     return true;
 
   // Only allow new args have a *strictly bigger* frame_time value and statisfy
   // minimum interval requirement.
   return (args.frame_time >=
-          observer_->LastUsedBeginFrameArgs().frame_time + minimum_interval_);
+          LastUsedBeginFrameArgs().frame_time + minimum_interval_);
 }
 
 }  // namespace cc