cc: Set sync token on resource even if context is lost.

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 17b9f1dd173770a2833eda5090982634fd6d8336..f41f18f9689c8335a93a7cde970de9fbdd13d6ae 100644
--- a/cc/trees/layer_tree_host_unittest_context.cc
+++ b/cc/trees/layer_tree_host_unittest_context.cc
@@ -76,41 +76,31 @@ class LayerTreeHostContextTest : public LayerTreeTest {
     context3d_ = nullptr;
   }
 
-  virtual std::unique_ptr<TestWebGraphicsContext3D> CreateContext3d() {
-    return TestWebGraphicsContext3D::Create();
-  }
-
   std::unique_ptr<TestCompositorFrameSink> CreateCompositorFrameSink(
       scoped_refptr<ContextProvider> compositor_context_provider,
       scoped_refptr<ContextProvider> worker_context_provider) override {
+    base::AutoLock lock(context3d_lock_);
+
+    std::unique_ptr<TestWebGraphicsContext3D> compositor_context3d =
+        TestWebGraphicsContext3D::Create();
+    if (context_should_support_io_surface_) {
+      compositor_context3d->set_have_extension_io_surface(true);
+      compositor_context3d->set_have_extension_egl_image(true);
+    }
+    context3d_ = compositor_context3d.get();
+
     if (times_to_fail_create_) {
       --times_to_fail_create_;
       ExpectCreateToFail();
-      auto test_compositor_context_provider = TestContextProvider::Create();
-      test_compositor_context_provider->UnboundTestContext3d()
-          ->loseContextCHROMIUM(GL_GUILTY_CONTEXT_RESET_ARB,
-                                GL_INNOCENT_CONTEXT_RESET_ARB);
-      compositor_context_provider = std::move(test_compositor_context_provider);
+      context3d_->loseContextCHROMIUM(GL_GUILTY_CONTEXT_RESET_ARB,
+                                      GL_INNOCENT_CONTEXT_RESET_ARB);
     }
+
     return LayerTreeTest::CreateCompositorFrameSink(
-        std::move(compositor_context_provider),
+        TestContextProvider::Create(std::move(compositor_context3d)),

[danakj, 2016/10/06 22:59:59]

Can you explicitly point out that the context is not in a share group with the
worker and why? It's accidental in the base class but here we would actually not
want to change it or at least we'd have to think about it.

         std::move(worker_context_provider));
   }
 
