Use RunLoop to wait for readback on compositor unittest

ui/compositor/layer_unittest.cc

 // Copyright (c) 2012 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/bind.h"
 #include "base/compiler_specific.h"
 #include "base/debug/trace_event.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
@@ skipping 114 matching lines @@
     layer->SetBounds(bounds);
     return layer;
   }
 
   void DrawTree(Layer* root) {
     GetCompositor()->SetRootLayer(root);
     GetCompositor()->ScheduleDraw();
     WaitForSwap();
   }
 
-  bool ReadPixels(SkBitmap* bitmap) {
-    return ReadPixels(bitmap, gfx::Rect(GetCompositor()->size()));
+  void ReadPixels(SkBitmap* bitmap) {
+    ReadPixels(bitmap, gfx::Rect(GetCompositor()->size()));
   }
 
-  bool ReadPixels(SkBitmap* bitmap, gfx::Rect source_rect) {
+  void ReadPixels(SkBitmap* bitmap, gfx::Rect source_rect) {
     scoped_refptr<ReadbackHolder> holder(new ReadbackHolder);
     scoped_ptr<cc::CopyOutputRequest> request =
         cc::CopyOutputRequest::CreateBitmapRequest(
             base::Bind(&ReadbackHolder::OutputRequestCallback, holder));
     request->set_area(source_rect);
 
     GetCompositor()->root_layer()->RequestCopyOfOutput(request.Pass());
 
     // Wait for copy response.  The copy output request will get committed
     // before the first draw, but may not be part of the first draw's frame.
     // The second draw will perform the async copy request, post the callback.
-    // The second loop finishes before the callback is run, so a third
-    // loop is needed.
-    for (int i = 0; i < 3; i++) {
+    for (int i = 0; i < 2; i++) {
       GetCompositor()->ScheduleFullRedraw();
       WaitForDraw();
     }
 
-    if (holder->completed()) {
-      *bitmap = holder->result();
-      return true;
-    }
+    // Waits for the callback to finish run and return result.
+    holder->WaitForReadback();
 
-    // Callback never called.
-    NOTREACHED();
-    return false;
+    *bitmap = holder->result();
   }
 
   void WaitForDraw() {
     ui::DrawWaiterForTest::WaitForCompositingStarted(GetCompositor());
   }
 
   void WaitForSwap() {
     DrawWaiterForTest::WaitForCompositingEnded(GetCompositor());
   }
 
   void WaitForCommit() {
     ui::DrawWaiterForTest::WaitForCommit(GetCompositor());
   }
 
   // Invalidates the entire contents of the layer.
   void SchedulePaintForLayer(Layer* layer) {
     layer->SchedulePaint(
         gfx::Rect(0, 0, layer->bounds().width(), layer->bounds().height()));
   }
 
   const base::FilePath& test_data_directory() const {
     return test_data_directory_;
   }
 
  private:
   class ReadbackHolder : public base::RefCountedThreadSafe<ReadbackHolder> {
    public:
-    ReadbackHolder() : completed_(false) {}
+    ReadbackHolder() : run_loop_(new base::RunLoop) {}
 
     void OutputRequestCallback(scoped_ptr<cc::CopyOutputResult> result) {
-      DCHECK(!completed_);
       result_ = result->TakeBitmap();
-      completed_ = true;
+      base::MessageLoop::current()->PostTask(FROM_HERE,

[danakj, 2014/09/19 22:51:15]

you could just QuitClosure().Run() instead of posttasking it.

but even better just run_loop_.Quit(); :)

[danakj, 2014/09/19 23:36:17]

LGTM once you Quit() more directly here.

+                                             run_loop_->QuitClosure());
     }
-    bool completed() const {
-      return completed_;
-    };
+
+    void WaitForReadback() { run_loop_->Run(); }
+
     const SkBitmap& result() const { return *result_; }
 
    private:
     friend class base::RefCountedThreadSafe<ReadbackHolder>;
 
     virtual ~ReadbackHolder() {}
 
     scoped_ptr<SkBitmap> result_;
-    bool completed_;
+    scoped_ptr<base::RunLoop> run_loop_;
   };
 
   scoped_ptr<TestCompositorHost> compositor_host_;
 
   // The root directory for test files.
   base::FilePath test_data_directory_;
 
   DISALLOW_COPY_AND_ASSIGN(LayerWithRealCompositorTest);
 };
 
@@ skipping 665 matching lines @@
       CreateColorLayer(SK_ColorRED, gfx::Rect(viewport_size)));
   scoped_ptr<Layer> layer2(
       CreateColorLayer(SK_ColorBLUE,
                        gfx::Rect(0, 0, viewport_size.width(), blue_height)));
 
   layer->Add(layer2.get());
 
   DrawTree(layer.get());
 
   SkBitmap bitmap;
