Rewrite WebContentsVideoCaptureDeviceTest.

content/browser/renderer_host/media/web_contents_video_capture_device_unittest.cc

Index: content/browser/renderer_host/media/web_contents_video_capture_device_unittest.cc
diff --git a/content/browser/renderer_host/media/web_contents_video_capture_device_unittest.cc b/content/browser/renderer_host/media/web_contents_video_capture_device_unittest.cc
index 4f41283b1ac6791562bafbfd7cbf43f1feeca7d3..65a63aa620c86ffc8d7aff899c0fb7595260b201 100644
--- a/content/browser/renderer_host/media/web_contents_video_capture_device_unittest.cc
+++ b/content/browser/renderer_host/media/web_contents_video_capture_device_unittest.cc
@@ -5,14 +5,18 @@
 #include "content/browser/renderer_host/media/web_contents_video_capture_device.h"
 
 #include "base/bind_helpers.h"
-#include "base/synchronization/condition_variable.h"
-#include "base/synchronization/waitable_event.h"
+#include "base/debug/debugger.h"
 #include "base/time.h"
+#include "base/timer.h"
 #include "content/browser/browser_thread_impl.h"
-#include "content/browser/renderer_host/render_widget_host_delegate.h"
+#include "content/browser/renderer_host/media/web_contents_capture_util.h"
+#include "content/browser/renderer_host/render_view_host_factory.h"
 #include "content/browser/renderer_host/render_widget_host_impl.h"
-#include "content/public/test/mock_render_process_host.h"
+#include "content/browser/renderer_host/test_render_view_host.h"
+#include "content/browser/web_contents/test_web_contents.h"
 #include "content/public/test/test_browser_context.h"
+#include "content/public/test/test_browser_thread.h"
+#include "content/public/test/test_utils.h"
 #include "media/base/video_util.h"
 #include "media/video/capture/video_capture_types.h"
 #include "skia/ext/platform_canvas.h"
