gpu: Reset share group in LoseAllContexts

Follow up to
https://chromium-review.googlesource.com/c/chromium/src/+/1960465

With change above and with virtual context enabled, it is fairly likely
that when a context is lost, the new context is created before the old
context is destroyed. Since GpuChannelManager keeps using the same
GLShareGroup, the new context will end up in the same share group as the
old context.

Avoid this by creating a new GLShareGroup in LoseAllContexts.

Bug: 1059286,1052485
Change-Id: I1d60578fa6359329406d634fe04c7f242314839a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2090137
Commit-Queue: Bo <boliu@chromium.org>
Auto-Submit: Bo <boliu@chromium.org>
Reviewed-by: kylechar <kylechar@chromium.org>
Reviewed-by: Khushal <khushalsagar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#748708}

android_webview/browser/gfx/deferred_gpu_command_service.cc

@@ -31,7 +31,6 @@ DeferredGpuCommandService::DeferredGpuCommandService(
                                      gpu_service->gpu_feature_info(),
                                      gpu_service->sync_point_manager(),
                                      gpu_service->mailbox_manager(),
-                                     nullptr,
                                      gl::GLSurfaceFormat(),
                                      gpu_service->shared_image_manager(),
                                      nullptr),
@@ -74,4 +73,10 @@ DeferredGpuCommandService::GetSharedContextState() {
   return nullptr;
 }
 
+scoped_refptr<gl::GLShareGroup> DeferredGpuCommandService::GetShareGroup() {
+  if (!share_group_)
+    share_group_ = base::MakeRefCounted<gl::GLShareGroup>();
+  return share_group_;
+}
+
 }  // namespace android_webview

android_webview/browser/gfx/deferred_gpu_command_service.h

@@ -8,6 +8,10 @@
 #include "base/macros.h"
 #include "gpu/ipc/command_buffer_task_executor.h"
 
