Add CHECKs to diagnose SubmitCompositorFrame deserialization failures

Add CHECKS to the StructTraits of objects that are sent via
SubmitCompositorFrame to get a better understanding of what is failing.
These CHECKs are only there temporarily and will be reverted before
M80's branch cut.

Bug: 979564
Change-Id: I184830991ad2cfacd3f3ccb1d30e74f89f13a838
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1890000
Commit-Queue: Saman Sami <samans@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#712576}

services/viz/public/cpp/compositing/compositor_frame_metadata_mojom_traits.cc

@@ -18,18 +18,16 @@ bool StructTraits<viz::mojom::CompositorFrameMetadataDataView,
                   viz::CompositorFrameMetadata>::
     Read(viz::mojom::CompositorFrameMetadataDataView data,
          viz::CompositorFrameMetadata* out) {
-  if (data.device_scale_factor() <= 0)
-    return false;
+  // TODO(samans): Replace CHECKs with early-outs once we find out the cause for
+  // deserialization failures. https://crbug.com/979564
+  CHECK_GT(data.device_scale_factor(), 0);
   out->device_scale_factor = data.device_scale_factor();
-  if (!data.ReadRootScrollOffset(&out->root_scroll_offset))
-    return false;
+  CHECK(data.ReadRootScrollOffset(&out->root_scroll_offset));
 
   out->page_scale_factor = data.page_scale_factor();
-  if (!data.ReadScrollableViewportSize(&out->scrollable_viewport_size))
-    return false;
+  CHECK(data.ReadScrollableViewportSize(&out->scrollable_viewport_size));
 
-  if (data.frame_token() == 0u)
-    return false;
+  CHECK(data.frame_token());
   out->frame_token = data.frame_token();
 
   out->may_contain_video = data.may_contain_video();
@@ -45,15 +43,16 @@ bool StructTraits<viz::mojom::CompositorFrameMetadataDataView,
   out->bottom_controls_shown_ratio = data.bottom_controls_shown_ratio();
 #endif
 
-  return data.ReadLatencyInfo(&out->latency_info) &&
-         data.ReadReferencedSurfaces(&out->referenced_surfaces) &&
-         data.ReadDeadline(&out->deadline) &&
-         data.ReadActivationDependencies(&out->activation_dependencies) &&
-         data.ReadBeginFrameAck(&out->begin_frame_ack) &&
-         data.ReadLocalSurfaceIdAllocationTime(
-             &out->local_surface_id_allocation_time) &&
-         !out->local_surface_id_allocation_time.is_null() &&
-         data.ReadPreferredFrameInterval(&out->preferred_frame_interval);
+  CHECK(data.ReadLatencyInfo(&out->latency_info));
+  CHECK(data.ReadReferencedSurfaces(&out->referenced_surfaces));
+  CHECK(data.ReadDeadline(&out->deadline));
+  CHECK(data.ReadActivationDependencies(&out->activation_dependencies));
+  CHECK(data.ReadBeginFrameAck(&out->begin_frame_ack));
+  CHECK(data.ReadLocalSurfaceIdAllocationTime(
+      &out->local_surface_id_allocation_time));
+  CHECK(!out->local_surface_id_allocation_time.is_null());
+  CHECK(data.ReadPreferredFrameInterval(&out->preferred_frame_interval));
+  return true;
 }
 
 }  // namespace mojo

services/viz/public/cpp/compositing/compositor_frame_mojom_traits.cc

@@ -9,11 +9,12 @@ namespace mojo {
 // static
 bool StructTraits<viz::mojom::CompositorFrameDataView, viz::CompositorFrame>::
     Read(viz::mojom::CompositorFrameDataView data, viz::CompositorFrame* out) {
-  return data.ReadPasses(&out->render_pass_list) &&
-         !out->render_pass_list.empty() &&
-         !out->render_pass_list.back()->output_rect.size().IsEmpty() &&
-         data.ReadMetadata(&out->metadata) &&
-         data.ReadResources(&out->resource_list);
+  CHECK(data.ReadPasses(&out->render_pass_list));
+  CHECK(!out->render_pass_list.empty());
+  CHECK(!out->render_pass_list.back()->output_rect.size().IsEmpty());
+  CHECK(data.ReadMetadata(&out->metadata));
+  CHECK(data.ReadResources(&out->resource_list));
+  return true;
 }
 
 }  // namespace mojo

services/viz/public/cpp/compositing/local_surface_id_mojom_traits.h

@@ -32,7 +32,9 @@ struct StructTraits<viz::mojom::LocalSurfaceIdDataView, viz::LocalSurfaceId> {
                    viz::LocalSurfaceId* out) {
     out->parent_sequence_number_ = data.parent_sequence_number();
     out->child_sequence_number_ = data.child_sequence_number();
-    return data.ReadEmbedToken(&out->embed_token_) && out->is_valid();
+    CHECK(data.ReadEmbedToken(&out->embed_token_));
+    CHECK(out->is_valid());
+    return true;
   }
 };
 

services/viz/public/cpp/compositing/render_pass_mojom_traits.cc

