cc: Setup API to release OutputSurface from LTHClient.

cc/trees/layer_tree_host_unittest_context.cc

Index: cc/trees/layer_tree_host_unittest_context.cc
diff --git a/cc/trees/layer_tree_host_unittest_context.cc b/cc/trees/layer_tree_host_unittest_context.cc
index 7c766fe8bedc60376fafb543e8f51667e2c40682..06719989aee376f605aa1e959e1cb7d029d5b208 100644
--- a/cc/trees/layer_tree_host_unittest_context.cc
+++ b/cc/trees/layer_tree_host_unittest_context.cc
@@ -382,6 +382,71 @@ class LayerTreeHostClientNotReadyDoesNotCreateOutputSurface
 SINGLE_AND_MULTI_THREAD_TEST_F(
     LayerTreeHostClientNotReadyDoesNotCreateOutputSurface);
 
+// This tests the OutputSurface release logic in the following sequence.
+// SetUp LTH and create and init OutputSurface
+// LTH::SetVisible(false);
+// LTH::ReleaseOutputSurface();
+// ...
+// Create and init new OutputSurface
+// LTH::SetVisible(true);
+class LayerTreeHostClientTakeAwayOutputSurface
+    : public LayerTreeHostContextTest {
+ public:
+  LayerTreeHostClientTakeAwayOutputSurface()
+      : LayerTreeHostContextTest(), setos_counter_(0) {}
+
+  void BeginTest() override { PostSetNeedsCommitToMainThread(); }
+
+  void RequestNewOutputSurface() override {
+    if (layer_tree_host()->visible())
+      CreateAndSetOutputSurface();
+    else
+      EndTest();

[no sievers, 2015/09/14 21:28:13]

Can we still go through and create another OutputSurface after making the LTH
visible again?

I think it's worth testing also that there are no broken assumptions and that
succeeds without blowing up.

Don't you just have to move the EndTest() to DidInitializeOutputSurface() for
when |setos_counter == 2|?

You'd also have to handle races between RequestNewOutputSurface() and
MakeVisible() to trigger the new OutputSurface creation (you'd probably just
need to track a |requested_| bool). But maybe it's easier if you just rebase
this on top of https://codereview.chromium.org/1336813002/? Then RequestNewOS()
will not happen until after MakeVisible().

[sohanjg, 2015/09/15 06:39:34]

Yes, in the current form we were getting RequestNewOutputSurface, before
MakeVisible, hence the EndTest here.
But, you are right, there is scope of race between the 2 calls.
If i rebase on top of your scheduler changes, the order is maintained well i.e
new OS request comes only after makevisible is executed.
I will move this to didinitializeos.. .

So, what should be the plan ? we wait for the scheduler patch to land? and then
push this?

+  }
+
+  void CreateAndSetOutputSurface() {
+    scoped_ptr<OutputSurface> surface =
+        LayerTreeHostContextTest::CreateOutputSurface();
+    CHECK(surface);
+    setos_counter_++;
+    layer_tree_host()->SetOutputSurface(surface.Pass());
+  }
+
+  void HideAndReleaseOutputSurface() {
+    EXPECT_TRUE(layer_tree_host()->proxy()->IsMainThread());
+    layer_tree_host()->SetVisible(false);
+    scoped_ptr<OutputSurface> surface =
+        layer_tree_host()->ReleaseOutputSurface();
+    CHECK(surface);
+    surface->context_provider()->DetachFromThread();
+    MainThreadTaskRunner()->PostTask(
+        FROM_HERE,
+        base::Bind(&LayerTreeHostClientTakeAwayOutputSurface::MakeVisible,
+                   base::Unretained(this)));
+  }
+
+  void DidInitializeOutputSurface() override {
+    if (setos_counter_ == 1) {
+      EXPECT_TRUE(layer_tree_host()->visible());

[no sievers, 2015/09/14 21:28:13]

I think you can pull this out of the if-block regardless now.

[sohanjg, 2015/09/15 06:39:34]

Done.

+      MainThreadTaskRunner()->PostTask(
+          FROM_HERE, base::Bind(&LayerTreeHostClientTakeAwayOutputSurface::
+                                    HideAndReleaseOutputSurface,
+                                base::Unretained(this)));
+    }
+  }
+
+  void MakeVisible() {
+    EXPECT_TRUE(layer_tree_host()->proxy()->IsMainThread());
+    layer_tree_host()->SetVisible(true);
+  }
+
+  void AfterTest() override {}
+
+  int setos_counter_;
+};
+
+SINGLE_AND_MULTI_THREAD_TEST_F(LayerTreeHostClientTakeAwayOutputSurface);
+
 class MultipleCompositeDoesNotCreateOutputSurface
     : public LayerTreeHostContextTest {
  public: