cc: Be robust against invalid RenderPassDrawQuads.

When the LayerTreeHostImpl drops RenderPasses from its output, it does
not also drop the RenderPassDrawQuads that point to them. The direct
renderers ignore these quads, so this has never been a problem. However
the DelegatedRendererLayerImpl was not able to deal with this correctly
and would crash. More problematic is that a compromised renderer could
send invalid RenderPassDrawQuads, which the DelegatedRendererLayerImpl
must be able to handle gracefully.

So, for both cases, we here make the DelegatedRendererLayerImpl ignore
invalid RenderPassDrawQuads, and just drop the from its own output.

Tests:
DelegatedRendererLayerImplTest.InvalidRenderPassDrawQuad

R=piman
BUG=283630

Review URL: https://chromiumcodereview.appspot.com/23891003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@221493 0039d316-1c4b-4281-b951-d872f2087c98

cc/layers/delegated_renderer_layer_impl.cc

@@ -240,13 +240,19 @@ RenderPass::Id DelegatedRendererLayerImpl::NextContributingRenderPassId(
   return RenderPass::Id(previous.layer_id, previous.index + 1);
 }
 
-RenderPass::Id DelegatedRendererLayerImpl::ConvertDelegatedRenderPassId(
-    RenderPass::Id delegated_render_pass_id) const {
+bool DelegatedRendererLayerImpl::ConvertDelegatedRenderPassId(
+    RenderPass::Id delegated_render_pass_id,
+    RenderPass::Id* output_render_pass_id) const {
   base::hash_map<RenderPass::Id, int>::const_iterator found =
       render_passes_index_by_id_.find(delegated_render_pass_id);
-  DCHECK(found != render_passes_index_by_id_.end());
+  if (found == render_passes_index_by_id_.end()) {
+    // Be robust against a RenderPass id that isn't part of the frame.
+    return false;
+  }
   unsigned delegated_render_pass_index = found->second;
-  return RenderPass::Id(id(), IndexToId(delegated_render_pass_index));
+  *output_render_pass_id =
+      RenderPass::Id(id(), IndexToId(delegated_render_pass_index));
+  return true;
 }
 
 void DelegatedRendererLayerImpl::AppendContributingRenderPasses(
@@ -254,10 +260,14 @@ void DelegatedRendererLayerImpl::AppendContributingRenderPasses(
   DCHECK(HasContributingDelegatedRenderPasses());
 
   for (size_t i = 0; i < render_passes_in_draw_order_.size() - 1; ++i) {
-    RenderPass::Id output_render_pass_id =
-        ConvertDelegatedRenderPassId(render_passes_in_draw_order_[i]->id);
+    RenderPass::Id output_render_pass_id(-1, -1);
+    bool present =
+        ConvertDelegatedRenderPassId(render_passes_in_draw_order_[i]->id,
+                                     &output_render_pass_id);
 
     // Don't clash with the RenderPass we generate if we own a RenderSurface.
+    DCHECK(present) << render_passes_in_draw_order_[i]->id.layer_id << ", "
+                    << render_passes_in_draw_order_[i]->id.index;
     DCHECK_GT(output_render_pass_id.index, 0);
 
     render_pass_sink->AppendRenderPass(
@@ -447,18 +457,26 @@ void DelegatedRendererLayerImpl::AppendRenderPassQuads(
     } else {
       RenderPass::Id delegated_contributing_render_pass_id =
           RenderPassDrawQuad::MaterialCast(delegated_quad)->render_pass_id;
-      RenderPass::Id output_contributing_render_pass_id =
-          ConvertDelegatedRenderPassId(delegated_contributing_render_pass_id);
-      DCHECK(output_contributing_render_pass_id !=
-             append_quads_data->render_pass_id);
-
-      output_quad = RenderPassDrawQuad::MaterialCast(delegated_quad)->Copy(
-          output_shared_quad_state,
-          output_contributing_render_pass_id).PassAs<DrawQuad>();
+      RenderPass::Id output_contributing_render_pass_id(-1, -1);
+
+      bool present =
+          ConvertDelegatedRenderPassId(delegated_contributing_render_pass_id,
+                                       &output_contributing_render_pass_id);
+
+      // The frame may have a RenderPassDrawQuad that points to a RenderPass not
+      // part of the frame. Just ignore these quads.
+      if (present) {
+        DCHECK(output_contributing_render_pass_id !=
+               append_quads_data->render_pass_id);
+
+        output_quad = RenderPassDrawQuad::MaterialCast(delegated_quad)->Copy(
+            output_shared_quad_state,
+            output_contributing_render_pass_id).PassAs<DrawQuad>();
+      }
     }
-    DCHECK(output_quad.get());
 
-    quad_sink->Append(output_quad.Pass(), append_quads_data);
+    if (output_quad)
+      quad_sink->Append(output_quad.Pass(), append_quads_data);
   }
 }
 

cc/layers/delegated_renderer_layer_impl.h

@@ -71,8 +71,11 @@ class CC_EXPORT DelegatedRendererLayerImpl : public LayerImpl {
       ScopedPtrVector<RenderPass>* render_passes_in_draw_order);
   void ClearRenderPasses();
 
-  RenderPass::Id ConvertDelegatedRenderPassId(
-      RenderPass::Id delegated_render_pass_id) const;
+  // Returns |true| if the delegated_render_pass_id is part of the current
+  // frame and can be converted.
+  bool ConvertDelegatedRenderPassId(
+      RenderPass::Id delegated_render_pass_id,
+      RenderPass::Id* output_render_pass_id) const;
 
   gfx::Transform DelegatedFrameToLayerSpaceTransform(gfx::Size frame_size)
       const;

cc/layers/delegated_renderer_layer_impl_unittest.cc

@@ -1245,5 +1245,58 @@ TEST_F(DelegatedRendererLayerImplTestClip, QuadsClipped_LayerClipped_Surface) {
   host_impl_->DidDrawAllLayers(frame);
 }
 
+TEST_F(DelegatedRendererLayerImplTest, InvalidRenderPassDrawQuad) {
+  scoped_ptr<LayerImpl> root_layer = LayerImpl::Create(
+      host_impl_->active_tree(), 1).PassAs<LayerImpl>();
+  scoped_ptr<FakeDelegatedRendererLayerImpl> delegated_renderer_layer =
+      FakeDelegatedRendererLayerImpl::Create(host_impl_->active_tree(), 4);
+
+  host_impl_->SetViewportSize(gfx::Size(100, 100));
+
+  delegated_renderer_layer->SetPosition(gfx::Point(3, 3));
+  delegated_renderer_layer->SetBounds(gfx::Size(10, 10));
+  delegated_renderer_layer->SetContentBounds(gfx::Size(10, 10));
+  delegated_renderer_layer->SetDrawsContent(true);
+
+  ScopedPtrVector<RenderPass> delegated_render_passes;
+  TestRenderPass* pass1 = AddRenderPass(
+      &delegated_render_passes,
+      RenderPass::Id(9, 6),
+      gfx::Rect(0, 0, 10, 10),
+      gfx::Transform());
+  AddQuad(pass1, gfx::Rect(0, 0, 6, 6), 33u);
+
+  // This render pass isn't part of the frame.
+  scoped_ptr<TestRenderPass> missing_pass(TestRenderPass::Create());
+  missing_pass->SetNew(RenderPass::Id(9, 7),
+                       gfx::Rect(7, 7, 7, 7),
+                       gfx::Rect(7, 7, 7, 7),
+                       gfx::Transform());
+
+  // But a render pass quad refers to it.
+  AddRenderPassQuad(pass1, missing_pass.get());
+
+  delegated_renderer_layer->SetFrameDataForRenderPasses(
+      &delegated_render_passes);
+
+  // The RenderPasses should be taken by the layer.
+  EXPECT_EQ(0u, delegated_render_passes.size());
+
+  root_layer->AddChild(delegated_renderer_layer.PassAs<LayerImpl>());
+  host_impl_->active_tree()->SetRootLayer(root_layer.Pass());
+
+  LayerTreeHostImpl::FrameData frame;
+  EXPECT_TRUE(host_impl_->PrepareToDraw(&frame, gfx::Rect()));
+
+  // The DelegatedRendererLayerImpl should drop the bad RenderPassDrawQuad.
+  ASSERT_EQ(1u, frame.render_passes.size());
+  ASSERT_EQ(1u, frame.render_passes[0]->quad_list.size());
+  EXPECT_EQ(DrawQuad::SOLID_COLOR,
+            frame.render_passes[0]->quad_list[0]->material);
+
+  host_impl_->DrawLayers(&frame, base::TimeTicks::Now());
+  host_impl_->DidDrawAllLayers(frame);
+}
+
 }  // namespace
 }  // namespace cc

cc/test/render_pass_test_utils.h

@@ -50,7 +50,7 @@ void AddRenderPassQuad(TestRenderPass* to_pass,
 
 // Adds a render pass quad with the given mask resource, filter, and transform.
 void AddRenderPassQuad(TestRenderPass* toPass,
-                       TestRenderPass* contributingPass,
+                       TestRenderPass* contributing_pass,
                        ResourceProvider::ResourceId mask_resource_id,
                        skia::RefPtr<SkImageFilter> filter,
                        gfx::Transform transform);