@@ -15,20 +15,17 @@ bool StructTraits<viz::mojom::RenderPassDataView,
     Read(viz::mojom::RenderPassDataView data,
          std::unique_ptr<viz::RenderPass>* out) {
   *out = viz::RenderPass::Create();
-  if (!data.ReadOutputRect(&(*out)->output_rect) ||
-      !data.ReadDamageRect(&(*out)->damage_rect) ||
-      !data.ReadTransformToRootTarget(&(*out)->transform_to_root_target) ||
-      !data.ReadFilters(&(*out)->filters) ||
-      !data.ReadBackdropFilters(&(*out)->backdrop_filters) ||
-      !data.ReadBackdropFilterBounds(&(*out)->backdrop_filter_bounds) ||
-      !data.ReadColorSpace(&(*out)->color_space) ||
-      !data.ReadCopyRequests(&(*out)->copy_requests)) {
-    return false;
-  }
+  CHECK(data.ReadOutputRect(&(*out)->output_rect));
+  CHECK(data.ReadDamageRect(&(*out)->damage_rect));
+  CHECK(data.ReadTransformToRootTarget(&(*out)->transform_to_root_target));
+  CHECK(data.ReadFilters(&(*out)->filters));
+  CHECK(data.ReadBackdropFilters(&(*out)->backdrop_filters));
+  CHECK(data.ReadBackdropFilterBounds(&(*out)->backdrop_filter_bounds));
+  CHECK(data.ReadColorSpace(&(*out)->color_space));
+  CHECK(data.ReadCopyRequests(&(*out)->copy_requests));
   (*out)->id = data.id();
   // RenderPass ids are never zero.
-  if (!(*out)->id)
-    return false;
+  CHECK((*out)->id);
   (*out)->has_transparent_background = data.has_transparent_background();
   (*out)->cache_render_pass = data.cache_render_pass();
   (*out)->has_damage_from_contributing_content =
@@ -46,10 +43,8 @@ bool StructTraits<viz::mojom::RenderPassDataView,
 
     viz::DrawQuad* quad =
         AllocateAndConstruct(quad_state_data_view.tag(), &(*out)->quad_list);
-    if (!quad)
-      return false;
-    if (!quads.Read(i, quad))
-      return false;
+    CHECK(quad);
+    CHECK(quads.Read(i, quad));
 
     // Read the SharedQuadState.
     viz::mojom::SharedQuadStateDataView sqs_data_view;
@@ -58,12 +53,10 @@ bool StructTraits<viz::mojom::RenderPassDataView,
     // one.
     if (!sqs_data_view.is_null()) {
       last_sqs = (*out)->CreateAndAppendSharedQuadState();
-      if (!quad_data_view.ReadSqs(last_sqs))
-        return false;
+      CHECK(quad_data_view.ReadSqs(last_sqs));
     }
     quad->shared_quad_state = last_sqs;
-    if (!quad->shared_quad_state)
-      return false;
+    CHECK(quad->shared_quad_state);
   }
   return true;
 }

services/viz/public/cpp/compositing/transferable_resource_mojom_traits.cc

@@ -17,12 +17,10 @@ bool StructTraits<viz::mojom::TransferableResourceDataView,
                   viz::TransferableResource>::
     Read(viz::mojom::TransferableResourceDataView data,
          viz::TransferableResource* out) {
-  if (!data.ReadSize(&out->size) ||
-      !data.ReadMailboxHolder(&out->mailbox_holder) ||
-      !data.ReadColorSpace(&out->color_space) ||
-      !data.ReadYcbcrInfo(&out->ycbcr_info)) {
-    return false;
-  }
+  CHECK(data.ReadSize(&out->size));
+  CHECK(data.ReadMailboxHolder(&out->mailbox_holder));
+  CHECK(data.ReadColorSpace(&out->color_space));
+  CHECK(data.ReadYcbcrInfo(&out->ycbcr_info));
   out->id = data.id();
   out->format = static_cast<viz::ResourceFormat>(data.format());
   out->filter = data.filter();

services/viz/public/cpp/hit_test/hit_test_region_list_mojom_traits.cc

@@ -10,12 +10,9 @@ namespace mojo {
 bool StructTraits<viz::mojom::HitTestRegionDataView, viz::HitTestRegion>::Read(
     viz::mojom::HitTestRegionDataView data,
     viz::HitTestRegion* out) {
-  if (!data.ReadFrameSinkId(&out->frame_sink_id))
-    return false;
-  if (!data.ReadRect(&out->rect))
-    return false;
-  if (!data.ReadTransform(&out->transform))
-    return false;
+  CHECK(data.ReadFrameSinkId(&out->frame_sink_id));
+  CHECK(data.ReadRect(&out->rect));
+  CHECK(data.ReadTransform(&out->transform));
   out->flags = data.flags();
   out->async_hit_test_reasons = data.async_hit_test_reasons();
   return true;
@@ -26,12 +23,9 @@ bool StructTraits<
     viz::mojom::HitTestRegionListDataView,
     viz::HitTestRegionList>::Read(viz::mojom::HitTestRegionListDataView data,
                                   viz::HitTestRegionList* out) {
-  if (!data.ReadRegions(&out->regions))
-    return false;
-  if (!data.ReadBounds(&out->bounds))
-    return false;
-  if (!data.ReadTransform(&out->transform))
-    return false;
+  CHECK(data.ReadRegions(&out->regions));
+  CHECK(data.ReadBounds(&out->bounds));
+  CHECK(data.ReadTransform(&out->transform));
   out->flags = data.flags();
   out->async_hit_test_reasons = data.async_hit_test_reasons();
   return true;