Make MailboxManager, ShaderTranslatorCache, FramebufferCompletenessCa…

…che single-ownership

Those classes used by ContextGroup can always be owned by a higher level
component (e.g. GpuChannelManager or InProcessCommandBuffer::Service) that
manages how they are shared anyway.

This reduces refcount trashing, removes indirections and simplifies the code overall.

Bug: None
Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I6df052c9713a373acdb0b040d46874f92141deff
Reviewed-on: https://chromium-review.googlesource.com/531859
Commit-Queue: Antoine Labour <piman@chromium.org>
Reviewed-by: Bo Liu <boliu@chromium.org>
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#479250}

android_webview/browser/deferred_gpu_command_service.cc

@@ -13,10 +13,8 @@
 #include "base/trace_event/trace_event.h"
 #include "content/public/browser/gpu_utils.h"
 #include "content/public/common/content_switches.h"
-#include "gpu/command_buffer/service/framebuffer_completeness_cache.h"
 #include "gpu/command_buffer/service/gpu_preferences.h"
 #include "gpu/command_buffer/service/gpu_switches.h"
-#include "gpu/command_buffer/service/shader_translator_cache.h"
 #include "gpu/command_buffer/service/sync_point_manager.h"
 #include "gpu/config/gpu_switches.h"
 #include "ui/gl/gl_switches.h"
@@ -156,24 +154,6 @@ void DeferredGpuCommandService::PerformAllIdleWork() {
 
 bool DeferredGpuCommandService::UseVirtualizedGLContexts() { return true; }
 
-scoped_refptr<gpu::gles2::ShaderTranslatorCache>
-DeferredGpuCommandService::shader_translator_cache() {
-  if (!shader_translator_cache_.get()) {
-    shader_translator_cache_ =
-        new gpu::gles2::ShaderTranslatorCache(gpu_preferences());
-  }
-  return shader_translator_cache_;
-}
-
-scoped_refptr<gpu::gles2::FramebufferCompletenessCache>
-DeferredGpuCommandService::framebuffer_completeness_cache() {
-  if (!framebuffer_completeness_cache_.get()) {
-    framebuffer_completeness_cache_ =
-        new gpu::gles2::FramebufferCompletenessCache;
-  }
-  return framebuffer_completeness_cache_;
-}
-
 gpu::SyncPointManager* DeferredGpuCommandService::sync_point_manager() {
   return sync_point_manager_.get();
 }

android_webview/browser/deferred_gpu_command_service.h

@@ -48,10 +48,6 @@ class DeferredGpuCommandService
   void ScheduleTask(const base::Closure& task) override;
   void ScheduleDelayedWork(const base::Closure& task) override;
   bool UseVirtualizedGLContexts() override;
-  scoped_refptr<gpu::gles2::ShaderTranslatorCache> shader_translator_cache()
-      override;
-  scoped_refptr<gpu::gles2::FramebufferCompletenessCache>
-  framebuffer_completeness_cache() override;
   gpu::SyncPointManager* sync_point_manager() override;
 
   void RunTasks();
@@ -82,9 +78,6 @@ class DeferredGpuCommandService
   std::queue<std::pair<base::Time, base::Closure> > idle_tasks_;
 
   std::unique_ptr<gpu::SyncPointManager> sync_point_manager_;
-  scoped_refptr<gpu::gles2::ShaderTranslatorCache> shader_translator_cache_;
-  scoped_refptr<gpu::gles2::FramebufferCompletenessCache>
-      framebuffer_completeness_cache_;
   DISALLOW_COPY_AND_ASSIGN(DeferredGpuCommandService);
 };
 

gpu/command_buffer/service/context_group.cc

@@ -16,7 +16,6 @@
 #include "gpu/command_buffer/service/framebuffer_manager.h"
 #include "gpu/command_buffer/service/gles2_cmd_decoder_passthrough.h"
 #include "gpu/command_buffer/service/gpu_preferences.h"
-#include "gpu/command_buffer/service/mailbox_manager_impl.h"
 #include "gpu/command_buffer/service/path_manager.h"
 #include "gpu/command_buffer/service/program_manager.h"
 #include "gpu/command_buffer/service/progress_reporter.h"