-  std::unique_ptr<OutputSurface> CreateDisplayOutputSurface(
-      scoped_refptr<ContextProvider> compositor_context_provider) override {
-    std::unique_ptr<TestWebGraphicsContext3D> context3d = CreateContext3d();
-    if (context_should_support_io_surface_) {
-      context3d->set_have_extension_io_surface(true);
-      context3d->set_have_extension_egl_image(true);
-    }
-
-    base::AutoLock lock(context3d_lock_);
-    context3d_ = context3d.get();
-    return LayerTreeTest::CreateDisplayOutputSurface(
-        TestContextProvider::Create(std::move(context3d)));
-  }
-
   DrawResult PrepareToDrawOnThread(LayerTreeHostImpl* host_impl,
                                    LayerTreeHostImpl::FrameData* frame,
                                    DrawResult draw_result) override {
@@ -484,7 +474,9 @@ class FailedCreateDoesNotCreateExtraCompositorFrameSink
     : public LayerTreeHostContextTest {
  public:
   FailedCreateDoesNotCreateExtraCompositorFrameSink()
-      : LayerTreeHostContextTest(), num_requests_(0), has_failed_(false) {}
+      : LayerTreeHostContextTest(), num_requests_(0), has_failed_(false) {
+    times_to_fail_create_ = 1;
+  }
 
   void InitializeSettings(LayerTreeSettings* settings) override {
     settings->single_thread_proxy_scheduler = false;
@@ -503,19 +495,6 @@ class FailedCreateDoesNotCreateExtraCompositorFrameSink
     LayerTreeHostContextTest::RequestNewCompositorFrameSink();
   }
 
-  std::unique_ptr<TestCompositorFrameSink> CreateCompositorFrameSink(
-      scoped_refptr<ContextProvider> compositor_context_provider,
-      scoped_refptr<ContextProvider> worker_context_provider) override {
-    ExpectCreateToFail();
-    auto test_compositor_context_provider = TestContextProvider::Create();
-    test_compositor_context_provider->UnboundTestContext3d()
-        ->loseContextCHROMIUM(GL_GUILTY_CONTEXT_RESET_ARB,
-                              GL_INNOCENT_CONTEXT_RESET_ARB);
-    return LayerTreeTest::CreateCompositorFrameSink(
-        std::move(test_compositor_context_provider),
-        std::move(worker_context_provider));
-  }
-
   void BeginTest() override {
     // First composite tries to create a surface.
     layer_tree_host()->Composite(base::TimeTicks::FromInternalValue(1));
@@ -729,7 +708,7 @@ class LayerTreeHostContextTestLostContextAndEvictTextures
       ImplThreadTaskRunner()->PostTask(
           FROM_HERE,
           base::Bind(&LayerTreeHostContextTestLostContextAndEvictTextures::
-                          EvictTexturesOnImplThread,
+                         EvictTexturesOnImplThread,
                      base::Unretained(this)));
     } else {
       DebugScopedSetImplThread impl(task_runner_provider());
@@ -906,16 +885,12 @@ class LayerTreeHostContextTestDontUseLostResources
     std::unique_ptr<RenderPass> pass_for_quad = RenderPass::Create();
     pass_for_quad->SetNew(
         // AppendOneOfEveryQuadType() makes a RenderPass quad with this id.
-        RenderPassId(2, 1),
-        gfx::Rect(0, 0, 10, 10),
-        gfx::Rect(0, 0, 10, 10),
+        RenderPassId(2, 1), gfx::Rect(0, 0, 10, 10), gfx::Rect(0, 0, 10, 10),
         gfx::Transform());
 
     std::unique_ptr<RenderPass> pass = RenderPass::Create();
-    pass->SetNew(RenderPassId(1, 1),
-                 gfx::Rect(0, 0, 10, 10),
-                 gfx::Rect(0, 0, 10, 10),
-                 gfx::Transform());
+    pass->SetNew(RenderPassId(1, 1), gfx::Rect(0, 0, 10, 10),
+                 gfx::Rect(0, 0, 10, 10), gfx::Transform());
     gpu::SyncToken mailbox_sync_token;
     AddOneOfEveryQuadType(pass.get(), child_resource_provider_.get(),
                           RenderPassId(2, 1), &mailbox_sync_token);
@@ -1198,9 +1173,8 @@ class UIResourceLostTest : public LayerTreeHostContextTest {
   void PostLoseContextToImplThread() {
     EXPECT_TRUE(layer_tree_host()->GetTaskRunnerProvider()->IsMainThread());
     ImplThreadTaskRunner()->PostTask(
-        FROM_HERE,
-        base::Bind(&LayerTreeHostContextTest::LoseContext,
-                   base::Unretained(this)));
+        FROM_HERE, base::Bind(&LayerTreeHostContextTest::LoseContext,
+                              base::Unretained(this)));
   }
 
  protected:
@@ -1591,9 +1565,7 @@ SINGLE_AND_MULTI_THREAD_TEST_F(UIResourceLostEviction);
 class LayerTreeHostContextTestLoseAfterSendingBeginMainFrame
     : public LayerTreeHostContextTest {
  protected:
-  void BeginTest() override {
-    PostSetNeedsCommitToMainThread();
-  }
+  void BeginTest() override { PostSetNeedsCommitToMainThread(); }
 
   void WillBeginMainFrame() override {
     // Don't begin a frame with a lost surface.
@@ -1641,5 +1613,50 @@ class LayerTreeHostContextTestLoseAfterSendingBeginMainFrame
 SINGLE_AND_MULTI_THREAD_TEST_F(
     LayerTreeHostContextTestLoseAfterSendingBeginMainFrame);
 
+// This test causes context loss on the worker context but not the compositor
+// context and checks that draw still occurs. The resources might be in a bad
+// state e.g. null sync tokens but that shouldn't cause the draw to fail.

[danakj, 2016/10/06 22:09:04]

High level question, should we be looking for a GL error on the worker context
and not drawing? Or what do you think the best thing to be doing in this
situation is?

[sunnyps, 2016/10/06 22:29:21]

In prod worker context loss implies compositor context loss because they're in a
share group. In test worker and compositor contexts aren't in a share group
which is why the DCHECK(resource_provider_->IsGLContextLost()) wasn't working.
We could check the worker context instead but even that's an implementation
detail that the resource provider shouldn't worry about.

We could abort the draw if worker context is lost (by explicitly checking
instead of a callback) but since the context can be lost at any time it's better
to make the code safe and wait for a callback.

As part of enabling async worker context we'll have to plumb worker context loss
to the compositor somehow. I don't know if this means binding the worker context
to compositor thread or if there's some other way.

[danakj, 2016/10/06 22:47:42]

They could be put into a share group (I thought I had done that but maybe I only
did it for pixel tests).
I agree.
Yeah I'm really wondering what it means to have worker context lost and the
compositor doesn't know. It's going to draw black tiles essentially, and think
that the resources are complete so it won't recreate them or anything. So if
worker context is lost and we're trying to support compositor context /not/
being lost too, then I feel like we need to check and release the output surface
anyway at some point here or risk really bad stuff.
This is the heart of what I am wondering about. Should our unit tests be testing
non-share-group contexts if we're not using them yet and don't have code to deal
with that situation fully formed?

+class LayerTreeHostContextTestLoseWorkerContextDuringPrepareTiles
+    : public LayerTreeTest {
+ protected:
+  void SetupTree() override {
+    SkPaint paint;
+    client_.set_fill_with_nonsolid_color(true);
+    client_.add_draw_rect(gfx::Rect(5, 5), paint);
+
+    scoped_refptr<FakePictureLayer> picture_layer =
+        FakePictureLayer::Create(&client_);
+    picture_layer->SetBounds(gfx::Size(10, 20));
+    client_.set_bounds(picture_layer->bounds());
+    layer_tree()->SetRootLayer(picture_layer);
+
+    LayerTreeTest::SetupTree();
+  }
+
+  void BeginTest() override { PostSetNeedsCommitToMainThread(); }
+
+  void WillPrepareTilesOnThread(LayerTreeHostImpl* host_impl) override {
+    ContextProvider::ScopedContextLock scoped_context(
+        host_impl->compositor_frame_sink()->worker_context_provider());
+    gpu::gles2::GLES2Interface* gl = scoped_context.ContextGL();
+    gl->LoseContextCHROMIUM(GL_GUILTY_CONTEXT_RESET_ARB,
+                            GL_INNOCENT_CONTEXT_RESET_ARB);
+  }
+
+  void DrawLayersOnThread(LayerTreeHostImpl* host_impl) override {
+    ++num_draws_;
+  }
+
+  void DidCommitAndDrawFrame() override { EndTest(); }
+
+  void AfterTest() override { EXPECT_EQ(1, num_draws_); }
+
+  FakeContentLayerClient client_;
+  int num_draws_ = 0;
+};
+
+MULTI_THREAD_TEST_F(
+    LayerTreeHostContextTestLoseWorkerContextDuringPrepareTiles);
+
 }  // namespace
 }  // namespace cc