Revert of cc: Move worker context BindToCurrentThread/SetupLock calls out of cc::OutputSurface. (patchset #5 id:80001 of https://codereview.chromium.org/1317743002/ )

Reason for revert:
Likely causes crash on Mac: https://build.chromium.org/p/chromium.gpu/builders/Mac%20Release%20%28Intel%29/builds/54667

Going to see if reverting helps.
BUG=527186

Original issue's description:
> cc: Move worker context BindToCurrentThread/SetupLock calls out of cc::OutputSurface.
>
> This moves the responsibility to call these functions from
> cc:OutputSurface to the code that creates the context.
>
> BUG=523411
> CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
>
> Committed: https://crrev.com/10b73d7c140bc67ddeeaf716eb31df0977fe2497
> Cr-Commit-Position: refs/heads/master@{#346712}

TBR=avi@chromium.org,danakj@chromium.org,reveman@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=523411

Review URL: https://codereview.chromium.org/1317533003

Cr-Commit-Position: refs/heads/master@{#346736}

14 files changed, 33 insertions, 80 deletions

diff --git a/cc/output/output_surface.cc b/cc/output/output_surface.cc
index 65f0ef3..056879f 100644
--- a/cc/output/output_surface.cc
+++ b/cc/output/output_surface.cc
@@ -128,6 +128,12 @@ bool OutputSurface::BindToClient(OutputSurfaceClient* client) {
     }
   }
 
+  if (success && worker_context_provider_.get()) {
+    success = worker_context_provider_->BindToCurrentThread();
+    if (success)
+      worker_context_provider_->SetupLock();
+  }
+
   if (!success)
     client_ = NULL;
 
diff --git a/cc/raster/tile_task_worker_pool_unittest.cc b/cc/raster/tile_task_worker_pool_unittest.cc
index 73f22df..da411f6 100644
--- a/cc/raster/tile_task_worker_pool_unittest.cc
+++ b/cc/raster/tile_task_worker_pool_unittest.cc
@@ -133,10 +133,9 @@ class TileTaskWorkerPoolTest
 
   TileTaskWorkerPoolTest()
       : context_provider_(TestContextProvider::Create()),
-        worker_context_provider_(TestContextProvider::CreateWorker()),
+        worker_context_provider_(TestContextProvider::Create()),
         all_tile_tasks_finished_(
-            base::ThreadTaskRunnerHandle::Get()
-                .get(),
+            base::ThreadTaskRunnerHandle::Get().get(),
             base::Bind(&TileTaskWorkerPoolTest::AllTileTasksFinished,
                        base::Unretained(this))),
         timeout_seconds_(5),
diff --git a/cc/test/fake_output_surface.h b/cc/test/fake_output_surface.h
index d2af2e3..066f027 100644
--- a/cc/test/fake_output_surface.h
+++ b/cc/test/fake_output_surface.h
@@ -23,15 +23,14 @@ class FakeOutputSurface : public OutputSurface {
   ~FakeOutputSurface() override;
 
   static scoped_ptr<FakeOutputSurface> Create3d() {
-    return make_scoped_ptr(
-        new FakeOutputSurface(TestContextProvider::Create(),
-                              TestContextProvider::CreateWorker(), false));
+    return make_scoped_ptr(new FakeOutputSurface(
+        TestContextProvider::Create(), TestContextProvider::Create(), false));
   }
 
   static scoped_ptr<FakeOutputSurface> Create3d(
       scoped_refptr<ContextProvider> context_provider) {
     return make_scoped_ptr(new FakeOutputSurface(
-        context_provider, TestContextProvider::CreateWorker(), false));
+        context_provider, TestContextProvider::Create(), false));
   }
 
   static scoped_ptr<FakeOutputSurface> Create3d(
@@ -45,7 +44,7 @@ class FakeOutputSurface : public OutputSurface {
       scoped_ptr<TestWebGraphicsContext3D> context) {
     return make_scoped_ptr(
         new FakeOutputSurface(TestContextProvider::Create(context.Pass()),
-                              TestContextProvider::CreateWorker(), false));
+                              TestContextProvider::Create(), false));
   }
 
   static scoped_ptr<FakeOutputSurface> CreateSoftware(
@@ -55,22 +54,21 @@ class FakeOutputSurface : public OutputSurface {
   }
 
   static scoped_ptr<FakeOutputSurface> CreateDelegating3d() {
-    return make_scoped_ptr(
-        new FakeOutputSurface(TestContextProvider::Create(),
-                              TestContextProvider::CreateWorker(), true));
+    return make_scoped_ptr(new FakeOutputSurface(
+        TestContextProvider::Create(), TestContextProvider::Create(), true));
   }
 
   static scoped_ptr<FakeOutputSurface> CreateDelegating3d(
       scoped_refptr<TestContextProvider> context_provider) {
     return make_scoped_ptr(new FakeOutputSurface(
-        context_provider, TestContextProvider::CreateWorker(), true));
+        context_provider, TestContextProvider::Create(), true));
   }
 
   static scoped_ptr<FakeOutputSurface> CreateDelegating3d(
       scoped_ptr<TestWebGraphicsContext3D> context) {
     return make_scoped_ptr(
         new FakeOutputSurface(TestContextProvider::Create(context.Pass()),
-                              TestContextProvider::CreateWorker(), true));
+                              TestContextProvider::Create(), true));
   }
 
   static scoped_ptr<FakeOutputSurface> CreateDelegatingSoftware(
diff --git a/cc/test/test_context_provider.cc b/cc/test/test_context_provider.cc
index cbc92a5..ca7714d 100644
--- a/cc/test/test_context_provider.cc
+++ b/cc/test/test_context_provider.cc
@@ -19,18 +19,7 @@ namespace cc {
 
 // static
 scoped_refptr<TestContextProvider> TestContextProvider::Create() {
-  return Create(TestWebGraphicsContext3D::Create());
-}
-
-// static
-scoped_refptr<TestContextProvider> TestContextProvider::CreateWorker() {
-  scoped_refptr<TestContextProvider> worker_context_provider =
-      Create(TestWebGraphicsContext3D::Create());
-  if (!worker_context_provider)
-    return nullptr;
-  // Worker context is initially bound but not attached to a specific thread.
-  worker_context_provider->bound_ = true;
-  return worker_context_provider;
+  return Create(TestWebGraphicsContext3D::Create().Pass());
 }
 
 // static
diff --git a/cc/test/test_context_provider.h b/cc/test/test_context_provider.h
index 8f128f1..82b3e61 100644
--- a/cc/test/test_context_provider.h
+++ b/cc/test/test_context_provider.h
@@ -25,10 +25,6 @@ class TestContextProvider : public ContextProvider {
     CreateCallback;
 
   static scoped_refptr<TestContextProvider> Create();
-  // Creates a worker context provider that doesn't need to be bound to a
-  // thread and can be used on any thread. This is equivalent to:
-  // Create(); BindToCurrentThread(); SetupLock(); DetachFromThread().
-  static scoped_refptr<TestContextProvider> CreateWorker();
   static scoped_refptr<TestContextProvider> Create(
       scoped_ptr<TestWebGraphicsContext3D> context);
 
diff --git a/cc/trees/layer_tree_host_impl_unittest.cc b/cc/trees/layer_tree_host_impl_unittest.cc
index 9271cff..3b5765f 100644
--- a/cc/trees/layer_tree_host_impl_unittest.cc
+++ b/cc/trees/layer_tree_host_impl_unittest.cc
@@ -8479,8 +8479,7 @@ class MockReclaimResourcesOutputSurface : public FakeOutputSurface {
  public:
   static scoped_ptr<MockReclaimResourcesOutputSurface> Create3d() {
     return make_scoped_ptr(new MockReclaimResourcesOutputSurface(
-        TestContextProvider::Create(), TestContextProvider::CreateWorker(),
-        false));
+        TestContextProvider::Create(), TestContextProvider::Create(), false));
   }
 
   MOCK_METHOD0(ForceReclaimResources, void());
diff --git a/cc/trees/layer_tree_host_unittest.cc b/cc/trees/layer_tree_host_unittest.cc
index 2b8f11a..9636f38 100644
--- a/cc/trees/layer_tree_host_unittest.cc
+++ b/cc/trees/layer_tree_host_unittest.cc
@@ -463,7 +463,7 @@ class LayerTreeHostFreeWorkerContextResourcesTest : public LayerTreeHostTest {
     explicit MockSetWorkerContextShouldAggressivelyFreeResourcesOutputSurface(
         bool delegated_rendering)
         : FakeOutputSurface(TestContextProvider::Create(),
-                            TestContextProvider::CreateWorker(),
+                            TestContextProvider::Create(),
                             delegated_rendering) {}
     MOCK_METHOD1(SetWorkerContextShouldAggressivelyFreeResources,
                  void(bool is_visible));
diff --git a/content/browser/android/in_process/synchronous_compositor_factory_impl.cc b/content/browser/android/in_process/synchronous_compositor_factory_impl.cc
index 18c544c..220c8624 100644
--- a/content/browser/android/in_process/synchronous_compositor_factory_impl.cc
+++ b/content/browser/android/in_process/synchronous_compositor_factory_impl.cc
@@ -173,14 +173,6 @@ SynchronousCompositorFactoryImpl::CreateOutputSurface(
       CreateContextProviderForCompositor(surface_id, RENDER_COMPOSITOR_CONTEXT);
   scoped_refptr<cc::ContextProvider> worker_context =
       CreateContextProviderForCompositor(0, RENDER_WORKER_CONTEXT);
-  if (!worker_context->BindToCurrentThread())
-    worker_context = nullptr;
-  if (worker_context) {
-    worker_context->SetupLock();
-    // Detach from thread to allow context to be destroyed on a
-    // different thread without being used.
-    worker_context->DetachFromThread();
-  }
 
   return make_scoped_ptr(new SynchronousCompositorOutputSurface(
       onscreen_context, worker_context, routing_id, frame_swap_message_queue));
diff --git a/content/browser/compositor/gpu_process_transport_factory.cc b/content/browser/compositor/gpu_process_transport_factory.cc
index bf1af95..4c0b363 100644
--- a/content/browser/compositor/gpu_process_transport_factory.cc
+++ b/content/browser/compositor/gpu_process_transport_factory.cc
@@ -265,13 +265,6 @@ void GpuProcessTransportFactory::EstablishedGpuChannel(
         if (shared_worker_context_provider_ &&
             !shared_worker_context_provider_->BindToCurrentThread())
           shared_worker_context_provider_ = nullptr;
-
-        if (shared_worker_context_provider_) {
-          shared_worker_context_provider_->SetupLock();
-          // Detach from thread to allow context to be destroyed on a
-          // different thread without being used.
-          shared_worker_context_provider_->DetachFromThread();
-        }
       }
     }
 
diff --git a/content/common/gpu/client/context_provider_command_buffer.cc b/content/common/gpu/client/context_provider_command_buffer.cc
index 172b1e8..ba9f0eb 100644
--- a/content/common/gpu/client/context_provider_command_buffer.cc
+++ b/content/common/gpu/client/context_provider_command_buffer.cc
@@ -182,9 +182,7 @@ void ContextProviderCommandBuffer::DeleteCachedResources() {
 }
 
 void ContextProviderCommandBuffer::OnLostContext() {
-  // Note: no thread check here as this should not change the thread for which
-  // this context is currently bound. e.g. a worker context might be unbound
-  // and this should not result in it being bound to the current thread.
+  DCHECK(context_thread_checker_.CalledOnValidThread());
   {
     base::AutoLock lock(main_thread_lock_);
     if (destroyed_)
@@ -199,9 +197,8 @@ void ContextProviderCommandBuffer::OnLostContext() {
 
 void ContextProviderCommandBuffer::OnMemoryAllocationChanged(
     const gpu::MemoryAllocation& allocation) {
-  // Note: no thread check here as this should not change the thread for which
-  // this context is currently bound. e.g. a worker context might be unbound
-  // and this should not result in it being bound to the current thread.
+  DCHECK(context_thread_checker_.CalledOnValidThread());
+
   if (memory_policy_changed_callback_.is_null())
     return;
 
diff --git a/content/renderer/render_widget.cc b/content/renderer/render_widget.cc
index 90e394b..285ca7f 100644
--- a/content/renderer/render_widget.cc
+++ b/content/renderer/render_widget.cc
@@ -1022,15 +1022,10 @@ scoped_ptr<cc::OutputSurface> RenderWidget::CreateOutputSurface(bool fallback) {
 
     worker_context_provider = ContextProviderCommandBuffer::Create(
         CreateGraphicsContext3D(false), RENDER_WORKER_CONTEXT);
-    if (!worker_context_provider.get() ||
-        !worker_context_provider->BindToCurrentThread()) {
+    if (!worker_context_provider.get()) {
       // Cause the compositor to wait and try again.
       return scoped_ptr<cc::OutputSurface>();
     }
-    worker_context_provider->SetupLock();
-    // Detach from thread to allow context to be destroyed on a different
-    // thread without being used.
-    worker_context_provider->DetachFromThread();
   }
 
   uint32 output_surface_id = next_output_surface_id_++;
diff --git a/ui/compositor/test/in_process_context_factory.cc b/ui/compositor/test/in_process_context_factory.cc
index 3bb555a..942947d 100644
--- a/ui/compositor/test/in_process_context_factory.cc
+++ b/ui/compositor/test/in_process_context_factory.cc
@@ -115,15 +115,6 @@ void InProcessContextFactory::CreateOutputSurface(
   scoped_refptr<InProcessContextProvider> worker_context_provider =
       InProcessContextProvider::CreateOffscreen(&gpu_memory_buffer_manager_,
                                                 &image_factory_);
-  if (worker_context_provider &&
-      !worker_context_provider->BindToCurrentThread())
-    worker_context_provider = nullptr;
-  if (worker_context_provider) {
-    worker_context_provider->SetupLock();
-    // Detach from thread to allow context to be destroyed on a different
-    // thread without being used.
-    worker_context_provider->DetachFromThread();
-  }
 
   scoped_ptr<cc::OutputSurface> real_output_surface;
 
diff --git a/ui/compositor/test/in_process_context_provider.cc b/ui/compositor/test/in_process_context_provider.cc
index a6c8d3a..76cd19d 100644
--- a/ui/compositor/test/in_process_context_provider.cc
+++ b/ui/compositor/test/in_process_context_provider.cc
@@ -218,9 +218,7 @@ void InProcessContextProvider::SetMemoryPolicyChangedCallback(
 }
 
 void InProcessContextProvider::OnLostContext() {
-  // Note: no thread check here as this should not change the thread for which
-  // this context is currently bound. e.g. a worker context might be unbound
-  // and this should not result in it being bound to the current thread.
+  DCHECK(context_thread_checker_.CalledOnValidThread());
   {
     base::AutoLock lock(destroyed_lock_);
     if (destroyed_)
diff --git a/ui/compositor/test/in_process_context_provider.h b/ui/compositor/test/in_process_context_provider.h
index 628eaec..0f5f35f 100644
--- a/ui/compositor/test/in_process_context_provider.h
+++ b/ui/compositor/test/in_process_context_provider.h
@@ -38,6 +38,15 @@ class InProcessContextProvider : public cc::ContextProvider {
       gpu::GpuMemoryBufferManager* gpu_memory_buffer_manager,
       gpu::ImageFactory* image_factory);
 
+ private:
+  InProcessContextProvider(
+      const gpu::gles2::ContextCreationAttribHelper& attribs,
+      gpu::GpuMemoryBufferManager* gpu_memory_buffer_manager,
+      gpu::ImageFactory* image_factory,
+      gfx::AcceleratedWidget window,
+      const std::string& debug_name);
+  ~InProcessContextProvider() override;
+
   // cc::ContextProvider:
   bool BindToCurrentThread() override;
   void DetachFromThread() override;
@@ -57,15 +66,6 @@ class InProcessContextProvider : public cc::ContextProvider {
       const MemoryPolicyChangedCallback& memory_policy_changed_callback)
       override;
 
- private:
-  InProcessContextProvider(
-      const gpu::gles2::ContextCreationAttribHelper& attribs,
-      gpu::GpuMemoryBufferManager* gpu_memory_buffer_manager,
-      gpu::ImageFactory* image_factory,
-      gfx::AcceleratedWidget window,
-      const std::string& debug_name);
-  ~InProcessContextProvider() override;
-
   void OnLostContext();
 
   base::ThreadChecker main_thread_checker_;