Adding drawable to CRD andorid and iOS gl rendering pipeline.

remoting/client/gl_renderer_unittest.cc

 // Copyright 2016 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 "remoting/client/gl_renderer.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/memory/ptr_util.h"
 #include "base/message_loop/message_loop.h"
@@ skipping 63 matching lines @@
   int on_size_changed_call_count_ = 0;
 
   int canvas_width_ = 0;
   int canvas_height_ = 0;
 
   base::Closure on_frame_rendered_callback_;
 
   base::WeakPtrFactory<FakeGlRendererDelegate> weak_factory_;
 };
 
+class FakeGlDrawable : public GlDrawable {
+ public:
+  FakeGlDrawable() : drawn_(0), id_(-1), weak_factory_(this) {}

[joedow, 2016/12/22 00:29:03]

nit: init the primitives inline and clean up the initializer list.

[nicholss, 2017/01/09 18:50:24]

Done.

+
+  void SetId(int id) { id_ = id; }
+  int GetId() { return id_; }
+
+  base::WeakPtr<GlDrawable> GetWeakPtr() override {
+    return weak_factory_.GetWeakPtr();
+  }
+
+  void SetCanvas(GlCanvas* canvas) override { canvas_ = canvas; }
+
+  bool Draw() override {
+    drawn_++;
+    return false;
+  }
+
+  int DrawnCount() { return drawn_; }
+
+ private:
+  int drawn_;
+  int id_;
+  GlCanvas* canvas_;
+
+  base::WeakPtrFactory<FakeGlDrawable> weak_factory_;

[joedow, 2016/12/22 00:29:03]

DISALLOW_COPY_AND_ASSIGN macro?

[nicholss, 2017/01/09 18:50:24]

Done.

+};
+
 class GlRendererTest : public testing::Test {
  public:
   void SetUp() override;
   void SetDesktopFrameWithSize(const webrtc::DesktopSize& size);
   void PostSetDesktopFrameTasks(const webrtc::DesktopSize& size, int count);
+  int GetDrawablesCount();
+  std::vector<base::WeakPtr<GlDrawable>> GetDrawables();

[joedow, 2016/12/22 00:29:03]

Can this return a const ref so it doesn't need to make a copy of the vector and
contents?

[nicholss, 2017/01/09 18:50:24]

I don't think it matters for the test. It is 5 elements.

 
  protected:
   void RequestRender();
   void OnDesktopFrameProcessed();
   void RunTasksInCurrentQueue();
   void RunUntilRendered();
   int on_desktop_frame_processed_call_count() {
     return on_desktop_frame_processed_call_count_;
   }
 
   base::MessageLoop message_loop_;
   std::unique_ptr<GlRenderer> renderer_;
   FakeGlRendererDelegate delegate_;
 
  private:
   int on_desktop_frame_processed_call_count_ = 0;
 };
 
 void GlRendererTest::SetUp() {
   renderer_.reset(new GlRenderer());
   renderer_->SetDelegate(delegate_.GetWeakPtr());
 }
 
 void GlRendererTest::RequestRender() {
   renderer_->RequestRender();
 }
 
