media/gpu/test: more s/LOG_ASSERT/ADD_FAILURE/

This CL continues substituting LOG_ASSERT()s to avoid crashing
the test binary in Tast test runs. This last patch in the list
intervenes image_processor-client.cc, by adding a local macro
ASSERT_TRUE_OR_RETURN_NULLPTR.

This CL also takes the chance to rename ASSERT_TRUE_AND_RETURN
to ASSERT_TRUE_OR_RETURN in video_frame_helpers.cc, since that
name makes more sense (it was introduced in this series of
patches in crrev.com/c/1900559).

Bug: 1020776
Change-Id: I7f2a9c19d3eba474e8f7551854b3ec6ebd0fc943
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1907130
Auto-Submit: Miguel Casas <mcasas@chromium.org>
Reviewed-by: David Staessens <dstaessens@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#714234}

media/gpu/test/image_processor/image_processor_client.cc

@@ -32,6 +32,15 @@
 namespace media {
 namespace test {
 namespace {
+
+#define ASSERT_TRUE_OR_RETURN_NULLPTR(predicate) \
+  do {                                           \
+    if (!(predicate)) {                          \
+      ADD_FAILURE();                             \
+      return nullptr;                            \
+    }                                            \
+  } while (0)
+
 base::Optional<VideoFrameLayout> CreateLayout(
     const ImageProcessor::PortConfig& config) {
   const VideoPixelFormat pixel_format = config.fourcc.ToVideoPixelFormat();
@@ -127,15 +136,15 @@ void ImageProcessorClient::CreateImageProcessorTask(
 scoped_refptr<VideoFrame> ImageProcessorClient::CreateInputFrame(
     const Image& input_image) const {
   DCHECK_CALLED_ON_VALID_THREAD(test_main_thread_checker_);
-  LOG_ASSERT(image_processor_);
-  LOG_ASSERT(input_image.IsLoaded());
+  ASSERT_TRUE_OR_RETURN_NULLPTR(image_processor_);
+  ASSERT_TRUE_OR_RETURN_NULLPTR(input_image.IsLoaded());
 
   const ImageProcessor::PortConfig& input_config =
       image_processor_->input_config();
   const VideoFrame::StorageType input_storage_type =
       input_config.storage_type();
   base::Optional<VideoFrameLayout> input_layout = CreateLayout(input_config);
-  LOG_ASSERT(input_layout);
+  ASSERT_TRUE_OR_RETURN_NULLPTR(input_layout);
 
   if (VideoFrame::IsStorageTypeMappable(input_storage_type)) {
     return CloneVideoFrame(gpu_memory_buffer_factory_.get(),
@@ -143,7 +152,8 @@ scoped_refptr<VideoFrame> ImageProcessorClient::CreateInputFrame(
                            *input_layout, VideoFrame::STORAGE_OWNED_MEMORY);
   } else {
 #if defined(OS_CHROMEOS)
-    LOG_ASSERT(input_storage_type == VideoFrame::STORAGE_DMABUFS ||
+    ASSERT_TRUE_OR_RETURN_NULLPTR(
+        input_storage_type == VideoFrame::STORAGE_DMABUFS ||
         input_storage_type == VideoFrame::STORAGE_GPU_MEMORY_BUFFER);
     // NV12 and YV12 are the only formats that can be allocated with
     // gfx::BufferUsage::SCANOUT_VEA_READ_CAMERA_AND_CPU_READ_WRITE. So
@@ -163,22 +173,23 @@ scoped_refptr<VideoFrame> ImageProcessorClient::CreateInputFrame(
 scoped_refptr<VideoFrame> ImageProcessorClient::CreateOutputFrame(
     const Image& output_image) const {
   DCHECK_CALLED_ON_VALID_THREAD(test_main_thread_checker_);
-  LOG_ASSERT(output_image.IsMetadataLoaded());
-  LOG_ASSERT(image_processor_);
+  ASSERT_TRUE_OR_RETURN_NULLPTR(output_image.IsMetadataLoaded());
+  ASSERT_TRUE_OR_RETURN_NULLPTR(image_processor_);
 
   const ImageProcessor::PortConfig& output_config =
       image_processor_->output_config();
   const VideoFrame::StorageType output_storage_type =
       output_config.storage_type();
   base::Optional<VideoFrameLayout> output_layout = CreateLayout(output_config);
-  LOG_ASSERT(output_layout);
+  ASSERT_TRUE_OR_RETURN_NULLPTR(output_layout);
   if (VideoFrame::IsStorageTypeMappable(output_storage_type)) {
     return VideoFrame::CreateFrameWithLayout(
         *output_layout, gfx::Rect(output_image.Size()), output_image.Size(),
         base::TimeDelta(), false /* zero_initialize_memory*/);
   } else {
 #if BUILDFLAG(USE_CHROMEOS_MEDIA_ACCELERATION)
-    LOG_ASSERT(output_storage_type == VideoFrame::STORAGE_DMABUFS ||
+    ASSERT_TRUE_OR_RETURN_NULLPTR(
+        output_storage_type == VideoFrame::STORAGE_DMABUFS ||
         output_storage_type == VideoFrame::STORAGE_GPU_MEMORY_BUFFER);
     scoped_refptr<VideoFrame> output_frame = CreatePlatformVideoFrame(
         gpu_memory_buffer_factory_.get(), output_layout->format(),

media/gpu/test/video_frame_helpers.cc

@@ -32,7 +32,7 @@ namespace test {
 
 namespace {
 
-#define ASSERT_TRUE_AND_RETURN(predicate, return_value) \
+#define ASSERT_TRUE_OR_RETURN(predicate, return_value) \
   do {                                                 \
     if (!(predicate)) {                                \
       ADD_FAILURE();                                   \
@@ -42,9 +42,9 @@ namespace {
 
 bool ConvertVideoFrameToI420(const VideoFrame* src_frame,
                              VideoFrame* dst_frame) {
-  ASSERT_TRUE_AND_RETURN(src_frame->visible_rect() == dst_frame->visible_rect(),
+  ASSERT_TRUE_OR_RETURN(src_frame->visible_rect() == dst_frame->visible_rect(),
                         false);
-  ASSERT_TRUE_AND_RETURN(dst_frame->format() == PIXEL_FORMAT_I420, false);
+  ASSERT_TRUE_OR_RETURN(dst_frame->format() == PIXEL_FORMAT_I420, false);
 
   const auto& visible_rect = src_frame->visible_rect();
   const int width = visible_rect.width();
@@ -91,9 +91,9 @@ bool ConvertVideoFrameToI420(const VideoFrame* src_frame,
 
 bool ConvertVideoFrameToARGB(const VideoFrame* src_frame,
                              VideoFrame* dst_frame) {
-  ASSERT_TRUE_AND_RETURN(src_frame->visible_rect() == dst_frame->visible_rect(),
+  ASSERT_TRUE_OR_RETURN(src_frame->visible_rect() == dst_frame->visible_rect(),
                         false);
-  ASSERT_TRUE_AND_RETURN(dst_frame->format() == PIXEL_FORMAT_ARGB, false);
+  ASSERT_TRUE_OR_RETURN(dst_frame->format() == PIXEL_FORMAT_ARGB, false);
 
   const auto& visible_rect = src_frame->visible_rect();
   const int width = visible_rect.width();
@@ -137,7 +137,7 @@ bool ConvertVideoFrameToARGB(const VideoFrame* src_frame,
 // Copy memory based |src_frame| buffer to |dst_frame| buffer.
 bool CopyVideoFrame(const VideoFrame* src_frame,
                     scoped_refptr<VideoFrame> dst_frame) {
-  ASSERT_TRUE_AND_RETURN(src_frame->IsMappable(), false);
+  ASSERT_TRUE_OR_RETURN(src_frame->IsMappable(), false);
 #if BUILDFLAG(USE_CHROMEOS_MEDIA_ACCELERATION)
   // If |dst_frame| is a Dmabuf-backed VideoFrame, we need to map its underlying
   // buffer into memory. We use a VideoFrameMapper to create a memory-based
@@ -145,7 +145,7 @@ bool CopyVideoFrame(const VideoFrame* src_frame,
   if (dst_frame->storage_type() == VideoFrame::STORAGE_DMABUFS) {
     auto video_frame_mapper = VideoFrameMapperFactory::CreateMapper(
         dst_frame->format(), VideoFrame::STORAGE_DMABUFS, true);
-    ASSERT_TRUE_AND_RETURN(video_frame_mapper, false);
+    ASSERT_TRUE_OR_RETURN(video_frame_mapper, false);
     dst_frame = video_frame_mapper->Map(std::move(dst_frame));
     if (!dst_frame) {
       LOG(ERROR) << "Failed to map DMABuf video frame.";
@@ -153,14 +153,14 @@ bool CopyVideoFrame(const VideoFrame* src_frame,
     }
   }
 #endif  // BUILDFLAG(USE_CHROMEOS_MEDIA_ACCELERATION)
-  ASSERT_TRUE_AND_RETURN(dst_frame->IsMappable(), false);
-  ASSERT_TRUE_AND_RETURN(src_frame->format() == dst_frame->format(), false);
+  ASSERT_TRUE_OR_RETURN(dst_frame->IsMappable(), false);
+  ASSERT_TRUE_OR_RETURN(src_frame->format() == dst_frame->format(), false);
 
   // Copy every plane's content from |src_frame| to |dst_frame|.
   const size_t num_planes = VideoFrame::NumPlanes(dst_frame->format());
-  ASSERT_TRUE_AND_RETURN(dst_frame->layout().planes().size() == num_planes,
+  ASSERT_TRUE_OR_RETURN(dst_frame->layout().planes().size() == num_planes,
                         false);
-  ASSERT_TRUE_AND_RETURN(src_frame->layout().planes().size() == num_planes,
+  ASSERT_TRUE_OR_RETURN(src_frame->layout().planes().size() == num_planes,
                         false);
   for (size_t i = 0; i < num_planes; ++i) {
     // |width| in libyuv::CopyPlane() is in bytes, not pixels.
@@ -177,9 +177,9 @@ bool CopyVideoFrame(const VideoFrame* src_frame,
 }  // namespace
 
 bool ConvertVideoFrame(const VideoFrame* src_frame, VideoFrame* dst_frame) {
-  ASSERT_TRUE_AND_RETURN(src_frame->visible_rect() == dst_frame->visible_rect(),
+  ASSERT_TRUE_OR_RETURN(src_frame->visible_rect() == dst_frame->visible_rect(),
                         false);
-  ASSERT_TRUE_AND_RETURN(src_frame->IsMappable() && dst_frame->IsMappable(),
+  ASSERT_TRUE_OR_RETURN(src_frame->IsMappable() && dst_frame->IsMappable(),
                         false);
 
   // Writing into non-owned memory might produce some unexpected side effects.
@@ -362,13 +362,13 @@ base::Optional<VideoFrameLayout> CreateVideoFrameLayout(VideoPixelFormat format,
 size_t CompareFramesWithErrorDiff(const VideoFrame& frame1,
                                   const VideoFrame& frame2,
                                   uint8_t tolerance) {
-  ASSERT_TRUE_AND_RETURN(frame1.format() == frame2.format(),
+  ASSERT_TRUE_OR_RETURN(frame1.format() == frame2.format(),
                         std::numeric_limits<std::size_t>::max());
-  ASSERT_TRUE_AND_RETURN(frame1.visible_rect() == frame2.visible_rect(),
+  ASSERT_TRUE_OR_RETURN(frame1.visible_rect() == frame2.visible_rect(),
                         std::numeric_limits<std::size_t>::max());
-  ASSERT_TRUE_AND_RETURN(frame1.visible_rect().origin() == gfx::Point(0, 0),
+  ASSERT_TRUE_OR_RETURN(frame1.visible_rect().origin() == gfx::Point(0, 0),
                         std::numeric_limits<std::size_t>::max());
-  ASSERT_TRUE_AND_RETURN(frame1.IsMappable() && frame2.IsMappable(),
+  ASSERT_TRUE_OR_RETURN(frame1.IsMappable() && frame2.IsMappable(),
                         std::numeric_limits<std::size_t>::max());
   size_t diff_cnt = 0;