@@ -62,11 +61,10 @@ DisallowedFeatures AdjustDisallowedFeatures(
 
 ContextGroup::ContextGroup(
     const GpuPreferences& gpu_preferences,
-    const scoped_refptr<MailboxManager>& mailbox_manager,
+    MailboxManager* mailbox_manager,
     const scoped_refptr<MemoryTracker>& memory_tracker,
-    const scoped_refptr<ShaderTranslatorCache>& shader_translator_cache,
-    const scoped_refptr<FramebufferCompletenessCache>&
-        framebuffer_completeness_cache,
+    ShaderTranslatorCache* shader_translator_cache,
+    FramebufferCompletenessCache* framebuffer_completeness_cache,
     const scoped_refptr<FeatureInfo>& feature_info,
     bool bind_generates_resource,
     ImageManager* image_manager,
@@ -118,8 +116,7 @@ ContextGroup::ContextGroup(
       discardable_manager_(discardable_manager) {
   DCHECK(discardable_manager);
   DCHECK(feature_info_);
-  if (!mailbox_manager_.get())
-    mailbox_manager_ = new MailboxManagerImpl;
+  DCHECK(mailbox_manager_);
   transfer_buffer_manager_ =
       base::MakeUnique<TransferBufferManager>(memory_tracker_.get());
 }

gpu/command_buffer/service/context_group.h

@@ -57,20 +57,18 @@ DisallowedFeatures AdjustDisallowedFeatures(
 // resources.
 class GPU_EXPORT ContextGroup : public base::RefCounted<ContextGroup> {
  public:
-  ContextGroup(
-      const GpuPreferences& gpu_preferences,
-      const scoped_refptr<MailboxManager>& mailbox_manager,
-      const scoped_refptr<MemoryTracker>& memory_tracker,
-      const scoped_refptr<ShaderTranslatorCache>& shader_translator_cache,
-      const scoped_refptr<FramebufferCompletenessCache>&
-          framebuffer_completeness_cache,
-      const scoped_refptr<FeatureInfo>& feature_info,
-      bool bind_generates_resource,
-      ImageManager* image_manager,
-      gpu::ImageFactory* image_factory,
-      ProgressReporter* progress_reporter,
-      const GpuFeatureInfo& gpu_feature_info,
-      ServiceDiscardableManager* discardable_manager);
+  ContextGroup(const GpuPreferences& gpu_preferences,
+               MailboxManager* mailbox_manager,
+               const scoped_refptr<MemoryTracker>& memory_tracker,
+               ShaderTranslatorCache* shader_translator_cache,
+               FramebufferCompletenessCache* framebuffer_completeness_cache,
+               const scoped_refptr<FeatureInfo>& feature_info,
+               bool bind_generates_resource,
+               ImageManager* image_manager,
+               gpu::ImageFactory* image_factory,
+               ProgressReporter* progress_reporter,
+               const GpuFeatureInfo& gpu_feature_info,
+               ServiceDiscardableManager* discardable_manager);
 
   // This should only be called by GLES2Decoder. This must be paired with a
   // call to destroy if it succeeds.
@@ -83,20 +81,16 @@ class GPU_EXPORT ContextGroup : public base::RefCounted<ContextGroup> {
   // It should only be called by GLES2Decoder.
   void Destroy(GLES2Decoder* decoder, bool have_context);
 
-  MailboxManager* mailbox_manager() const {
-    return mailbox_manager_.get();
-  }
+  MailboxManager* mailbox_manager() const { return mailbox_manager_; }
 
-  MemoryTracker* memory_tracker() const {
-    return memory_tracker_.get();
-  }
+  MemoryTracker* memory_tracker() const { return memory_tracker_.get(); }
 
   ShaderTranslatorCache* shader_translator_cache() const {
-    return shader_translator_cache_.get();
+    return shader_translator_cache_;
   }
 
   FramebufferCompletenessCache* framebuffer_completeness_cache() const {
-    return framebuffer_completeness_cache_.get();
+    return framebuffer_completeness_cache_;
   }
 
   bool bind_generates_resource() {
@@ -252,10 +246,10 @@ class GPU_EXPORT ContextGroup : public base::RefCounted<ContextGroup> {
   void ReportProgress();
 
   const GpuPreferences& gpu_preferences_;
-  scoped_refptr<MailboxManager> mailbox_manager_;
+  MailboxManager* mailbox_manager_;
   scoped_refptr<MemoryTracker> memory_tracker_;
-  scoped_refptr<ShaderTranslatorCache> shader_translator_cache_;
-  scoped_refptr<FramebufferCompletenessCache> framebuffer_completeness_cache_;
+  ShaderTranslatorCache* shader_translator_cache_;
+  FramebufferCompletenessCache* framebuffer_completeness_cache_;
   std::unique_ptr<TransferBufferManager> transfer_buffer_manager_;
 
   bool enforce_gl_minimums_;

gpu/command_buffer/service/context_group_unittest.cc

@@ -12,7 +12,7 @@
 #include "gpu/command_buffer/service/gles2_cmd_decoder_mock.h"
 #include "gpu/command_buffer/service/gpu_service_test.h"
 #include "gpu/command_buffer/service/image_manager.h"
-#include "gpu/command_buffer/service/mailbox_manager.h"
+#include "gpu/command_buffer/service/mailbox_manager_impl.h"
 #include "gpu/command_buffer/service/service_discardable_manager.h"
 #include "gpu/command_buffer/service/test_helper.h"
 #include "gpu/command_buffer/service/texture_manager.h"
@@ -46,8 +46,8 @@ class ContextGroupTest : public GpuServiceTest {
     decoder_.reset(new MockGLES2Decoder(&command_buffer_service_));
     scoped_refptr<FeatureInfo> feature_info = new FeatureInfo;
     group_ = scoped_refptr<ContextGroup>(new ContextGroup(
-        gpu_preferences_, nullptr /* mailbox_manager */,
-        nullptr /* memory_tracker */, nullptr /* shader_translator_cache */,
+        gpu_preferences_, &mailbox_manager_, nullptr /* memory_tracker */,
+        nullptr /* shader_translator_cache */,
         nullptr /* framebuffer_completeness_cache */, feature_info,
         kBindGeneratesResource, &image_manager_, nullptr /* image_factory */,
         nullptr /* progress_reporter */, GpuFeatureInfo(),
@@ -58,6 +58,7 @@ class ContextGroupTest : public GpuServiceTest {
   ImageManager image_manager_;
   ServiceDiscardableManager discardable_manager_;
   FakeCommandBufferServiceBase command_buffer_service_;
+  MailboxManagerImpl mailbox_manager_;
   std::unique_ptr<MockGLES2Decoder> decoder_;
   scoped_refptr<ContextGroup> group_;
 };

gpu/command_buffer/service/framebuffer_completeness_cache.h

@@ -9,7 +9,6 @@
 
 #include "base/containers/hash_tables.h"
 #include "base/macros.h"
-#include "base/memory/ref_counted.h"
 #include "gpu/gpu_export.h"
 
 namespace gpu {
@@ -18,20 +17,15 @@ namespace gles2 {
 // Refcounted wrapper for a hash_set of framebuffer format signatures
 // representing framebuffer configurations that are reported by the GL
 // driver as complete according to glCheckFramebufferStatusEXT.
-class GPU_EXPORT FramebufferCompletenessCache
-    : public base::RefCounted<FramebufferCompletenessCache> {
+class GPU_EXPORT FramebufferCompletenessCache {
  public:
   FramebufferCompletenessCache();
+  ~FramebufferCompletenessCache();
 
   bool IsComplete(const std::string& signature) const;
   void SetComplete(const std::string& signature);
 
- protected:
-  virtual ~FramebufferCompletenessCache();
-
  private:
-  friend class base::RefCounted<FramebufferCompletenessCache>;
-
   typedef base::hash_set<std::string> Map;
 
   Map cache_;

gpu/command_buffer/service/framebuffer_manager.cc

@@ -318,8 +318,7 @@ class TextureAttachment
 FramebufferManager::FramebufferManager(
     uint32_t max_draw_buffers,
     uint32_t max_color_attachments,
-    const scoped_refptr<FramebufferCompletenessCache>&
-        framebuffer_combo_complete_cache)
+    FramebufferCompletenessCache* framebuffer_combo_complete_cache)
     : framebuffer_state_change_count_(1),
       framebuffer_count_(0),
       have_context_(true),

gpu/command_buffer/service/framebuffer_manager.h

@@ -317,10 +317,10 @@ struct DecoderFramebufferState {
 // so we can correctly clear them.
 class GPU_EXPORT FramebufferManager {
  public:
-  FramebufferManager(uint32_t max_draw_buffers,
-                     uint32_t max_color_attachments,
-                     const scoped_refptr<FramebufferCompletenessCache>&
-                         framebuffer_combo_complete_cache);
+  FramebufferManager(
+      uint32_t max_draw_buffers,
+      uint32_t max_color_attachments,
+      FramebufferCompletenessCache* framebuffer_combo_complete_cache);
   ~FramebufferManager();
 
   // Must call before destruction.
@@ -360,7 +360,7 @@ class GPU_EXPORT FramebufferManager {
   void StopTracking(Framebuffer* framebuffer);
 
   FramebufferCompletenessCache* GetFramebufferComboCompleteCache() {
-    return framebuffer_combo_complete_cache_.get();
+    return framebuffer_combo_complete_cache_;
   }
 
   // Info for each framebuffer in the system.
@@ -381,7 +381,7 @@ class GPU_EXPORT FramebufferManager {
   uint32_t max_draw_buffers_;
   uint32_t max_color_attachments_;
 
-  scoped_refptr<FramebufferCompletenessCache> framebuffer_combo_complete_cache_;
+  FramebufferCompletenessCache* framebuffer_combo_complete_cache_;
 
   DISALLOW_COPY_AND_ASSIGN(FramebufferManager);
 };

gpu/command_buffer/service/framebuffer_manager_unittest.cc

@@ -119,7 +119,7 @@ class FramebufferInfoTestBase : public GpuServiceTest {
       : context_type_(context_type),
         manager_(kMaxDrawBuffers,
                  kMaxColorAttachments,
-                 new FramebufferCompletenessCache),
+                 &framebuffer_completeness_cache_),
         feature_info_(new FeatureInfo()) {
     texture_manager_.reset(new TextureManager(
         nullptr, feature_info_.get(), kMaxTextureSize, kMaxCubemapSize,
@@ -158,6 +158,7 @@ class FramebufferInfoTestBase : public GpuServiceTest {
   }
 
   ContextType context_type_;
+  FramebufferCompletenessCache framebuffer_completeness_cache_;
   FramebufferManager manager_;
   Framebuffer* framebuffer_;
   scoped_refptr<FeatureInfo> feature_info_;

gpu/command_buffer/service/gles2_cmd_decoder_passthrough.h

@@ -357,7 +357,7 @@ class GLES2DecoderPassthroughImpl : public GLES2Decoder {
   ClientServiceMap<GLuint, GLuint> vertex_array_id_map_;
 
   // Mailboxes
-  scoped_refptr<MailboxManager> mailbox_manager_;
+  MailboxManager* mailbox_manager_;
 
   // State tracking of currently bound 2D textures (client IDs)
   size_t active_texture_unit_;

gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.cc

@@ -126,7 +126,8 @@ GLES2DecoderTestBase::GLES2DecoderTestBase()
       cached_depth_mask_(true),
       cached_stencil_front_mask_(static_cast<GLuint>(-1)),
       cached_stencil_back_mask_(static_cast<GLuint>(-1)),
-      shader_language_version_(100) {
+      shader_language_version_(100),
+      shader_translator_cache_(gpu_preferences_) {
   memset(immediate_buffer_, 0xEE, sizeof(immediate_buffer_));
 }
 
@@ -208,9 +209,8 @@ void GLES2DecoderTestBase::InitDecoderWithCommandLine(
   }
 
   group_ = scoped_refptr<ContextGroup>(new ContextGroup(
-      gpu_preferences_, nullptr /* mailbox_manager */, memory_tracker_,
-      new ShaderTranslatorCache(gpu_preferences_),
-      new FramebufferCompletenessCache, feature_info,
+      gpu_preferences_, &mailbox_manager_, memory_tracker_,
+      &shader_translator_cache_, &framebuffer_completeness_cache_, feature_info,
       normalized_init.bind_generates_resource, &image_manager_,
       nullptr /* image_factory */, nullptr /* progress_reporter */,
       GpuFeatureInfo(), &discardable_manager_));

gpu/command_buffer/service/gles2_cmd_decoder_unittest_base.h

@@ -22,6 +22,7 @@
 #include "gpu/command_buffer/service/gles2_cmd_decoder_mock.h"
 #include "gpu/command_buffer/service/gpu_preferences.h"
 #include "gpu/command_buffer/service/image_manager.h"
+#include "gpu/command_buffer/service/mailbox_manager_impl.h"
 #include "gpu/command_buffer/service/program_manager.h"
 #include "gpu/command_buffer/service/query_manager.h"
 #include "gpu/command_buffer/service/renderbuffer_manager.h"
@@ -767,7 +768,10 @@ class GLES2DecoderTestBase : public ::testing::TestWithParam<bool>,
   void SetupInitStateManualExpectationsForDoLineWidth(GLfloat width);
 
   GpuPreferences gpu_preferences_;
-  gles2::ImageManager image_manager_;
+  MailboxManagerImpl mailbox_manager_;
+  ShaderTranslatorCache shader_translator_cache_;
+  FramebufferCompletenessCache framebuffer_completeness_cache_;
+  ImageManager image_manager_;
   ServiceDiscardableManager discardable_manager_;
   scoped_refptr<ContextGroup> group_;
   MockGLStates gl_states_;

gpu/command_buffer/service/mailbox_manager.cc

@@ -5,6 +5,7 @@
 #include "gpu/command_buffer/service/mailbox_manager.h"
 
 #include "base/command_line.h"
+#include "base/memory/ptr_util.h"
 #include "gpu/command_buffer/service/gpu_preferences.h"
 #include "gpu/command_buffer/service/mailbox_manager_impl.h"
 #include "gpu/command_buffer/service/mailbox_manager_sync.h"
@@ -13,11 +14,11 @@ namespace gpu {
 namespace gles2 {
 
 // static
-scoped_refptr<MailboxManager> MailboxManager::Create(
+std::unique_ptr<MailboxManager> MailboxManager::Create(
     const GpuPreferences& gpu_preferences) {
   if (gpu_preferences.enable_threaded_texture_mailboxes)
-    return scoped_refptr<MailboxManager>(new MailboxManagerSync);
-  return scoped_refptr<MailboxManager>(new MailboxManagerImpl);
+    return base::MakeUnique<MailboxManagerSync>();
+  return base::MakeUnique<MailboxManagerImpl>();
 }
 
 }  // namespage gles2

gpu/command_buffer/service/mailbox_manager.h

@@ -5,8 +5,9 @@
 #ifndef GPU_COMMAND_BUFFER_SERVICE_MAILBOX_MANAGER_H_
 #define GPU_COMMAND_BUFFER_SERVICE_MAILBOX_MANAGER_H_
 
+#include <memory>
+
 #include "base/macros.h"
-#include "base/memory/ref_counted.h"
 #include "gpu/command_buffer/common/mailbox.h"
 #include "gpu/gpu_export.h"
 
@@ -20,10 +21,9 @@ namespace gles2 {
 class TextureBase;
 
 // Manages resources scoped beyond the context or context group level.
-class GPU_EXPORT MailboxManager : public base::RefCounted<MailboxManager> {
+class GPU_EXPORT MailboxManager {
  public:
-  static scoped_refptr<MailboxManager> Create(
-      const GpuPreferences& gpu_preferences);
+  virtual ~MailboxManager() {}
 
   // Look up the texture definition from the named mailbox.
   virtual TextureBase* ConsumeTexture(const Mailbox& mailbox) = 0;
@@ -41,14 +41,8 @@ class GPU_EXPORT MailboxManager : public base::RefCounted<MailboxManager> {
   // Destroy any mailbox that reference the given texture.
   virtual void TextureDeleted(TextureBase* texture) = 0;
 
- protected:
-  MailboxManager() {}
-  virtual ~MailboxManager() {}
-
- private:
-  friend class base::RefCounted<MailboxManager>;
-
-  DISALLOW_COPY_AND_ASSIGN(MailboxManager);
+  static std::unique_ptr<MailboxManager> Create(
+      const GpuPreferences& gpu_preferences);
 };
 
 }  // namespage gles2