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

cc/trees/layer_tree_host_unittest_context.cc

 // Copyright 2012 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 <stddef.h>
 #include <stdint.h>
 
 #include "base/memory/ptr_util.h"
 #include "cc/layers/heads_up_display_layer.h"
 #include "cc/layers/layer_impl.h"
@@ skipping 58 matching lines @@
     // there.
     base::AutoLock lock(context3d_lock_);
     // For sanity-checking tests, they should only call this when the
     // context is not lost.
     CHECK(context3d_);
     context3d_->loseContextCHROMIUM(GL_GUILTY_CONTEXT_RESET_ARB,
                                     GL_INNOCENT_CONTEXT_RESET_ARB);
     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);
-    }
-    return LayerTreeTest::CreateCompositorFrameSink(
-        std::move(compositor_context_provider),
-        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);
+      context3d_->loseContextCHROMIUM(GL_GUILTY_CONTEXT_RESET_ARB,
+                                      GL_INNOCENT_CONTEXT_RESET_ARB);
     }
 
-    base::AutoLock lock(context3d_lock_);
-    context3d_ = context3d.get();
-    return LayerTreeTest::CreateDisplayOutputSurface(
-        TestContextProvider::Create(std::move(context3d)));
+    return LayerTreeTest::CreateCompositorFrameSink(
+        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));
   }
 
   DrawResult PrepareToDrawOnThread(LayerTreeHostImpl* host_impl,
                                    LayerTreeHostImpl::FrameData* frame,
                                    DrawResult draw_result) override {
     if (draw_result == DRAW_ABORTED_MISSING_HIGH_RES_CONTENT) {
       // Only valid for single-threaded compositing, which activates
       // immediately and will try to draw again when content has finished.
       DCHECK(!host_impl->task_runner_provider()->HasImplThread());
       return draw_result;
@@ skipping 355 matching lines @@
 // This test uses Composite() which only exists for single thread.
 SINGLE_THREAD_TEST_F(MultipleCompositeDoesNotCreateCompositorFrameSink);
 
 // This test makes sure that once a SingleThreadProxy issues a
 // DidFailToInitializeCompositorFrameSink, that future Composite calls will not
 // trigger additional requests for output surfaces.
 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;
     settings->use_zero_copy = true;
   }
 
   void RequestNewCompositorFrameSink() override {
     num_requests_++;
     // There should be one initial request and then one request from
     // the LayerTreeTest test hooks DidFailToInitializeCompositorFrameSink
     // (which is hard to skip).  This second request is just ignored and is test
     // cruft.
     EXPECT_LE(num_requests_, 2);
     if (num_requests_ > 1)
       return;
     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));
     EXPECT_EQ(num_requests_, 2);
     EXPECT_TRUE(has_failed_);
 
     // Second composite should not request or fail.
     layer_tree_host()->Composite(base::TimeTicks::FromInternalValue(2));
     EXPECT_EQ(num_requests_, 2);
     EndTest();
@@ skipping 193 matching lines @@
     LayerTreeHostContextTest::SetupTree();
   }
 
   void BeginTest() override { PostSetNeedsCommitToMainThread(); }
 
   void PostEvictTextures() {
     if (HasImplThread()) {
       ImplThreadTaskRunner()->PostTask(
           FROM_HERE,
           base::Bind(&LayerTreeHostContextTestLostContextAndEvictTextures::
-                          EvictTexturesOnImplThread,
+                         EvictTexturesOnImplThread,
                      base::Unretained(this)));
     } else {
       DebugScopedSetImplThread impl(task_runner_provider());
       EvictTexturesOnImplThread();
     }
   }
 
   void EvictTexturesOnImplThread() {
     impl_host_->EvictTexturesForTesting();
 
@@ skipping 156 matching lines @@
                                    bool lost) {}
 
   void SetupTree() override {
     gpu::gles2::GLES2Interface* gl = child_context_provider_->ContextGL();
 
     std::unique_ptr<DelegatedFrameData> frame_data(new DelegatedFrameData);
 
     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);
 
     frame_data->render_pass_list.push_back(std::move(pass_for_quad));
     frame_data->render_pass_list.push_back(std::move(pass));
 
     ResourceId resource = child_resource_provider_->CreateResource(
         gfx::Size(4, 4), ResourceProvider::TEXTURE_HINT_IMMUTABLE, RGBA_8888,
         gfx::ColorSpace());
@@ skipping 262 matching lines @@
   void PostStepCompleteToMainThread() {
     task_runner_provider()->MainThreadTaskRunner()->PostTask(
         FROM_HERE,
         base::Bind(&UIResourceLostTest::StepCompleteOnMainThreadInternal,
                    base::Unretained(this), time_step_));
   }
 
   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:
   int time_step_;
   std::unique_ptr<FakeScopedUIResource> ui_resource_;
 
  private:
   void StepCompleteOnMainThreadInternal(int step) {
     EXPECT_TRUE(layer_tree_host()->GetTaskRunnerProvider()->IsMainThread());
     StepCompleteOnMainThread(step);
@@ skipping 370 matching lines @@
  private:
   std::unique_ptr<FakeScopedUIResource> ui_resource2_;
   std::unique_ptr<FakeScopedUIResource> ui_resource3_;
 };
 
 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.
     EXPECT_FALSE(lost_);
 
     if (deferred_)
       return;
     deferred_ = true;
 
     // Defer commits before the BeginFrame completes, causing it to be delayed.
@@ skipping 27 matching lines @@
 
   void AfterTest() override {}
 
   bool deferred_ = false;
   bool lost_ = true;
 };
 
 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