canvas: Cleanup mailbox sharing across threads and synchronization.

This updates 2 code-paths pertaining to sharing mailboxes wrapped in
AcceleratedStaticBitmapImage:

1) Always use a sync token when wrapping a canvas mailbox to an image.
This was earlier removed to be lazily generated inside the image, if the
mailbox is used on the same context, with the expectation that avoiding
an ordering barrier in the process would be beneficial. But this doesn't
seem to be the case and the extra complexity is not worth it.

2) Only verify sync tokens when shared cross-process, instead of
cross-thread usage. All callers which acquire the image mailbox directly
already verify sync tokens before dispatching them cross-process so this
simply removes this requirement from AcceleratedStaticBitmapImage.

R=sunnyps@chromium.org

Change-Id: I8c8d5befdf8741ddcecb11309927eb76356dcd86
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2423269
Commit-Queue: Khushal <khushalsagar@chromium.org>
Auto-Submit: Khushal <khushalsagar@chromium.org>
Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org>
Cr-Commit-Position: refs/heads/master@{#809965}

third_party/blink/renderer/platform/graphics/accelerated_static_bitmap_image.cc

@@ -46,7 +46,7 @@ AcceleratedStaticBitmapImage::MailboxRef::MailboxRef(
       context_thread_ref_(context_thread_ref),
       context_task_runner_(std::move(context_task_runner)),
       release_callback_(std::move(release_callback)) {
-  DCHECK(!is_cross_thread() || sync_token_.verified_flush());
+  DCHECK(sync_token.HasData());
 }
 
 AcceleratedStaticBitmapImage::MailboxRef::~MailboxRef() {
@@ -59,20 +59,6 @@ AcceleratedStaticBitmapImage::MailboxRef::~MailboxRef() {
   }
 }
 
-const gpu::SyncToken&
-AcceleratedStaticBitmapImage::MailboxRef::GetOrCreateSyncToken(
-    base::WeakPtr<WebGraphicsContext3DProviderWrapper>
-        context_provider_wrapper) {
-  if (!sync_token_.HasData()) {
-    DCHECK(!is_cross_thread());
-    DCHECK(context_provider_wrapper);
-    context_provider_wrapper->ContextProvider()
-        ->InterfaceBase()
-        ->GenUnverifiedSyncTokenCHROMIUM(sync_token_.GetData());
-  }
-  return sync_token_;
-}
-
 // static
 void AcceleratedStaticBitmapImage::ReleaseTexture(void* ctx) {
   auto* release_ctx = static_cast<ReleaseContext*>(ctx);
@@ -174,9 +160,7 @@ bool AcceleratedStaticBitmapImage::CopyToTexture(
   DCHECK(mailbox_.IsSharedImage());
 
   // Get a texture id that |destProvider| knows about and copy from it.
-  dest_gl->WaitSyncTokenCHROMIUM(
-      mailbox_ref_->GetOrCreateSyncToken(ContextProviderWrapper())
-          .GetConstData());
+  dest_gl->WaitSyncTokenCHROMIUM(mailbox_ref_->sync_token().GetConstData());
   GLuint source_texture_id =
       dest_gl->CreateAndTexStorage2DSharedImageCHROMIUM(mailbox_.name);
   dest_gl->BeginSharedImageAccessDirectCHROMIUM(
@@ -279,14 +263,12 @@ void AcceleratedStaticBitmapImage::InitializeTextureBacking(
 
   gpu::raster::RasterInterface* shared_ri =
       context_provider_wrapper->ContextProvider()->RasterInterface();
+  shared_ri->WaitSyncTokenCHROMIUM(mailbox_ref_->sync_token().GetConstData());
 
   if (context_provider_wrapper->ContextProvider()
           ->GetCapabilities()
           .supports_oop_raster) {
     DCHECK_EQ(shared_image_texture_id, 0u);
-    shared_ri->WaitSyncTokenCHROMIUM(
-        mailbox_ref_->GetOrCreateSyncToken(context_provider_wrapper)
-            .GetConstData());
     skia_context_provider_wrapper_ = context_provider_wrapper;
     texture_backing_ = sk_make_sp<MailboxTextureBacking>(
         mailbox_, sk_image_info_, std::move(context_provider_wrapper));
@@ -305,9 +287,6 @@ void AcceleratedStaticBitmapImage::InitializeTextureBacking(
     shared_context_texture_id = shared_image_texture_id;
     should_delete_texture_on_release = false;
   } else {
-    shared_ri->WaitSyncTokenCHROMIUM(
-        mailbox_ref_->GetOrCreateSyncToken(context_provider_wrapper)
-            .GetConstData());
     shared_context_texture_id =
         shared_ri->CreateAndConsumeForGpuRaster(mailbox_);
     shared_ri->BeginSharedImageAccessDirectCHROMIUM(
@@ -351,18 +330,16 @@ void AcceleratedStaticBitmapImage::EnsureSyncTokenVerified() {
   if (mailbox_ref_->verified_flush())
     return;
 
-  if (mailbox_ref_->is_cross_thread()) {
-    // Was originally created on another thread. Should already have a sync
-    // token from the original source context, already verified if needed.
-    NOTREACHED() << "Cross-thread SyncToken should already be verified.";
-    return;
-  }
-
-  if (!ContextProviderWrapper())
+  // If the original context was created on a different thread, we need to
+  // fallback to using the shared GPU context.
+  auto context_provider_wrapper =
+      mailbox_ref_->is_cross_thread()
+          ? SharedGpuContext::ContextProviderWrapper()
+          : ContextProviderWrapper();
+  if (!context_provider_wrapper)
     return;
 
-  auto sync_token =
-      mailbox_ref_->GetOrCreateSyncToken(ContextProviderWrapper());
+  auto sync_token = mailbox_ref_->sync_token();
   int8_t* token_data = sync_token.GetData();
   ContextProvider()->InterfaceBase()->VerifySyncTokensCHROMIUM(&token_data, 1);
   sync_token.SetVerifyFlush();
@@ -373,14 +350,12 @@ gpu::MailboxHolder AcceleratedStaticBitmapImage::GetMailboxHolder() const {
   if (!IsValid())
     return gpu::MailboxHolder();
 
-  return gpu::MailboxHolder(
-      mailbox_, mailbox_ref_->GetOrCreateSyncToken(ContextProviderWrapper()),
-      texture_target_);
+  return gpu::MailboxHolder(mailbox_, mailbox_ref_->sync_token(),
+                            texture_target_);
 }
 
 void AcceleratedStaticBitmapImage::Transfer() {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
-  EnsureSyncTokenVerified();
 
   // SkImage is bound to the current thread so is no longer valid to use
   // cross-thread.

third_party/blink/renderer/platform/graphics/accelerated_static_bitmap_image.h

@@ -131,9 +131,11 @@ class PLATFORM_EXPORT AcceleratedStaticBitmapImage final
     bool is_cross_thread() const {
       return base::PlatformThread::CurrentRef() != context_thread_ref_;
     }
-    void set_sync_token(gpu::SyncToken token) { sync_token_ = token; }
-    const gpu::SyncToken& GetOrCreateSyncToken(
-        base::WeakPtr<WebGraphicsContext3DProviderWrapper>);
+    void set_sync_token(gpu::SyncToken token) {
+      DCHECK(sync_token_.HasData());
+      sync_token_ = token;
+    }
+    const gpu::SyncToken& sync_token() const { return sync_token_; }
     bool verified_flush() { return sync_token_.verified_flush(); }
 
    private:

third_party/blink/renderer/platform/graphics/accelerated_static_bitmap_image_test.cc

@@ -42,8 +42,8 @@ GLbyte SyncTokenMatcher(const gpu::SyncToken& token) {
 
 gpu::SyncToken GenTestSyncToken(GLbyte id) {
   gpu::SyncToken token;
-  // Store id in the first byte
-  reinterpret_cast<GLbyte*>(&token)[0] = id;
+  token.Set(gpu::CommandBufferNamespace::GPU_IO,
+            gpu::CommandBufferId::FromUnsafeValue(64), id);
   return token;
 }
 
@@ -51,6 +51,7 @@ scoped_refptr<StaticBitmapImage> CreateBitmap() {
   auto mailbox = gpu::Mailbox::GenerateForSharedImage();
   auto release_callback = viz::SingleReleaseCallback::Create(
       base::BindOnce([](const gpu::SyncToken&, bool) {}));
+
   return AcceleratedStaticBitmapImage::CreateFromCanvasMailbox(
       mailbox, GenTestSyncToken(100), 0, SkImageInfo::MakeN32Premul(100, 100),
       GL_TEXTURE_2D, true, SharedGpuContext::ContextProviderWrapper(),

third_party/blink/renderer/platform/graphics/canvas_resource.cc

@@ -626,11 +626,9 @@ scoped_refptr<StaticBitmapImage> CanvasResourceRasterSharedImage::Bitmap() {
 
   scoped_refptr<StaticBitmapImage> image;
 
-  // If its cross thread, then the sync token was already verified. If not, then
-  // we don't need one. The image lazily generates a token if needed.
-  gpu::SyncToken token = is_cross_thread() ? sync_token() : gpu::SyncToken();
+  // If its cross thread, then the sync token was already verified.
   image = AcceleratedStaticBitmapImage::CreateFromCanvasMailbox(
-      mailbox(), token, texture_id_for_image, image_info, texture_target_,
+      mailbox(), GetSyncToken(), texture_id_for_image, image_info, texture_target_,
       is_origin_top_left_, context_provider_wrapper_, owning_thread_ref_,
       owning_thread_task_runner_, std::move(release_callback));
 
@@ -1172,11 +1170,8 @@ scoped_refptr<StaticBitmapImage> CanvasResourceSwapChain::Bitmap() {
       },
       base::RetainedRef(this)));
 
-  // Use an empty sync token so that the image lazily generates its own sync
-  // token so that it can synchronize with Skia commands as well as the copy
-  // from the front buffer to back buffer after present.
   return AcceleratedStaticBitmapImage::CreateFromCanvasMailbox(
-      back_buffer_mailbox_, gpu::SyncToken(), shared_texture_id, image_info,
+      back_buffer_mailbox_, GetSyncToken(), shared_texture_id, image_info,
       GL_TEXTURE_2D, true /*is_origin_top_left*/, context_provider_wrapper_,
       owning_thread_ref_, owning_thread_task_runner_,
       std::move(release_callback));

third_party/blink/renderer/platform/graphics/gpu/webgpu_image_bitmap_handler_test.cc

@@ -27,8 +27,8 @@ namespace blink {
 namespace {
 gpu::SyncToken GenTestSyncToken(GLbyte id) {
   gpu::SyncToken token;
-  // Store id in the first byte
-  reinterpret_cast<GLbyte*>(&token)[0] = id;
+  token.Set(gpu::CommandBufferNamespace::GPU_IO,
+            gpu::CommandBufferId::FromUnsafeValue(64), id);
   return token;
 }