-  ASSERT_TRUE(ReadPixels(&bitmap, gfx::Rect(viewport_size)));
+  ReadPixels(&bitmap, gfx::Rect(viewport_size));
   ASSERT_FALSE(bitmap.empty());
 
   SkAutoLockPixels lock(bitmap);
   for (int x = 0; x < viewport_size.width(); x++) {
     for (int y = 0; y < viewport_size.height(); y++) {
       SkColor actual_color = bitmap.getColor(x, y);
       SkColor expected_color = y < blue_height ? SK_ColorBLUE : SK_ColorRED;
       ExpectRgba(x, y, expected_color, actual_color);
     }
   }
@@ skipping 17 matching lines @@
   scoped_ptr<Layer> foreground_layer(
       CreateColorLayer(blue_with_alpha, gfx::Rect(viewport_size)));
 
   // This must be set to false for layers with alpha to be blended correctly.
   foreground_layer->SetFillsBoundsOpaquely(false);
 
   background_layer->Add(foreground_layer.get());
   DrawTree(background_layer.get());
 
   SkBitmap bitmap;
-  ASSERT_TRUE(ReadPixels(&bitmap, gfx::Rect(viewport_size)));
+  ReadPixels(&bitmap, gfx::Rect(viewport_size));
   ASSERT_FALSE(bitmap.empty());
 
   SkAutoLockPixels lock(bitmap);
   for (int x = 0; x < test_size; x++) {
     for (int y = 0; y < test_size; y++) {
       SkColor actual_color = bitmap.getColor(x, y);
       ExpectRgba(x, y, blend_color, actual_color);
     }
   }
 }
@@ skipping 20 matching lines @@
   SkRegion shape;
   shape.setRect(0, 0, viewport_size.width(), blue_height);
   foreground_layer->SetAlphaShape(make_scoped_ptr(new SkRegion(shape)));
 
   foreground_layer->SetFillsBoundsOpaquely(false);
 
   background_layer->Add(foreground_layer.get());
   DrawTree(background_layer.get());
 
   SkBitmap bitmap;
-  ASSERT_TRUE(ReadPixels(&bitmap, gfx::Rect(viewport_size)));
+  ReadPixels(&bitmap, gfx::Rect(viewport_size));
   ASSERT_FALSE(bitmap.empty());
 
   SkAutoLockPixels lock(bitmap);
   for (int x = 0; x < test_size; x++) {
     for (int y = 0; y < test_size; y++) {
       SkColor actual_color = bitmap.getColor(x, y);
       ExpectRgba(x, y, actual_color,
                  y < blue_height ? blend_color : SK_ColorRED);
     }
   }
@@ skipping 125 matching lines @@
   base::FilePath ref_img1 =
       test_data_directory().AppendASCII("ModifyHierarchy1.png");
   base::FilePath ref_img2 =
       test_data_directory().AppendASCII("ModifyHierarchy2.png");
   SkBitmap bitmap;
 
   l0->Add(l11.get());
   l11->Add(l21.get());
   l0->Add(l12.get());
   DrawTree(l0.get());
-  ASSERT_TRUE(ReadPixels(&bitmap));
+  ReadPixels(&bitmap);
   ASSERT_FALSE(bitmap.empty());
   // WritePNGFile(bitmap, ref_img1);
   EXPECT_TRUE(MatchesPNGFile(bitmap, ref_img1, cc::ExactPixelComparator(true)));
 
   l0->StackAtTop(l11.get());
   DrawTree(l0.get());
