Add BeginFrameObserverProxy

ui/compositor/compositor.cc

Index: ui/compositor/compositor.cc
diff --git a/ui/compositor/compositor.cc b/ui/compositor/compositor.cc
index f050a64e1a74dede9134fcfe075fd614dcd9841c..31a50efcb221c36026599105a632d2166bb62c4c 100644
--- a/ui/compositor/compositor.cc
+++ b/ui/compositor/compositor.cc
@@ -156,6 +156,8 @@ Compositor::~Compositor() {
   FOR_EACH_OBSERVER(CompositorObserver, observer_list_,
                     OnCompositingShuttingDown(this));
 
+  DCHECK(begin_frame_observer_list_.empty());
+
   if (root_layer_)
     root_layer_->SetCompositor(NULL);
 
@@ -283,6 +285,39 @@ bool Compositor::HasAnimationObserver(
   return animation_observer_list_.HasObserver(observer);
 }
 
+void Compositor::AddBeginFrameObserver(
+    CompositorBeginFrameObserver* observer,
+    const cc::BeginFrameArgs& last_begin_frame_args_sent_to_observer) {
+  // If |last_begin_frame_args_| is still effective, send it to the new

[danakj, 2015/03/19 22:36:41]

what does "still effective" mean? You're checking that it's new here? This
doesn't clarify for me why Compositor is doing this check instead of the
observer..

[simonhong, 2015/03/20 16:07:36]

Ah... I got your meaning now.
You mean that making Compositor just OnSendBeginFrame(missed_begin_frame_) to
newly added
observer and let observer verify about it whether or not to send to child
scheduler.

+  // |observer| immediately.
+  if (last_begin_frame_args_sent_to_observer.frame_time !=

[brianderson, 2015/03/19 22:48:59]

Now that there's a shared CompositorBeginFrameObserverImpl we can use, should
the BeginFrame de-dupe logic and test move there?

[simonhong, 2015/03/20 16:07:36]

Yep, moved to there.

+      missed_begin_frame_args_.frame_time) {
+    observer->OnSendBeginFrame(missed_begin_frame_args_);

[brianderson, 2015/03/19 22:48:59]

Then the call to OnSendBeginFrame can move to below.

[simonhong, 2015/03/20 16:07:36]

Done.

+  }
+
+  if (begin_frame_observer_list_.empty())
+    host_->SetChildrenNeedBeginFrames(true);

[brianderson, 2015/03/19 22:48:59]

else if (missed_begin_frame_args_.IsValid())
  observer->OnSendBeginFrame(missed_begin_frame_args_);

We should also make missed_begin_frame_args_ invalid after calling
SetChildrenNeedBeginFrames(false) to avoid sending a reallly old missed begin
frame from a previous active period.

Can you add a test case for that? I think this patch will fail that test.

[simonhong, 2015/03/20 16:07:36]

Done.

+
+  std::list<CompositorBeginFrameObserver*>::iterator it =

[danakj, 2015/03/19 22:36:41]

do all of this inside the DCHECK please, temp vars are good but keep DCHECK-only
code in DCHECKs

[brianderson, 2015/03/19 22:48:59]

auto? Here and elsewhere.

[simonhong, 2015/03/20 16:07:36]

Done.

+      std::find(begin_frame_observer_list_.begin(),
+                begin_frame_observer_list_.end(), observer);
+  DCHECK(begin_frame_observer_list_.end() == it);

[danakj, 2015/03/19 22:36:41]

DCHECK(it != end)

Like we do foo != 5 not 5 != foo. end is the "fixed" thing here.

[simonhong, 2015/03/20 16:07:36]

Done.

+
+  begin_frame_observer_list_.push_back(observer);
+}
+
+void Compositor::RemoveBeginFrameObserver(
+    CompositorBeginFrameObserver* observer) {
+  std::list<CompositorBeginFrameObserver*>::iterator it =
+      std::find(begin_frame_observer_list_.begin(),
+                begin_frame_observer_list_.end(), observer);
+  DCHECK(begin_frame_observer_list_.end() != it);

[danakj, 2015/03/19 22:36:40]

DCHECK(it != end)

[simonhong, 2015/03/20 16:07:36]

Done.

+  begin_frame_observer_list_.erase(it);
+
+  if (begin_frame_observer_list_.empty())
+    host_->SetChildrenNeedBeginFrames(false);

[brianderson, 2015/03/19 22:48:59]

To make missed_begin_frame_args_ invalid:
missed_begin_frame_args_ = BeginFrameArgs();

[simonhong, 2015/03/20 16:07:36]

Done.

+}
+
 void Compositor::BeginMainFrame(const cc::BeginFrameArgs& args) {
   FOR_EACH_OBSERVER(CompositorAnimationObserver,
                     animation_observer_list_,
@@ -345,6 +380,17 @@ void Compositor::DidAbortSwapBuffers() {
                     OnCompositingAborted(this));
 }
 
+void Compositor::SendBeginFramesToChildren(const cc::BeginFrameArgs& args) {
+  std::list<CompositorBeginFrameObserver*>::iterator it =
+      begin_frame_observer_list_.begin();
+  while(it != begin_frame_observer_list_.end()) {

[danakj, 2015/03/19 22:36:41]

for (auto* observer : begin_..._list_)

[simonhong, 2015/03/20 16:07:36]

Done.

+    (*it)->OnSendBeginFrame(args);
+    it++;
+  }
+  missed_begin_frame_args_ = args;
+  missed_begin_frame_args_.type = cc::BeginFrameArgs::MISSED;
+}
+
 const cc::LayerTreeDebugState& Compositor::GetLayerTreeDebugState() const {
   return host_->debug_state();
 }