Keeping track of descendants that draw content instead of recalcualting

cc/layers/image_layer.cc

Index: cc/layers/image_layer.cc
diff --git a/cc/layers/image_layer.cc b/cc/layers/image_layer.cc
index cada84df3ccb4a0ce63731db532f2e0cd1f08358..281d33fbefd0ecff676b5733ba594ef3b5f6b640 100644
--- a/cc/layers/image_layer.cc
+++ b/cc/layers/image_layer.cc
@@ -30,9 +30,14 @@ void ImageLayer::SetBitmap(const SkBitmap& bitmap) {
     return;
 
   bitmap_ = bitmap;
+  UpdateDrawsContent(true);
   SetNeedsDisplay();
 }
 
+void ImageLayer::UpdateDrawsContent(bool drawsContent) {

[danakj, 2014/07/14 14:56:31]

draws_content thoughout

[awoloszyn, 2014/07/14 19:38:32]

Done.

+  TiledLayer::UpdateDrawsContent(drawsContent && !bitmap_.isNull());

[danakj, 2014/07/14 14:56:31]

so I think what you're trying to do here, you want DrawsContent to be true only
if someone did SetDrawsContent(true) externally on the layer? But to get that
effect you should be && with the current DrawsContent() value, not with |true|
as you would here. Do I have that right?

However, can be simplify this a bit? We have 2 things:
1. SetIsDrawable() which is an external flag saying whether the layer should
draw anything at all.
2. Layer-type-specific internal flag saying whether the layer has anything to
draw.

Can we just use 2 bools on Layer to represent these two things instead of doing
boolean logic to store that in one?

Then Layer::DrawsContent() would be is_drawable_ && has_drawable_content_ or
whatever. WDYT?

[awoloszyn, 2014/07/14 15:25:09]

So how I had set UpateDrawsContent up, currently, is that it is originally
called with true, and then every class in the hierarchy has the ability to veto
the fact that before delegating onto its super-class. Once we hit
Layer::UpdateDrawsContent we will have the actual value of draws_content (with
respect to every subclass on the way down), and set the value.

This could probably be made clearer with a "UpdateDrawsContent" function with no
parameters, and then a helper PropagateDrawsContent(bool) that does this work.
My hope with how this was set up was that we can keep the "truth" if you will,
of whether or not we draw content, in a single variable. This was so that we
could do the required book-keeping to notify our ancestors of our drawing in a
single place, SetDrawsContent. Otherwise we will have to track changes to the
result of (is_drawable_ && has_drawable_content_), both whenever is_drawable_
changes and when has_drawable_content_ changes.

I am certainly amenable to change, I just felt it would be nice to keep the
book-keeping self-contained.

+}
+
 void ImageLayer::SetTexturePriorities(const PriorityCalculator& priority_calc) {
   // Update the tile data before creating all the layer's tiles.
   UpdateTileSizeAndTilingOption();
@@ -77,10 +82,6 @@ void ImageLayer::CalculateContentsScale(float ideal_contents_scale,
   *content_bounds = gfx::Size(bitmap_.width(), bitmap_.height());
 }
 
-bool ImageLayer::DrawsContent() const {
-  return !bitmap_.isNull() && TiledLayer::DrawsContent();
-}
-
 void ImageLayer::OnOutputSurfaceCreated() {
   SetTextureFormat(
       layer_tree_host()->GetRendererCapabilities().best_texture_format);