cc: Cleanup Layer::SetNeedsDisplayInRect

cc/layers/layer.cc

 // Copyright 2010 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/layers/layer.h"
 
 #include <algorithm>
 
 #include "base/location.h"
 #include "base/metrics/histogram.h"
@@ skipping 602 matching lines @@
 void Layer::SetHideLayerAndSubtree(bool hide) {
   DCHECK(IsPropertyChangeAllowed());
   if (hide_layer_and_subtree_ == hide)
     return;
 
   hide_layer_and_subtree_ = hide;
   SetNeedsCommit();
 }
 
 void Layer::SetNeedsDisplayRect(const gfx::RectF& dirty_rect) {
+  if (dirty_rect.IsEmpty())

[slavi, 2013/07/11 21:58:09]

Don't do this. When I wrote that function, the intended semantics was that
dirty_rect will be appended to the region that need update. Passing empty rect
still should issue a commit.

See 
https://code.google.com/p/chromium/codesearch#chromium/src/ui/views/widget/de...
for example of such usage.

[danakj, 2013/07/11 21:59:12]

Why not just call SetNeedsCommit on the layer's host?

[tomhudson, 2013/07/24 09:02:43]

If people can't determine the intended semantics by reading the code, do we need
to add comments? Or more descriptive naming?

+    return;
+
   update_rect_.Union(dirty_rect);
   needs_display_ = true;
 
-  // Simply mark the contents as dirty. For non-root layers, the call to
-  // SetNeedsCommit will schedule a fresh compositing pass.
-  // For the root layer, SetNeedsCommit has no effect.
-  if (DrawsContent() && !update_rect_.IsEmpty())
+  if (DrawsContent())
     SetNeedsCommit();
 }
 
 bool Layer::DescendantIsFixedToContainerLayer() const {
   for (size_t i = 0; i < children_.size(); ++i) {
     if (children_[i]->position_constraint_.is_fixed_position() ||
         children_[i]->DescendantIsFixedToContainerLayer())
       return true;
   }
   return false;
@@ skipping 260 matching lines @@
 
 RenderingStatsInstrumentation* Layer::rendering_stats_instrumentation() const {
   return layer_tree_host_->rendering_stats_instrumentation();
 }
 
 bool Layer::SupportsLCDText() const {
   return false;
 }
 
 }  // namespace cc