+namespace gl {
+class GLShareGroup;
+}
+
 namespace android_webview {
 
 class GpuServiceWebView;
@@ -26,6 +30,7 @@ class DeferredGpuCommandService : public gpu::CommandBufferTaskExecutor {
   void ScheduleDelayedWork(base::OnceClosure task) override;
   void PostNonNestableToClient(base::OnceClosure callback) override;
   scoped_refptr<gpu::SharedContextState> GetSharedContextState() override;
+  scoped_refptr<gl::GLShareGroup> GetShareGroup() override;
 
  protected:
   ~DeferredGpuCommandService() override;
@@ -40,6 +45,7 @@ class DeferredGpuCommandService : public gpu::CommandBufferTaskExecutor {
 
   TaskQueueWebView* task_queue_;
   GpuServiceWebView* gpu_service_;
+  scoped_refptr<gl::GLShareGroup> share_group_;
 
   DISALLOW_COPY_AND_ASSIGN(DeferredGpuCommandService);
 };

components/viz/service/main/viz_main_impl.cc

@@ -19,7 +19,6 @@
 #include "components/viz/service/gl/gpu_service_impl.h"
 #include "gpu/command_buffer/common/activity_flags.h"
 #include "gpu/config/gpu_finch_features.h"
-#include "gpu/ipc/gpu_in_process_thread_service.h"
 #include "gpu/ipc/service/gpu_init.h"
 #include "gpu/ipc/service/gpu_watchdog_thread.h"
 #include "media/gpu/buildflags.h"
@@ -235,15 +234,12 @@ void VizMainImpl::CreateFrameSinkManagerInternal(
   // the same signature. https://crbug.com/928845
   CHECK(!task_executor_);
   task_executor_ = std::make_unique<gpu::GpuInProcessThreadService>(
-      gpu_thread_task_runner_, gpu_service_->GetGpuScheduler(),
+      this, gpu_thread_task_runner_, gpu_service_->GetGpuScheduler(),
       gpu_service_->sync_point_manager(), gpu_service_->mailbox_manager(),
-      gpu_service_->share_group(), format, gpu_service_->gpu_feature_info(),
+      format, gpu_service_->gpu_feature_info(),
       gpu_service_->gpu_channel_manager()->gpu_preferences(),
       gpu_service_->shared_image_manager(),
-      gpu_service_->gpu_channel_manager()->program_cache(),
-      // Unretained is safe since |gpu_service_| outlives |task_executor_|.
-      base::BindRepeating(&GpuServiceImpl::GetContextState,
-                          base::Unretained(gpu_service_.get())));
+      gpu_service_->gpu_channel_manager()->program_cache());
 
   viz_compositor_thread_runner_->CreateFrameSinkManager(
       std::move(params), task_executor_.get(), gpu_service_.get());
@@ -255,6 +251,14 @@ void VizMainImpl::CreateVizDevTools(mojom::VizDevToolsParamsPtr params) {
 #endif
 }
 
+scoped_refptr<gpu::SharedContextState> VizMainImpl::GetSharedContextState() {
+  return gpu_service_->GetContextState();
+}
+
+scoped_refptr<gl::GLShareGroup> VizMainImpl::GetShareGroup() {
+  return gpu_service_->share_group();
+}
+
 void VizMainImpl::ExitProcess(bool immediately) {
   DCHECK(gpu_thread_task_runner_->BelongsToCurrentThread());
 

components/viz/service/main/viz_main_impl.h

@@ -14,6 +14,7 @@
 #include "build/build_config.h"
 #include "components/discardable_memory/client/client_discardable_shared_memory_manager.h"
 #include "components/viz/service/main/viz_compositor_thread_runner_impl.h"
+#include "gpu/ipc/gpu_in_process_thread_service.h"
 #include "gpu/ipc/in_process_command_buffer.h"
 #include "mojo/public/cpp/bindings/associated_receiver_set.h"
 #include "mojo/public/cpp/bindings/pending_associated_receiver.h"
@@ -41,7 +42,8 @@ class MojoUkmRecorder;
 namespace viz {
 class GpuServiceImpl;
 
-class VizMainImpl : public mojom::VizMain {
+class VizMainImpl : public mojom::VizMain,
+                    public gpu::GpuInProcessThreadServiceDelegate {
  public:
   struct LogMessage {
     int severity;
@@ -120,6 +122,10 @@ class VizMainImpl : public mojom::VizMain {
   void CreateFrameSinkManager(mojom::FrameSinkManagerParamsPtr params) override;
   void CreateVizDevTools(mojom::VizDevToolsParamsPtr params) override;
 
+  // gpu::GpuInProcessThreadServiceDelegate implementation:
+  scoped_refptr<gpu::SharedContextState> GetSharedContextState() override;
+  scoped_refptr<gl::GLShareGroup> GetShareGroup() override;
+
   GpuServiceImpl* gpu_service() { return gpu_service_.get(); }
   const GpuServiceImpl* gpu_service() const { return gpu_service_.get(); }
 

components/viz/test/DEPS

@@ -49,13 +49,13 @@ specific_include_rules = {
   ],
 
   "test_gpu_service_holder\.h": [
+    "+gpu/ipc/gpu_in_process_thread_service.h",
     "+gpu/vulkan/buildflags.h",
   ],
 
   "test_gpu_service_holder\.cc": [
     "+gpu/command_buffer/service/service_utils.h",
     "+gpu/config",
-    "+gpu/ipc/gpu_in_process_thread_service.h",
     "+gpu/ipc/service/gpu_watchdog_thread.h",
     "+gpu/vulkan/init/vulkan_factory.h",
     "+gpu/vulkan/vulkan_implementation.h",

components/viz/test/test_gpu_service_holder.cc

@@ -24,7 +24,6 @@
 #include "gpu/config/gpu_info_collector.h"
 #include "gpu/config/gpu_preferences.h"
 #include "gpu/config/gpu_util.h"
-#include "gpu/ipc/gpu_in_process_thread_service.h"
 #include "gpu/ipc/service/gpu_watchdog_thread.h"
 #include "mojo/public/cpp/bindings/strong_binding.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -162,6 +161,15 @@ TestGpuServiceHolder::~TestGpuServiceHolder() {
   io_thread_.Stop();
 }
 
+scoped_refptr<gpu::SharedContextState>
+TestGpuServiceHolder::GetSharedContextState() {
+  return gpu_service_->GetContextState();
+}
+
+scoped_refptr<gl::GLShareGroup> TestGpuServiceHolder::GetShareGroup() {
+  return gpu_service_->share_group();
+}
+
 void TestGpuServiceHolder::ScheduleGpuTask(base::OnceClosure callback) {
   DCHECK(gpu_task_sequence_);
   gpu_task_sequence_->ScheduleTask(std::move(callback), {});
@@ -236,19 +244,15 @@ void TestGpuServiceHolder::InitializeOnGpuThread(
       /*shutdown_event=*/nullptr);
 
   task_executor_ = std::make_unique<gpu::GpuInProcessThreadService>(
-      gpu_thread_.task_runner(), gpu_service_->GetGpuScheduler(),
+      this, gpu_thread_.task_runner(), gpu_service_->GetGpuScheduler(),
       gpu_service_->sync_point_manager(), gpu_service_->mailbox_manager(),
-      gpu_service_->share_group(),
       gpu_service_->gpu_channel_manager()
           ->default_offscreen_surface()
           ->GetFormat(),
       gpu_service_->gpu_feature_info(),
       gpu_service_->gpu_channel_manager()->gpu_preferences(),
       gpu_service_->shared_image_manager(),
-      gpu_service_->gpu_channel_manager()->program_cache(),
-      // Unretained is safe since |gpu_service_| outlives |task_executor_|.
-      base::BindRepeating(&GpuServiceImpl::GetContextState,
-                          base::Unretained(gpu_service_.get())));
+      gpu_service_->gpu_channel_manager()->program_cache());
 
   // TODO(weiliangc): Since SkiaOutputSurface should not depend on command
   // buffer, the |gpu_task_sequence_| should be coming from

components/viz/test/test_gpu_service_holder.h

@@ -10,6 +10,7 @@
 #include "base/macros.h"
 #include "base/memory/scoped_refptr.h"
 #include "base/threading/thread.h"
+#include "gpu/ipc/gpu_in_process_thread_service.h"
 #include "gpu/vulkan/buildflags.h"
 
 namespace gpu {
@@ -32,7 +33,7 @@ class GpuServiceImpl;
 // Starts GPU Main and IO threads, and creates a GpuServiceImpl that can be used
 // to create a SkiaOutputSurfaceImpl. This isn't a full GPU service
 // implementation and should only be used in tests.
-class TestGpuServiceHolder {
+class TestGpuServiceHolder : public gpu::GpuInProcessThreadServiceDelegate {
  public:
   class ScopedResetter {
    public:
@@ -62,7 +63,7 @@ class TestGpuServiceHolder {
   static void DoNotResetOnTestExit();
 
   explicit TestGpuServiceHolder(const gpu::GpuPreferences& preferences);
-  ~TestGpuServiceHolder();
+  ~TestGpuServiceHolder() override;
 
   scoped_refptr<base::SingleThreadTaskRunner> gpu_thread_task_runner() {
     return gpu_thread_.task_runner();
@@ -86,6 +87,10 @@ class TestGpuServiceHolder {
 #endif
   }
 
+  // gpu::GpuInProcessThreadServiceDelegate implementation:
+  scoped_refptr<gpu::SharedContextState> GetSharedContextState() override;
+  scoped_refptr<gl::GLShareGroup> GetShareGroup() override;
+
  private:
   friend struct base::DefaultSingletonTraits<TestGpuServiceHolder>;
 

gpu/ipc/command_buffer_task_executor.cc

@@ -18,7 +18,6 @@ CommandBufferTaskExecutor::CommandBufferTaskExecutor(
     const GpuFeatureInfo& gpu_feature_info,
     SyncPointManager* sync_point_manager,
     MailboxManager* mailbox_manager,
-    scoped_refptr<gl::GLShareGroup> share_group,
     gl::GLSurfaceFormat share_group_surface_format,
     SharedImageManager* shared_image_manager,
     gles2::ProgramCache* program_cache)
@@ -26,7 +25,6 @@ CommandBufferTaskExecutor::CommandBufferTaskExecutor(
       gpu_feature_info_(gpu_feature_info),
       sync_point_manager_(sync_point_manager),
       mailbox_manager_(mailbox_manager),
-      share_group_(share_group),
       share_group_surface_format_(share_group_surface_format),
       program_cache_(program_cache),
       discardable_manager_(gpu_preferences_),
@@ -39,12 +37,6 @@ CommandBufferTaskExecutor::CommandBufferTaskExecutor(
 
 CommandBufferTaskExecutor::~CommandBufferTaskExecutor() = default;
 
-scoped_refptr<gl::GLShareGroup> CommandBufferTaskExecutor::share_group() {
-  if (!share_group_)
-    share_group_ = base::MakeRefCounted<gl::GLShareGroup>();
-  return share_group_;
-}
-
 gles2::Outputter* CommandBufferTaskExecutor::outputter() {
   if (!outputter_) {
     outputter_ =

gpu/ipc/command_buffer_task_executor.h

@@ -47,7 +47,6 @@ class GL_IN_PROCESS_CONTEXT_EXPORT CommandBufferTaskExecutor {
                             const GpuFeatureInfo& gpu_feature_info,
                             SyncPointManager* sync_point_manager,
                             MailboxManager* mailbox_manager,
-                            scoped_refptr<gl::GLShareGroup> share_group,
                             gl::GLSurfaceFormat share_group_surface_format,
                             SharedImageManager* shared_image_manager,
                             gles2::ProgramCache* program_cache);
@@ -75,6 +74,8 @@ class GL_IN_PROCESS_CONTEXT_EXPORT CommandBufferTaskExecutor {
   // Returns the shared offscreen context state.
   virtual scoped_refptr<SharedContextState> GetSharedContextState() = 0;
 
+  virtual scoped_refptr<gl::GLShareGroup> GetShareGroup() = 0;
+
   const GpuPreferences& gpu_preferences() const { return gpu_preferences_; }
   const GpuFeatureInfo& gpu_feature_info() const { return gpu_feature_info_; }
   gl::GLSurfaceFormat share_group_surface_format() const {
@@ -100,7 +101,6 @@ class GL_IN_PROCESS_CONTEXT_EXPORT CommandBufferTaskExecutor {
   SharedImageManager* shared_image_manager() { return shared_image_manager_; }
 
   // These methods construct accessed fields if not already initialized.
-  scoped_refptr<gl::GLShareGroup> share_group();
   gles2::Outputter* outputter();
   gles2::ProgramCache* program_cache();
 
@@ -110,7 +110,6 @@ class GL_IN_PROCESS_CONTEXT_EXPORT CommandBufferTaskExecutor {
   SyncPointManager* sync_point_manager_;
   MailboxManager* mailbox_manager_;
   std::unique_ptr<gles2::Outputter> outputter_;
-  scoped_refptr<gl::GLShareGroup> share_group_;
   gl::GLSurfaceFormat share_group_surface_format_;
   std::unique_ptr<gles2::ProgramCache> owned_program_cache_;
   gles2::ProgramCache* program_cache_;

gpu/ipc/gpu_in_process_thread_service.cc

@@ -14,29 +14,32 @@
 
 namespace gpu {
 
+GpuInProcessThreadServiceDelegate::GpuInProcessThreadServiceDelegate() =
+    default;
+GpuInProcessThreadServiceDelegate::~GpuInProcessThreadServiceDelegate() =
+    default;
+
 GpuInProcessThreadService::GpuInProcessThreadService(
+    GpuInProcessThreadServiceDelegate* delegate,
     scoped_refptr<base::SingleThreadTaskRunner> task_runner,
     Scheduler* scheduler,
     SyncPointManager* sync_point_manager,
     MailboxManager* mailbox_manager,
-    scoped_refptr<gl::GLShareGroup> share_group,
     gl::GLSurfaceFormat share_group_surface_format,
     const GpuFeatureInfo& gpu_feature_info,
     const GpuPreferences& gpu_preferences,
     SharedImageManager* shared_image_manager,
-    gles2::ProgramCache* program_cache,
-    SharedContextStateGetter shared_context_state_getter)
+    gles2::ProgramCache* program_cache)
     : CommandBufferTaskExecutor(gpu_preferences,
                                 gpu_feature_info,
                                 sync_point_manager,
                                 mailbox_manager,
-                                share_group,
                                 share_group_surface_format,
                                 shared_image_manager,
                                 program_cache),
+      delegate_(delegate),
       task_runner_(task_runner),
-      scheduler_(scheduler),
-      shared_context_state_getter_(std::move(shared_context_state_getter)) {}
+      scheduler_(scheduler) {}
 
 GpuInProcessThreadService::~GpuInProcessThreadService() = default;
 
@@ -69,7 +72,11 @@ void GpuInProcessThreadService::PostNonNestableToClient(
 
 scoped_refptr<SharedContextState>
 GpuInProcessThreadService::GetSharedContextState() {
-  return shared_context_state_getter_.Run();
+  return delegate_->GetSharedContextState();
+}
+
+scoped_refptr<gl::GLShareGroup> GpuInProcessThreadService::GetShareGroup() {
+  return delegate_->GetShareGroup();
 }
 
 }  // namespace gpu

gpu/ipc/gpu_in_process_thread_service.h

@@ -25,27 +25,37 @@ namespace gles2 {
 class ProgramCache;
 }  // namespace gles2
 
+class GL_IN_PROCESS_CONTEXT_EXPORT GpuInProcessThreadServiceDelegate {
+ public:
+  GpuInProcessThreadServiceDelegate();
+
+  GpuInProcessThreadServiceDelegate(const GpuInProcessThreadServiceDelegate&) =
+      delete;
+  GpuInProcessThreadServiceDelegate& operator=(
+      const GpuInProcessThreadServiceDelegate&) = delete;
+
+  virtual ~GpuInProcessThreadServiceDelegate();
+
+  virtual scoped_refptr<SharedContextState> GetSharedContextState() = 0;
+  virtual scoped_refptr<gl::GLShareGroup> GetShareGroup() = 0;
+};
+
 // Default Service class when no service is specified. GpuInProcessThreadService
 // is used by Mus and unit tests.
 class GL_IN_PROCESS_CONTEXT_EXPORT GpuInProcessThreadService
     : public CommandBufferTaskExecutor {
  public:
-  // Must be valid to call through lifetime of GpuInProcessThreadService.
-  using SharedContextStateGetter =
-      base::RepeatingCallback<scoped_refptr<SharedContextState>()>;
-
   GpuInProcessThreadService(
+      GpuInProcessThreadServiceDelegate* delegate,
       scoped_refptr<base::SingleThreadTaskRunner> task_runner,
       Scheduler* scheduler,
       SyncPointManager* sync_point_manager,
       MailboxManager* mailbox_manager,
-      scoped_refptr<gl::GLShareGroup> share_group,
       gl::GLSurfaceFormat share_group_surface_format,
       const GpuFeatureInfo& gpu_feature_info,
       const GpuPreferences& gpu_preferences,
       SharedImageManager* shared_image_manager,
-      gles2::ProgramCache* program_cache,
-      SharedContextStateGetter shared_context_state_getter);
+      gles2::ProgramCache* program_cache);
   ~GpuInProcessThreadService() override;
 
   // CommandBufferTaskExecutor implementation.
@@ -56,11 +66,12 @@ class GL_IN_PROCESS_CONTEXT_EXPORT GpuInProcessThreadService
   void ScheduleDelayedWork(base::OnceClosure task) override;
   void PostNonNestableToClient(base::OnceClosure callback) override;
   scoped_refptr<SharedContextState> GetSharedContextState() override;
+  scoped_refptr<gl::GLShareGroup> GetShareGroup() override;
 
  private:
+  GpuInProcessThreadServiceDelegate* const delegate_;
   scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
   Scheduler* scheduler_;
-  SharedContextStateGetter shared_context_state_getter_;
 
   DISALLOW_COPY_AND_ASSIGN(GpuInProcessThreadService);
 };

gpu/ipc/in_process_command_buffer.cc

@@ -469,7 +469,7 @@ gpu::ContextResult InProcessCommandBuffer::InitializeOnGpuThread(
   } else {
     // When using the validating command decoder, always use the global share
     // group.
-    gl_share_group_ = task_executor_->share_group();
+    gl_share_group_ = task_executor_->GetShareGroup();
   }
 
   if (params.attribs.context_type == CONTEXT_TYPE_WEBGPU) {

gpu/ipc/in_process_gpu_thread_holder.cc

@@ -17,7 +17,6 @@
 #include "gpu/command_buffer/service/sync_point_manager.h"
 #include "gpu/config/gpu_info_collector.h"
 #include "gpu/config/gpu_util.h"
-#include "gpu/ipc/gpu_in_process_thread_service.h"
 #include "ui/gl/init/gl_factory.h"
 
 namespace gpu {
@@ -103,11 +102,9 @@ void InProcessGpuThreadHolder::InitializeOnGpuThread(
                                       gpu_driver_bug_workarounds, nullptr);
 
   task_executor_ = std::make_unique<GpuInProcessThreadService>(
-      task_runner(), scheduler_.get(), sync_point_manager_.get(),
-      mailbox_manager_.get(), nullptr, gl::GLSurfaceFormat(), gpu_feature_info_,
-      gpu_preferences_, shared_image_manager_.get(), nullptr,
-      base::BindRepeating(&InProcessGpuThreadHolder::GetSharedContextState,
-                          base::Unretained(this)));
+      this, task_runner(), scheduler_.get(), sync_point_manager_.get(),
+      mailbox_manager_.get(), gl::GLSurfaceFormat(), gpu_feature_info_,
+      gpu_preferences_, shared_image_manager_.get(), nullptr);
 
   completion->Signal();
 }
@@ -130,4 +127,10 @@ InProcessGpuThreadHolder::GetSharedContextState() {
   return context_state_;
 }
 
+scoped_refptr<gl::GLShareGroup> InProcessGpuThreadHolder::GetShareGroup() {
+  if (!share_group_)
+    share_group_ = base::MakeRefCounted<gl::GLShareGroup>();
+  return share_group_;
+}
+
 }  // namespace gpu

gpu/ipc/in_process_gpu_thread_holder.h

@@ -14,6 +14,7 @@
 #include "gpu/command_buffer/service/shared_context_state.h"
 #include "gpu/config/gpu_feature_info.h"
 #include "gpu/config/gpu_preferences.h"
+#include "gpu/ipc/gpu_in_process_thread_service.h"
 
 namespace gpu {
 class CommandBufferTaskExecutor;
@@ -27,7 +28,8 @@ class SyncPointManager;
 // default GpuPreferences and GpuFeatureInfo will be constructed from the
 // command line when this class is first created.
 class COMPONENT_EXPORT(GPU_THREAD_HOLDER) InProcessGpuThreadHolder
-    : public base::Thread {
+    : public base::Thread,
+      public GpuInProcessThreadServiceDelegate {
  public:
   InProcessGpuThreadHolder();
   ~InProcessGpuThreadHolder() override;
@@ -44,10 +46,13 @@ class COMPONENT_EXPORT(GPU_THREAD_HOLDER) InProcessGpuThreadHolder
   // executor will be created the first time this is called.
   CommandBufferTaskExecutor* GetTaskExecutor();
 
+  // gpu::GpuInProcessThreadServiceDelegate implementation:
+  scoped_refptr<gpu::SharedContextState> GetSharedContextState() override;
+  scoped_refptr<gl::GLShareGroup> GetShareGroup() override;
+
  private:
   void InitializeOnGpuThread(base::WaitableEvent* completion);
   void DeleteOnGpuThread();
-  scoped_refptr<SharedContextState> GetSharedContextState();
 
   GpuPreferences gpu_preferences_;
   GpuFeatureInfo gpu_feature_info_;

gpu/ipc/service/gpu_channel_manager.cc

@@ -388,6 +388,7 @@ void GpuChannelManager::PopulateShaderCache(int32_t client_id,
 void GpuChannelManager::LoseAllContexts() {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
 
+  share_group_ = base::MakeRefCounted<gl::GLShareGroup>();
   for (auto& kv : gpu_channels_) {
     kv.second->MarkAllContextsLost();
   }