cc: Make BackToBackBeginFrameSource a SyntheticBeginFrameSource.

content/renderer/gpu/compositor_external_begin_frame_source.cc

Index: content/renderer/gpu/compositor_external_begin_frame_source.cc
diff --git a/content/renderer/gpu/compositor_external_begin_frame_source.cc b/content/renderer/gpu/compositor_external_begin_frame_source.cc
index a8b76cf2e87366d30dde269a15e5a982054592ba..45149d1fb9e80363d88fbdfd1c14cf23bc25ea3b 100644
--- a/content/renderer/gpu/compositor_external_begin_frame_source.cc
+++ b/content/renderer/gpu/compositor_external_begin_frame_source.cc
@@ -32,19 +32,21 @@ CompositorExternalBeginFrameSource::~CompositorExternalBeginFrameSource() {
   }
 }
 
-void CompositorExternalBeginFrameSource::OnNeedsBeginFramesChanged(
-    bool needs_begin_frames) {
-  DCHECK(CalledOnValidThread());
-  if (!needs_begin_frames)
-    missed_begin_frame_args_ = cc::BeginFrameArgs();
-  Send(new ViewHostMsg_SetNeedsBeginFrames(routing_id_, needs_begin_frames));
-}
-
 void CompositorExternalBeginFrameSource::AddObserver(
     cc::BeginFrameObserver* obs) {
   DCHECK(CalledOnValidThread());
+  DCHECK(obs);
+  DCHECK(observers_.find(obs) == observers_.end());
+
+  DEBUG_FRAMES("CompositorExternalBeginFrameSource::AddObserver",
+               "num observers", observers_.size(), "to add observer", obs);
   SetClientReady();
-  BeginFrameSourceBase::AddObserver(obs);
+  bool observers_was_empty = observers_.empty();
+  observers_.insert(obs);
+  obs->OnBeginFrameSourcePausedChanged(paused_);
+  if (observers_was_empty)
+    Send(new ViewHostMsg_SetNeedsBeginFrames(routing_id_, true));
+
   // Send a MISSED begin frame if necessary.
   if (missed_begin_frame_args_.IsValid()) {
     cc::BeginFrameArgs last_args = obs->LastUsedBeginFrameArgs();
@@ -55,6 +57,20 @@ void CompositorExternalBeginFrameSource::AddObserver(
   }
 }
 
+void CompositorExternalBeginFrameSource::RemoveObserver(
+    cc::BeginFrameObserver* obs) {
+  DCHECK(obs);
+  DCHECK(observers_.find(obs) != observers_.end());
+
+  DEBUG_FRAMES("CompositorExternalBeginFrameSource::RemoveObserver",
+               "num observers", observers_.size(), "removed observer", obs);
+  observers_.erase(obs);
+  if (observers_.empty()) {
+    missed_begin_frame_args_ = cc::BeginFrameArgs();
+    Send(new ViewHostMsg_SetNeedsBeginFrames(routing_id_, false));
+  }
+}
+
 void CompositorExternalBeginFrameSource::SetClientReady() {
   DCHECK(CalledOnValidThread());
   if (begin_frame_source_proxy_)
@@ -74,17 +90,35 @@ void CompositorExternalBeginFrameSource::OnMessageReceived(
   DCHECK(CalledOnValidThread());
   DCHECK(begin_frame_source_proxy_.get());
   IPC_BEGIN_MESSAGE_MAP(CompositorExternalBeginFrameSource, message)
-    IPC_MESSAGE_HANDLER(ViewMsg_SetBeginFramePaused, SetBeginFrameSourcePaused)
+    IPC_MESSAGE_HANDLER(ViewMsg_SetBeginFramePaused,
+                        OnSetBeginFrameSourcePaused)
     IPC_MESSAGE_HANDLER(ViewMsg_BeginFrame, OnBeginFrame)
   IPC_END_MESSAGE_MAP()
 }
 
+void CompositorExternalBeginFrameSource::OnSetBeginFrameSourcePaused(
+    bool paused) {
+  if (paused_ == paused)
+    return;
+  DEBUG_FRAMES(
+      "CompositorExternalBeginFrameSource::OnSetBeginFrameSourcePaused",
+      "num observers", observers_.size(), "paused", paused);
+  paused_ = paused;
+  std::set<cc::BeginFrameObserver*> observers(observers_);

[piman, 2016/06/15 19:48:14]

Would it make sense to use ObserverList + FOR_EACH_OBSERVER instead?
It deals with an observer removing itself from the list while called back, and
is probably generally cheaper (and more idiomatic) than a std::set.

[danakj, 2016/06/15 19:56:05]

I think it didn't work because of OnBeginFrame being const. I looked at some
patches to change, or add another, ObserverList when the original code for this
went into BFSBase.

[danakj, 2016/06/15 19:58:00]

OK.. so I found the email and it said:

"The important part is that inside a **const method** on
ThingWhichHasObservers we want to be able to loop over the observer
list and call const methods on the observers.

As observers are normally allowed to modify the observer list while
you are iterating over them (so they can remove themselves after being
notified) the ObserverList::Iterator currently only works on non-const
ObserverLists. This means you can't loop over ObserverLists in a const
method!"

But these methods aren't const. So ya let me try that.

[danakj, 2016/06/15 20:23:41]

It gets sketchy around "if the list was empty then do this after adding". We can
check "might_have_observers()", but if this is being added during iteration,
then it might return true when there wasn't and we fail to act correctly, or
after removing it checks if it's now empty. I think I prefer to leave it using
set().

+  for (auto* obs : observers)
+    obs->OnBeginFrameSourcePausedChanged(paused_);
+}
+
 void CompositorExternalBeginFrameSource::OnBeginFrame(
   const cc::BeginFrameArgs& args) {
+  DEBUG_FRAMES("CompositorExternalBeginFrameSource::OnBeginFrame",
+               "num observers", observers_.size(), "args", args.AsValue());
   DCHECK(CalledOnValidThread());
   missed_begin_frame_args_ = args;
   missed_begin_frame_args_.type = cc::BeginFrameArgs::MISSED;
-  CallOnBeginFrame(args);
+  std::set<cc::BeginFrameObserver*> observers(observers_);
+  for (auto* obs : observers)
+    obs->OnBeginFrame(args);
 }
 
 bool CompositorExternalBeginFrameSource::Send(IPC::Message* message) {