Releasing Track & Thumb UIResources based on their Geometry

cc/layers/scrollbar_layer_unittest.cc

Index: cc/layers/scrollbar_layer_unittest.cc
diff --git a/cc/layers/scrollbar_layer_unittest.cc b/cc/layers/scrollbar_layer_unittest.cc
index 4c5e5b480366f2f239dcc804d3f8bc55bf951e8b..2a38e0045c5af896d2744772c9268629029f6a71 100644
--- a/cc/layers/scrollbar_layer_unittest.cc
+++ b/cc/layers/scrollbar_layer_unittest.cc
@@ -626,9 +626,9 @@ TEST_F(ScrollbarLayerTestMaxTextureSize, DelegatingRenderer) {
   RunTest(true, true, true);
 }
 
-class MockLayerTreeHost : public LayerTreeHost {
+class FakeLayerTreeHost : public LayerTreeHost {

[MuVen, 2014/09/10 15:40:58]

Done.

Comments by dana:
(probably wanna rename this to Fake to avoid name colliding with an actual mock
class)

  public:
-  MockLayerTreeHost(FakeLayerTreeHostClient* client,
+  FakeLayerTreeHost(FakeLayerTreeHostClient* client,
                     const LayerTreeSettings& settings)
       : LayerTreeHost(client, NULL, settings),
         next_id_(1),
@@ -682,10 +682,9 @@ class MockLayerTreeHost : public LayerTreeHost {
   int total_ui_resource_deleted_;
 };
 
-
-class ScrollbarLayerTestResourceCreation : public testing::Test {
+class ScrollbarLayerTestResourceCreationAndRelease : public testing::Test {
  public:
-  ScrollbarLayerTestResourceCreation()
+  ScrollbarLayerTestResourceCreationAndRelease()
       : fake_client_(FakeLayerTreeHostClient::DIRECT_3D) {}
 
   void TestResourceUpload(int num_updates,
@@ -694,7 +693,7 @@ class ScrollbarLayerTestResourceCreation : public testing::Test {
                           int expected_deleted,
                           bool use_solid_color_scrollbar) {
     layer_tree_host_.reset(
-        new MockLayerTreeHost(&fake_client_, layer_tree_settings_));
+        new FakeLayerTreeHost(&fake_client_, layer_tree_settings_));
 
     scoped_ptr<Scrollbar> scrollbar(new FakeScrollbar(false, true, false));
     scoped_refptr<Layer> layer_tree_root = Layer::Create();
@@ -751,13 +750,73 @@ class ScrollbarLayerTestResourceCreation : public testing::Test {
     scrollbar_layer->ClearRenderSurface();
   }
 
+  void TestResourceRelease() {
+    layer_tree_host_.reset(
+        new FakeLayerTreeHost(&fake_client_, layer_tree_settings_));
+
+    gfx::Point scrollbar_location(0, 185);
+    scoped_refptr<Layer> layer_tree_root = Layer::Create();
+    scoped_refptr<Layer> content_layer = Layer::Create();
+    scoped_refptr<FakePaintedScrollbarLayer> scrollbar_layer =
+        FakePaintedScrollbarLayer::Create(false, true, layer_tree_root->id());
+
+    layer_tree_root->AddChild(content_layer);
+    layer_tree_root->AddChild(scrollbar_layer);
+
+    layer_tree_host_->SetRootLayer(layer_tree_root);
+
+    scrollbar_layer->SetIsDrawable(true);
+    scrollbar_layer->SetBounds(gfx::Size(100, 15));
+    scrollbar_layer->SetPosition(scrollbar_location);
+    layer_tree_root->SetBounds(gfx::Size(100, 200));
+    content_layer->SetBounds(gfx::Size(100, 200));
+
+    scrollbar_layer->draw_properties().content_bounds = gfx::Size(100, 200);
+    scrollbar_layer->draw_properties().visible_content_rect =
+        gfx::Rect(0, 0, 100, 200);
+
+    scrollbar_layer->CreateRenderSurface();
+    scrollbar_layer->draw_properties().render_target = scrollbar_layer.get();
+
+    testing::Mock::VerifyAndClearExpectations(layer_tree_host_.get());
+    EXPECT_EQ(scrollbar_layer->layer_tree_host(), layer_tree_host_.get());
+
+    ResourceUpdateQueue queue;
+    gfx::Rect screen_space_clip_rect;
+    OcclusionTracker<Layer> occlusion_tracker(screen_space_clip_rect);
+    scrollbar_layer->SavePaintProperties();
+    scrollbar_layer->Update(&queue, &occlusion_tracker);

[MuVen, 2014/09/10 15:40:58]

FakePaintedScrollbarLayer doesnt have Impl implementation to check this. If you
want me to do that i shall write FakePaintedScrollbarLayerImpl implementation
for this.

Comments by dana:
can you verify this is calling SetNeedsPushProperties() also? see
layer_unittest.cc for example of a MockLayerTreeHost to test similar things.

[danakj, 2014/09/10 16:15:03]

how about verifying the Update return value?

[MuVen, 2014/09/11 14:07:21]

Instead of verifying on Update i have verified on
layer_tree_host_->UIResourceCount, TotalUIResourceCreated,
TotalUIResourceDeleted. As we are returning true in Update Api upon reset of
resources, i felt this way we can verify it as done in the above method
TestResourceUpload.

On 2014/09/10 16:15:03, danakj wrote:

+    EXPECT_NE(0, scrollbar_layer->track_resource_id());

[MuVen, 2014/09/10 15:40:58]

Need IMPL implementation for this.

comments by dana:
can we verify that we push the resource and push a 0 to the impl side
appropriately also? maybe see the TiledLayer tests for examples.

+    EXPECT_NE(0, scrollbar_layer->thumb_resource_id());
+
+    scrollbar_layer->fake_scrollbar()->set_track_rect(gfx::Rect(0, 0, 0, 0));
+    scrollbar_layer->Update(&queue, &occlusion_tracker);
+    EXPECT_EQ(0, scrollbar_layer->track_resource_id());
+    EXPECT_EQ(0, scrollbar_layer->thumb_resource_id());
+
+    scrollbar_layer->fake_scrollbar()->set_track_rect(
+        gfx::Rect(30, 10, 50, 10));
+    scrollbar_layer->Update(&queue, &occlusion_tracker);
+    EXPECT_NE(0, scrollbar_layer->track_resource_id());
+    EXPECT_NE(0, scrollbar_layer->thumb_resource_id());
+
+    scrollbar_layer->fake_scrollbar()->set_has_thumb(false);
+    scrollbar_layer->Update(&queue, &occlusion_tracker);
+    EXPECT_NE(0, scrollbar_layer->track_resource_id());

[MuVen, 2014/09/10 15:40:58]

I feel it should be present to give user an idea that on resizing window thumb
will be showed on the track.If you are ok to do that we can remove track even.

comments by dana:
make sure track can be released too?

[danakj, 2014/09/10 16:15:03]

Oh,I agree. I meant another test case that shows the track being freed, right
now you only show it being created and that the thumb resource can be freed

[MuVen, 2014/09/11 14:07:21]

Done.

+    EXPECT_EQ(0, scrollbar_layer->thumb_resource_id());
+
+    testing::Mock::VerifyAndClearExpectations(layer_tree_host_.get());
+
+    scrollbar_layer->ClearRenderSurface();
+  }
+
  protected:
   FakeLayerTreeHostClient fake_client_;
   LayerTreeSettings layer_tree_settings_;
-  scoped_ptr<MockLayerTreeHost> layer_tree_host_;
+  scoped_ptr<FakeLayerTreeHost> layer_tree_host_;
 };
 
-TEST_F(ScrollbarLayerTestResourceCreation, ResourceUpload) {
+TEST_F(ScrollbarLayerTestResourceCreationAndRelease, ResourceUpload) {
   bool use_solid_color_scrollbars = false;
   TestResourceUpload(0, 0, 0, 0, use_solid_color_scrollbars);
   int num_updates[3] = {1, 5, 10};
@@ -770,12 +829,18 @@ TEST_F(ScrollbarLayerTestResourceCreation, ResourceUpload) {
   }
 }
 
-TEST_F(ScrollbarLayerTestResourceCreation, SolidColorNoResourceUpload) {
+TEST_F(ScrollbarLayerTestResourceCreationAndRelease,
+       SolidColorNoResourceUpload) {
   bool use_solid_color_scrollbars = true;
   TestResourceUpload(0, 0, 0, 0, use_solid_color_scrollbars);
   TestResourceUpload(1, 0, 0, 0, use_solid_color_scrollbars);
 }
 
+TEST_F(ScrollbarLayerTestResourceCreationAndRelease,
+       ReleaseThumbAndTrackUIResources) {
+  TestResourceRelease();

[MuVen, 2014/09/10 15:40:58]

I have added the test function to ScrollbarLayerTestResourceCreation, to keep
creation and release test case should be under one roof.

comments by dana:
what's the reason for the class indirection here? can't its member variables
just be locals in the test and all the code lives here?

[danakj, 2014/09/10 16:15:03]

The TestResourceUpdate code makes sense in the class since the test calls it
with different inputs. I'd rather just put this test inside the TEST_F() { } as
that's a lot easier to read than having to redirect to some method in a class
above.

[MuVen, 2014/09/11 14:07:21]

Done.

+}
+
 class ScaledScrollbarLayerTestResourceCreation : public testing::Test {
  public:
   ScaledScrollbarLayerTestResourceCreation()
@@ -783,7 +848,7 @@ class ScaledScrollbarLayerTestResourceCreation : public testing::Test {
 
   void TestResourceUpload(const float test_scale) {
     layer_tree_host_.reset(
-        new MockLayerTreeHost(&fake_client_, layer_tree_settings_));
+        new FakeLayerTreeHost(&fake_client_, layer_tree_settings_));
 
     gfx::Point scrollbar_location(0, 185);
     scoped_refptr<Layer> layer_tree_root = Layer::Create();
@@ -847,7 +912,7 @@ class ScaledScrollbarLayerTestResourceCreation : public testing::Test {
  protected:
   FakeLayerTreeHostClient fake_client_;
   LayerTreeSettings layer_tree_settings_;
-  scoped_ptr<MockLayerTreeHost> layer_tree_host_;
+  scoped_ptr<FakeLayerTreeHost> layer_tree_host_;
 };
 
 TEST_F(ScaledScrollbarLayerTestResourceCreation, ScaledResourceUpload) {
@@ -865,7 +930,7 @@ class ScaledScrollbarLayerTestScaledRasterization : public testing::Test {
 
   void TestScale(const gfx::Rect scrollbar_rect, const float test_scale) {
     layer_tree_host_.reset(
-        new MockLayerTreeHost(&fake_client_, layer_tree_settings_));
+        new FakeLayerTreeHost(&fake_client_, layer_tree_settings_));
 
     bool paint_during_update = true;
     bool has_thumb = false;
@@ -943,7 +1008,7 @@ class ScaledScrollbarLayerTestScaledRasterization : public testing::Test {
 
   FakeLayerTreeHostClient fake_client_;
   LayerTreeSettings layer_tree_settings_;
-  scoped_ptr<MockLayerTreeHost> layer_tree_host_;
+  scoped_ptr<FakeLayerTreeHost> layer_tree_host_;
 };
 
 TEST_F(ScaledScrollbarLayerTestScaledRasterization, TestLostPrecisionInClip) {