cc: Adding a BeginFrameObserverMultiplexer.

cc/scheduler/begin_frame_source.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/scheduler/begin_frame_source.h"
 
 #include "base/auto_reset.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/strings/string_number_conversions.h"
@@ skipping 42 matching lines @@
 }
 
 void BeginFrameObserverMixIn::AsValueInto(
     base::trace_event::TracedValue* dict) const {
   dict->BeginDictionary("last_begin_frame_args_");
   last_begin_frame_args_.AsValueInto(dict);
   dict->EndDictionary();
   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),
       inside_as_value_into_(false) {
   DCHECK(!observer_);
   DCHECK_EQ(inside_as_value_into_, false);
 }
 
 bool BeginFrameSourceMixIn::NeedsBeginFrames() const {
   return needs_begin_frames_;
@@ skipping 397 matching lines @@
   if (!observer_->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_);
 }
 
 }  // namespace cc