Revert "Remove WaitSyncTokenCHROMIUM command"

This reverts commit c19106d.

Reason for revert: Causing multiple mac gpu fyi failures crbug.com/895984

Original change's description:
> Remove WaitSyncTokenCHROMIUM command
> 
> After migrating InProcessCommandBuffer to use GPU scheduler, most tasks
> are scheduled after their sync token dependencies are satisifed (see
> crrev.com/c/1157874).
> 
> The one exception was the WaitSyncToken IPC used by ReturnFrontBuffer
> for pepper, which specifies a sync token, and waits while handling the
> message.
> 
> Change ReturnFrontBuffer to contain the sync token, and use it to
> specify the dependency to the scheduler.
> 
> Remove WaitSyncTokenCHROMIUM command, since sync token dependencies are
> specified as task metadata in all cases.
> 
> Make other cleanups such as removing unnecessary sync token tests, and
> make sure the sync token code paths are not used where unsupported by
> using NOTREACHED.
> 
> Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
> Change-Id: Ieee4f6e2427a59a4e0c4b3c983cb489741241272
> Bug: 778753
> Reviewed-on: https://chromium-review.googlesource.com/c/1168155
> Reviewed-by: Bo <boliu@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: Antoine Labour <piman@chromium.org>
> Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#599849}

TBR=dcheng@chromium.org,boliu@chromium.org,sunnyps@chromium.org,piman@chromium.org

Change-Id: I87347c05dded27955410b08e40f37388a484d5f9
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 778753, 895984
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/c/1284394
Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org>
Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600102}

Author: sunnyps

android_webview/browser/deferred_gpu_command_service.cc

