cc: Making BeginFrameSources support multiple BeginFrameObservers.

cc/scheduler/begin_frame_source_unittest.cc

 // Copyright 2011 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 <deque>
 #include <string>
 
 #include "base/basictypes.h"
 #include "base/gtest_prod_util.h"
 #include "base/test/test_simple_task_runner.h"
@@ skipping 213 matching lines @@
 // BeginFrameSource testing ----------------------------------------------
 TEST(BeginFrameSourceBaseTest, ObserverManipulation) {
   MockBeginFrameObserver obs;
   MockBeginFrameObserver otherObs;
   FakeBeginFrameSource source;
 
   source.AddObserver(&obs);
   EXPECT_EQ(&obs, source.GetObserver());
 
 #ifndef NDEBUG
-  // Adding an observer when an observer already exists should DCHECK fail.
-  EXPECT_DEATH({ source.AddObserver(&otherObs); }, "");
-
   // Removing wrong observer should DCHECK fail.
   EXPECT_DEATH({ source.RemoveObserver(&otherObs); }, "");
 
   // Removing an observer when there is no observer should DCHECK fail.
   EXPECT_DEATH({
                  source.RemoveObserver(&obs);
                  source.RemoveObserver(&obs);
                },
                "");
 #endif
@@ skipping 17 matching lines @@
   SEND_BEGIN_FRAME_DROP(source, 400, 600, 300);
   SEND_BEGIN_FRAME_DROP(source, 450, 650, 300);
   SEND_BEGIN_FRAME_USED(source, 700, 900, 300);
 }
 
 TEST(BeginFrameSourceBaseTest, NoObserver) {
   FakeBeginFrameSource source;
   SEND_BEGIN_FRAME_DROP(source, 100, 200, 300);
 }
 
+TEST(BeginFrameSourceBaseTest, ObserverMulti) {

[brianderson, 2015/09/12 00:23:50]

Looks like the same test as ObserverMultiSimple?

+  MockBeginFrameObserver obs;
+  MockBeginFrameObserver otherObs;
+  FakeBeginFrameSource source;
+
+  EXPECT_BEGIN_FRAME_USED(obs, 100, 200, 300);
+  EXPECT_BEGIN_FRAME_USED(obs, 400, 600, 300);
+  EXPECT_BEGIN_FRAME_USED(obs, 450, 650, 300);
+  EXPECT_BEGIN_FRAME_USED(obs, 700, 900, 300);
+
+  EXPECT_BEGIN_FRAME_USED(otherObs, 400, 600, 300);
+  EXPECT_BEGIN_FRAME_USED(otherObs, 450, 650, 300);
+
+  source.AddObserver(&obs);
+  SEND_BEGIN_FRAME_USED(source, 100, 200, 300);
+  source.AddObserver(&otherObs);
+  SEND_BEGIN_FRAME_USED(source, 400, 600, 300);
+  SEND_BEGIN_FRAME_USED(source, 450, 650, 300);
+  source.RemoveObserver(&otherObs);
+  SEND_BEGIN_FRAME_USED(source, 700, 900, 300);
+}
+
+TEST(BeginFrameSourceBaseTest, ObserverMultiSimple) {
+  MockBeginFrameObserver obs;
+  MockBeginFrameObserver otherObs;
+  FakeBeginFrameSource source;
+
+  EXPECT_BEGIN_FRAME_USED(obs, 100, 200, 300);
+  EXPECT_BEGIN_FRAME_USED(obs, 400, 600, 300);
+  EXPECT_BEGIN_FRAME_USED(obs, 450, 650, 300);
+  EXPECT_BEGIN_FRAME_USED(obs, 700, 900, 300);
+
+  EXPECT_BEGIN_FRAME_USED(otherObs, 400, 600, 300);
+  EXPECT_BEGIN_FRAME_USED(otherObs, 450, 650, 300);
+
+  source.AddObserver(&obs);
+  SEND_BEGIN_FRAME_USED(source, 100, 200, 300);
+  source.AddObserver(&otherObs);
+  SEND_BEGIN_FRAME_USED(source, 400, 600, 300);
+  SEND_BEGIN_FRAME_USED(source, 450, 650, 300);
+  source.RemoveObserver(&otherObs);
+  SEND_BEGIN_FRAME_USED(source, 700, 900, 300);
+
+  scoped_refptr<base::trace_event::TracedValue> state1 =
+      new base::trace_event::TracedValue();
+  source.AsValueInto(state1.get());
+
+  source.AddObserver(&otherObs);
+  scoped_refptr<base::trace_event::TracedValue> state2 =
+      new base::trace_event::TracedValue();
+  source.AsValueInto(state2.get());
+}
+
+class RemovingBeginFrameObserver : public BeginFrameObserverBase {
+ public:
+  explicit RemovingBeginFrameObserver(BeginFrameSource* bfs) : source_(bfs) {}
+
+ protected:
+  // BeginFrameObserverBase
+  bool OnBeginFrameDerivedImpl(const BeginFrameArgs& args) override {
+    source_->RemoveObserver(this);
+    return true;
+  }
+
+  BeginFrameSource* source_;
+};
+
+TEST(BeginFrameSourceBaseTest, ObserverMultiRemoveOnBeginFrame) {

[brianderson, 2015/09/12 00:23:50]

If you are going to add a WillLoseBeginFrameSource as part of this patch alo add
a test similar to this one.

BeginFrameObserverMap supports adding observers while triggering an observer. I
can't think of a case where that's needed. If it's not, remove it from
BeginFrameObserverMap. If it is, there should probably be a test in here for
that.

+  MockBeginFrameObserver obs;
+  FakeBeginFrameSource source;
+  RemovingBeginFrameObserver otherObs(&source);
+
+  EXPECT_BEGIN_FRAME_USED(obs, 100, 200, 300);
+  EXPECT_BEGIN_FRAME_USED(obs, 400, 600, 300);
+  EXPECT_BEGIN_FRAME_USED(obs, 700, 900, 300);
+
+  source.AddObserver(&obs);
+  SEND_BEGIN_FRAME_USED(source, 100, 200, 300);
+  source.AddObserver(&otherObs);
+  SEND_BEGIN_FRAME_USED(source, 400, 600, 300);
+  SEND_BEGIN_FRAME_USED(source, 700, 900, 300);
+  EXPECT_EQ(
+      otherObs.LastUsedBeginFrameArgs(),
+      CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE, 400, 600, 300));
+}
+
 TEST(BeginFrameSourceBaseTest, NeedsBeginFrames) {
   FakeBeginFrameSource source;
   EXPECT_FALSE(source.NeedsBeginFrames());
   source.SetNeedsBeginFrames(true);
   EXPECT_TRUE(source.NeedsBeginFrames());
   source.SetNeedsBeginFrames(false);
   EXPECT_FALSE(source.NeedsBeginFrames());
 }
 
 class LoopingBeginFrameObserver : public BeginFrameObserverBase {
@@ skipping 486 matching lines @@
   mux_->SetActiveSource(source2_);
   SEND_BEGIN_FRAME_DROP(*source2_, 750, 1050, 300);
   SEND_BEGIN_FRAME_USED(*source2_, 1050, 1250, 300);
 
   mux_->SetActiveSource(source1_);
   SEND_BEGIN_FRAME_DROP(*source2_, 1100, 1400, 300);
 }
 
 }  // namespace
 }  // namespace cc