cc: Add mailbox support to ResourceProvider write locks.

cc/resources/resource_provider.cc

Index: cc/resources/resource_provider.cc
diff --git a/cc/resources/resource_provider.cc b/cc/resources/resource_provider.cc
index 8d7fb6428ba375b8a2425417a83921fdcd3a2995..d98d20001f8832338bd7b143bf6b8c2d27a6f9ed 100644
--- a/cc/resources/resource_provider.cc
+++ b/cc/resources/resource_provider.cc
@@ -695,11 +695,21 @@ void ResourceProvider::DeleteResourceInternal(ResourceMap::iterator it,
                                               DeleteStyle style) {
   TRACE_EVENT0("cc", "ResourceProvider::DeleteResourceInternal");
   Resource* resource = &it->second;
-  bool lost_resource = resource->lost;
-
   DCHECK(resource->exported_count == 0 || style != NORMAL);
-  if (style == FOR_SHUTDOWN && resource->exported_count > 0)
-    lost_resource = true;
+
+  bool lost_resource =
+      resource->lost ||
+      (style == FOR_SHUTDOWN && resource->exported_count > 0) ||
+      (IsGpuResourceType(resource->type) && lost_output_surface_);
+
+  if (!lost_resource &&
+      resource->synchronization_state() == Resource::NEEDS_WAIT) {
+    DCHECK(resource->allocated);
+    DCHECK(IsGpuResourceType(resource->type));
+    GLES2Interface* gl = ContextGL();
+    DCHECK(gl);
+    resource->WaitSyncToken(gl);
+  }
 
   if (resource->image_id) {
     DCHECK(resource->origin == Resource::INTERNAL);
@@ -730,7 +740,6 @@ void ResourceProvider::DeleteResourceInternal(ResourceMap::iterator it,
     gpu::SyncToken sync_token = resource->mailbox().sync_token();
     if (IsGpuResourceType(resource->type)) {
       DCHECK(resource->mailbox().IsTexture());
-      lost_resource |= lost_output_surface_;
       GLES2Interface* gl = ContextGL();
       DCHECK(gl);
       if (resource->gl_id) {
@@ -816,11 +825,12 @@ void ResourceProvider::CopyToResource(ResourceId id,
     SkCanvas dest(lock.sk_bitmap());
     dest.writePixels(source_info, image, image_stride, 0, 0);
   } else {
-    ScopedWriteLockGL lock(this, id);
-    DCHECK(lock.texture_id());
+    ScopedWriteLockGL lock(this, id, false);
+    unsigned resource_texture_id = lock.texture_id();
+    DCHECK(resource_texture_id);
     GLES2Interface* gl = ContextGL();
     DCHECK(gl);
-    gl->BindTexture(resource->target, lock.texture_id());
+    gl->BindTexture(resource->target, resource_texture_id);
     if (resource->format == ETC1) {
       DCHECK_EQ(resource->target, static_cast<GLenum>(GL_TEXTURE_2D));
       int image_bytes = ResourceUtil::CheckedSizeInBytes<int>(image_size, ETC1);
@@ -836,7 +846,7 @@ void ResourceProvider::CopyToResource(ResourceId id,
     gl->OrderingBarrierCHROMIUM();
     gpu::SyncToken sync_token;
     gl->GenUnverifiedSyncTokenCHROMIUM(fence_sync, sync_token.GetData());
-    lock.UpdateResourceSyncToken(sync_token);
+    lock.set_sync_token(sync_token);
   }
 }
 
@@ -907,7 +917,9 @@ const ResourceProvider::Resource* ResourceProvider::LockForRead(
 
   // Mailbox sync_tokens must be processed by a call to
   // WaitSyncTokenIfNeeded() prior to calling LockForRead().
-  DCHECK_NE(Resource::NEEDS_WAIT, resource->synchronization_state());
+  // DCHECK_NE(Resource::NEEDS_WAIT, resource->synchronization_state());

[piman, 2016/06/10 22:57:01]

nit: I think you should leave this (and remove the wait).

This is used on the browser compositor, in the parent, and we do want to make
sure we correctly synchronized in GLRenderer::BeginDrawingFrame, because
otherwise we could cause a context switch in the middle of drawing, which has
very bad performance on mobile.

[sunnyps, 2016/06/11 01:29:52]

Done.

+
+  WaitSyncTokenIfNeeded(id);
 
   LazyCreate(resource);
 
@@ -982,7 +994,9 @@ void ResourceProvider::UnlockForRead(ResourceId id) {
 ResourceProvider::Resource* ResourceProvider::LockForWrite(ResourceId id) {
   Resource* resource = GetResource(id);
   DCHECK(CanLockForWrite(id));
-  DCHECK_NE(Resource::NEEDS_WAIT, resource->synchronization_state());
+  // DCHECK_NE(Resource::NEEDS_WAIT, resource->synchronization_state());

[piman, 2016/06/10 22:57:01]

nit: remove

[sunnyps, 2016/06/11 01:29:52]

Done.

+  if (resource->allocated)
+    WaitSyncTokenIfNeeded(id);
   resource->locked_for_write = true;
   resource->SetLocallyUsed();
   return resource;
@@ -1000,12 +1014,12 @@ bool ResourceProvider::IsOverlayCandidate(ResourceId id) {
   return resource->is_overlay_candidate;
 }
 
-void ResourceProvider::UnlockForWrite(ResourceProvider::Resource* resource) {
+void ResourceProvider::UnlockForWrite(Resource* resource) {
   DCHECK(resource->locked_for_write);
   DCHECK_EQ(resource->exported_count, 0);
   DCHECK(resource->origin == Resource::INTERNAL);
   resource->locked_for_write = false;
-  resource->SetSynchronized();
+  // resource->SetSynchronized();

[piman, 2016/06/10 22:57:01]

nit: remove.

[sunnyps, 2016/06/11 01:29:52]

Done.

 }
 
 void ResourceProvider::EnableReadLockFencesForTesting(ResourceId id) {
@@ -1017,10 +1031,11 @@ void ResourceProvider::EnableReadLockFencesForTesting(ResourceId id) {
 ResourceProvider::ScopedReadLockGL::ScopedReadLockGL(
     ResourceProvider* resource_provider,
     ResourceId resource_id)
-    : resource_provider_(resource_provider),
-      resource_id_(resource_id),
-      resource_(resource_provider->LockForRead(resource_id, false)) {
-  DCHECK(resource_);
+    : resource_provider_(resource_provider), resource_id_(resource_id) {
+  const Resource* resource = resource_provider->LockForRead(resource_id, false);
+  texture_id_ = resource->gl_id;
+  target_ = resource->target;
+  size_ = resource->size;
 }
 
 ResourceProvider::ScopedReadLockGL::~ScopedReadLockGL() {
@@ -1031,42 +1046,111 @@ ResourceProvider::ScopedSamplerGL::ScopedSamplerGL(
     ResourceProvider* resource_provider,
     ResourceId resource_id,
     GLenum filter)
-    : ScopedReadLockGL(resource_provider, resource_id),
+    : resource_lock_(resource_provider, resource_id),
       unit_(GL_TEXTURE0),
-      target_(resource_provider->BindForSampling(resource_id, unit_, filter)) {
-}
+      target_(resource_provider->BindForSampling(resource_id, unit_, filter)) {}
 
 ResourceProvider::ScopedSamplerGL::ScopedSamplerGL(
     ResourceProvider* resource_provider,
     ResourceId resource_id,
     GLenum unit,
     GLenum filter)
-    : ScopedReadLockGL(resource_provider, resource_id),
+    : resource_lock_(resource_provider, resource_id),
       unit_(unit),
-      target_(resource_provider->BindForSampling(resource_id, unit_, filter)) {
-}
+      target_(resource_provider->BindForSampling(resource_id, unit_, filter)) {}
 
 ResourceProvider::ScopedSamplerGL::~ScopedSamplerGL() {
 }
 
 ResourceProvider::ScopedWriteLockGL::ScopedWriteLockGL(
     ResourceProvider* resource_provider,
-    ResourceId resource_id)
-    : resource_provider_(resource_provider),
-      resource_(resource_provider->LockForWrite(resource_id)),
-      texture_id_(0),
-      set_sync_token_(false) {
-  resource_provider_->LazyAllocate(resource_);
-  texture_id_ = resource_->gl_id;
-  DCHECK(texture_id_);
-  if (resource_->image_id && resource_->dirty_image)
-    resource_provider_->BindImageForSampling(resource_);
+    ResourceId resource_id,
+    bool create_mailbox)
+    : resource_provider_(resource_provider), resource_id_(resource_id) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  Resource* resource = resource_provider->LockForWrite(resource_id);
+  resource_provider_->LazyAllocate(resource);
+  if (resource->image_id && resource->dirty_image)
+    resource_provider_->BindImageForSampling(resource);
+  if (create_mailbox) {
+    resource_provider_->CreateMailboxAndBindResource(
+        resource_provider_->ContextGL(), resource);
+  }
+  texture_id_ = resource->gl_id;
+  target_ = resource->target;
+  format_ = resource->format;
+  size_ = resource->size;
+  mailbox_ = resource->mailbox();
 }
 
 ResourceProvider::ScopedWriteLockGL::~ScopedWriteLockGL() {
-  if (set_sync_token_)
-    resource_->UpdateSyncToken(sync_token_);
-  resource_provider_->UnlockForWrite(resource_);
+  DCHECK(thread_checker_.CalledOnValidThread());
+  Resource* resource = resource_provider_->GetResource(resource_id_);
+  DCHECK(resource->locked_for_write);
+  if (sync_token_.HasData())
+    resource->UpdateSyncToken(sync_token_);
+  resource_provider_->UnlockForWrite(resource);
+}
+
+ResourceProvider::ScopedTextureProvider::ScopedTextureProvider(
+    gpu::gles2::GLES2Interface* gl,
+    ScopedWriteLockGL* resource_lock,
+    bool use_mailbox)
+    : gl_(gl), use_mailbox_(use_mailbox) {
+  if (use_mailbox_) {
+    texture_id_ = gl_->CreateAndConsumeTextureCHROMIUM(
+        resource_lock->target(), resource_lock->mailbox().name());
+  } else {
+    texture_id_ = resource_lock->texture_id();
+  }
+  DCHECK(texture_id_);
+}
+
+ResourceProvider::ScopedTextureProvider::~ScopedTextureProvider() {
+  if (use_mailbox_)
+    gl_->DeleteTextures(1, &texture_id_);
+}
+
+ResourceProvider::ScopedSkSurfaceProvider::ScopedSkSurfaceProvider(
+    ContextProvider* context_provider,
+    ScopedWriteLockGL* resource_lock,
+    bool use_mailbox,
+    bool use_distance_field_text,
+    bool can_use_lcd_text,
+    int msaa_sample_count)
+    : texture_provider_(context_provider->ContextGL(),
+                        resource_lock,
+                        use_mailbox) {
+  GrGLTextureInfo texture_info;
+  texture_info.fID = texture_provider_.texture_id();
+  texture_info.fTarget = resource_lock->target();
+  GrBackendTextureDesc desc;
+  desc.fFlags = kRenderTarget_GrBackendTextureFlag;
+  desc.fWidth = resource_lock->size().width();
+  desc.fHeight = resource_lock->size().height();
+  desc.fConfig = ToGrPixelConfig(resource_lock->format());
+  desc.fOrigin = kTopLeft_GrSurfaceOrigin;
+  desc.fTextureHandle = skia::GrGLTextureInfoToGrBackendObject(texture_info);
+  desc.fSampleCnt = msaa_sample_count;
+
+  uint32_t flags =
+      use_distance_field_text ? SkSurfaceProps::kUseDistanceFieldFonts_Flag : 0;
+  // Use unknown pixel geometry to disable LCD text.
+  SkSurfaceProps surface_props(flags, kUnknown_SkPixelGeometry);
+  if (can_use_lcd_text) {
+    // LegacyFontHost will get LCD text and skia figures out what type to use.
+    surface_props =
+        SkSurfaceProps(flags, SkSurfaceProps::kLegacyFontHost_InitType);
+  }
+  sk_surface_ = SkSurface::MakeFromBackendTextureAsRenderTarget(
+      context_provider->GrContext(), desc, &surface_props);
+}
+
+ResourceProvider::ScopedSkSurfaceProvider::~ScopedSkSurfaceProvider() {
+  if (sk_surface_.get()) {
+    sk_surface_->prepareForExternalIO();
+    sk_surface_.reset();
+  }
 }
 
 void ResourceProvider::PopulateSkBitmapWithResource(
@@ -1092,49 +1176,50 @@ ResourceProvider::ScopedReadLockSoftware::~ScopedReadLockSoftware() {
 ResourceProvider::ScopedWriteLockSoftware::ScopedWriteLockSoftware(
     ResourceProvider* resource_provider,
     ResourceId resource_id)
-    : resource_provider_(resource_provider),
-      resource_(resource_provider->LockForWrite(resource_id)) {
-  ResourceProvider::PopulateSkBitmapWithResource(&sk_bitmap_, resource_);
+    : resource_provider_(resource_provider), resource_id_(resource_id) {
+  ResourceProvider::PopulateSkBitmapWithResource(
+      &sk_bitmap_, resource_provider->LockForWrite(resource_id));
   DCHECK(valid());
 }
 
 ResourceProvider::ScopedWriteLockSoftware::~ScopedWriteLockSoftware() {
   DCHECK(thread_checker_.CalledOnValidThread());
-  resource_provider_->UnlockForWrite(resource_);
+  resource_provider_->UnlockForWrite(
+      resource_provider_->GetResource(resource_id_));
 }
 
 ResourceProvider::ScopedWriteLockGpuMemoryBuffer::
     ScopedWriteLockGpuMemoryBuffer(ResourceProvider* resource_provider,
                                    ResourceId resource_id)
-    : resource_provider_(resource_provider),
-      resource_(resource_provider->LockForWrite(resource_id)) {
-  DCHECK(IsGpuResourceType(resource_->type));
-  gpu_memory_buffer_ = std::move(resource_->gpu_memory_buffer);
-  resource_->gpu_memory_buffer = nullptr;
+    : resource_provider_(resource_provider), resource_id_(resource_id) {
+  Resource* resource = resource_provider->LockForWrite(resource_id);
+  DCHECK(IsGpuResourceType(resource->type));
+  format_ = resource->format;
+  size_ = resource->size;
+  gpu_memory_buffer_ = std::move(resource->gpu_memory_buffer);
+  resource->gpu_memory_buffer = nullptr;
 }
 
 ResourceProvider::ScopedWriteLockGpuMemoryBuffer::
     ~ScopedWriteLockGpuMemoryBuffer() {
   DCHECK(thread_checker_.CalledOnValidThread());
-  resource_provider_->UnlockForWrite(resource_);
-  if (!gpu_memory_buffer_)
-    return;
-
-  DCHECK(!resource_->gpu_memory_buffer);
-  resource_provider_->LazyCreate(resource_);
-  resource_->gpu_memory_buffer = std::move(gpu_memory_buffer_);
-  if (resource_->gpu_memory_buffer)
-    resource_->gpu_memory_buffer_id = resource_->gpu_memory_buffer->GetId();
-  resource_->allocated = true;
-  resource_provider_->LazyCreateImage(resource_);
-  resource_->dirty_image = true;
-  resource_->is_overlay_candidate = true;
-  resource_->SetSynchronized();
-
-  // GpuMemoryBuffer provides direct access to the memory used by the GPU.
-  // Read lock fences are required to ensure that we're not trying to map a
-  // buffer that is currently in-use by the GPU.
-  resource_->read_lock_fences_enabled = true;
+  Resource* resource = resource_provider_->GetResource(resource_id_);
+  if (gpu_memory_buffer_) {
+    DCHECK(!resource->gpu_memory_buffer);
+    resource_provider_->LazyCreate(resource);
+    resource->gpu_memory_buffer = std::move(gpu_memory_buffer_);
+    if (resource->gpu_memory_buffer)
+      resource->gpu_memory_buffer_id = resource->gpu_memory_buffer->GetId();
+    resource->allocated = true;
+    resource_provider_->LazyCreateImage(resource);
+    resource->dirty_image = true;
+    resource->is_overlay_candidate = true;
+    // GpuMemoryBuffer provides direct access to the memory used by the GPU.
+    // Read lock fences are required to ensure that we're not trying to map a
+    // buffer that is currently in-use by the GPU.
+    resource->read_lock_fences_enabled = true;
+  }
+  resource_provider_->UnlockForWrite(resource);
 }
 
 gfx::GpuMemoryBuffer*
@@ -1142,7 +1227,7 @@ ResourceProvider::ScopedWriteLockGpuMemoryBuffer::GetGpuMemoryBuffer() {
   if (!gpu_memory_buffer_) {
     gpu_memory_buffer_ =
         resource_provider_->gpu_memory_buffer_manager_->AllocateGpuMemoryBuffer(
-            resource_->size, BufferFormat(resource_->format),
+            size_, BufferFormat(format_),
             gfx::BufferUsage::GPU_READ_CPU_READ_WRITE, gpu::kNullSurfaceHandle);
   }
   return gpu_memory_buffer_.get();
@@ -1151,9 +1236,11 @@ ResourceProvider::ScopedWriteLockGpuMemoryBuffer::GetGpuMemoryBuffer() {
 ResourceProvider::ScopedReadLockGpuMemoryBuffer::ScopedReadLockGpuMemoryBuffer(
     ResourceProvider* resource_provider,
     ResourceId resource_id)
-    : resource_provider_(resource_provider),
-      resource_id_(resource_id),
-      resource_(resource_provider->LockForRead(resource_id, true)) {}
+    : resource_provider_(resource_provider), resource_id_(resource_id) {
+  const Resource* resource = resource_provider->LockForRead(resource_id, true);
+  gpu_memory_buffer_ = resource->gpu_memory_buffer.get();
+  texture_id_ = resource->gl_id;
+}
 
 ResourceProvider::ScopedReadLockGpuMemoryBuffer::
     ~ScopedReadLockGpuMemoryBuffer() {
@@ -1161,79 +1248,6 @@ ResourceProvider::ScopedReadLockGpuMemoryBuffer::
   resource_provider_->UnlockForRead(resource_id_);
 }
 
-gfx::GpuMemoryBuffer*
-ResourceProvider::ScopedReadLockGpuMemoryBuffer::GetGpuMemoryBuffer() const {
-  return resource_->gpu_memory_buffer.get();
-}
-
-unsigned ResourceProvider::ScopedReadLockGpuMemoryBuffer::GetTextureId() const {
-  return resource_->gl_id;
-}
-
-ResourceId ResourceProvider::ScopedReadLockGpuMemoryBuffer::GetResourceId()
-    const {
-  return resource_id_;
-}
-
-ResourceProvider::ScopedWriteLockGr::ScopedWriteLockGr(
-    ResourceProvider* resource_provider,
-    ResourceId resource_id)
-    : resource_provider_(resource_provider),
-      resource_(resource_provider->LockForWrite(resource_id)),
-      set_sync_token_(false) {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  resource_provider_->LazyAllocate(resource_);
-  if (resource_->image_id && resource_->dirty_image)
-    resource_provider_->BindImageForSampling(resource_);
-}
-
-ResourceProvider::ScopedWriteLockGr::~ScopedWriteLockGr() {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  DCHECK(resource_->locked_for_write);
-  if (set_sync_token_)
-    resource_->UpdateSyncToken(sync_token_);
-
-  resource_provider_->UnlockForWrite(resource_);
-}
-
-void ResourceProvider::ScopedWriteLockGr::InitSkSurface(
-    GrContext* gr_context,
-    bool use_distance_field_text,
-    bool can_use_lcd_text,
-    int msaa_sample_count) {
-  DCHECK(resource_->locked_for_write);
-
-  GrGLTextureInfo texture_info;
-  texture_info.fID = resource_->gl_id;
-  texture_info.fTarget = resource_->target;
-  GrBackendTextureDesc desc;
-  desc.fFlags = kRenderTarget_GrBackendTextureFlag;
-  desc.fWidth = resource_->size.width();
-  desc.fHeight = resource_->size.height();
-  desc.fConfig = ToGrPixelConfig(resource_->format);
-  desc.fOrigin = kTopLeft_GrSurfaceOrigin;
-  desc.fTextureHandle = skia::GrGLTextureInfoToGrBackendObject(texture_info);
-  desc.fSampleCnt = msaa_sample_count;
-
-  uint32_t flags =
-      use_distance_field_text ? SkSurfaceProps::kUseDistanceFieldFonts_Flag : 0;
-  // Use unknown pixel geometry to disable LCD text.
-  SkSurfaceProps surface_props(flags, kUnknown_SkPixelGeometry);
-  if (can_use_lcd_text) {
-    // LegacyFontHost will get LCD text and skia figures out what type to use.
-    surface_props =
-        SkSurfaceProps(flags, SkSurfaceProps::kLegacyFontHost_InitType);
-  }
-  sk_surface_ = SkSurface::MakeFromBackendTextureAsRenderTarget(
-      gr_context, desc, &surface_props);
-}
-
-void ResourceProvider::ScopedWriteLockGr::ReleaseSkSurface() {
-  DCHECK(sk_surface_);
-  sk_surface_->prepareForExternalIO();
-  sk_surface_.reset();
-}
-
 ResourceProvider::SynchronousFence::SynchronousFence(
     gpu::gles2::GLES2Interface* gl)
     : gl_(gl), has_synchronized_(true) {
@@ -1373,10 +1387,8 @@ void ResourceProvider::PrepareSendToParent(const ResourceIdArray& resource_ids,
                                    unverified_sync_tokens.size());
     }
   }
-  for (Resource* resource : need_synchronization_resources) {
+  for (Resource* resource : need_synchronization_resources)
     resource->UpdateSyncToken(new_sync_token);
-    resource->SetSynchronized();
-  }
 
   // Transfer Resources
   DCHECK_EQ(resources.size(), resource_ids.size());
@@ -1391,6 +1403,7 @@ void ResourceProvider::PrepareSendToParent(const ResourceIdArray& resource_ids,
     TransferableResource resource;
     TransferResource(source, id, &resource);
 
+    source->SetSynchronized();

[piman, 2016/06/10 22:57:01]

Is this right?
If a resource comes from the raster thread, we would have generated the sync
token on the worker context, but nothing here would wait for it on the
compositor thread, so I don't think it's right to SetSynchronized here. It's
valid to do on the need_synchronization_resources ones, because they were
created on the compositor context, and we just generated a token for them.

[sunnyps, 2016/06/11 01:29:52]

Done.

     source->exported_count++;
     list->push_back(resource);
   }
@@ -1790,6 +1803,10 @@ void ResourceProvider::LazyAllocate(Resource* resource) {
     LazyCreateImage(resource);
     resource->dirty_image = true;
     resource->is_overlay_candidate = true;
+    // GpuMemoryBuffer provides direct access to the memory used by the GPU.
+    // Read lock fences are required to ensure that we're not trying to map a
+    // buffer that is currently in-use by the GPU.
+    resource->read_lock_fences_enabled = true;
   } else if (use_texture_storage_ext_ &&
              IsFormatSupportedForStorage(format, use_texture_format_bgra_) &&
              (resource->hint & TEXTURE_HINT_IMMUTABLE)) {