cc: Cleanup Layer::SetNeedsDisplayInRect

This comment is no longer true.  Also, simplified the early out
condition and added a test.

R=danakj@chromium.org
BUG=none

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=214103

Review URL: https://chromiumcodereview.appspot.com/5831706594508800

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@214819 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 27 insertions, 13 deletions

diff --git a/cc/layers/image_layer.cc b/cc/layers/image_layer.cc
index e02fc62..8ba1908 100644
--- a/cc/layers/image_layer.cc
+++ b/cc/layers/image_layer.cc
@@ -43,11 +43,10 @@ void ImageLayer::SetTexturePriorities(const PriorityCalculator& priority_calc) {
 bool ImageLayer::Update(ResourceUpdateQueue* queue,
                         const OcclusionTracker* occlusion) {
   CreateUpdaterIfNeeded();
-  if (needs_display_) {
-    updater_->set_bitmap(bitmap_);
+  if (!updater_->UsingBitmap(bitmap_)) {
+    updater_->SetBitmap(bitmap_);
     UpdateTileSizeAndTilingOption();
     InvalidateContentRect(gfx::Rect(content_bounds()));
-    needs_display_ = false;
   }
   return TiledLayer::Update(queue, occlusion);
 }
diff --git a/cc/layers/layer.cc b/cc/layers/layer.cc
index 9d1cde5..9e181db 100644
--- a/cc/layers/layer.cc
+++ b/cc/layers/layer.cc
@@ -671,16 +671,15 @@ void Layer::SetHideLayerAndSubtree(bool hide) {
 }
 
 void Layer::SetNeedsDisplayRect(const gfx::RectF& dirty_rect) {
-  if (!update_rect_.Contains(dirty_rect)) {
-    SetNeedsPushProperties();
-  }
+  if (dirty_rect.IsEmpty())
+    return;
 
+  SetNeedsPushProperties();
   update_rect_.Union(dirty_rect);
   needs_display_ = true;
 
-  if (DrawsContent() && !update_rect_.IsEmpty()) {
+  if (DrawsContent())
     SetNeedsUpdate();
-  }
 }
 
 bool Layer::DescendantIsFixedToContainerLayer() const {
diff --git a/cc/layers/layer.h b/cc/layers/layer.h
index 09f7f71..defbf17 100644
--- a/cc/layers/layer.h
+++ b/cc/layers/layer.h
@@ -419,6 +419,7 @@ class CC_EXPORT Layer : public base::RefCounted<Layer>,
   void reset_raster_scale_to_unknown() { raster_scale_ = 0.f; }
 
   // This flag is set when layer need repainting/updating.
+  // TODO(enne): Remove this flag and just check update_rect_.
   bool needs_display_;
 
   // This flag is set when the layer needs to push properties to the impl
diff --git a/cc/layers/layer_unittest.cc b/cc/layers/layer_unittest.cc
index 0c2fca6..c338195 100644
--- a/cc/layers/layer_unittest.cc
+++ b/cc/layers/layer_unittest.cc
@@ -510,7 +510,13 @@ TEST_F(LayerTest, CheckSetNeedsDisplayCausesCorrectBehavior) {
   EXPECT_TRUE(test_layer->NeedsDisplayForTesting());
   test_layer->ResetNeedsDisplayForTesting();
 
-  // Case 3: SetNeedsDisplay() with a non-drawable layer
+  // Case 3: SetNeedsDisplay() with an empty rect.
+  test_layer->ResetNeedsDisplayForTesting();
+  EXPECT_FALSE(test_layer->NeedsDisplayForTesting());
+  EXPECT_SET_NEEDS_COMMIT(0, test_layer->SetNeedsDisplayRect(gfx::Rect()));
+  EXPECT_FALSE(test_layer->NeedsDisplayForTesting());
+
+  // Case 4: SetNeedsDisplay() with a non-drawable layer
   EXPECT_SET_NEEDS_COMMIT(1, test_layer->SetIsDrawable(false));
   test_layer->ResetNeedsDisplayForTesting();
   EXPECT_FALSE(test_layer->NeedsDisplayForTesting());
diff --git a/cc/layers/nine_patch_layer.cc b/cc/layers/nine_patch_layer.cc
index c75316a..e88a95a 100644
--- a/cc/layers/nine_patch_layer.cc
+++ b/cc/layers/nine_patch_layer.cc
@@ -33,7 +33,7 @@ void NinePatchLayer::SetTexturePriorities(
     resource_.reset();
     if (!bitmap_.isNull())
       CreateResource();
-  } else if (needs_display_ && bitmap_dirty_ && DrawsContent()) {
+  } else if (bitmap_dirty_ && DrawsContent()) {
     CreateResource();
   }
 
@@ -83,8 +83,7 @@ void NinePatchLayer::CreateUpdaterIfNeeded() {
 void NinePatchLayer::CreateResource() {
   DCHECK(!bitmap_.isNull());
   CreateUpdaterIfNeeded();
-  updater_->set_bitmap(bitmap_);
-  needs_display_ = false;
+  updater_->SetBitmap(bitmap_);
 
   if (!resource_) {
     resource_ = updater_->CreateResource(
diff --git a/cc/resources/image_layer_updater.cc b/cc/resources/image_layer_updater.cc
index 6e9cdfea..470f518 100644
--- a/cc/resources/image_layer_updater.cc
+++ b/cc/resources/image_layer_updater.cc
@@ -56,4 +56,13 @@ void ImageLayerUpdater::UpdateTexture(ResourceUpdateQueue* queue,
     queue->AppendFullUpload(upload);
 }
 
+void ImageLayerUpdater::SetBitmap(const SkBitmap& bitmap) {
+  DCHECK(bitmap.pixelRef());
+  bitmap_ = bitmap;
+}
+
+bool ImageLayerUpdater::UsingBitmap(const SkBitmap& bitmap) const {
+  return bitmap.pixelRef() == bitmap_.pixelRef();
+}
+
 }  // namespace cc
diff --git a/cc/resources/image_layer_updater.h b/cc/resources/image_layer_updater.h
index ec5da7a..44e541d 100644
--- a/cc/resources/image_layer_updater.h
+++ b/cc/resources/image_layer_updater.h
@@ -43,7 +43,8 @@ class CC_EXPORT ImageLayerUpdater : public LayerUpdater {
                      gfx::Vector2d dest_offset,
                      bool partial_update);
 
-  void set_bitmap(const SkBitmap& bitmap) { bitmap_ = bitmap; }
+  void SetBitmap(const SkBitmap& bitmap);
+  bool UsingBitmap(const SkBitmap& bitmap) const;
 
  private:
   ImageLayerUpdater() {}