cc: Implement unified BeginFrame on aura

ui/compositor/compositor_unittest.cc

Index: ui/compositor/compositor_unittest.cc
diff --git a/ui/compositor/compositor_unittest.cc b/ui/compositor/compositor_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..6e4fb01ce0c4c8bccde67c5994a8836e0be7e970
--- /dev/null
+++ b/ui/compositor/compositor_unittest.cc
@@ -0,0 +1,102 @@
+// Copyright 2015 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 "base/basictypes.h"
+#include "base/compiler_specific.h"
+#include "base/memory/scoped_ptr.h"
+#include "cc/output/begin_frame_args.h"
+#include "cc/test/begin_frame_args_test.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/compositor/compositor.h"
+#include "ui/compositor/test/compositor_test_api.h"
+#include "ui/compositor/test/context_factories_for_test.h"
+#include "ui/compositor/test/test_compositor_host.h"
+#include "ui/gfx/geometry/rect.h"
+
+namespace ui {
+
+namespace {
+
+class TestCompositorBeginFrameObserver : public CompositorBeginFrameObserver {
+ public:
+  TestCompositorBeginFrameObserver() : send_begin_frame_called_(false) {}
+  ~TestCompositorBeginFrameObserver() override {}
+
+  void OnSendBeginFrame(const cc::BeginFrameArgs& args) override {
+    send_begin_frame_called_ = true;
+  }
+
+  void Reset() {
+    send_begin_frame_called_ = false;
+  }
+
+  bool send_begin_frame_called() { return send_begin_frame_called_; }
+
+ private:
+  bool send_begin_frame_called_;
+};
+
+class BeginFrameWithCompositorTest : public testing::Test {
+ public:
+  BeginFrameWithCompositorTest() {}
+  ~BeginFrameWithCompositorTest() override {}
+
+  // Overridden from testing::Test:
+  void SetUp() override {
+    bool enable_pixel_output = false;
+    ui::ContextFactory* context_factory =
+        InitializeContextFactoryForTests(enable_pixel_output);
+
+    const gfx::Rect host_bounds(10, 10, 500, 500);

[danakj, 2015/03/10 00:40:44]

kHostBounds for compile time constants

+    compositor_host_.reset(
+        TestCompositorHost::Create(host_bounds, context_factory));

[brianderson, 2015/03/06 20:37:00]

It looks like TestCompositorHost is used to test platform specific code.

Is it possible to create a ui::Compositor directly in a platform agnostic way?
For example, by using a kNullAcceleratedWidget?

Alternatively, if creating a ui::Compositor directly is too hard/invasive, maybe
pull the more complicated Observer logic out of ui::Compositor and put it into a
delegate class that you can unit test more easily?

I see why you may have gone the CompositorTestAPI approach now...

danakj: Do you have any thoughts on which approach would be best?

[danakj, 2015/03/10 00:40:44]

I think brian is right here. If you don't need pixels on the screen to debug
things, you don't need the TestCompositorHost.

Here's another unit test example that has a compositor.
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/co...

And instead of a wrapper class that is a friend you could do 2 other things:
1. subclass ui::Compositor, make it protected, expose it in the subclass with
"using"
or
2. just expose it on ui::Compositor (if it's const and simple maybe that's fine)
or
3. expose it as FooForTesting() which will not allow calls to it outside of test
targets


lastly.. if you can split this patch up that would be hugely awesome, it's quite
big right now.

+    compositor_host_->Show();
+
+    compositor_test_api_.reset(new CompositorTestAPI(GetCompositor()));
+  }
+
+  void TearDown() override {
+    compositor_host_.reset();
+    TerminateContextFactoryForTests();
+  }
+
+  Compositor* GetCompositor() { return compositor_host_->GetCompositor(); }
+
+  cc::BeginFrameArgs missed_begin_frame_args() {
+    return compositor_test_api_->missed_begin_frame_args();
+  }
+
+ private:
+  scoped_ptr<TestCompositorHost> compositor_host_;
+  scoped_ptr<CompositorTestAPI> compositor_test_api_;
+
+  DISALLOW_COPY_AND_ASSIGN(BeginFrameWithCompositorTest);
+};
+
+}  // namespace
+
+TEST_F(BeginFrameWithCompositorTest, AddBeginFrameObserver) {

[brianderson, 2015/03/06 20:36:59]

The test looks good, but please add comments explaining what this test is
testing and why, both at the top and in the body.

+  cc::BeginFrameArgs last_args =
+    cc::CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE,
+                                       base::TimeTicks::FromInternalValue(33));
+  cc::BeginFrameArgs new_args =
+    cc::CreateBeginFrameArgsForTesting(BEGINFRAME_FROM_HERE,
+                                       base::TimeTicks::FromInternalValue(55));
+  TestCompositorBeginFrameObserver test_observer;
+
+  GetCompositor()->SendBeginFramesToChildren(new_args);
+  EXPECT_EQ(new_args.frame_time,
+            missed_begin_frame_args().frame_time);
+
+  GetCompositor()->AddBeginFrameObserver(&test_observer, last_args);
+  EXPECT_TRUE(test_observer.send_begin_frame_called());
+  GetCompositor()->RemoveBeginFrameObserver(&test_observer);
+
+  test_observer.Reset();
+  GetCompositor()->AddBeginFrameObserver(&test_observer, new_args);
+  EXPECT_FALSE(test_observer.send_begin_frame_called());
+  GetCompositor()->RemoveBeginFrameObserver(&test_observer);
+}
+
+}  // namespace ui