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

4 files changed, 93 insertions, 19 deletions

diff --git a/cc/layers/delegated_renderer_layer_impl.cc b/cc/layers/delegated_renderer_layer_impl.cc
index 16d4770..f88885e 100644
--- a/cc/layers/delegated_renderer_layer_impl.cc
+++ b/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);
   }
 }
 
diff --git a/cc/layers/delegated_renderer_layer_impl.h b/cc/layers/delegated_renderer_layer_impl.h
index 89b2537..5f55050 100644
--- a/cc/layers/delegated_renderer_layer_impl.h
+++ b/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;
diff --git a/cc/layers/delegated_renderer_layer_impl_unittest.cc b/cc/layers/delegated_renderer_layer_impl_unittest.cc
index 401ac3c..400a442 100644
--- a/cc/layers/delegated_renderer_layer_impl_unittest.cc
+++ b/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
diff --git a/cc/test/render_pass_test_utils.h b/cc/test/render_pass_test_utils.h
index 01a7ca0..b427e5a 100644
--- a/cc/test/render_pass_test_utils.h
+++ b/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);