@@ -24,106 +28,252 @@ namespace {
 const int kTestWidth = 1280;
 const int kTestHeight = 720;
 const int kBytesPerPixel = 4;
-const int kTestFramesPerSecond = 8;
-const base::TimeDelta kWaitTimeout =
-    base::TimeDelta::FromMilliseconds(2000);
+const int kTestFramesPerSecond = 20;
+const base::TimeDelta kWaitTimeout = base::TimeDelta::FromMilliseconds(2000);
 const SkColor kNothingYet = 0xdeadbeef;
 const SkColor kNotInterested = ~kNothingYet;
+
+void DeadlineExceeded(base::Closure quit_closure) {
+  if (!base::debug::BeingDebugged()) {
+    FAIL() << "Deadline exceeded while waiting, quitting";
+    quit_closure.Run();
+  } else {
+    LOG(WARNING) << "Deadline exceeded; test would fail if debugger weren't "
+                 << "attached.";
+  }
 }
 
-// A stub implementation which returns solid-color bitmaps in calls to
-// CopyFromBackingStore().  The unit tests can change the color for successive
-// captures.
-class StubRenderWidgetHost : public RenderWidgetHostImpl {
+void RunCurrentLoopWithDeadline() {
+  base::Timer deadline(false, false);
+  deadline.Start(FROM_HERE, kWaitTimeout, base::Bind(
+      &DeadlineExceeded, MessageLoop::current()->QuitClosure()));
+  MessageLoop::current()->Run();
+  deadline.Stop();
+}
+
+// Thread-safe class that controls the source pattern to be captured by the
+// system under test. The lifetime of this class is greater than the lifetime
+// of all objects that reference it, so it does not need to be reference
+// counted.
+class CaptureTestSourceController {
  public:
-  StubRenderWidgetHost(RenderProcessHost* process, int routing_id)
-      : RenderWidgetHostImpl(&delegate_, process, routing_id),
-        color_(kNothingYet),
+  CaptureTestSourceController()
+      : color_(SK_ColorMAGENTA),
         copy_result_size_(kTestWidth, kTestHeight),
-        copy_event_(false, false) {}
+        can_copy_to_video_frame_(false) {}
 
   void SetSolidColor(SkColor color) {
     base::AutoLock guard(lock_);
     color_ = color;
   }
 
+  SkColor GetSolidColor() {
+    base::AutoLock guard(lock_);
+    return color_;
+  }
+
   void SetCopyResultSize(int width, int height) {
     base::AutoLock guard(lock_);
     copy_result_size_ = gfx::Size(width, height);
   }
 
-  bool WaitForNextBackingStoreCopy() {
-    if (!copy_event_.TimedWait(kWaitTimeout)) {
-      ADD_FAILURE() << "WaitForNextBackingStoreCopy: wait deadline exceeded";
-      return false;
+  gfx::Size GetCopyResultSize() {
+    base::AutoLock guard(lock_);
+    return copy_result_size_;
+  }
+
+  void SignalBackingStoreCopy() {
+    // TODO(nick): This actually should always be happening on the UI thread.
+    base::AutoLock guard(lock_);
+    if (!copy_done_.is_null()) {
+      BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, copy_done_);
+      copy_done_.Reset();
+    }
+  }
+
+  void SetCanCopyToVideoFrame(bool value) {
+    base::AutoLock guard(lock_);
+    can_copy_to_video_frame_ = value;
+  }
+
+  bool CanCopyToVideoFrame() {
+    base::AutoLock guard(lock_);
+    return can_copy_to_video_frame_;
+  }
+
+  void WaitForNextBackingStoreCopy() {
+    {
+      base::AutoLock guard(lock_);
+      copy_done_ = MessageLoop::current()->QuitClosure();
     }
-    return true;
+    RunCurrentLoopWithDeadline();
   }
 
-  // RenderWidgetHostImpl overrides.
+ private:
+  base::Lock lock_;  // Guards changes to |color_| and |copy_result_size_|

[miu, 2013/02/20 06:45:10]

Comment should just say "guards changes to all members" or some such.

[ncarter (slow), 2013/02/22 02:16:11]

Done.

+  SkColor color_;
+  gfx::Size copy_result_size_;
+  bool can_copy_to_video_frame_;
+  base::Closure copy_done_;
+};

[miu, 2013/02/20 06:45:10]

nit: Add DISALLOW_COPY_AND_ASSIGN.

[ncarter (slow), 2013/02/22 02:16:11]

Done.

+
+// A stub implementation which returns solid-color bitmaps in calls to

[miu, 2013/02/20 06:45:10]

Comment needs adjusting.  FWICT, this class simulates the GPU-accelerated path.

[ncarter (slow), 2013/02/22 02:16:11]

Done.

+// CopyFromBackingStore().  The unit tests can change the color for successive
+// captures.
+class CaptureTestView : public TestRenderWidgetHostView {
+ public:
+  explicit CaptureTestView(RenderWidgetHostImpl* rwh,
+                           CaptureTestSourceController* controller)
+      : TestRenderWidgetHostView(rwh),
+        controller_(controller) {}
+  virtual ~CaptureTestView() {}
+
+  // TestRenderWidgetHostView overrides.
+  virtual gfx::Rect GetViewBounds() const OVERRIDE {
+    return gfx::Rect(100, 100, 100 + kTestWidth, 100 + kTestHeight);
+  }
+
+  //

[miu, 2013/02/20 06:45:10]

Unfinished comment.  Looks like this returns true if the accelerated path can be
used?

[ncarter (slow), 2013/02/22 02:16:11]

Done.

+  virtual bool CanCopyToVideoFrame() const OVERRIDE {
+    return controller_->CanCopyToVideoFrame();
+  }
+
+  virtual void CopyFromCompositingSurfaceToVideoFrame(
+      const gfx::Rect& src_subrect,
+      const scoped_refptr<media::VideoFrame>& target,
+      const base::Callback<void(bool)>& callback) OVERRIDE {
+    SkColor c = controller_->GetSolidColor();
+    media::FillYUV(target, SkColorGetR(c), SkColorGetG(c), SkColorGetB(c));
+    callback.Run(true);
+    controller_->SignalBackingStoreCopy();
+  }
+
+ private:
+  CaptureTestSourceController* controller_;

[miu, 2013/02/20 06:45:10]

nit: Type should be "CaptureTestSourceController* const" since this is only set
by the ctor.  And in the other classes below.

[ncarter (slow), 2013/02/22 02:16:11]

Done.

+  DISALLOW_IMPLICIT_CONSTRUCTORS(CaptureTestView);

[miu, 2013/02/20 06:45:10]

nit: Add newline above this line.

[ncarter (slow), 2013/02/22 02:16:11]

Done.

+};
+
+#if defined(COMPILER_MSVC)
+// See comment for same warning on RenderViewHostImpl.

[miu, 2013/02/20 06:45:10]

nit: Just so people don't have to go looking it up, maybe add a quick sentence
like "MSVC complier hates our intentional/safe diamond inheritance."

[ncarter (slow), 2013/02/22 02:16:11]

Done.

+#pragma warning(push)
+#pragma warning(disable: 4250)
+#endif
+
+// A stub implementation which returns solid-color bitmaps in calls to
+// CopyFromBackingStore(). The behavior is controlled by a
+// CaptureTestSourceController.
+class CaptureTestRenderViewHost : public TestRenderViewHost {
+ public:
+  CaptureTestRenderViewHost(SiteInstance* instance,
+                            RenderViewHostDelegate* delegate,
+                            RenderWidgetHostDelegate* widget_delegate,
+                            int routing_id,
+                            bool swapped_out,
+                            CaptureTestSourceController* controller)
+      : TestRenderViewHost(instance, delegate, widget_delegate, routing_id,
+                           swapped_out),
+        controller_(controller) {
+    // Override the default view installed by TestRenderViewHost; we need
+    // our special subclass which has mocked-out tab capture support.
+    SetView(new CaptureTestView(this, controller));
+  }
+
+  // TestRenderViewHost overrides.
   virtual void CopyFromBackingStore(
       const gfx::Rect& src_rect,
       const gfx::Size& accelerated_dst_size,
       const base::Callback<void(bool, const SkBitmap&)>& callback) OVERRIDE {
+    gfx::Size size = controller_->GetCopyResultSize();
+    SkColor color = controller_->GetSolidColor();
+
     // Although it's not necessary, use a PlatformBitmap here (instead of a
     // regular SkBitmap) to exercise possible threading issues.
-    scoped_ptr<skia::PlatformBitmap> platform_bitmap(new skia::PlatformBitmap);
-    EXPECT_TRUE(platform_bitmap->Allocate(
-        copy_result_size_.width(), copy_result_size_.height(), false));
+    skia::PlatformBitmap output;
+    EXPECT_TRUE(output.Allocate(size.width(), size.height(), false));
     {
-      SkAutoLockPixels locker(platform_bitmap->GetBitmap());
-      base::AutoLock guard(lock_);
-      platform_bitmap->GetBitmap().eraseColor(color_);
+      SkAutoLockPixels locker(output.GetBitmap());
+      output.GetBitmap().eraseColor(color);
     }
-
-    callback.Run(true, platform_bitmap->GetBitmap());
-    copy_event_.Signal();
+    callback.Run(true, output.GetBitmap());
+    controller_->SignalBackingStoreCopy();
   }
 
  private:
-  class StubRenderWidgetHostDelegate : public RenderWidgetHostDelegate {
-   public:
-    StubRenderWidgetHostDelegate() {}
-    virtual ~StubRenderWidgetHostDelegate() {}
+  CaptureTestSourceController* controller_;
 
-   private:
-    DISALLOW_COPY_AND_ASSIGN(StubRenderWidgetHostDelegate);
-  };
+  DISALLOW_IMPLICIT_CONSTRUCTORS(CaptureTestRenderViewHost);
+};
 
-  StubRenderWidgetHostDelegate delegate_;
-  base::Lock lock_;  // Guards changes to color_.
-  SkColor color_;
-  gfx::Size copy_result_size_;
-  base::WaitableEvent copy_event_;
+#if defined(COMPILER_MSVC)
+// Re-enable warning 4250
+#pragma warning(pop)
+#endif
 
-  DISALLOW_IMPLICIT_CONSTRUCTORS(StubRenderWidgetHost);
+class CaptureTestRenderViewHostFactory : public RenderViewHostFactory {
+ public:
+  explicit CaptureTestRenderViewHostFactory(
+      CaptureTestSourceController* controller) : controller_(controller) {
+    RegisterFactory(this);
+  }
+
+  virtual ~CaptureTestRenderViewHostFactory() {
+    UnregisterFactory();
+  }
+
+  // RenderViewHostFactory implementation.
+  virtual RenderViewHost* CreateRenderViewHost(
+      SiteInstance* instance,
+      RenderViewHostDelegate* delegate,
+      RenderWidgetHostDelegate* widget_delegate,
+      int routing_id,
+      bool swapped_out,
+      SessionStorageNamespace* session_storage_namespace) {
+    return new CaptureTestRenderViewHost(instance, delegate, widget_delegate,
+                                         routing_id, swapped_out, controller_);
+  }
+ private:
+  CaptureTestSourceController* controller_;
+
+  DISALLOW_IMPLICIT_CONSTRUCTORS(CaptureTestRenderViewHostFactory);
 };
 
 // A stub consumer of captured video frames, which checks the output of
 // WebContentsVideoCaptureDevice.
 class StubConsumer : public media::VideoCaptureDevice::EventHandler {
  public:
-  StubConsumer() : output_changed_(&lock_),
-                   picture_color_(kNothingYet),
-                   error_encountered_(false) {}
+  StubConsumer() : error_encountered_(false), wait_color_(0xcafe1950) {}
   virtual ~StubConsumer() {}
 
-  // Returns false if an error was encountered.
-  bool WaitForNextColorOrError(SkColor expected_color) {
-    base::TimeTicks deadline = base::TimeTicks::Now() + kWaitTimeout;
+  void QuitIfConditionMet(SkColor color) {
     base::AutoLock guard(lock_);
-    while (picture_color_ != expected_color && !error_encountered_) {
-      output_changed_.TimedWait(kWaitTimeout);
-      if (base::TimeTicks::Now() >= deadline) {
-        ADD_FAILURE() << "WaitForNextColorOrError: wait deadline exceeded";
-        return false;
-      }
+
+    if (wait_color_ == color || error_encountered_)
+      MessageLoop::current()->Quit();
+  }
+
+  void WaitForNextColor(SkColor expected_color) {
+    {
+      base::AutoLock guard(lock_);
+      wait_color_ = expected_color;
+      error_encountered_ = false;
     }
-    if (!error_encountered_) {
-      EXPECT_EQ(expected_color, picture_color_);
-      return true;
-    } else {
-      return false;
+    RunCurrentLoopWithDeadline();
+    {
+      base::AutoLock guard(lock_);
+      ASSERT_FALSE(error_encountered_);
+    }
+  }
+
+  void WaitForError() {
+    {
+      base::AutoLock guard(lock_);
+      wait_color_ = kNotInterested;
+      error_encountered_ = false;
+    }
+    RunCurrentLoopWithDeadline();
+    {
+      base::AutoLock guard(lock_);
+      ASSERT_TRUE(error_encountered_);
     }
   }
 
@@ -143,49 +293,34 @@ class StubConsumer : public media::VideoCaptureDevice::EventHandler {
       }
     }
     EXPECT_TRUE(all_pixels_are_the_same_color);
-
-    {
-      base::AutoLock guard(lock_);
-      if (color != picture_color_) {
-        picture_color_ = color;
-        output_changed_.Signal();
-      }
-    }
+    PostColorOrError(color);
   }
 
   virtual void OnIncomingCapturedVideoFrame(media::VideoFrame* frame,
                                             base::Time timestamp) OVERRIDE {
     EXPECT_EQ(gfx::Size(kTestWidth, kTestHeight), frame->coded_size());
     EXPECT_EQ(media::VideoFrame::YV12, frame->format());
-    bool all_pixels_are_the_same_color = true;
     uint8 yuv[3] = {0};
     for (int plane = 0; plane < 3; ++plane) {
       yuv[plane] = frame->data(plane)[0];
-      for (int y = 0; y < frame->rows(plane); ++y) {
-        for (int x = 0; x < frame->row_bytes(plane); ++x) {
-          if (yuv[plane] != frame->data(plane)[x + y * frame->stride(plane)]) {
-            all_pixels_are_the_same_color = false;
-            break;
-          }
-        }
-      }
     }
-    EXPECT_TRUE(all_pixels_are_the_same_color);
-    const SkColor color = SkColorSetRGB(yuv[0], yuv[1], yuv[2]);
+    // TODO(nick): We just look at the first pixel presently, because if
+    // the analysis is too slow, the backlog of frames will grow without bound
+    // and trouble erupts. http://crbug.com/174519
+    PostColorOrError(SkColorSetRGB(yuv[0], yuv[1], yuv[2]));
+  }
 
-    {
-      base::AutoLock guard(lock_);
-      if (color != picture_color_) {
-        picture_color_ = color;
-        output_changed_.Signal();
-      }
-    }
+  void PostColorOrError(SkColor new_color) {
+    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE, base::Bind(
+        &StubConsumer::QuitIfConditionMet, base::Unretained(this), new_color));
   }
 
   virtual void OnError() OVERRIDE {
-    base::AutoLock guard(lock_);
-    error_encountered_ = true;
-    output_changed_.Signal();
+    {
+      base::AutoLock guard(lock_);
+      error_encountered_ = true;
+    }
+    PostColorOrError(kNothingYet);
   }
 
   virtual void OnFrameInfo(const media::VideoCaptureCapability& info) OVERRIDE {
@@ -197,13 +332,14 @@ class StubConsumer : public media::VideoCaptureDevice::EventHandler {
 
  private:
   base::Lock lock_;
-  base::ConditionVariable output_changed_;
-  SkColor picture_color_;
   bool error_encountered_;
+  SkColor wait_color_;
 
   DISALLOW_COPY_AND_ASSIGN(StubConsumer);
 };
 
+}  // namespace
+
 // Test harness that sets up a minimal environment with necessary stubs.
 class WebContentsVideoCaptureDeviceTest : public testing::Test {
  public:
@@ -211,55 +347,75 @@ class WebContentsVideoCaptureDeviceTest : public testing::Test {
 
  protected:
   virtual void SetUp() {
-    // This is a MessageLoop for the current thread.  The MockRenderProcessHost
-    // will schedule its destruction in this MessageLoop during TearDown().
-    message_loop_.reset(new MessageLoop(MessageLoop::TYPE_IO));
+    // The main thread will serve as the UI thread as well as the test thread.
+    // We'll manually pump the run loop at appropriate times in the test.
+    ui_thread_.reset(new TestBrowserThread(BrowserThread::UI, &message_loop_));
 
-    // The CopyFromBackingStore and WebContents tracking occur on the UI thread.
-    ui_thread_.reset(new BrowserThreadImpl(BrowserThread::UI));
-    ui_thread_->Start();
+    // Create our (self-registering) RVH factory, so that when we create a
+    // WebContents, it in turn creates CaptureTestRenderViewHosts.
+    render_view_host_factory_.reset(
+        new CaptureTestRenderViewHostFactory(&controller_));
 
-    // And the rest...
     browser_context_.reset(new TestBrowserContext());
-    source_.reset(new StubRenderWidgetHost(
-        new MockRenderProcessHost(browser_context_.get()), MSG_ROUTING_NONE));
-    destroyed_.reset(new base::WaitableEvent(true, false));
-    device_.reset(WebContentsVideoCaptureDevice::CreateForTesting(
-        source_.get(),
-        base::Bind(&base::WaitableEvent::Signal,
-                   base::Unretained(destroyed_.get()))));
-    consumer_.reset(new StubConsumer);
+
+    scoped_refptr<SiteInstance> site_instance =
+        SiteInstance::Create(browser_context_.get());
+    web_contents_.reset(
+        TestWebContents::Create(browser_context_.get(), site_instance));
+
+    // This is actually a CaptureTestRenderViewHost.
+    RenderWidgetHostImpl* rwh =
+        RenderWidgetHostImpl::From(web_contents_->GetRenderViewHost());
+
+    std::string device_id =
+        WebContentsCaptureUtil::AppendWebContentsDeviceScheme(
+            base::StringPrintf("%d:%d", rwh->GetProcess()->GetID(),
+                               rwh->GetRoutingID()));
+    device_.reset(WebContentsVideoCaptureDevice::Create(device_id));
   }
 
   virtual void TearDown() {
     // Tear down in opposite order of set-up.
-    device_->DeAllocate();  // Guarantees no more use of consumer_.
-    consumer_.reset();
+    device_->DeAllocate();
     device_.reset();  // Release reference to internal CaptureMachine.
-    message_loop_->RunUntilIdle();  // Just in case.
-    destroyed_->Wait();  // Wait until CaptureMachine is fully destroyed.
-    destroyed_.reset();
-    source_.reset();
+    content::RunAllPendingInMessageLoop();
+
+    web_contents_.reset();
     browser_context_.reset();
-    ui_thread_->Stop();
-    ui_thread_.reset();
-    message_loop_->RunUntilIdle();  // Deletes MockRenderProcessHost.
-    message_loop_.reset();
+
+    content::RunAllPendingInMessageLoop();
+    message_loop_.RunUntilIdle();

[miu, 2013/02/20 06:45:10]

Can you confirm whether this is still needed?  Since we're not creating a
MockRenderProcess host anymore, I think we're avoiding the
destruction-posted-to-message-loop weirdness now.

[ncarter (slow), 2013/02/22 02:16:11]

Reworked this.

+
+    render_view_host_factory_.reset();
   }
 
   // Accessors.
-  StubRenderWidgetHost* source() const { return source_.get(); }
-  media::VideoCaptureDevice* device() const { return device_.get(); }
-  StubConsumer* consumer() const { return consumer_.get(); }
+  CaptureTestSourceController* source() { return &controller_; }
+  media::VideoCaptureDevice* device() { return device_.get(); }
+  StubConsumer* consumer() { return &consumer_; }
 
  private:
-  scoped_ptr<MessageLoop> message_loop_;
-  scoped_ptr<BrowserThreadImpl> ui_thread_;
+  // The consumer is the ultimate recipient of captured pixel data.
+  StubConsumer consumer_;
+
+  // The controller controls which pixel patterns to produce.
+  CaptureTestSourceController controller_;
+
+  // We run the UI message loop on the main thread. The capture device
+  // will also spin up its own threads.
+  MessageLoopForUI message_loop_;
+  scoped_ptr<TestBrowserThread> ui_thread_;
+
+  // Creates capture-capable RenderViewHosts whose pixel content production is
+  // under the control of |controller_|.
+  scoped_ptr<CaptureTestRenderViewHostFactory> render_view_host_factory_;
+
+  // A mocked-out browser and tab.
   scoped_ptr<TestBrowserContext> browser_context_;
-  scoped_ptr<StubRenderWidgetHost> source_;
-  scoped_ptr<base::WaitableEvent> destroyed_;
+  scoped_ptr<WebContents> web_contents_;
+
+  // Finally, the WebContentsVideoCaptureDevice under test.
   scoped_ptr<media::VideoCaptureDevice> device_;
-  scoped_ptr<StubConsumer> consumer_;
 
   DISALLOW_COPY_AND_ASSIGN(WebContentsVideoCaptureDeviceTest);
 };
@@ -270,28 +426,24 @@ class WebContentsVideoCaptureDeviceTest : public testing::Test {
 // paths, just as RenderWidgetHost might if the content falls in and out of
 // accelerated compositing.
 TEST_F(WebContentsVideoCaptureDeviceTest, GoesThroughAllTheMotions) {
-  device()->Allocate(kTestWidth, kTestHeight, kTestFramesPerSecond,
-                     consumer());
+  device()->Allocate(kTestWidth, kTestHeight, kTestFramesPerSecond, consumer());
 
   device()->Start();
 
   bool use_video_frames = false;
   for (int i = 0; i < 4; i++, use_video_frames = !use_video_frames) {
-    SCOPED_TRACE(
-        testing::Message() << "Using "
-                           << (use_video_frames ? "VideoFrame" : "SkBitmap")
-                           << " path, iteration #" << i);
-    // TODO(nick): Implement this.
-    // source()->SetUseVideoFrames(use_video_frames);
+    SCOPED_TRACE(StringPrintf("Using %s path, iteration #%d",
+                              use_video_frames ? "VideoFrame" : "SkBitmap", i));
+    source()->SetCanCopyToVideoFrame(use_video_frames);
     source()->SetSolidColor(SK_ColorRED);
-    source()->WaitForNextBackingStoreCopy();
-    ASSERT_TRUE(consumer()->WaitForNextColorOrError(SK_ColorRED));
+    ASSERT_NO_FATAL_FAILURE(source()->WaitForNextBackingStoreCopy());
+    ASSERT_NO_FATAL_FAILURE(consumer()->WaitForNextColor(SK_ColorRED));
     source()->SetSolidColor(SK_ColorGREEN);
-    ASSERT_TRUE(consumer()->WaitForNextColorOrError(SK_ColorGREEN));
+    ASSERT_NO_FATAL_FAILURE(consumer()->WaitForNextColor(SK_ColorGREEN));
     source()->SetSolidColor(SK_ColorBLUE);
-    ASSERT_TRUE(consumer()->WaitForNextColorOrError(SK_ColorBLUE));
+    ASSERT_NO_FATAL_FAILURE(consumer()->WaitForNextColor(SK_ColorBLUE));
     source()->SetSolidColor(SK_ColorBLACK);
-    ASSERT_TRUE(consumer()->WaitForNextColorOrError(SK_ColorBLACK));
+    ASSERT_NO_FATAL_FAILURE(consumer()->WaitForNextColor(SK_ColorBLACK));
   }
 
   device()->DeAllocate();
@@ -299,13 +451,11 @@ TEST_F(WebContentsVideoCaptureDeviceTest, GoesThroughAllTheMotions) {
 
 TEST_F(WebContentsVideoCaptureDeviceTest, RejectsInvalidAllocateParams) {
   device()->Allocate(1280, 720, -2, consumer());
-  EXPECT_FALSE(consumer()->WaitForNextColorOrError(kNotInterested));
+  ASSERT_NO_FATAL_FAILURE(consumer()->WaitForError());
 }
 
 TEST_F(WebContentsVideoCaptureDeviceTest, BadFramesGoodFrames) {
-  device()->Allocate(kTestWidth, kTestHeight, kTestFramesPerSecond,
-                     consumer());
-
+  device()->Allocate(kTestWidth, kTestHeight, kTestFramesPerSecond, consumer());
 
   // 1x1 is too small to process; we intend for this to result in an error.
   source()->SetCopyResultSize(1, 1);
@@ -314,18 +464,20 @@ TEST_F(WebContentsVideoCaptureDeviceTest, BadFramesGoodFrames) {
 
   // These frames ought to be dropped during the Render stage. Let
   // several captures to happen.
-  ASSERT_TRUE(source()->WaitForNextBackingStoreCopy());
-  ASSERT_TRUE(source()->WaitForNextBackingStoreCopy());
-  ASSERT_TRUE(source()->WaitForNextBackingStoreCopy());
-  ASSERT_TRUE(source()->WaitForNextBackingStoreCopy());
-  ASSERT_TRUE(source()->WaitForNextBackingStoreCopy());
+  ASSERT_NO_FATAL_FAILURE(source()->WaitForNextBackingStoreCopy());
+  ASSERT_NO_FATAL_FAILURE(source()->WaitForNextBackingStoreCopy());
+  ASSERT_NO_FATAL_FAILURE(source()->WaitForNextBackingStoreCopy());
+  ASSERT_NO_FATAL_FAILURE(source()->WaitForNextBackingStoreCopy());
+  ASSERT_NO_FATAL_FAILURE(source()->WaitForNextBackingStoreCopy());
 
   // Now push some good frames through; they should be processed normally.
   source()->SetCopyResultSize(kTestWidth, kTestHeight);
   source()->SetSolidColor(SK_ColorGREEN);
-  EXPECT_TRUE(consumer()->WaitForNextColorOrError(SK_ColorGREEN));
+  ASSERT_NO_FATAL_FAILURE(consumer()->WaitForNextColor(SK_ColorGREEN));
   source()->SetSolidColor(SK_ColorRED);
-  EXPECT_TRUE(consumer()->WaitForNextColorOrError(SK_ColorRED));
+  ASSERT_NO_FATAL_FAILURE(consumer()->WaitForNextColor(SK_ColorRED));
+
+  device()->Stop();
   device()->DeAllocate();
 }