cc: Fix hit-testing in zero-opacity layers.

cc/trees/layer_tree_host_common.cc

Index: cc/trees/layer_tree_host_common.cc
diff --git a/cc/trees/layer_tree_host_common.cc b/cc/trees/layer_tree_host_common.cc
index d483cc697bd76d44e0b7d9353dbb0974c6fdfc5d..8e48b2bc8de6fe53701743c3478e5b5da282bd13 100644
--- a/cc/trees/layer_tree_host_common.cc
+++ b/cc/trees/layer_tree_host_common.cc
@@ -63,6 +63,12 @@ static gfx::Vector2dF GetEffectiveTotalScrollOffset(LayerType* layer) {
   return offset;
 }
 
+template <typename LayerType>
+static inline bool LayerCanAcceptInput(LayerType* layer) {
+  return !layer->touch_event_handler_region().IsEmpty() ||
+         layer->have_wheel_event_handlers();
+}
+
 inline gfx::Rect CalculateVisibleRectWithCachedLayerRect(
     gfx::Rect target_surface_rect,
     gfx::Rect layer_bound_rect,
@@ -452,14 +458,14 @@ static bool LayerShouldBeSkipped(LayerType* layer,
       IsLayerBackFaceVisible(backface_test_layer))
     return true;
 
-  // The layer is visible to events.  If it's subject to hit testing, then
-  // we can't skip it.
-  bool can_accept_input = !layer->touch_event_handler_region().IsEmpty() ||
-      layer->have_wheel_event_handlers();
-  if (!layer->DrawsContent() && !can_accept_input)
+  // A layer cannot be skipped if it is subject to hit-testing.
+  if (LayerCanAcceptInput(layer))
+    return false;
+
+  if (!layer->DrawsContent())
     return true;
 
-  return false;
+  return !layer->draw_opacity() && !layer->draw_opacity_is_animating();

[enne (OOO), 2013/10/07 17:27:23]

I'm not sure this is adequate.

If you have:
-A
  -B
  -C

...and C accepts input and A has zero opacity, then B will now appear in the
RSLL where it didn't before.  If B is a PictureLayerImpl then it'll go ahead and
create resources wastefully.

Does there need to be some "in skipped subtree" boolean that gets passed to
children and considered in LayerShouldBeSkipped? 

[danakj, 2013/10/07 17:37:35]

Can you make this a if () return true; and keep return false at the end of the
method?

[danakj, 2013/10/07 17:37:35]

PictureLayerImpl should probably consider its draw opacity the same way it
considers DrawsContent?

[enne (OOO), 2013/10/07 17:45:51]

It's more than that.  What if you've set a subtree to not be visible and some
descendant has a copy request?

I'm suggesting that maybe this could be generalized to a "this subtree could not
be skipped because of some descendant" property and then every layer in such a
tree is individually skipped except for those specific descendants.

[sadrul, 2013/10/07 18:37:24]

Done.

[sadrul, 2013/10/07 18:37:24]

I am going through the code to completely grok this suggestion.

 }
 
 static inline bool SubtreeShouldBeSkipped(LayerImpl* layer,
@@ -481,9 +487,10 @@ static inline bool SubtreeShouldBeSkipped(LayerImpl* layer,
 
   // The opacity of a layer always applies to its children (either implicitly
   // via a render surface or explicitly if the parent preserves 3D), so the
-  // entire subtree can be skipped if this layer is fully transparent.
-  // TODO(sad): Don't skip layers used for hit testing crbug.com/295295.
-  return !layer->opacity();
+  // entire subtree can be skipped if this layer is fully transparent, and none
+  // of layers in the subtree has event handlers.
+  return !layer->opacity() &&
+         !layer->draw_properties().layer_or_descendant_has_event_handler;
 }
 
 static inline bool SubtreeShouldBeSkipped(Layer* layer,
@@ -503,9 +510,9 @@ static inline bool SubtreeShouldBeSkipped(Layer* layer,
   // In particular, it should not cause the subtree to be skipped.
   // Similarly, for layers that might animate opacity using an impl-only
   // animation, their subtree should also not be skipped.
-  // TODO(sad): Don't skip layers used for hit testing crbug.com/295295.
   return !layer->opacity() && !layer->OpacityIsAnimating() &&
-         !layer->OpacityCanAnimateOnImplThread();
+         !layer->OpacityCanAnimateOnImplThread() &&
+         !layer->draw_properties().layer_or_descendant_has_event_handler;

[enne (OOO), 2013/10/07 17:27:23]

Can you break out this condition from the rest of the opacity ones, since it's
not related to opacity? Same thing for the other function above?

[sadrul, 2013/10/07 18:37:24]

Done.

 }
 
 // Called on each layer that could be drawn after all information from
@@ -1014,15 +1021,19 @@ static inline void RemoveSurfaceForEarlyExit(
 
 struct PreCalculateMetaInformationRecursiveData {
   bool layer_or_descendant_has_copy_request;
+  bool layer_or_descendant_has_event_handler;
   int num_unclipped_descendants;
 
   PreCalculateMetaInformationRecursiveData()
       : layer_or_descendant_has_copy_request(false),
+        layer_or_descendant_has_event_handler(false),
         num_unclipped_descendants(0) {}
 
   void Merge(const PreCalculateMetaInformationRecursiveData& data) {
     layer_or_descendant_has_copy_request |=
         data.layer_or_descendant_has_copy_request;
+    layer_or_descendant_has_event_handler |=
+        data.layer_or_descendant_has_event_handler;
     num_unclipped_descendants +=
         data.num_unclipped_descendants;
   }
@@ -1084,6 +1095,9 @@ static void PreCalculateMetaInformation(
   if (layer->HasCopyRequest())
     recursive_data->layer_or_descendant_has_copy_request = true;
 
+  if (LayerCanAcceptInput(layer))
+    recursive_data->layer_or_descendant_has_event_handler = true;
+
   layer->draw_properties().num_descendants_that_draw_content =
       num_descendants_that_draw_content;
   layer->draw_properties().num_unclipped_descendants =
@@ -1092,6 +1106,8 @@ static void PreCalculateMetaInformation(
       descendants_can_clip_selves;
   layer->draw_properties().layer_or_descendant_has_copy_request =
       recursive_data->layer_or_descendant_has_copy_request;
+  layer->draw_properties().layer_or_descendant_has_event_handler =
+      recursive_data->layer_or_descendant_has_event_handler;
 }
 
 static void RoundTranslationComponents(gfx::Transform* transform) {