@@ -72,6 +72,10 @@ class TaskForwardingSequence : public gpu::CommandBufferTaskExecutor::Sequence {
 
   bool ShouldYield() override { return false; }
 
+  // Should not be called because BlockThreadOnWaitSyncToken() returns true,
+  // and the client should not disable sequences to wait for sync tokens.
+  void SetEnabled(bool enabled) override { NOTREACHED(); }
+
   void ScheduleTask(base::OnceClosure task,
                     std::vector<gpu::SyncToken> sync_token_fences) override {
     uint32_t order_num =
@@ -283,6 +287,10 @@ void DeferredGpuCommandService::RunTasks() {
   }
 }
 
+bool DeferredGpuCommandService::BlockThreadOnWaitSyncToken() const {
+  return true;
+}
+
 bool DeferredGpuCommandService::CanSupportThreadedTextureMailbox() const {
   return gpu_info_.can_support_threaded_texture_mailbox;
 }

android_webview/browser/deferred_gpu_command_service.h

@@ -49,6 +49,7 @@ class DeferredGpuCommandService : public gpu::CommandBufferTaskExecutor {
   // gpu::CommandBufferTaskExecutor implementation.
   bool ForceVirtualizedGLContexts() const override;
   bool ShouldCreateMemoryTracker() const override;
+  bool BlockThreadOnWaitSyncToken() const override;
   std::unique_ptr<gpu::CommandBufferTaskExecutor::Sequence> CreateSequence()
       override;
   void ScheduleOutOfOrderTask(base::OnceClosure task) override;

gpu/BUILD.gn

@@ -213,6 +213,7 @@ test("gl_tests") {
     "command_buffer/tests/gl_ext_multisample_compatibility_unittest.cc",
     "command_buffer/tests/gl_ext_srgb_unittest.cc",
     "command_buffer/tests/gl_ext_window_rectangles_unittest.cc",
+    "command_buffer/tests/gl_fence_sync_unittest.cc",
     "command_buffer/tests/gl_gpu_memory_buffer_unittest.cc",
     "command_buffer/tests/gl_iosurface_readback_workaround_unittest.cc",
     "command_buffer/tests/gl_lose_context_chromium_unittest.cc",

gpu/command_buffer/build_gles2_cmd_buffer.py

@@ -3707,7 +3707,12 @@
     'extension': "CHROMIUM_sync_point",
   },
   'WaitSyncTokenCHROMIUM': {
-    'type': 'NoCommand',
+    'type': 'Custom',
+    'impl_func': False,
+    'cmd_args': 'GLint namespace_id, '
+                'GLuint64 command_buffer_id, '
+                'GLuint64 release_count',
+    'client_test': False,
     'extension': "CHROMIUM_sync_point",
   },
   'DiscardBackbufferCHROMIUM': {

gpu/command_buffer/build_raster_cmd_buffer.py

@@ -365,7 +365,12 @@
     'type': 'NoCommand',
   },
   'WaitSyncTokenCHROMIUM': {
-    'type': 'NoCommand',
+    'type': 'Custom',
+    'impl_func': False,
+    'cmd_args': 'GLint namespace_id, '
+                'GLuint64 command_buffer_id, '
+                'GLuint64 release_count',
+    'client_test': False,
   },
   'InitializeDiscardableTextureCHROMIUM': {
     'type': 'Custom',

gpu/command_buffer/client/client_test_helper.h

@@ -137,7 +137,7 @@ class MockClientGpuControl : public GpuControl {
     DoSignalSyncToken(sync_token, &callback);
   }
 
-  MOCK_METHOD1(WaitSyncToken, void(const SyncToken&));
+  MOCK_METHOD1(WaitSyncTokenHint, void(const SyncToken&));
   MOCK_METHOD1(CanWaitUnverifiedSyncToken, bool(const SyncToken&));
   MOCK_METHOD2(CreateGpuFence,
                void(uint32_t gpu_fence_id, ClientGpuFence source));

gpu/command_buffer/client/gles2_cmd_helper_autogen.h

@@ -2734,6 +2734,16 @@ void InsertFenceSyncCHROMIUM(GLuint64 release_count) {
   }
 }
 
+void WaitSyncTokenCHROMIUM(GLint namespace_id,
+                           GLuint64 command_buffer_id,
+                           GLuint64 release_count) {
+  gles2::cmds::WaitSyncTokenCHROMIUM* c =
+      GetCmdSpace<gles2::cmds::WaitSyncTokenCHROMIUM>();
+  if (c) {
+    c->Init(namespace_id, command_buffer_id, release_count);
+  }
+}
+
 void UnpremultiplyAndDitherCopyCHROMIUM(GLuint source_id,
                                         GLuint dest_id,
                                         GLint x,

gpu/command_buffer/client/gles2_implementation.cc

@@ -6300,9 +6300,14 @@ void GLES2Implementation::WaitSyncTokenCHROMIUM(const GLbyte* sync_token_data) {
     return;
   }
 
+  helper_->WaitSyncTokenCHROMIUM(
+      static_cast<GLint>(sync_token.namespace_id()),
+      sync_token.command_buffer_id().GetUnsafeValue(),
+      sync_token.release_count());
+
   // Enqueue sync token in flush after inserting command so that it's not
   // included in an automatic flush.
-  gpu_control_->WaitSyncToken(verified_sync_token);
+  gpu_control_->WaitSyncTokenHint(verified_sync_token);
 }
 
 namespace {

gpu/command_buffer/client/gles2_implementation_unittest.cc

@@ -3931,9 +3931,12 @@ TEST_F(GLES2ImplementationTest, WaitSyncTokenCHROMIUM) {
 
   struct Cmds {
     cmds::InsertFenceSyncCHROMIUM insert_fence_sync;
+    cmds::WaitSyncTokenCHROMIUM wait_sync_token;
   };
   Cmds expected;
   expected.insert_fence_sync.Init(kFenceSync);
+  expected.wait_sync_token.Init(kNamespaceId, kCommandBufferId.GetUnsafeValue(),
+                                kFenceSync);
 
   EXPECT_CALL(*gpu_control_, GetNamespaceID()).WillOnce(Return(kNamespaceId));
   EXPECT_CALL(*gpu_control_, GetCommandBufferID())
@@ -3943,7 +3946,7 @@ TEST_F(GLES2ImplementationTest, WaitSyncTokenCHROMIUM) {
   EXPECT_CALL(*gpu_control_, EnsureWorkVisible());
   gl_->GenSyncTokenCHROMIUM(sync_token_data);
 
-  EXPECT_CALL(*gpu_control_, WaitSyncToken(sync_token));
+  EXPECT_CALL(*gpu_control_, WaitSyncTokenHint(sync_token));
   gl_->WaitSyncTokenCHROMIUM(sync_token_data);
   EXPECT_EQ(0, memcmp(&expected, commands_, sizeof(expected)));
 }

gpu/command_buffer/client/gpu_control.h

@@ -105,9 +105,11 @@ class GPU_EXPORT GpuControl {
                                base::OnceClosure callback) = 0;
 
   // This allows the command buffer proxy to mark the next flush with sync token
-  // dependencies for the gpu scheduler, or to block prior to the flush in case
-  // of android webview.
-  virtual void WaitSyncToken(const SyncToken& sync_token) = 0;
+  // dependencies for the gpu scheduler. This is used in addition to the
+  // WaitSyncToken command in the command buffer which is still needed. For
+  // example, the WaitSyncToken command is used to pull texture updates when
+  // used in conjunction with MailboxManagerSync.
+  virtual void WaitSyncTokenHint(const SyncToken& sync_token) = 0;
 
   // Under some circumstances a sync token may be used which has not been
   // verified to have been flushed. For example, fence syncs queued on the same

gpu/command_buffer/client/raster_cmd_helper_autogen.h

@@ -103,6 +103,16 @@ void InsertFenceSyncCHROMIUM(GLuint64 release_count) {
   }
 }
 
+void WaitSyncTokenCHROMIUM(GLint namespace_id,
+                           GLuint64 command_buffer_id,
+                           GLuint64 release_count) {
+  raster::cmds::WaitSyncTokenCHROMIUM* c =
+      GetCmdSpace<raster::cmds::WaitSyncTokenCHROMIUM>();
+  if (c) {
+    c->Init(namespace_id, command_buffer_id, release_count);
+  }
+}
+
 void UnpremultiplyAndDitherCopyCHROMIUM(GLuint source_id,
                                         GLuint dest_id,
                                         GLint x,

gpu/command_buffer/client/raster_implementation.cc

@@ -955,7 +955,14 @@ void RasterImplementation::WaitSyncTokenCHROMIUM(
     return;
   }
 
-  gpu_control_->WaitSyncToken(verified_sync_token);
+  helper_->WaitSyncTokenCHROMIUM(
+      static_cast<GLint>(sync_token.namespace_id()),
+      sync_token.command_buffer_id().GetUnsafeValue(),
+      sync_token.release_count());
+
+  // Enqueue sync token in flush after inserting command so that it's not
+  // included in an automatic flush.
+  gpu_control_->WaitSyncTokenHint(verified_sync_token);
 }
 
 namespace {

gpu/command_buffer/client/raster_implementation_unittest.cc

@@ -736,9 +736,12 @@ TEST_F(RasterImplementationTest, WaitSyncTokenCHROMIUM) {
 
   struct Cmds {
     cmds::InsertFenceSyncCHROMIUM insert_fence_sync;
+    cmds::WaitSyncTokenCHROMIUM wait_sync_token;
   };
   Cmds expected;
   expected.insert_fence_sync.Init(kFenceSync);
+  expected.wait_sync_token.Init(kNamespaceId, kCommandBufferId.GetUnsafeValue(),
+                                kFenceSync);
 
   EXPECT_CALL(*gpu_control_, GetNamespaceID()).WillOnce(Return(kNamespaceId));
   EXPECT_CALL(*gpu_control_, GetCommandBufferID())
@@ -748,7 +751,7 @@ TEST_F(RasterImplementationTest, WaitSyncTokenCHROMIUM) {
   EXPECT_CALL(*gpu_control_, EnsureWorkVisible());
   gl_->GenSyncTokenCHROMIUM(sync_token_data);
 
-  EXPECT_CALL(*gpu_control_, WaitSyncToken(sync_token));
+  EXPECT_CALL(*gpu_control_, WaitSyncTokenHint(sync_token));
   gl_->WaitSyncTokenCHROMIUM(sync_token_data);
   EXPECT_EQ(0, memcmp(&expected, commands_, sizeof(expected)));
 }

gpu/command_buffer/common/gles2_cmd_format_autogen.h

@@ -13519,6 +13519,73 @@ static_assert(offsetof(InsertFenceSyncCHROMIUM, release_count_0) == 4,
 static_assert(offsetof(InsertFenceSyncCHROMIUM, release_count_1) == 8,
               "offset of InsertFenceSyncCHROMIUM release_count_1 should be 8");
 
+struct WaitSyncTokenCHROMIUM {
+  typedef WaitSyncTokenCHROMIUM ValueType;
+  static const CommandId kCmdId = kWaitSyncTokenCHROMIUM;
+  static const cmd::ArgFlags kArgFlags = cmd::kFixed;
+  static const uint8_t cmd_flags = CMD_FLAG_SET_TRACE_LEVEL(3);
+
+  static uint32_t ComputeSize() {
+    return static_cast<uint32_t>(sizeof(ValueType));  // NOLINT
+  }
+
+  void SetHeader() { header.SetCmd<ValueType>(); }
+
+  void Init(GLint _namespace_id,
+            GLuint64 _command_buffer_id,
+            GLuint64 _release_count) {
+    SetHeader();
+    namespace_id = _namespace_id;
+    GLES2Util::MapUint64ToTwoUint32(static_cast<uint64_t>(_command_buffer_id),
+                                    &command_buffer_id_0, &command_buffer_id_1);
+    GLES2Util::MapUint64ToTwoUint32(static_cast<uint64_t>(_release_count),
+                                    &release_count_0, &release_count_1);
+  }
+
+  void* Set(void* cmd,
+            GLint _namespace_id,
+            GLuint64 _command_buffer_id,
+            GLuint64 _release_count) {
+    static_cast<ValueType*>(cmd)->Init(_namespace_id, _command_buffer_id,
+                                       _release_count);
+    return NextCmdAddress<ValueType>(cmd);
+  }
+
+  GLuint64 command_buffer_id() const volatile {
+    return static_cast<GLuint64>(GLES2Util::MapTwoUint32ToUint64(
+        command_buffer_id_0, command_buffer_id_1));
+  }
+
+  GLuint64 release_count() const volatile {
+    return static_cast<GLuint64>(
+        GLES2Util::MapTwoUint32ToUint64(release_count_0, release_count_1));
+  }
+
+  gpu::CommandHeader header;
+  int32_t namespace_id;
+  uint32_t command_buffer_id_0;
+  uint32_t command_buffer_id_1;
+  uint32_t release_count_0;
+  uint32_t release_count_1;
+};
+
+static_assert(sizeof(WaitSyncTokenCHROMIUM) == 24,
+              "size of WaitSyncTokenCHROMIUM should be 24");
+static_assert(offsetof(WaitSyncTokenCHROMIUM, header) == 0,
+              "offset of WaitSyncTokenCHROMIUM header should be 0");
+static_assert(offsetof(WaitSyncTokenCHROMIUM, namespace_id) == 4,
+              "offset of WaitSyncTokenCHROMIUM namespace_id should be 4");
+static_assert(
+    offsetof(WaitSyncTokenCHROMIUM, command_buffer_id_0) == 8,
+    "offset of WaitSyncTokenCHROMIUM command_buffer_id_0 should be 8");
+static_assert(
+    offsetof(WaitSyncTokenCHROMIUM, command_buffer_id_1) == 12,
+    "offset of WaitSyncTokenCHROMIUM command_buffer_id_1 should be 12");
+static_assert(offsetof(WaitSyncTokenCHROMIUM, release_count_0) == 16,
+              "offset of WaitSyncTokenCHROMIUM release_count_0 should be 16");
+static_assert(offsetof(WaitSyncTokenCHROMIUM, release_count_1) == 20,
+              "offset of WaitSyncTokenCHROMIUM release_count_1 should be 20");
+
 struct UnpremultiplyAndDitherCopyCHROMIUM {
   typedef UnpremultiplyAndDitherCopyCHROMIUM ValueType;
   static const CommandId kCmdId = kUnpremultiplyAndDitherCopyCHROMIUM;

gpu/command_buffer/common/gles2_cmd_format_test_autogen.h

@@ -4520,6 +4520,21 @@ TEST_F(GLES2FormatTest, InsertFenceSyncCHROMIUM) {
   CheckBytesWrittenMatchesExpectedSize(next_cmd, sizeof(cmd));
 }
 
+TEST_F(GLES2FormatTest, WaitSyncTokenCHROMIUM) {
+  cmds::WaitSyncTokenCHROMIUM& cmd =
+      *GetBufferAs<cmds::WaitSyncTokenCHROMIUM>();
+  void* next_cmd =
+      cmd.Set(&cmd, static_cast<GLint>(11), static_cast<GLuint64>(12),
+              static_cast<GLuint64>(13));
+  EXPECT_EQ(static_cast<uint32_t>(cmds::WaitSyncTokenCHROMIUM::kCmdId),
+            cmd.header.command);
+  EXPECT_EQ(sizeof(cmd), cmd.header.size * 4u);
+  EXPECT_EQ(static_cast<GLint>(11), cmd.namespace_id);
+  EXPECT_EQ(static_cast<GLuint64>(12), cmd.command_buffer_id());
+  EXPECT_EQ(static_cast<GLuint64>(13), cmd.release_count());
+  CheckBytesWrittenMatchesExpectedSize(next_cmd, sizeof(cmd));
+}
+
 TEST_F(GLES2FormatTest, UnpremultiplyAndDitherCopyCHROMIUM) {
   cmds::UnpremultiplyAndDitherCopyCHROMIUM& cmd =
       *GetBufferAs<cmds::UnpremultiplyAndDitherCopyCHROMIUM>();