cc: Adding a BeginFrameObserverMultiplexer.

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 42b8f6f1c735db05dfca52c00ed6070bb8b63a36..43a84bda571190f47d2e87e4886abaa5996004d1 100644
--- a/cc/scheduler/begin_frame_source.cc
+++ b/cc/scheduler/begin_frame_source.cc
@@ -60,7 +60,91 @@ void BeginFrameObserverMixIn::AsValueInto(
   dict->SetInteger("dropped_begin_frame_args_", dropped_begin_frame_args_);
 }
 
-// BeginFrameSourceMixIn ------------------------------------------------------
+// BeginFrameObserverMultiplexer ------------------------------------------
+BeginFrameObserverMultiplexer::BeginFrameObserverMultiplexer()
+    : last_callback_state_(false), has_observers_callback_() {
+}
+
+BeginFrameObserverMultiplexer::~BeginFrameObserverMultiplexer() {

[simonhong, 2015/03/25 02:03:21]

How about checking empty() of observer list?

[mithro-old, 2015/03/25 02:34:47]

Do we (and should we) require a user to remove all observers before removing the
multiplexer?

+  observer_list_.Clear();
+  CallCallbackOnChange();
+}
+
+scoped_ptr<BeginFrameObserverMultiplexer>
+BeginFrameObserverMultiplexer::Create() {
+  return make_scoped_ptr(new BeginFrameObserverMultiplexer());
+}
+
+void BeginFrameObserverMultiplexer::AddObserver(BeginFrameObserver* obs) {

[simonhong, 2015/03/25 02:03:21]

As you can see in my previous landed cl
(https://codereview.chromium.org/1000503002/),
BeginFrameObserverProxy sends missed_begin_frame_args_ immediately when new
observer is added.
Each observer determines whether or not to send it to RenderWidget.

[mithro-old, 2015/03/25 02:34:47]

I'm not sure what you are saying here? This code also sends the last recorded
begin frame args as a missed begin frame args on observer addition?

The BeginFrameObserver contract is that any new frame_time sent is strictly
greater than the frame_time returned by an observers LastUsedBeginFrameArgs
method.

+  DCHECK(!observer_list_.HasObserver(obs));
+  // If have a LastBeginFrameArgs is still effective, send it to the new
+  // |obs| immediately.
+  if (last_begin_frame_args_.IsValid() &&
+      obs->LastUsedBeginFrameArgs().frame_time <
+          last_begin_frame_args_.frame_time) {
+    BeginFrameArgs missed = last_begin_frame_args_;
+    missed.type = BeginFrameArgs::MISSED;
+    obs->OnBeginFrame(missed);
+  }
+  observer_list_.AddObserver(obs);
+  CallCallbackOnChange();
+}
+
+void BeginFrameObserverMultiplexer::RemoveObserver(BeginFrameObserver* obs) {
+  DCHECK(observer_list_.HasObserver(obs));
+  observer_list_.RemoveObserver(obs);
+  CallCallbackOnChange();
+}
+
+bool BeginFrameObserverMultiplexer::HasObservers() {
+  return observer_list_.might_have_observers();

[simonhong, 2015/03/25 02:03:21]

Also in my previous cl, there is some suggestion.
"might" word can give confusion.
So, I replaced this with plane std::list.
We can use it here because we don't modify observer list during observer method
is called.

[mithro-old, 2015/03/25 02:34:47]

Looking at the ObserverList implementation, the reason it is "might" is because
if an observer is removed while iterating over the list this method won't return
false until the Compact() method is called (which happens at the end of
iteration).

Guess I should add some type of comment here?
A std::list (in fact pretty much all std::xxx structures) is not safe to modify
while being iterated over, something that is important for the idea of
Observers. This is why the ObserverList type exists.

+}
+
+void BeginFrameObserverMultiplexer::SetHasObserverCallback(
+    const HasObserversCallback& cb) {
+  if (last_callback_state_ != false)
+    CallCallback(false);
+  has_observers_callback_ = cb;
+  CallCallback(HasObservers());
+}
+
+void BeginFrameObserverMultiplexer::AsValueInto(
+    base::trace_event::TracedValue* dict) const {
+  BeginFrameObserverMixIn::AsValueInto(dict);
+  if (observer_list_.might_have_observers()) {
+    ObserverList<BeginFrameObserver>::Iterator it(
+        const_cast<ObserverList<BeginFrameObserver>*>(&observer_list_));
+    BeginFrameObserver* obs;
+    dict->BeginArray("observers");
+    while ((obs = it.GetNext()) != NULL) {
+      dict->BeginDictionary();
+      obs->AsValueInto(dict);
+      dict->EndDictionary();
+    }
+    dict->EndArray();
+  }
+}
+
+bool BeginFrameObserverMultiplexer::OnBeginFrameMixInDelegate(
+    const BeginFrameArgs& args) {
+  FOR_EACH_OBSERVER(BeginFrameObserver, observer_list_, OnBeginFrame(args));
+  return true;
+}
+
+void BeginFrameObserverMultiplexer::CallCallbackOnChange() {
+  if (HasObservers() == last_callback_state_)
+    return;
+  CallCallback(HasObservers());
+}
+
+void BeginFrameObserverMultiplexer::CallCallback(bool has_observers) {
+  if (!has_observers_callback_.is_null()) {
+    has_observers_callback_.Run(has_observers);
+    last_callback_state_ = has_observers;
+  }
+}
+
+// BeginFrameSourceMixIn --------------------------------------------------
 BeginFrameSourceMixIn::BeginFrameSourceMixIn()
     : observer_(NULL),
       needs_begin_frames_(false),