Improve checking for successful Decode() without returning a frame

There's been a bunch of crashes where it appears no video frame was returned
even though status = success. This change adds additional logging and converts
some of the DCHECKs to CHECK so that the problem is caught earlier.

BUG=828148
TEST=encrypted media browser_tests pass

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Ib15465179f4931860b6b6fb4192c320e09a30d7e
Reviewed-on: https://chromium-review.googlesource.com/993711Reviewed-by: Chris Palmer <palmer@chromium.org>
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Commit-Queue: John Rummell <jrummell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548665}

media/cdm/cdm_adapter.cc

@@ -878,6 +878,12 @@ void CdmAdapter::DecryptAndDecodeVideo(
 
   scoped_refptr<VideoFrame> decoded_frame =
       video_frame->TransformToVideoFrame(natural_size_);
+  if (!decoded_frame) {
+    DLOG(ERROR) << __func__ << ": TransformToVideoFrame failed.";
+    video_decode_cb.Run(Decryptor::kError, nullptr);
+    return;
+  }
+
   video_decode_cb.Run(Decryptor::kSuccess, decoded_frame);
 }
 

media/filters/decrypting_video_decoder.cc

@@ -294,6 +294,8 @@ void DecryptingVideoDecoder::DeliverFrame(
   }
 
   DCHECK_EQ(status, Decryptor::kSuccess);
+  CHECK(frame);
+
   // Frame returned with kSuccess should not be an end-of-stream frame.
   DCHECK(!frame->metadata()->IsTrue(VideoFrameMetadata::END_OF_STREAM));
 

media/mojo/common/mojo_shared_buffer_video_frame.cc

@@ -36,8 +36,10 @@ MojoSharedBufferVideoFrame::CreateDefaultI420(const gfx::Size& dimensions,
   const size_t allocation_size = VideoFrame::AllocationSize(format, coded_size);
   mojo::ScopedSharedBufferHandle handle =
       mojo::SharedBufferHandle::Create(allocation_size);
-  if (!handle.is_valid())
+  if (!handle.is_valid()) {
+    DLOG(ERROR) << __func__ << " Unable to allocate memory.";
     return nullptr;
+  }
 
   // Create and initialize the frame. As this is I420 format, the U and V
   // planes have samples for each 2x2 block. The memory is laid out as follows:
@@ -125,8 +127,11 @@ scoped_refptr<MojoSharedBufferVideoFrame> MojoSharedBufferVideoFrame::Create(
       new MojoSharedBufferVideoFrame(format, coded_size, visible_rect,
                                      natural_size, std::move(handle), data_size,
                                      timestamp));
-  if (!frame->Init(y_stride, u_stride, v_stride, y_offset, u_offset, v_offset))
+  if (!frame->Init(y_stride, u_stride, v_stride, y_offset, u_offset,
+                   v_offset)) {
+    DLOG(ERROR) << __func__ << " MojoSharedBufferVideoFrame::Init failed.";
     return nullptr;
+  }
 
   return frame;
 }

media/mojo/interfaces/video_frame_struct_traits.cc

@@ -140,6 +140,9 @@ bool StructTraits<media::mojom::VideoFrameDataView,
     return false;
   }
 
+  if (!frame)
+    return false;
+
   std::unique_ptr<base::DictionaryValue> metadata;
   if (!input.ReadMetadata(&metadata))
     return false;

media/mojo/services/mojo_cdm_allocator.cc

@@ -148,6 +148,13 @@ class MojoCdmVideoFrame : public VideoFrameImpl {
             PlaneOffset(kYPlane), PlaneOffset(kUPlane), PlaneOffset(kVPlane),
             Stride(kYPlane), Stride(kUPlane), Stride(kVPlane),
             base::TimeDelta::FromMicroseconds(Timestamp()));
+
+    // If |frame| is not created something is wrong with the video frame data
+    // returned by the CDM. Catch it here rather than returning a null frame
+    // to the renderer.
+    // TODO(crbug.com/829443). Monitor crashes to see if this happens.
+    CHECK(frame);
+
     frame->SetMojoSharedBufferDoneCB(mojo_shared_buffer_done_cb_);
     return frame;
   }