-  ASSERT_TRUE(ReadPixels(&bitmap));
+  ReadPixels(&bitmap);
   ASSERT_FALSE(bitmap.empty());
   // WritePNGFile(bitmap, ref_img2);
   EXPECT_TRUE(MatchesPNGFile(bitmap, ref_img2, cc::ExactPixelComparator(true)));
 
   // should restore to original configuration
   l0->StackAbove(l12.get(), l11.get());
   DrawTree(l0.get());
-  ASSERT_TRUE(ReadPixels(&bitmap));
+  ReadPixels(&bitmap);
   ASSERT_FALSE(bitmap.empty());
   EXPECT_TRUE(MatchesPNGFile(bitmap, ref_img1, cc::ExactPixelComparator(true)));
 
   // l11 back to front
   l0->StackAtTop(l11.get());
   DrawTree(l0.get());
-  ASSERT_TRUE(ReadPixels(&bitmap));
+  ReadPixels(&bitmap);
   ASSERT_FALSE(bitmap.empty());
   EXPECT_TRUE(MatchesPNGFile(bitmap, ref_img2, cc::ExactPixelComparator(true)));
 
   // should restore to original configuration
   l0->StackAbove(l12.get(), l11.get());
   DrawTree(l0.get());
-  ASSERT_TRUE(ReadPixels(&bitmap));
+  ReadPixels(&bitmap);
   ASSERT_FALSE(bitmap.empty());
   EXPECT_TRUE(MatchesPNGFile(bitmap, ref_img1, cc::ExactPixelComparator(true)));
 
   // l11 back to front
   l0->StackAbove(l11.get(), l12.get());
   DrawTree(l0.get());
-  ASSERT_TRUE(ReadPixels(&bitmap));
+  ReadPixels(&bitmap);
   ASSERT_FALSE(bitmap.empty());
   EXPECT_TRUE(MatchesPNGFile(bitmap, ref_img2, cc::ExactPixelComparator(true)));
 }
 
 // Opacity is rendered correctly.
 // Checks that modifying the hierarchy correctly affects final composite.
 TEST_F(LayerWithRealCompositorTest, Opacity) {
   GetCompositor()->SetScaleAndSize(1.0f, gfx::Size(50, 50));
 
   // l0
   //  +-l11
   scoped_ptr<Layer> l0(CreateColorLayer(SK_ColorRED,
                                         gfx::Rect(0, 0, 50, 50)));
   scoped_ptr<Layer> l11(CreateColorLayer(SK_ColorGREEN,
                                          gfx::Rect(0, 0, 25, 25)));
 
   base::FilePath ref_img = test_data_directory().AppendASCII("Opacity.png");
 
   l11->SetOpacity(0.75);
   l0->Add(l11.get());
   DrawTree(l0.get());
   SkBitmap bitmap;
-  ASSERT_TRUE(ReadPixels(&bitmap));
+  ReadPixels(&bitmap);
   ASSERT_FALSE(bitmap.empty());
   // WritePNGFile(bitmap, ref_img);
   EXPECT_TRUE(MatchesPNGFile(bitmap, ref_img, cc::ExactPixelComparator(true)));
 }
 
 namespace {
 
 class SchedulePaintLayerDelegate : public LayerDelegate {
  public:
   SchedulePaintLayerDelegate() : paint_count_(0), layer_(NULL) {}
@@ skipping 540 matching lines @@
                                      MakeFrameData(gfx::Size(10, 10))));
   layer->SetShowDelegatedContent(frame_provider.get(), gfx::Size(10, 10));
 
   EXPECT_FALSE(delegate.delegated_frame_damage_called());
   layer->OnDelegatedFrameDamage(damage_rect);
   EXPECT_TRUE(delegate.delegated_frame_damage_called());
   EXPECT_EQ(damage_rect, delegate.delegated_frame_damage_rect());
 }
 
 }  // namespace ui