+int GlRendererTest::GetDrawablesCount() {
+  return (int)renderer_->drawables_.size();
+}
+
+std::vector<base::WeakPtr<GlDrawable>> GlRendererTest::GetDrawables() {
+  return renderer_->drawables_;
+}
+
 void GlRendererTest::SetDesktopFrameWithSize(const webrtc::DesktopSize& size) {
   renderer_->OnFrameReceived(
       base::MakeUnique<webrtc::BasicDesktopFrame>(size),
       base::Bind(&GlRendererTest::OnDesktopFrameProcessed,
                  base::Unretained(this)));
 }
 
 void GlRendererTest::PostSetDesktopFrameTasks(
     const webrtc::DesktopSize& size, int count) {
   for (int i = 0; i < count; i++) {
@@ skipping 83 matching lines @@
   EXPECT_EQ(1, on_desktop_frame_processed_call_count());
 
   PostSetDesktopFrameTasks(webrtc::DesktopSize(16, 16), 20);
   RunUntilRendered();
   EXPECT_EQ(2, delegate_.on_frame_rendered_call_count());
   EXPECT_EQ(21, on_desktop_frame_processed_call_count());
 }
 
 // TODO(yuweih): Add tests to validate the rendered output.
 
+TEST_F(GlRendererTest, TestAddDrawable) {
+  FakeGlDrawable* drawable0 = new FakeGlDrawable();
+  drawable0->SetId(0);
+  renderer_->AddDrawable(drawable0->GetWeakPtr());
+  EXPECT_EQ(1, GetDrawablesCount());
+}
+
+TEST_F(GlRendererTest, TestAddDrawableDefaultOrder) {
+  FakeGlDrawable* drawable0 = new FakeGlDrawable();
+  drawable0->SetId(0);
+  renderer_->AddDrawable(drawable0->GetWeakPtr());
+  EXPECT_EQ(1, GetDrawablesCount());

[joedow, 2016/12/22 00:29:03]

Why use EXPECT instead of ASSERT?  Do you want the test to continue even though
this line failed (same with the rest of the file)?

[nicholss, 2017/01/09 18:50:24]

I do not know how to unit test in chromium... Don't know what the magic macros
mean. I was unable to find any documentation about this on the chromium site and
the file where ASSERT is defined is HUGE and really not well documented either.
Seems like if you have something to teach, we should have you present this at a
chromoting tech talk, that would be really awesome.

[Yuwei, 2017/01/09 20:28:52]

I think ASSERT will fail the test case and make the function immediately return
while EXPECT will just print out the error and still allow the rest of the test
to be run. Use ASSERT if the error is critical to the rest of the test or if you
just want to speed up the test.

Just to mention IIRC ASSERT can't be used inside destructor (might be the other
way around :P) so you'll get stuck with EXPECT in that case.

+
+  FakeGlDrawable* drawable1 = new FakeGlDrawable();
+  drawable1->SetId(1);
+  renderer_->AddDrawable(drawable1->GetWeakPtr());
+  EXPECT_EQ(2, GetDrawablesCount());
+
+  FakeGlDrawable* drawable2 = new FakeGlDrawable();
+  drawable2->SetId(2);
+  renderer_->AddDrawable(drawable2->GetWeakPtr());
+  EXPECT_EQ(3, GetDrawablesCount());
+
+  std::vector<base::WeakPtr<GlDrawable>> drawables = GetDrawables();
+  int i = 0;
+  for (auto& drawable : drawables) {

[joedow, 2016/12/22 00:29:03]

replace 'drawable' with GetDrawables() instead?

[nicholss, 2017/01/09 18:50:24]

Done.

+    FakeGlDrawable* fg = (FakeGlDrawable*)drawable.get();

[joedow, 2016/12/22 00:29:03]

can you use a c++ style cast instead a c style? 
reinterpret_cast<FakeGlDrawable*>(drawable)

[nicholss, 2017/01/09 18:50:24]

Done.

+    EXPECT_EQ(i, fg->GetId());
+    i++;
+  }
+  EXPECT_EQ(3, i);
+}
+
+TEST_F(GlRendererTest, TestAddDrawableOrder) {
+  FakeGlDrawable* drawable2 = new FakeGlDrawable();
+  drawable2->SetId(2);
+  drawable2->SetZIndex(2);
+  renderer_->AddDrawable(drawable2->GetWeakPtr());
+  EXPECT_EQ(1, GetDrawablesCount());
+
+  FakeGlDrawable* drawable0 = new FakeGlDrawable();
+  drawable0->SetId(0);
+  renderer_->AddDrawable(drawable0->GetWeakPtr());
+  EXPECT_EQ(2, GetDrawablesCount());
+
+  FakeGlDrawable* drawable1 = new FakeGlDrawable();
+  drawable1->SetId(1);
+  drawable1->SetZIndex(1);
+  renderer_->AddDrawable(drawable1->GetWeakPtr());
+  EXPECT_EQ(3, GetDrawablesCount());
+
+  std::vector<base::WeakPtr<GlDrawable>> drawables = GetDrawables();
+  int i = 0;
+  for (auto& drawable : drawables) {
+    FakeGlDrawable* fg = (FakeGlDrawable*)drawable.get();
+    EXPECT_EQ(i, fg->GetId());
+    i++;
+  }
+  EXPECT_EQ(3, i);
+}
+
+TEST_F(GlRendererTest, TestAddDrawableDrawn) {
+  renderer_->OnSurfaceCreated(-1);
+
+  delegate_.can_render_frame_ = true;
+  PostSetDesktopFrameTasks(webrtc::DesktopSize(16, 16), 1);
+
+  FakeGlDrawable* drawable0 = new FakeGlDrawable();
+  drawable0->SetId(3);
+  renderer_->AddDrawable(drawable0->GetWeakPtr());
+
+  RequestRender();
+  RunTasksInCurrentQueue();
+
+  FakeGlDrawable* drawable1 = new FakeGlDrawable();
+  drawable1->SetId(2);
+  drawable1->SetZIndex(1);
+  renderer_->AddDrawable(drawable1->GetWeakPtr());
+
+  RequestRender();
+  RunTasksInCurrentQueue();
+
+  FakeGlDrawable* drawable2 = new FakeGlDrawable();
+  drawable2->SetId(1);
+  drawable2->SetZIndex(2);
+  renderer_->AddDrawable(drawable2->GetWeakPtr());
+  EXPECT_EQ(3, GetDrawablesCount());
+
+  RequestRender();
+  RunTasksInCurrentQueue();
+  std::vector<base::WeakPtr<GlDrawable>> drawables = GetDrawables();
+  for (auto& drawable : drawables) {
+    FakeGlDrawable* fg = (FakeGlDrawable*)drawable.get();
+    EXPECT_EQ(fg->GetId(), fg->DrawnCount());
+  }
+}
+
+TEST_F(GlRendererTest, TestCreateGlRendererWithDesktop) {
+  renderer_.reset(GlRenderer::CreateGlRendererWithDesktop());
+  renderer_->SetDelegate(delegate_.GetWeakPtr());
+  EXPECT_EQ(3, GetDrawablesCount());
+}
+
+// TODO(nicholss): Add a test where the drawable is destructed and the renderer
+// gets a dead weakptr.
+
 }  // namespace remoting