Revert of Compute if a layer is clipped outside CalcDrawProps (patchset #9 id:160001 of https://codereview.chromium.org/1231453002/)

Reason for revert:
It looks like this is causing MSAN failures from an uninitialized variable in the webkit_tests:

http://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20MSAN/builds/4378/steps/webkit_tests/logs/stdio

10:00:36.214 29049   ==29058==WARNING: MemorySanitizer: use-of-uninitialized-value
10:00:36.214 29049       #0 0x7f620bf62c3e in set_is_clipped cc/layers/layer.h:503:9
10:00:36.214 29049       #1 0x7f620bf62c3e in AddClipNodeIfNeeded<cc::Layer> cc/trees/property_tree_builder.cc:149:0
10:00:36.214 29049       #2 0x7f620bf62c3e in void cc::(anonymous namespace)::BuildPropertyTreesInternal<cc::Layer>(cc::Layer*, cc::(anonymous namespace)::DataForRecursion<cc::Layer> const&) cc/trees/property_tree_builder.cc:386:0
10:00:36.214 29049       #3 0x7f620bf5d128 in void cc::BuildPropertyTreesTopLevelInternal<cc::Layer>(cc::Layer*, cc::Layer const*, cc::Layer const*, cc::Layer const*, float, float, gfx::Rect const&, gfx::Transform const&, cc::PropertyTrees*) cc/trees/property_tree_builder.cc:466:3
10:00:36.214 29049       #4 0x7f620c206ee6 in cc::BuildPropertyTreesAndComputeVisibleRects(cc::Layer*, cc::Layer const*, cc::Layer const*, cc::Layer const*, float, float, gfx::Rect const&, gfx::Transform const&, cc::PropertyTrees*, std::__1::vector<scoped_refptr<cc::Layer>, std::__1::allocator<scoped_refptr<cc::Layer> > >*) cc/trees/draw_property_utils.cc:473:3
10:00:36.214 29049       #5 0x7f620be94a2f in cc::LayerTreeHost::DoUpdateLayers(cc::Layer*) cc/trees/layer_tree_host.cc:789:5

on pretty much every test.

So I'm reverting this. Sorry!

Original issue's description:
> Compute if a layer is clipped outside CalcDrawProps
>
> We need to know if a layer is clipped to compute its drawable
> visible rect. So, we need to compute it outside CalcDrawProps to move
> drawable visible rect computation outside CalcDrawProps.
>
> Committed: https://crrev.com/2af7226e29be65a8ea4a279b9358a6fcfada5cd3
> Cr-Commit-Position: refs/heads/master@{#338515}

TBR=ajuma@chromium.org,enne@chromium.org,vollick@chromium.org,jaydasika@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true

Review URL: https://codereview.chromium.org/1236943002

Cr-Commit-Position: refs/heads/master@{#338542}

6 files changed, 19 insertions, 32 deletions

diff --git a/cc/trees/layer_tree_host_common.cc b/cc/trees/layer_tree_host_common.cc
index 9989031..427a3c8 100644
--- a/cc/trees/layer_tree_host_common.cc
+++ b/cc/trees/layer_tree_host_common.cc
@@ -2047,7 +2047,7 @@ static void CalculateDrawPropertiesInternal(
   // reduce how much would be drawn, and instead it would create unnecessary
   // changes to scissor state affecting GPU performance. Our clip information
   // is used in the recursion below, so we must set it beforehand.
-  DCHECK_EQ(layer_or_ancestor_clips_descendants, layer->is_clipped());
+  layer_draw_properties.is_clipped = layer_or_ancestor_clips_descendants;
   if (layer_or_ancestor_clips_descendants) {
     layer_draw_properties.clip_rect = clip_rect_in_target_space;
   } else {
@@ -2487,7 +2487,6 @@ void VerifyPropertyTreeValuesForLayer(LayerImpl* current_layer,
       << "expected: " << current_layer->draw_opacity()
       << " actual: " << DrawOpacityFromPropertyTrees(
                             current_layer, property_trees->opacity_tree);
-
   const bool can_use_lcd_text_match =
       CanUseLcdTextFromPropertyTrees(
           current_layer, layers_always_allowed_lcd_text, can_use_lcd_text,
@@ -2537,6 +2536,22 @@ void CalculateDrawPropertiesAndVerify(LayerTreeHostCommon::CalcDrawPropsInputs<
       inputs->verify_property_trees &&
       (property_tree_option == BUILD_PROPERTY_TREES_IF_NEEDED);
 
+  if (should_measure_property_tree_performance) {
+    TRACE_EVENT_BEGIN0(TRACE_DISABLED_BY_DEFAULT("cc.debug.cdp-perf"),
+                       "LayerTreeHostCommon::CalculateDrawProperties");
+  }
+
+  std::vector<AccumulatedSurfaceState<LayerType>> accumulated_surface_state;
+  CalculateDrawPropertiesInternal<LayerType>(
+      inputs->root_layer, globals, data_for_recursion,
+      inputs->render_surface_layer_list, &dummy_layer_list,
+      &accumulated_surface_state, inputs->current_render_surface_layer_list_id);
+
+  if (should_measure_property_tree_performance) {
+    TRACE_EVENT_END0(TRACE_DISABLED_BY_DEFAULT("cc.debug.cdp-perf"),
+                     "LayerTreeHostCommon::CalculateDrawProperties");
+  }
+
   if (inputs->verify_property_trees) {
     typename LayerType::LayerListType update_layer_list;
 
@@ -2579,26 +2594,9 @@ void CalculateDrawPropertiesAndVerify(LayerTreeHostCommon::CalcDrawPropsInputs<
         break;
       }
     }
-  }
 
-  if (should_measure_property_tree_performance) {
-    TRACE_EVENT_BEGIN0(TRACE_DISABLED_BY_DEFAULT("cc.debug.cdp-perf"),
-                       "LayerTreeHostCommon::CalculateDrawProperties");
-  }
-
-  std::vector<AccumulatedSurfaceState<LayerType>> accumulated_surface_state;
-  CalculateDrawPropertiesInternal<LayerType>(
-      inputs->root_layer, globals, data_for_recursion,
-      inputs->render_surface_layer_list, &dummy_layer_list,
-      &accumulated_surface_state, inputs->current_render_surface_layer_list_id);
-
-  if (should_measure_property_tree_performance) {
-    TRACE_EVENT_END0(TRACE_DISABLED_BY_DEFAULT("cc.debug.cdp-perf"),
-                     "LayerTreeHostCommon::CalculateDrawProperties");
-  }
-
-  if (inputs->verify_property_trees)
     VerifyPropertyTreeValues(inputs);
+  }
 
   // The dummy layer list should not have been used.
   DCHECK_EQ(0u, dummy_layer_list.size());
diff --git a/cc/trees/layer_tree_host_common_unittest.cc b/cc/trees/layer_tree_host_common_unittest.cc
index 8d6a659..43e2bb0 100644
--- a/cc/trees/layer_tree_host_common_unittest.cc
+++ b/cc/trees/layer_tree_host_common_unittest.cc
@@ -7176,7 +7176,6 @@ TEST_F(LayerTreeHostCommonTest, RenderSurfaceLayerListMembership) {
 
   // Add a mask layer to child.
   child_raw->SetMaskLayer(LayerImpl::Create(host_impl.active_tree(), 6).Pass());
-  child_raw->layer_tree_impl()->property_trees()->needs_rebuild = true;
 
   ExecuteCalculateDrawProperties(grand_parent_raw);
   member_id = render_surface_layer_list_count();
@@ -7268,7 +7267,6 @@ TEST_F(LayerTreeHostCommonTest, RenderSurfaceLayerListMembership) {
   EXPECT_EQ(expected, actual);
 
   child_raw->TakeMaskLayer();
-  child_raw->layer_tree_impl()->property_trees()->needs_rebuild = true;
 
   // Now everyone's a member!
   grand_parent_raw->SetDrawsContent(true);
diff --git a/cc/trees/layer_tree_host_impl_unittest.cc b/cc/trees/layer_tree_host_impl_unittest.cc
index e02990f..daa179f 100644
--- a/cc/trees/layer_tree_host_impl_unittest.cc
+++ b/cc/trees/layer_tree_host_impl_unittest.cc
@@ -2310,7 +2310,6 @@ TEST_F(LayerTreeHostImplTest, DidDrawNotCalledOnHiddenLayer) {
   EXPECT_FALSE(layer->will_draw_called());
   EXPECT_FALSE(layer->did_draw_called());
 
-  host_impl_->active_tree()->BuildPropertyTreesForTesting();
   EXPECT_EQ(DRAW_SUCCESS, host_impl_->PrepareToDraw(&frame));
   host_impl_->DrawLayers(&frame);
   host_impl_->DidDrawAllLayers(frame);
@@ -5633,7 +5632,6 @@ TEST_F(LayerTreeHostImplTest, NoPartialSwap) {
   harness.MustSetScissor(0, 0, 10, 10);
   {
     LayerTreeHostImpl::FrameData frame;
-    host_impl_->active_tree()->BuildPropertyTreesForTesting();
     EXPECT_EQ(DRAW_SUCCESS, host_impl_->PrepareToDraw(&frame));
     host_impl_->DrawLayers(&frame);
     host_impl_->DidDrawAllLayers(frame);
diff --git a/cc/trees/layer_tree_impl.cc b/cc/trees/layer_tree_impl.cc
index 14e9dc5..8f351f0 100644
--- a/cc/trees/layer_tree_impl.cc
+++ b/cc/trees/layer_tree_impl.cc
@@ -751,7 +751,6 @@ bool LayerTreeImpl::UpdateDrawProperties(bool update_lcd_text) {
 }
 
 void LayerTreeImpl::BuildPropertyTreesForTesting() {
-  LayerTreeHostCommon::PreCalculateMetaInformationForTesting(root_layer_.get());
   PropertyTreeBuilder::BuildPropertyTrees(
       root_layer_.get(), page_scale_layer_, inner_viewport_scroll_layer_,
       outer_viewport_scroll_layer_, current_page_scale_factor(),
diff --git a/cc/trees/layer_tree_impl_unittest.cc b/cc/trees/layer_tree_impl_unittest.cc
index 9e8fee5..81a566f 100644
--- a/cc/trees/layer_tree_impl_unittest.cc
+++ b/cc/trees/layer_tree_impl_unittest.cc
@@ -972,9 +972,6 @@ TEST_F(LayerTreeImplTest, HitTestingRespectsScrollParents) {
     // This should cause scroll child and its descendants to be affected by
     // |child|'s clip.
     scroll_child->SetScrollParent(child.get());
-    scoped_ptr<std::set<LayerImpl*>> scroll_children(new std::set<LayerImpl*>);
-    scroll_children->insert(scroll_child.get());
-    child->SetScrollChildren(scroll_children.release());
 
     SetLayerPropertiesForTesting(grand_child.get(), identity_matrix,
                                  transform_origin, position, bounds, true,
diff --git a/cc/trees/property_tree_builder.cc b/cc/trees/property_tree_builder.cc
index 8ad4407..28ad43f 100644
--- a/cc/trees/property_tree_builder.cc
+++ b/cc/trees/property_tree_builder.cc
@@ -100,8 +100,7 @@ void AddClipNodeIfNeeded(const DataForRecursion<LayerType>& data_from_ancestor,
   int parent_id = parent->id;
 
   bool ancestor_clips_subtree =
-      data_from_ancestor.ancestor_clips_subtree ||
-      (layer->clip_parent() && layer->clip_parent()->is_clipped());
+      data_from_ancestor.ancestor_clips_subtree || layer->clip_parent();
 
   data_for_children->ancestor_clips_subtree = false;
   bool has_unclipped_surface = false;
@@ -146,8 +145,6 @@ void AddClipNodeIfNeeded(const DataForRecursion<LayerType>& data_from_ancestor,
   layer->SetClipTreeIndex(
       has_unclipped_surface ? 0 : data_for_children->clip_tree_parent);
 
-  layer->set_is_clipped(data_for_children->ancestor_clips_subtree);
-
   // TODO(awoloszyn): Right now when we hit a node with a replica, we reset the
   // clip for all children since we may need to draw. We need to figure out a
   // better way, since we will need both the clipped and unclipped versions.